From b990b06f90c9c1516353e0337587f94e8d3920e1 Mon Sep 17 00:00:00 2001 From: Matt Button Date: Thu, 28 Feb 2013 12:46:54 +0000 Subject: [PATCH 1/4] Allow metrics to export their values as a Hash First step towards removing duplication in the reporters --- lib/metriks/counter.rb | 10 +++++++++- lib/metriks/exportable.rb | 42 +++++++++++++++++++++++++++++++++++++++ lib/metriks/histogram.rb | 12 +++++++++++ lib/metriks/meter.rb | 14 ++++++++++++- lib/metriks/timer.rb | 19 +++++++++++++++++- test/counter_test.rb | 6 ++++++ test/exportable_test.rb | 31 +++++++++++++++++++++++++++++ test/histogram_test.rb | 22 ++++++++++++++++++++ test/meter_test.rb | 21 ++++++++++++++++++++ test/timer_test.rb | 32 ++++++++++++++++++++++++++++- 10 files changed, 205 insertions(+), 4 deletions(-) create mode 100644 lib/metriks/exportable.rb create mode 100644 test/exportable_test.rb diff --git a/lib/metriks/counter.rb b/lib/metriks/counter.rb index 93b9662..6a759f5 100644 --- a/lib/metriks/counter.rb +++ b/lib/metriks/counter.rb @@ -1,9 +1,12 @@ require 'atomic' +require 'metriks/exportable' module Metriks # Public: Counters are one of the simplest metrics whose only operations # are increment and decrement. class Counter + include Metriks::Exportable + # Public: Initialize a new Counter. def initialize @count = Atomic.new(0) @@ -40,5 +43,10 @@ def decrement(decr = 1) def count @count.value end + + private + def exportable_metrics + [:count] + end end -end \ No newline at end of file +end diff --git a/lib/metriks/exportable.rb b/lib/metriks/exportable.rb new file mode 100644 index 0000000..6410006 --- /dev/null +++ b/lib/metriks/exportable.rb @@ -0,0 +1,42 @@ + +module Metriks + module Exportable + + # Public: Export all of the Metric's computed values as a hash + def export_values + values = capture_metrics(self, exportable_metrics) + + if respond_to? :snapshot + values = values.merge(capture_metrics(snapshot, exportable_snapshots)) + end + + values + end + + private + # Private: Array of metrics that this metric object can calculate + # + # Returns: Array + def exportable_metrics + raise NotImplementedError, "Metrics must declare a set of exportable values" + end + + # Private: Array of metrics that can be calculated from a snapshot + # of the data + # + # Returns: Array + def exportable_snapshots + [] + end + + def capture_metrics(source, metric) + Array(metric).inject({}) do |values,key| + name = String(key).gsub(/^get_/, '').to_sym + + values.tap do |h| + h[name] = source.send(key) + end + end + end + end +end diff --git a/lib/metriks/histogram.rb b/lib/metriks/histogram.rb index 7237c50..1a0548c 100644 --- a/lib/metriks/histogram.rb +++ b/lib/metriks/histogram.rb @@ -1,9 +1,12 @@ require 'atomic' require 'metriks/uniform_sample' require 'metriks/exponentially_decaying_sample' +require 'metriks/exportable' module Metriks class Histogram + include Metriks::Exportable + DEFAULT_SAMPLE_SIZE = 1028 DEFAULT_ALPHA = 0.015 @@ -108,5 +111,14 @@ def update_variance(value) new_values end end + + private + def exportable_metrics + [:count, :min, :max, :mean, :stddev] + end + + def exportable_snapshots + [:median, :get_95th_percentile] + end end end diff --git a/lib/metriks/meter.rb b/lib/metriks/meter.rb index 3e4d64d..2ec56d6 100644 --- a/lib/metriks/meter.rb +++ b/lib/metriks/meter.rb @@ -1,9 +1,12 @@ require 'atomic' require 'metriks/ewma' +require 'metriks/exportable' module Metriks class Meter + include Metriks::Exportable + TICK_INTERVAL = 5.0 def initialize(averager_klass = Metriks::EWMA) @@ -81,5 +84,14 @@ def mean_rate def stop end + + private + def exportable_metrics + [ + :count, + :one_minute_rate, :five_minute_rate, :fifteen_minute_rate, + :mean_rate + ] + end end -end \ No newline at end of file +end diff --git a/lib/metriks/timer.rb b/lib/metriks/timer.rb index cf1181f..13c9f8e 100644 --- a/lib/metriks/timer.rb +++ b/lib/metriks/timer.rb @@ -1,11 +1,14 @@ require 'atomic' require 'hitimes' +require 'metriks/exportable' require 'metriks/meter' require 'metriks/histogram' module Metriks class Timer + include Metriks::Exportable + class Context def initialize(timer) @timer = timer @@ -97,5 +100,19 @@ def stddev def stop @meter.stop end + + private + def exportable_metrics + [ + :count, :min, :max, :mean, :stddev, + :one_minute_rate, :five_minute_rate, :fifteen_minute_rate, :mean_rate + ] + end + + def exportable_snapshots + [ + :median, :get_95th_percentile + ] + end end -end \ No newline at end of file +end diff --git a/test/counter_test.rb b/test/counter_test.rb index 8540a43..b24094e 100644 --- a/test/counter_test.rb +++ b/test/counter_test.rb @@ -36,4 +36,10 @@ def test_increment_by_more_threaded assert_equal 10000, @counter.count end + + def test_export_values + @counter.increment 10 + + assert_equal({:count => 10}, @counter.export_values) + end end diff --git a/test/exportable_test.rb b/test/exportable_test.rb new file mode 100644 index 0000000..fe809aa --- /dev/null +++ b/test/exportable_test.rb @@ -0,0 +1,31 @@ +require 'test_helper' +require 'metriks/exportable' + +class ExportableTest < Test::Unit::TestCase + + def test_export_values_raises_error_if_no_exportables_defined + metric = Class.new do + include Metriks::Exportable + end + + assert_raise NotImplementedError do + metric.new.export_values + end + end + + def test_export_values_returns_metric_values + metric = Class.new do + include Metriks::Exportable + + def exportable_metrics + [:count] + end + + def count + 42 + end + end + + assert_equal({:count => 42}, metric.new.export_values) + end +end diff --git a/test/histogram_test.rb b/test/histogram_test.rb index 41642ba..741a49c 100644 --- a/test/histogram_test.rb +++ b/test/histogram_test.rb @@ -196,4 +196,26 @@ def test_long_idle_sample assert_equal 5, @histogram.min end + + def test_export_values + @histogram = Metriks::Histogram.new(Metriks::ExponentiallyDecayingSample.new(Metriks::Histogram::DEFAULT_SAMPLE_SIZE, Metriks::Histogram::DEFAULT_ALPHA)) + + thread 10 do + 100.times do |idx| + @histogram.update(idx) + end + end + + expected = { + :count => 1000, + :min => 0, + :max => 99, + :mean => 49, + :stddev => 57.323642591866054, + :median => 49.5, + :"95th_percentile" => 94.94999999999993, + } + + assert_equal expected, @histogram.export_values + end end diff --git a/test/meter_test.rb b/test/meter_test.rb index 8d4e0c0..732d645 100644 --- a/test/meter_test.rb +++ b/test/meter_test.rb @@ -35,4 +35,25 @@ def test_one_minute_rate assert_equal 200, @meter.one_minute_rate end + + def test_export_values + @meter.mark 1000 + @meter.tick + + expected = { + :count => 1000, + :one_minute_rate => 200.0, + :five_minute_rate => 200.0, + :fifteen_minute_rate => 200.0, + } + + exported = @meter.export_values + + expected.each_pair do |metric, value| + assert_equal value, exported[metric] + end + + # This value changes wildly + assert exported[:mean_rate] > 0 + end end diff --git a/test/timer_test.rb b/test/timer_test.rb index 87eb0d4..c38bfc8 100644 --- a/test/timer_test.rb +++ b/test/timer_test.rb @@ -29,4 +29,34 @@ def test_timer_without_block assert_in_delta 0.1, @timer.mean, 0.01 end -end \ No newline at end of file + + def test_exportable_metrics + 2.times do + @timer.time do + sleep 0.1 + end + end + + values = @timer.export_values + + assert_equal 2, values[:count] + + expected = { + :min => [0.1, 0.1], + :max => [0.1, 0.1], + :mean => [0.1, 0.1], + :stddev => [1, 1], + :one_minute_rate => [0, 0], + :five_minute_rate => [0, 0], + :fifteen_minute_rate => [0, 0], + :median => [0.1, 0.1], + :"95th_percentile" => [0.1, 0.1] + } + + expected.each_pair do |metric, expectation| + target,delta = expectation + + assert_in_delta target, values[metric], delta, "#{metric}" + end + end +end From 283f0d89190fdf49db6a7a33a810f9a10ef1132d Mon Sep 17 00:00:00 2001 From: Matt Button Date: Thu, 28 Feb 2013 13:21:23 +0000 Subject: [PATCH 2/4] Partial work on refactoring the reporters to use Exportable --- lib/metriks/reporter/graphite.rb | 53 ++-------------------- lib/metriks/reporter/librato_metrics.rb | 59 ++----------------------- lib/metriks/reporter/logger.rb | 52 ++-------------------- lib/metriks/reporter/riemann.rb | 53 ++-------------------- 4 files changed, 13 insertions(+), 204 deletions(-) diff --git a/lib/metriks/reporter/graphite.rb b/lib/metriks/reporter/graphite.rb index e8ec7fc..f422258 100644 --- a/lib/metriks/reporter/graphite.rb +++ b/lib/metriks/reporter/graphite.rb @@ -48,45 +48,11 @@ def restart def write @registry.each do |name, metric| - case metric - when Metriks::Meter - write_metric name, metric, [ - :count, :one_minute_rate, :five_minute_rate, - :fifteen_minute_rate, :mean_rate - ] - when Metriks::Counter - write_metric name, metric, [ - :count - ] - when Metriks::UtilizationTimer - write_metric name, metric, [ - :count, :one_minute_rate, :five_minute_rate, - :fifteen_minute_rate, :mean_rate, - :min, :max, :mean, :stddev, - :one_minute_utilization, :five_minute_utilization, - :fifteen_minute_utilization, :mean_utilization, - ], [ - :median, :get_95th_percentile - ] - when Metriks::Timer - write_metric name, metric, [ - :count, :one_minute_rate, :five_minute_rate, - :fifteen_minute_rate, :mean_rate, - :min, :max, :mean, :stddev - ], [ - :median, :get_95th_percentile - ] - when Metriks::Histogram - write_metric name, metric, [ - :count, :min, :max, :mean, :stddev - ], [ - :median, :get_95th_percentile - ] - end + write_metrics name, metric.export_values end end - def write_metric(base_name, metric, keys, snapshot_keys = []) + def write_metrics(base_name, metrics) time = Time.now.to_i base_name = base_name.to_s.gsub(/ +/, '_') @@ -94,22 +60,9 @@ def write_metric(base_name, metric, keys, snapshot_keys = []) base_name = "#{@prefix}.#{base_name}" end - keys.flatten.each do |key| - name = key.to_s.gsub(/^get_/, '') - value = metric.send(key) + metrics.each_pair do |name, value| socket.write("#{base_name}.#{name} #{value} #{time}\n") end - - unless snapshot_keys.empty? - snapshot = metric.snapshot - snapshot_keys.flatten.each do |key| - name = key.to_s.gsub(/^get_/, '') - value = snapshot.send(key) - socket.write("#{base_name}.#{name} #{value} #{time}\n") - end - end - rescue Errno::EPIPE - socket.close end end end diff --git a/lib/metriks/reporter/librato_metrics.rb b/lib/metriks/reporter/librato_metrics.rb index 3299e8e..1081053 100644 --- a/lib/metriks/reporter/librato_metrics.rb +++ b/lib/metriks/reporter/librato_metrics.rb @@ -46,41 +46,7 @@ def restart def write gauges = [] @registry.each do |name, metric| - gauges << case metric - when Metriks::Meter - prepare_metric name, metric, [ - :count, :one_minute_rate, :five_minute_rate, - :fifteen_minute_rate, :mean_rate - ] - when Metriks::Counter - prepare_metric name, metric, [ - :count - ] - when Metriks::UtilizationTimer - prepare_metric name, metric, [ - :count, :one_minute_rate, :five_minute_rate, - :fifteen_minute_rate, :mean_rate, - :min, :max, :mean, :stddev, - :one_minute_utilization, :five_minute_utilization, - :fifteen_minute_utilization, :mean_utilization, - ], [ - :median, :get_95th_percentile - ] - when Metriks::Timer - prepare_metric name, metric, [ - :count, :one_minute_rate, :five_minute_rate, - :fifteen_minute_rate, :mean_rate, - :min, :max, :mean, :stddev - ], [ - :median, :get_95th_percentile - ] - when Metriks::Histogram - prepare_metric name, metric, [ - :count, :min, :max, :mean, :stddev - ], [ - :median, :get_95th_percentile - ] - end + gauges << prepare_metrics(name, metric.export_values) end gauges.flatten! @@ -137,7 +103,7 @@ def form_data(metrics) data end - def prepare_metric(base_name, metric, keys, snapshot_keys = []) + def prepare_metrics(base_name, metrics) results = [] time = @time_tracker.now_floored @@ -146,10 +112,7 @@ def prepare_metric(base_name, metric, keys, snapshot_keys = []) base_name = "#{@prefix}.#{base_name}" end - keys.flatten.each do |key| - name = key.to_s.gsub(/^get_/, '') - value = metric.send(key) - + metrics.each_pair do |name, value| results << { :type => name.to_s == "count" ? "counter" : "gauge", :name => "#{base_name}.#{name}", @@ -159,22 +122,6 @@ def prepare_metric(base_name, metric, keys, snapshot_keys = []) } end - unless snapshot_keys.empty? - snapshot = metric.snapshot - snapshot_keys.flatten.each do |key| - name = key.to_s.gsub(/^get_/, '') - value = snapshot.send(key) - - results << { - :type => name.to_s == "count" ? "counter" : "gauge", - :name => "#{base_name}.#{name}", - :source => @source, - :measure_time => time, - :value => value - } - end - end - results end end diff --git a/lib/metriks/reporter/logger.rb b/lib/metriks/reporter/logger.rb index cfa0b64..465ee9b 100644 --- a/lib/metriks/reporter/logger.rb +++ b/lib/metriks/reporter/logger.rb @@ -49,52 +49,11 @@ def write @last_write = Time.now @registry.each do |name, metric| - case metric - when Metriks::Meter - log_metric name, 'meter', metric, [ - :count, :one_minute_rate, :five_minute_rate, - :fifteen_minute_rate, :mean_rate - ] - when Metriks::Counter - log_metric name, 'counter', metric, [ - :count - ] - when Metriks::UtilizationTimer - log_metric name, 'utilization_timer', metric, [ - :count, :one_minute_rate, :five_minute_rate, - :fifteen_minute_rate, :mean_rate, - :min, :max, :mean, :stddev, - :one_minute_utilization, :five_minute_utilization, - :fifteen_minute_utilization, :mean_utilization, - ], [ - :median, :get_95th_percentile - ] - when Metriks::Timer - log_metric name, 'timer', metric, [ - :count, :one_minute_rate, :five_minute_rate, - :fifteen_minute_rate, :mean_rate, - :min, :max, :mean, :stddev - ], [ - :median, :get_95th_percentile - ] - when Metriks::Histogram - log_metric name, 'histogram', metric, [ - :count, :min, :max, :mean, :stddev - ], [ - :median, :get_95th_percentile - ] - end - end - end - - def extract_from_metric(metric, *keys) - keys.flatten.collect do |key| - name = key.to_s.gsub(/^get_/, '') - [ { name => metric.send(key) } ] + log_metrics name, metric.export_values end end - def log_metric(name, type, metric, keys, snapshot_keys = []) + def log_metrics(name, values) message = [] message << @prefix if @prefix @@ -102,12 +61,7 @@ def log_metric(name, type, metric, keys, snapshot_keys = []) message << { :name => name } message << { :type => type } - message += extract_from_metric(metric, keys) - - unless snapshot_keys.empty? - snapshot = metric.snapshot - message += extract_from_metric(snapshot, snapshot_keys) - end + message.merge!(values) @logger.add(@log_level, format_message(message)) end diff --git a/lib/metriks/reporter/riemann.rb b/lib/metriks/reporter/riemann.rb index aae5c39..01c7a23 100644 --- a/lib/metriks/reporter/riemann.rb +++ b/lib/metriks/reporter/riemann.rb @@ -53,63 +53,18 @@ def write @last_write = Time.now @registry.each do |name, metric| - case metric - when Metriks::Meter - send_metric name, 'meter', metric, [ - :count, :one_minute_rate, :five_minute_rate, - :fifteen_minute_rate, :mean_rate - ] - when Metriks::Counter - send_metric name, 'counter', metric, [ - :count - ] - when Metriks::UtilizationTimer - send_metric name, 'utilization_timer', metric, [ - :count, :one_minute_rate, :five_minute_rate, - :fifteen_minute_rate, :mean_rate, - :min, :max, :mean, :stddev, - :one_minute_utilization, :five_minute_utilization, - :fifteen_minute_utilization, :mean_utilization, - ], [ - :median, :get_95th_percentile - ] - when Metriks::Timer - send_metric name, 'timer', metric, [ - :count, :one_minute_rate, :five_minute_rate, - :fifteen_minute_rate, :mean_rate, - :min, :max, :mean, :stddev - ], [ - :median, :get_95th_percentile - ] - when Metriks::Histogram - send_metric name, 'histogram', metric, [ - :count, :min, :max, :mean, :stddev - ], [ - :median, :get_95th_percentile - ] - end + send_metric name, 'meter', metric.export_values end end - def send_metric(name, type, metric, keys, snapshot_keys = []) - keys.each do |key| + def send_metric(name, type, values) + values.each_pair do |key, value| @client << @default_event.merge( :service => "#{name} #{key}", - :metric => metric.send(key), + :metric => value, :tags => [type] ) end - - unless snapshot_keys.empty? - snapshot = metric.snapshot - snapshot_keys.each do |key| - @client << @default_event.merge( - :service => "#{name} #{key}", - :metric => snapshot.send(key), - :tags => [type] - ) - end - end end end end From f2545363d34544a70db307dbbd49384ff18df393 Mon Sep 17 00:00:00 2001 From: Matt Button Date: Sun, 3 Mar 2013 16:38:53 +0000 Subject: [PATCH 3/4] Introduce Exportable#metric_type Some reporters such as logger and riemann need to record the type of the metric. This type appears to be an underscored version of the class name, so for now that's the default implementation. --- lib/metriks/exportable.rb | 7 +++++++ lib/metriks/reporter/logger.rb | 6 +++--- lib/metriks/reporter/riemann.rb | 6 +++--- test/exportable_test.rb | 9 +++++++++ 4 files changed, 22 insertions(+), 6 deletions(-) diff --git a/lib/metriks/exportable.rb b/lib/metriks/exportable.rb index 6410006..89e1ab8 100644 --- a/lib/metriks/exportable.rb +++ b/lib/metriks/exportable.rb @@ -2,6 +2,13 @@ module Metriks module Exportable + # Public: Get the type of this metric + # + # This key is used by certain reporters such as logger + def metric_type + self.class.name.split('::').last.gsub(/(.)([A-Z])/, '\1_\2').downcase + end + # Public: Export all of the Metric's computed values as a hash def export_values values = capture_metrics(self, exportable_metrics) diff --git a/lib/metriks/reporter/logger.rb b/lib/metriks/reporter/logger.rb index 465ee9b..e1e1f10 100644 --- a/lib/metriks/reporter/logger.rb +++ b/lib/metriks/reporter/logger.rb @@ -49,11 +49,11 @@ def write @last_write = Time.now @registry.each do |name, metric| - log_metrics name, metric.export_values + log_metrics name, metric.metric_type, metric.export_values end end - def log_metrics(name, values) + def log_metrics(name, type, values) message = [] message << @prefix if @prefix @@ -61,7 +61,7 @@ def log_metrics(name, values) message << { :name => name } message << { :type => type } - message.merge!(values) + message << values @logger.add(@log_level, format_message(message)) end diff --git a/lib/metriks/reporter/riemann.rb b/lib/metriks/reporter/riemann.rb index 01c7a23..530cee1 100644 --- a/lib/metriks/reporter/riemann.rb +++ b/lib/metriks/reporter/riemann.rb @@ -11,7 +11,7 @@ def initialize(options = {}) @registry = options[:registry] || Metrics::Registry.default @interval = options[:interval] || 60 @on_error = options[:on_error] || proc { |ex| } - + @default_event = options[:default_event] || {} @default_event[:ttl] ||= @interval * 1.5 end @@ -20,7 +20,7 @@ def start @thread ||= Thread.new do loop do sleep @interval - + Thread.new do begin write @@ -53,7 +53,7 @@ def write @last_write = Time.now @registry.each do |name, metric| - send_metric name, 'meter', metric.export_values + send_metric name, metric.metric_type, metric.export_values end end diff --git a/test/exportable_test.rb b/test/exportable_test.rb index fe809aa..d1007cd 100644 --- a/test/exportable_test.rb +++ b/test/exportable_test.rb @@ -2,6 +2,15 @@ require 'metriks/exportable' class ExportableTest < Test::Unit::TestCase + class Foobar + class TestMetrik + include Metriks::Exportable + end + end + + def test_metric_type + assert_equal "test_metrik", Foobar::TestMetrik.new.metric_type + end def test_export_values_raises_error_if_no_exportables_defined metric = Class.new do From 495ac44c476668b905bb0bc1a507a5bdbe796907 Mon Sep 17 00:00:00 2001 From: Matt Button Date: Sun, 3 Mar 2013 16:50:30 +0000 Subject: [PATCH 4/4] Include tests to checker that logger reporter uses correct metric name/type --- test/logger_reporter_test.rb | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/test/logger_reporter_test.rb b/test/logger_reporter_test.rb index 83e7d77..d9ca65b 100644 --- a/test/logger_reporter_test.rb +++ b/test/logger_reporter_test.rb @@ -30,6 +30,22 @@ def teardown @registry.stop end + def test_includes_name + @reporter.write + + ['meter', 'counter', 'timer', 'histogram', 'utilization_timer'].each do |metric| + assert_match /\ name=#{metric}\.testing /, @stringio.string + end + end + + def test_includes_type + @reporter.write + + ['meter', 'counter', 'timer', 'histogram', 'utilization_timer'].each do |metric| + assert_match /\ type=#{metric} /, @stringio.string + end + end + def test_write @reporter.write @@ -43,4 +59,4 @@ def test_flush assert_match /time=\d/, @stringio.string assert_match /median=\d/, @stringio.string end -end \ No newline at end of file +end