From ad71d99e7d4379ff4809786121163aab19dd2b0d Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Wed, 24 Aug 2016 13:23:16 -0700 Subject: [PATCH 01/55] add getters/setters --- lib/librato/metrics/client.rb | 9 +++++++++ spec/unit/metrics/client_spec.rb | 33 ++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/lib/librato/metrics/client.rb b/lib/librato/metrics/client.rb index 0e35586..2da6f89 100644 --- a/lib/librato/metrics/client.rb +++ b/lib/librato/metrics/client.rb @@ -8,6 +8,15 @@ class Client attr_accessor :email, :api_key, :proxy + def tags + @tags ||= {} + end + + def tags=(tags) + raise ArgumentError, "tags must be a Hash" unless tags.is_a?(Hash) + @tags = tags + end + # Provide agent identifier for the developer program. See: # http://support.metrics.librato.com/knowledgebase/articles/53548-developer-program # diff --git a/spec/unit/metrics/client_spec.rb b/spec/unit/metrics/client_spec.rb index c611e19..880db93 100644 --- a/spec/unit/metrics/client_spec.rb +++ b/spec/unit/metrics/client_spec.rb @@ -5,6 +5,39 @@ module Metrics describe Client do + describe "#tags" do + context "when set" do + before { subject.tags = { instance: "i-1234567a" } } + it "gets @tags" do + expect(subject.tags).to be_a(Hash) + expect(subject.tags.keys).to include(:instance) + expect(subject.tags[:instance]).to eq("i-1234567a") + end + end + + context "when not set" do + it "defaults to empty hash" do + expect(subject.tags).to be_empty + end + end + end + + describe "#tags=" do + it "sets @tags" do + expected_tags = { instance: "i-1234567b" } + expect{subject.tags = expected_tags}.to change{subject.tags}.from({}).to(expected_tags) + expect(subject.tags).to be_a(Hash) + expect(subject.tags.keys).to eq(expected_tags.keys) + expect(subject.tags[:instance]).to eq(expected_tags[:instance]) + end + + context "when invalid argument" do + it "raises exception" do + expect { subject.tags = "invalid arg" }.to raise_error(ArgumentError) + end + end + end + describe "#agent_identifier" do context "when given a single string argument" do it "sets agent_identifier" do From 6dfde2338aa9ed0e4d1c237785e5ce0ba5b6891f Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Wed, 24 Aug 2016 13:37:18 -0700 Subject: [PATCH 02/55] add #add_tags --- lib/librato/metrics/client.rb | 5 ++++ spec/unit/metrics/client_spec.rb | 40 ++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/lib/librato/metrics/client.rb b/lib/librato/metrics/client.rb index 2da6f89..2f2ef28 100644 --- a/lib/librato/metrics/client.rb +++ b/lib/librato/metrics/client.rb @@ -17,6 +17,11 @@ def tags=(tags) @tags = tags end + def add_tags(tags) + raise ArgumentError, "tags must be a Hash" unless tags.is_a?(Hash) + self.tags.merge!(tags) + end + # Provide agent identifier for the developer program. See: # http://support.metrics.librato.com/knowledgebase/articles/53548-developer-program # diff --git a/spec/unit/metrics/client_spec.rb b/spec/unit/metrics/client_spec.rb index 880db93..746f8d2 100644 --- a/spec/unit/metrics/client_spec.rb +++ b/spec/unit/metrics/client_spec.rb @@ -8,6 +8,7 @@ module Metrics describe "#tags" do context "when set" do before { subject.tags = { instance: "i-1234567a" } } + after { Librato::Metrics.client.tags.clear } it "gets @tags" do expect(subject.tags).to be_a(Hash) expect(subject.tags.keys).to include(:instance) @@ -23,6 +24,7 @@ module Metrics end describe "#tags=" do + after { Librato::Metrics.client.tags.clear } it "sets @tags" do expected_tags = { instance: "i-1234567b" } expect{subject.tags = expected_tags}.to change{subject.tags}.from({}).to(expected_tags) @@ -38,6 +40,44 @@ module Metrics end end + describe "#add_tags" do + after { Librato::Metrics.client.tags.clear } + + context "when no existing tags" do + it "adds top-level tags" do + expected_tags = { instance: "i-1234567c" } + subject.add_tags expected_tags + + expect(subject.tags).to be_a(Hash) + expect(subject.tags.keys).to eq(expected_tags.keys) + expect(subject.tags[:instance]).to eq(expected_tags[:instance]) + end + end + + context "when existing tags" do + it "merges tags" do + tmp1 = { instance: "i-1234567d" } + tmp2 = { region: "us-east-1", elb: "metrics-stg" } + expected_tags = tmp1.merge(tmp2) + + subject.add_tags tmp1 + subject.add_tags tmp2 + + expect(subject.tags).to be_a(Hash) + expect(subject.tags.keys).to eq(expected_tags.keys) + expect(subject.tags[:instance]).to eq(expected_tags[:instance]) + expect(subject.tags[:region]).to eq(expected_tags[:region]) + expect(subject.tags[:elb]).to eq(expected_tags[:elb]) + end + end + + context "when invalid argument" do + it "raises exception" do + expect { subject.add_tags "invalid arg" }.to raise_error(ArgumentError) + end + end + end + describe "#agent_identifier" do context "when given a single string argument" do it "sets agent_identifier" do From 6b52228c282752f12a8e525bc4f21a85f5c2da1d Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Wed, 24 Aug 2016 13:45:54 -0700 Subject: [PATCH 03/55] add #clear_tags --- lib/librato/metrics/client.rb | 2 ++ spec/unit/metrics/client_spec.rb | 11 +++++++++++ 2 files changed, 13 insertions(+) diff --git a/lib/librato/metrics/client.rb b/lib/librato/metrics/client.rb index 2f2ef28..e596c22 100644 --- a/lib/librato/metrics/client.rb +++ b/lib/librato/metrics/client.rb @@ -6,6 +6,8 @@ class Client def_delegator :annotator, :add, :annotate + def_delegator :@tags, :clear, :clear_tags + attr_accessor :email, :api_key, :proxy def tags diff --git a/spec/unit/metrics/client_spec.rb b/spec/unit/metrics/client_spec.rb index 746f8d2..87356ef 100644 --- a/spec/unit/metrics/client_spec.rb +++ b/spec/unit/metrics/client_spec.rb @@ -78,6 +78,17 @@ module Metrics end end + describe "#clear_tags" do + context "when tags are set" do + it "empties Hash" do + expected_tags = { instance: "i-1234567c" } + subject.add_tags expected_tags + + expect{subject.clear_tags}.to change{subject.tags}.from(expected_tags).to({}) + end + end + end + describe "#agent_identifier" do context "when given a single string argument" do it "sets agent_identifier" do From e3db7ebbba418214e208661baf3cdcaac4d69083 Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Wed, 24 Aug 2016 13:54:56 -0700 Subject: [PATCH 04/55] add #has_tags? and #tags? alias --- lib/librato/metrics/client.rb | 5 +++++ spec/unit/metrics/client_spec.rb | 17 +++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/lib/librato/metrics/client.rb b/lib/librato/metrics/client.rb index e596c22..9f8703a 100644 --- a/lib/librato/metrics/client.rb +++ b/lib/librato/metrics/client.rb @@ -24,6 +24,11 @@ def add_tags(tags) self.tags.merge!(tags) end + def has_tags? + !self.tags.empty? + end + alias :tags? :has_tags? + # Provide agent identifier for the developer program. See: # http://support.metrics.librato.com/knowledgebase/articles/53548-developer-program # diff --git a/spec/unit/metrics/client_spec.rb b/spec/unit/metrics/client_spec.rb index 87356ef..5845790 100644 --- a/spec/unit/metrics/client_spec.rb +++ b/spec/unit/metrics/client_spec.rb @@ -89,6 +89,23 @@ module Metrics end end + describe "#has_tags?" do + context "when tags are set" do + after { Librato::Metrics.client.tags.clear } + it "returns true" do + subject.add_tags instance: "i-1234567c" + + expect(subject.has_tags?).to eq(true) + end + end + + context "when tags are not set" do + it "returns false" do + expect(subject.has_tags?).to eq(false) + end + end + end + describe "#agent_identifier" do context "when given a single string argument" do it "sets agent_identifier" do From edf6c34a1c04f1792d362af8bbb1e94186dc57f6 Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Wed, 24 Aug 2016 14:23:19 -0700 Subject: [PATCH 05/55] add :tags option to #initialize --- lib/librato/metrics/client.rb | 5 +++++ spec/unit/metrics/client_spec.rb | 29 +++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/lib/librato/metrics/client.rb b/lib/librato/metrics/client.rb index 9f8703a..bd91948 100644 --- a/lib/librato/metrics/client.rb +++ b/lib/librato/metrics/client.rb @@ -10,6 +10,11 @@ class Client attr_accessor :email, :api_key, :proxy + def initialize(options={}) + raise ArgumentError, ":tags must be a Hash" if options[:tags] && !options[:tags].is_a?(Hash) + self.tags = options.fetch(:tags, {}) + end + def tags @tags ||= {} end diff --git a/spec/unit/metrics/client_spec.rb b/spec/unit/metrics/client_spec.rb index 5845790..8ba1b4e 100644 --- a/spec/unit/metrics/client_spec.rb +++ b/spec/unit/metrics/client_spec.rb @@ -5,6 +5,35 @@ module Metrics describe Client do + describe "#initialize" do + context "when :tags are present" do + after { Librato::Metrics.client.tags.clear } + context "when :tags are valid" do + it "sets @tags" do + expected_tags = { environment: "staging" } + client = Client.new(tags: expected_tags) + + expect(client.tags).not_to be_empty + expect(client.tags).to eq(expected_tags) + end + end + + context "when :tags are invalid" do + it "raises exception" do + expect { Client.new(tags: "invalid arg") }.to raise_error(ArgumentError) + end + end + end + + context "when :tags are not present" do + it "does not set @tags" do + client = Client.new + + expect(client.tags).to be_empty + end + end + end + describe "#tags" do context "when set" do before { subject.tags = { instance: "i-1234567a" } } From 4eb65856060098e6bff672fb6ee6f13ce9bd80db Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Wed, 24 Aug 2016 14:25:13 -0700 Subject: [PATCH 06/55] reduce equivalence matchers --- spec/unit/metrics/client_spec.rb | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/spec/unit/metrics/client_spec.rb b/spec/unit/metrics/client_spec.rb index 8ba1b4e..2accaf5 100644 --- a/spec/unit/metrics/client_spec.rb +++ b/spec/unit/metrics/client_spec.rb @@ -58,8 +58,7 @@ module Metrics expected_tags = { instance: "i-1234567b" } expect{subject.tags = expected_tags}.to change{subject.tags}.from({}).to(expected_tags) expect(subject.tags).to be_a(Hash) - expect(subject.tags.keys).to eq(expected_tags.keys) - expect(subject.tags[:instance]).to eq(expected_tags[:instance]) + expect(subject.tags).to eq(expected_tags) end context "when invalid argument" do @@ -78,8 +77,7 @@ module Metrics subject.add_tags expected_tags expect(subject.tags).to be_a(Hash) - expect(subject.tags.keys).to eq(expected_tags.keys) - expect(subject.tags[:instance]).to eq(expected_tags[:instance]) + expect(subject.tags).to eq(expected_tags) end end @@ -93,10 +91,7 @@ module Metrics subject.add_tags tmp2 expect(subject.tags).to be_a(Hash) - expect(subject.tags.keys).to eq(expected_tags.keys) - expect(subject.tags[:instance]).to eq(expected_tags[:instance]) - expect(subject.tags[:region]).to eq(expected_tags[:region]) - expect(subject.tags[:elb]).to eq(expected_tags[:elb]) + expect(subject.tags).to eq(expected_tags) end end From 5f892f00b5de44e278bdf087f71fa49aba82e591 Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Wed, 24 Aug 2016 14:54:25 -0700 Subject: [PATCH 07/55] use /v1/measurements when client has tags --- lib/librato/metrics.rb | 4 ++-- lib/librato/metrics/persistence/direct.rb | 18 +++++++++++------- spec/unit/metrics/client_spec.rb | 6 +++--- 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/lib/librato/metrics.rb b/lib/librato/metrics.rb index dd72ba4..24cfac7 100644 --- a/lib/librato/metrics.rb +++ b/lib/librato/metrics.rb @@ -64,8 +64,8 @@ module Librato module Metrics extend SingleForwardable - TYPES = [:counter, :gauge] - PLURAL_TYPES = [:counters, :gauges] + TYPES = [:counter, :gauge, :measurement] + PLURAL_TYPES = TYPES.map { |type| "#{type}s".to_sym } MIN_MEASURE_TIME = (Time.now-(3600*24*365)).to_i # Most of the singleton methods of Librato::Metrics are actually diff --git a/lib/librato/metrics/persistence/direct.rb b/lib/librato/metrics/persistence/direct.rb index 79a1622..20bdfa4 100644 --- a/lib/librato/metrics/persistence/direct.rb +++ b/lib/librato/metrics/persistence/direct.rb @@ -5,8 +5,6 @@ module Librato module Metrics module Persistence class Direct - MEASUREMENT_TYPES = [:gauges, :counters] - # Persist the queued metrics directly to the # Metrics web API. # @@ -18,9 +16,15 @@ def persist(client, queued, options={}) requests = [queued] end requests.each do |request| - payload = SmartJSON.write(request) + if client.has_tags? + resource = "measurements" + payload = SmartJSON.write(request.merge({ tags: client.tags })) + else + resource = "metrics" + payload = SmartJSON.write(request) + end # expects 200 - client.connection.post('metrics', payload) + client.connection.post(resource, payload) end end @@ -31,7 +35,7 @@ def chunk_queued(queued, per_request) reqs = [] # separate metric-containing values from global values globals = fetch_globals(queued) - MEASUREMENT_TYPES.each do |metric_type| + Librato::Metrics::PLURAL_TYPES.each do |metric_type| metrics = queued[metric_type] next unless metrics if metrics.size <= per_request @@ -52,7 +56,7 @@ def build_request(type, metrics, globals) end def fetch_globals(queued) - queued.reject {|k, v| MEASUREMENT_TYPES.include?(k)} + queued.reject {|k, v| Librato::Metrics::PLURAL_TYPES.include?(k)} end def queue_count(queued) @@ -62,4 +66,4 @@ def queue_count(queued) end end end -end \ No newline at end of file +end diff --git a/spec/unit/metrics/client_spec.rb b/spec/unit/metrics/client_spec.rb index 2accaf5..9ad5e12 100644 --- a/spec/unit/metrics/client_spec.rb +++ b/spec/unit/metrics/client_spec.rb @@ -83,7 +83,7 @@ module Metrics context "when existing tags" do it "merges tags" do - tmp1 = { instance: "i-1234567d" } + tmp1 = { instance: "i-1234567c" } tmp2 = { region: "us-east-1", elb: "metrics-stg" } expected_tags = tmp1.merge(tmp2) @@ -105,7 +105,7 @@ module Metrics describe "#clear_tags" do context "when tags are set" do it "empties Hash" do - expected_tags = { instance: "i-1234567c" } + expected_tags = { instance: "i-1234567d" } subject.add_tags expected_tags expect{subject.clear_tags}.to change{subject.tags}.from(expected_tags).to({}) @@ -117,7 +117,7 @@ module Metrics context "when tags are set" do after { Librato::Metrics.client.tags.clear } it "returns true" do - subject.add_tags instance: "i-1234567c" + subject.add_tags instance: "i-1234567e" expect(subject.has_tags?).to eq(true) end From 38bf2a49e056aeb9f27cf23c6d9846a01f38ef78 Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Wed, 24 Aug 2016 16:32:51 -0700 Subject: [PATCH 08/55] validate options for Queue and Aggregator --- lib/librato/metrics/processor.rb | 19 +++++++++++++++++++ spec/unit/metrics/aggregator_spec.rb | 27 +++++++++++++++++++++++++++ spec/unit/metrics/queue_spec.rb | 27 +++++++++++++++++++++++++++ 3 files changed, 73 insertions(+) diff --git a/lib/librato/metrics/processor.rb b/lib/librato/metrics/processor.rb index 64616d1..43004a2 100644 --- a/lib/librato/metrics/processor.rb +++ b/lib/librato/metrics/processor.rb @@ -1,3 +1,5 @@ +require "set" + module Librato module Metrics @@ -82,11 +84,14 @@ def epoch_time end def setup_common_options(options) + validate_options(options) @autosubmit_interval = options[:autosubmit_interval] @client = options[:client] || Librato::Metrics.client @per_request = options[:per_request] || MEASUREMENTS_PER_REQUEST @source = options[:source] + @tags = options[:tags] @measure_time = options[:measure_time] && options[:measure_time].to_i + @time = options[:time] && options[:time].to_i @create_time = Time.now @clear_on_failure = options[:clear_failures] || false @prefix = options[:prefix] @@ -99,6 +104,20 @@ def autosubmit_check end end + def validate_options(options) + raise ArgumentError, ":options must be a Hash" unless options.is_a?(Hash) + check_compatibility(options, [:source, :tags]) + check_compatibility(options, [:measure_time, :time]) + check_compatibility(options, [:source, :time]) + check_compatibility(options, [:measure_time, :tags]) + end + + def check_compatibility(options, incompatible_options) + if incompatible_options.to_set.subset?(options.keys.to_set) + raise ArgumentError, "#{incompatible_options} cannot be simultaneously set" + end + end + end end diff --git a/spec/unit/metrics/aggregator_spec.rb b/spec/unit/metrics/aggregator_spec.rb index 8b82d6d..ffd144a 100644 --- a/spec/unit/metrics/aggregator_spec.rb +++ b/spec/unit/metrics/aggregator_spec.rb @@ -38,6 +38,33 @@ module Metrics expect(a.source).to be_nil end end + + context "with valid arguments" do + it "initializes Aggregator" do + expect { Aggregator.new }.not_to raise_error + expect { Aggregator.new(source: "metrics-web-stg-1") }.not_to raise_error + expect { Aggregator.new(tags: { hostname: "metrics-web-stg-1" }) }.not_to raise_error + end + end + + context "with invalid arguments" do + it "raises exception" do + expect { + Aggregator.new( + source: "metrics-web-stg-1", + tags: { hostname: "metrics-web-stg-1" } + ) + }.to raise_error(ArgumentError) + expect { Aggregator.new(measure_time: Time.now, time: Time.now) }.to raise_error(ArgumentError) + expect { Aggregator.new(source: "metrics-web-stg-1", time: Time.now) }.to raise_error(ArgumentError) + expect { + Aggregator.new( + measure_time: Time.now, + tags: { hostname: "metrics-web-stg-1" } + ) + }.to raise_error(ArgumentError) + end + end end describe "#add" do diff --git a/spec/unit/metrics/queue_spec.rb b/spec/unit/metrics/queue_spec.rb index 6a50c52..0771446 100644 --- a/spec/unit/metrics/queue_spec.rb +++ b/spec/unit/metrics/queue_spec.rb @@ -25,6 +25,33 @@ module Metrics expect(queue.client).to eq(Librato::Metrics.client) end end + + context "with valid arguments" do + it "initializes Queue" do + expect { Queue.new }.not_to raise_error + expect { Queue.new(source: "metrics-web-stg-1") }.not_to raise_error + expect { Queue.new(tags: { hostname: "metrics-web-stg-1" }) }.not_to raise_error + end + end + + context "with invalid arguments" do + it "raises exception" do + expect { + Queue.new( + source: "metrics-web-stg-1", + tags: { hostname: "metrics-web-stg-1" } + ) + }.to raise_error(ArgumentError) + expect { Queue.new(measure_time: Time.now, time: Time.now) }.to raise_error(ArgumentError) + expect { Queue.new(source: "metrics-web-stg-1", time: Time.now) }.to raise_error(ArgumentError) + expect { + Queue.new( + measure_time: Time.now, + tags: { hostname: "metrics-web-stg-1" } + ) + }.to raise_error(ArgumentError) + end + end end describe "#add" do From 5700eac3c1a0fe52bea2ab2e44eaf16a7a366c70 Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Wed, 24 Aug 2016 17:42:43 -0700 Subject: [PATCH 09/55] validate options for #add --- lib/librato/metrics/processor.rb | 1 + lib/librato/metrics/queue.rb | 23 +++++++++++++++++------ spec/unit/metrics/queue_spec.rb | 18 ++++++++++++++++++ 3 files changed, 36 insertions(+), 6 deletions(-) diff --git a/lib/librato/metrics/processor.rb b/lib/librato/metrics/processor.rb index 43004a2..3085d9f 100644 --- a/lib/librato/metrics/processor.rb +++ b/lib/librato/metrics/processor.rb @@ -95,6 +95,7 @@ def setup_common_options(options) @create_time = Time.now @clear_on_failure = options[:clear_failures] || false @prefix = options[:prefix] + @multidimensional = @client.has_tags? || @tags || @time end def autosubmit_check diff --git a/lib/librato/metrics/queue.rb b/lib/librato/metrics/queue.rb index ad46319..c0af073 100644 --- a/lib/librato/metrics/queue.rb +++ b/lib/librato/metrics/queue.rb @@ -29,6 +29,7 @@ def initialize(opts={}) def add(measurements) measurements.each do |key, value| if value.respond_to?(:each) + validate_options(value) metric = value metric[:name] = key.to_s type = metric.delete(:type) || metric.delete('type') || 'gauge' @@ -39,13 +40,17 @@ def add(measurements) if @prefix metric[:name] = "#{@prefix}.#{metric[:name]}" end + type = :measurement if @multidimensional type = ("#{type}s").to_sym - if metric[:measure_time] - metric[:measure_time] = metric[:measure_time].to_i + time_key = @multidimensional ? :time : :measure_time + + if metric[time_key] + metric[time_key] = metric[time_key].to_i check_measure_time(metric) elsif !skip_measurement_times - metric[:measure_time] = epoch_time + metric[time_key] = epoch_time end + @queued[type] ||= [] @queued[type] << metric end @@ -81,6 +86,10 @@ def gauges @queued[:gauges] || [] end + def measurements + @queued[:measurements] || [] + end + # Combines queueable measures from the given object # into this queue. # @@ -133,9 +142,11 @@ def size private def check_measure_time(data) - if data[:measure_time] < Metrics::MIN_MEASURE_TIME - raise InvalidMeasureTime, "Measure time for submitted metric (#{data}) is invalid." - end + invalid_time = + data[:measure_time] && data[:measure_time]< Metrics::MIN_MEASURE_TIME || + data[:time] && data[:time]< Metrics::MIN_MEASURE_TIME + + raise InvalidMeasureTime, "Measure time for submitted metric (#{data}) is invalid." if invalid_time end def reconcile_source(measurements, source) diff --git a/spec/unit/metrics/queue_spec.rb b/spec/unit/metrics/queue_spec.rb index 0771446..3dafdc8 100644 --- a/spec/unit/metrics/queue_spec.rb +++ b/spec/unit/metrics/queue_spec.rb @@ -14,6 +14,7 @@ module Metrics context "with specified client" do it "sets to client" do barney = Client + allow(barney).to receive(:has_tags?).and_return(false) queue = Queue.new(client: barney) expect(queue.client).to eq(barney) end @@ -59,6 +60,23 @@ module Metrics expect(subject.add(foo: 123)).to eq(subject) end + context "with invalid arguments" do + it "raises exception" do + expect { + subject.add foo: { source: "metrics-web-stg-1", tags: { hostname: "metrics-web-stg-1" }, value: 123 } + }.to raise_error(ArgumentError) + expect { + subject.add foo: { measure_time: Time.now, time: Time.now, value: 123 } + }.to raise_error(ArgumentError) + expect { + subject.add foo: { source: "metrics-web-stg-1", time: Time.now, value: 123 } + }.to raise_error(ArgumentError) + expect { + subject.add foo: { tags: { hostname: "metrics-web-stg-1" }, measure_time: Time.now, value: 123 } + }.to raise_error(ArgumentError) + end + end + context "with single hash argument" do it "records a key-value gauge" do expected = {gauges: [{name: 'foo', value: 3000, measure_time: @time}]} From b5b3ceb3ff5e8911a68cb4a08337d85b42a89853 Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Wed, 24 Aug 2016 17:54:41 -0700 Subject: [PATCH 10/55] add tags and time to globals --- lib/librato/metrics/queue.rb | 2 ++ spec/unit/metrics/queue_spec.rb | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/lib/librato/metrics/queue.rb b/lib/librato/metrics/queue.rb index c0af073..1d54d70 100644 --- a/lib/librato/metrics/queue.rb +++ b/lib/librato/metrics/queue.rb @@ -127,7 +127,9 @@ def queued return {} if @queued.empty? globals = {} globals[:source] = @source if @source + globals[:tags] = @tags if @tags globals[:measure_time] = @measure_time if @measure_time + globals[:time] = @time if @time @queued.merge(globals) end diff --git a/spec/unit/metrics/queue_spec.rb b/spec/unit/metrics/queue_spec.rb index 3dafdc8..2376150 100644 --- a/spec/unit/metrics/queue_spec.rb +++ b/spec/unit/metrics/queue_spec.rb @@ -350,6 +350,25 @@ module Metrics q.add foo: 12 expect(q.queued[:measure_time]).to eq(measure_time) end + + context "when tags are set" do + it "includes global tags" do + expected_tags = { region: "us-east-1" } + queue = Queue.new(tags: expected_tags) + queue.add test: 5 + expect(queue.queued[:tags]).to eq(expected_tags) + end + end + + context "when time is set" do + it "includes global time" do + expected_time = (Time.now-1000).to_i + queue = Queue.new(time: expected_time) + queue.add test: 10 + expect(queue.queued[:time]).to eq(expected_time) + end + end + end describe "#size" do From dd41cb7a181c10f19f2b0f857c3394095bd0e808 Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Wed, 24 Aug 2016 22:15:17 -0700 Subject: [PATCH 11/55] reconcile tags and/or source when merging queues --- lib/librato/metrics/queue.rb | 20 ++++++--- spec/unit/metrics/queue_spec.rb | 77 +++++++++++++++++++++++---------- 2 files changed, 68 insertions(+), 29 deletions(-) diff --git a/lib/librato/metrics/queue.rb b/lib/librato/metrics/queue.rb index 1d54d70..d69bcbf 100644 --- a/lib/librato/metrics/queue.rb +++ b/lib/librato/metrics/queue.rb @@ -40,7 +40,7 @@ def add(measurements) if @prefix metric[:name] = "#{@prefix}.#{metric[:name]}" end - type = :measurement if @multidimensional + type = :measurement if @multidimensional || metric[:tags] type = ("#{type}s").to_sym time_key = @multidimensional ? :time : :measure_time @@ -108,7 +108,12 @@ def merge!(mergeable) end Metrics::PLURAL_TYPES.each do |type| if to_merge[type] - measurements = reconcile_source(to_merge[type], to_merge[:source]) + measurements = + if @multidimensional + reconcile(to_merge[type], to_merge[:tags]) + else + reconcile(to_merge[type], to_merge[:source]) + end if @queued[type] @queued[type] += measurements else @@ -146,16 +151,17 @@ def size def check_measure_time(data) invalid_time = data[:measure_time] && data[:measure_time]< Metrics::MIN_MEASURE_TIME || - data[:time] && data[:time]< Metrics::MIN_MEASURE_TIME + data[:time] && data[:time]< Metrics::MIN_MEASURE_TIME raise InvalidMeasureTime, "Measure time for submitted metric (#{data}) is invalid." if invalid_time end - def reconcile_source(measurements, source) - return measurements if !source || source == @source + def reconcile(measurements, val) + arr = val.is_a?(Hash) ? [@tags, :tags] : [@source, :source] + return measurements if !val || val == arr.first measurements.map! do |measurement| - unless measurement[:source] - measurement[:source] = source + unless measurement[arr.last] + measurement[arr.last] = val end measurement end diff --git a/spec/unit/metrics/queue_spec.rb b/spec/unit/metrics/queue_spec.rb index 2376150..0a0f098 100644 --- a/spec/unit/metrics/queue_spec.rb +++ b/spec/unit/metrics/queue_spec.rb @@ -264,31 +264,64 @@ module Metrics expect(q2.queued).to equal_unordered(expected) end - it "maintains specified sources" do - q1 = Queue.new - q1.add neo: {source: 'matrix', value: 123} - q2 = Queue.new(source: 'red_pill') - q2.merge!(q1) - expect(q2.queued[:gauges][0][:source]).to eq('matrix') - end + context "when multidimensional is true" do + it "maintains specified tags" do + q1 = Queue.new + q1.add test: { tags: { db: "rr1" }, value: 123 } + q2 = Queue.new(tags: { db: "rr2" }) + q2.merge!(q1) + + expect(q2.queued[:measurements].first[:tags][:db]).to eq("rr1") + end - it "does not change default source" do - q1 = Queue.new(source: 'matrix') - q1.add neo: 456 - q2 = Queue.new(source: 'red_pill') - q2.merge!(q1) - expect(q2.queued[:source]).to eq('red_pill') + it "does not change default tags" do + q1 = Queue.new(tags: { db: "rr1" }) + q1.add test: 456 + q2 = Queue.new(tags: { db: "rr2" }) + q2.merge!(q1) + + expect(q2.queued[:tags][:db]).to eq("rr2") + end + + it "tracks previous default tags" do + q1 = Queue.new(tags: { db: "rr1" }) + q1.add test_1: 123 + q2 = Queue.new(tags: { db: "rr2" }) + q2.add test_2: 456 + q2.merge!(q1) + metric = q2.measurements.find { |measurement| measurement[:name] == "test_1" } + + expect(metric[:tags][:db]).to eq("rr1") + end end - it "tracks previous default source" do - q1 = Queue.new(source: 'matrix') - q1.add neo: 456 - q2 = Queue.new(source: 'red_pill') - q2.add morpheus: 678 - q2.merge!(q1) - q2.queued[:gauges].each do |gauge| - if gauge[:name] == 'neo' - expect(gauge[:source]).to eq('matrix') + context "when multidimensional is false" do + it "maintains specified sources" do + q1 = Queue.new + q1.add neo: {source: 'matrix', value: 123} + q2 = Queue.new(source: 'red_pill') + q2.merge!(q1) + expect(q2.queued[:gauges][0][:source]).to eq('matrix') + end + + it "does not change default source" do + q1 = Queue.new(source: 'matrix') + q1.add neo: 456 + q2 = Queue.new(source: 'red_pill') + q2.merge!(q1) + expect(q2.queued[:source]).to eq('red_pill') + end + + it "tracks previous default source" do + q1 = Queue.new(source: 'matrix') + q1.add neo: 456 + q2 = Queue.new(source: 'red_pill') + q2.add morpheus: 678 + q2.merge!(q1) + q2.queued[:gauges].each do |gauge| + if gauge[:name] == 'neo' + expect(gauge[:source]).to eq('matrix') + end end end end From e0c6713d503a3060e136e830ec895ad6f715e944 Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Thu, 25 Aug 2016 09:10:00 -0700 Subject: [PATCH 12/55] style changes --- lib/librato/metrics/queue.rb | 4 +- spec/unit/metrics/queue_spec.rb | 66 ++++++++++++++++----------------- 2 files changed, 34 insertions(+), 36 deletions(-) diff --git a/lib/librato/metrics/queue.rb b/lib/librato/metrics/queue.rb index d69bcbf..3ece01a 100644 --- a/lib/librato/metrics/queue.rb +++ b/lib/librato/metrics/queue.rb @@ -150,8 +150,8 @@ def size def check_measure_time(data) invalid_time = - data[:measure_time] && data[:measure_time]< Metrics::MIN_MEASURE_TIME || - data[:time] && data[:time]< Metrics::MIN_MEASURE_TIME + data[:measure_time] && data[:measure_time] < Metrics::MIN_MEASURE_TIME || + data[:time] && data[:time] < Metrics::MIN_MEASURE_TIME raise InvalidMeasureTime, "Measure time for submitted metric (#{data}) is invalid." if invalid_time end diff --git a/spec/unit/metrics/queue_spec.rb b/spec/unit/metrics/queue_spec.rb index 0a0f098..94219bc 100644 --- a/spec/unit/metrics/queue_spec.rb +++ b/spec/unit/metrics/queue_spec.rb @@ -295,46 +295,44 @@ module Metrics end end - context "when multidimensional is false" do - it "maintains specified sources" do - q1 = Queue.new - q1.add neo: {source: 'matrix', value: 123} - q2 = Queue.new(source: 'red_pill') - q2.merge!(q1) - expect(q2.queued[:gauges][0][:source]).to eq('matrix') - end + it "maintains specified sources" do + q1 = Queue.new + q1.add neo: {source: 'matrix', value: 123} + q2 = Queue.new(source: 'red_pill') + q2.merge!(q1) + expect(q2.queued[:gauges][0][:source]).to eq('matrix') + end - it "does not change default source" do - q1 = Queue.new(source: 'matrix') - q1.add neo: 456 - q2 = Queue.new(source: 'red_pill') - q2.merge!(q1) - expect(q2.queued[:source]).to eq('red_pill') - end + it "does not change default source" do + q1 = Queue.new(source: 'matrix') + q1.add neo: 456 + q2 = Queue.new(source: 'red_pill') + q2.merge!(q1) + expect(q2.queued[:source]).to eq('red_pill') + end - it "tracks previous default source" do - q1 = Queue.new(source: 'matrix') - q1.add neo: 456 - q2 = Queue.new(source: 'red_pill') - q2.add morpheus: 678 - q2.merge!(q1) - q2.queued[:gauges].each do |gauge| - if gauge[:name] == 'neo' - expect(gauge[:source]).to eq('matrix') - end + it "tracks previous default source" do + q1 = Queue.new(source: 'matrix') + q1.add neo: 456 + q2 = Queue.new(source: 'red_pill') + q2.add morpheus: 678 + q2.merge!(q1) + q2.queued[:gauges].each do |gauge| + if gauge[:name] == 'neo' + expect(gauge[:source]).to eq('matrix') end end end + end - it "handles empty cases" do - q1 = Queue.new - q1.add foo: 123, users: {type: :counter, value: 1000} - q2 = Queue.new - q2.merge!(q1) - expected = {counters: [{name:"users", value:1000, measure_time: @time}], - gauges: [{name:"foo", value:123, measure_time: @time}]} - expect(q2.queued).to eq(expected) - end + it "handles empty cases" do + q1 = Queue.new + q1.add foo: 123, users: {type: :counter, value: 1000} + q2 = Queue.new + q2.merge!(q1) + expected = {counters: [{name:"users", value:1000, measure_time: @time}], + gauges: [{name:"foo", value:123, measure_time: @time}]} + expect(q2.queued).to eq(expected) end context "with an aggregator" do From 55865f6bac4dff8504c4e89e54d18dcec245b7cd Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Thu, 25 Aug 2016 10:08:08 -0700 Subject: [PATCH 13/55] add multidimensional Aggregator --- lib/librato/metrics/aggregator.rb | 63 ++++++++++++++++++---------- lib/librato/metrics/processor.rb | 1 + spec/unit/metrics/aggregator_spec.rb | 54 ++++++++++++++++++++++++ spec/unit/metrics/queue_spec.rb | 2 +- 4 files changed, 97 insertions(+), 23 deletions(-) diff --git a/lib/librato/metrics/aggregator.rb b/lib/librato/metrics/aggregator.rb index 843eb89..8838c32 100644 --- a/lib/librato/metrics/aggregator.rb +++ b/lib/librato/metrics/aggregator.rb @@ -23,11 +23,11 @@ module Metrics # queue.merge!(aggregator) # class Aggregator - SOURCE_SEPARATOR = '%%' # must not be in valid source name criteria + SEPARATOR = '%%' # must not be in valid tags and/or source criteria include Processor - attr_reader :source + attr_reader :source, :tags # @option opts [Integer] :autosubmit_interval If set the aggregator will auto-submit if the given number of seconds has passed when a new metric is added. # @option opts [Boolean] :clear_failures Should the aggregator remove all stored data if it runs into problems with a request? (default: false) @@ -52,20 +52,33 @@ def initialize(opts={}) # @return [Aggregator] returns self def add(measurements) measurements.each do |metric, data| + entry = {} if @prefix metric = "#{@prefix}.#{metric}" end + entry[:name] = metric if data.respond_to?(:each) # hash form + validate_options(data) value = data[:value] if data[:source] - metric = "#{metric}#{SOURCE_SEPARATOR}#{data[:source]}" + metric = "#{metric}#{SEPARATOR}#{data[:source]}" + entry[:source] = data[:source] + end + if data[:tags] && data[:tags].respond_to?(:each) + metric = metric.to_s + data[:tags].each do |key, value| + metric = "#{metric}#{SEPARATOR}#{key}#{value}" + end + entry[:tags] = data[:tags] end else value = data end - @aggregated[metric] ||= Aggregate.new - @aggregated[metric] << value + @aggregated[metric] = {} unless @aggregated[metric] + @aggregated[metric][:aggregate] ||= Aggregate.new + @aggregated[metric][:aggregate] << value + @aggregated[metric].merge!(entry) end autosubmit_check self @@ -88,31 +101,37 @@ def clear # Returns currently queued data # def queued - gauges = [] + entries = [] + md_payload = false - @aggregated.each do |metric, data| - source = nil - metric = metric.to_s - if metric.include?(SOURCE_SEPARATOR) - metric, source = metric.split(SOURCE_SEPARATOR) - end + @aggregated.each_value do |data| entry = { - name: metric, - count: data.count, - sum: data.sum, - + name: data[:name].to_s, + count: data[:aggregate].count, + sum: data[:aggregate].sum, # TODO: make float/non-float consistent in the gem - min: data.min.to_f, - max: data.max.to_f + min: data[:aggregate].min.to_f, + max: data[:aggregate].max.to_f # TODO: expose v.sum2 and include } - entry[:source] = source if source - gauges << entry + entry[:source] = data[:source].to_s if data[:source] + if data[:tags] + md_payload = true + entry[:tags] = data[:tags] + end + md_payload = true if data[:time] + entries << entry end - - req = { gauges: gauges } + req = + if @multidimensional || md_payload + { measurements: entries } + else + { gauges: entries } + end req[:source] = @source if @source + req[:tags] = @tags if @tags req[:measure_time] = @measure_time if @measure_time + req[:time] = @time if @time req end diff --git a/lib/librato/metrics/processor.rb b/lib/librato/metrics/processor.rb index 3085d9f..c6b8688 100644 --- a/lib/librato/metrics/processor.rb +++ b/lib/librato/metrics/processor.rb @@ -111,6 +111,7 @@ def validate_options(options) check_compatibility(options, [:measure_time, :time]) check_compatibility(options, [:source, :time]) check_compatibility(options, [:measure_time, :tags]) + check_compatibility(options, [:type]) if options[:tags] || options[:time] # multidimensional is typeless end def check_compatibility(options, incompatible_options) diff --git a/spec/unit/metrics/aggregator_spec.rb b/spec/unit/metrics/aggregator_spec.rb index ffd144a..9809a84 100644 --- a/spec/unit/metrics/aggregator_spec.rb +++ b/spec/unit/metrics/aggregator_spec.rb @@ -72,6 +72,23 @@ module Metrics expect(subject.add(foo: 1234)).to eq(subject) end + context "with invalid arguments" do + it "raises exception" do + expect { + subject.add foo: { source: "metrics-web-stg-1", tags: { hostname: "metrics-web-stg-1" }, value: 123 } + }.to raise_error(ArgumentError) + expect { + subject.add foo: { measure_time: Time.now, time: Time.now, value: 123 } + }.to raise_error(ArgumentError) + expect { + subject.add foo: { source: "metrics-web-stg-1", time: Time.now, value: 123 } + }.to raise_error(ArgumentError) + expect { + subject.add foo: { tags: { hostname: "metrics-web-stg-1" }, measure_time: Time.now, value: 123 } + }.to raise_error(ArgumentError) + end + end + context "with single hash argument" do it "records a single aggregate" do subject.add foo: 3000 @@ -118,6 +135,23 @@ module Metrics expect(subject.queued).to equal_unordered(expected) end + context "when multidimensional is true" do + it "maintains specified tags" do + subject.add test: { tags: { db: "rr1" }, value: 1 } + subject.add test: 5 + subject.add test: { tags: { db: "rr1" }, value: 6 } + subject.add test: 10 + expected = { + measurements: [ + { name: "test", tags: { db: "rr1" }, count: 2, sum: 7.0, min: 1.0, max: 6.0 }, + { name: "test", count: 2, sum: 15.0, min: 5.0, max: 10.0 } + ] + } + + expect(subject.queued).to equal_unordered(expected) + end + end + context "with a prefix set" do it "auto-prepends names" do subject = Aggregator.new(prefix: 'foo') @@ -202,6 +236,26 @@ module Metrics a.add foo: 12 expect(a.queued[:measure_time]).to eq(measure_time) end + + context "when tags are set" do + it "includes global tags" do + expected_tags = { region: "us-east-1" } + subject = Aggregator.new(tags: expected_tags) + subject.add test: 5 + + expect(subject.queued[:tags]).to eq(expected_tags) + end + end + + context "when time is set" do + it "includes global time" do + expected_time = (Time.now-1000).to_i + subject = Aggregator.new(time: expected_time) + subject.add test: 10 + + expect(subject.queued[:time]).to eq(expected_time) + end + end end describe "#submit" do diff --git a/spec/unit/metrics/queue_spec.rb b/spec/unit/metrics/queue_spec.rb index 94219bc..a439fd4 100644 --- a/spec/unit/metrics/queue_spec.rb +++ b/spec/unit/metrics/queue_spec.rb @@ -265,7 +265,7 @@ module Metrics end context "when multidimensional is true" do - it "maintains specified tags" do + it "maintains specified tags" do q1 = Queue.new q1.add test: { tags: { db: "rr1" }, value: 123 } q2 = Queue.new(tags: { db: "rr2" }) From 460dc81c8303ea116d7819e55223cedb952a45aa Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Thu, 25 Aug 2016 10:14:32 -0700 Subject: [PATCH 14/55] add type test for MD --- spec/unit/metrics/aggregator_spec.rb | 11 +++++++---- spec/unit/metrics/queue_spec.rb | 11 +++++++---- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/spec/unit/metrics/aggregator_spec.rb b/spec/unit/metrics/aggregator_spec.rb index 9809a84..f2e7070 100644 --- a/spec/unit/metrics/aggregator_spec.rb +++ b/spec/unit/metrics/aggregator_spec.rb @@ -75,16 +75,19 @@ module Metrics context "with invalid arguments" do it "raises exception" do expect { - subject.add foo: { source: "metrics-web-stg-1", tags: { hostname: "metrics-web-stg-1" }, value: 123 } + subject.add test: { source: "metrics-web-stg-1", tags: { hostname: "metrics-web-stg-1" }, value: 123 } }.to raise_error(ArgumentError) expect { - subject.add foo: { measure_time: Time.now, time: Time.now, value: 123 } + subject.add test: { measure_time: Time.now, time: Time.now, value: 123 } }.to raise_error(ArgumentError) expect { - subject.add foo: { source: "metrics-web-stg-1", time: Time.now, value: 123 } + subject.add test: { source: "metrics-web-stg-1", time: Time.now, value: 123 } }.to raise_error(ArgumentError) expect { - subject.add foo: { tags: { hostname: "metrics-web-stg-1" }, measure_time: Time.now, value: 123 } + subject.add test: { tags: { hostname: "metrics-web-stg-1" }, measure_time: Time.now, value: 123 } + }.to raise_error(ArgumentError) + expect { + subject.add test: { type: "gauge", tags: { hostname: "metrics-web-stg-1" }, value: 123 } }.to raise_error(ArgumentError) end end diff --git a/spec/unit/metrics/queue_spec.rb b/spec/unit/metrics/queue_spec.rb index a439fd4..ef2402f 100644 --- a/spec/unit/metrics/queue_spec.rb +++ b/spec/unit/metrics/queue_spec.rb @@ -63,16 +63,19 @@ module Metrics context "with invalid arguments" do it "raises exception" do expect { - subject.add foo: { source: "metrics-web-stg-1", tags: { hostname: "metrics-web-stg-1" }, value: 123 } + subject.add test: { source: "metrics-web-stg-1", tags: { hostname: "metrics-web-stg-1" }, value: 123 } }.to raise_error(ArgumentError) expect { - subject.add foo: { measure_time: Time.now, time: Time.now, value: 123 } + subject.add test: { measure_time: Time.now, time: Time.now, value: 123 } }.to raise_error(ArgumentError) expect { - subject.add foo: { source: "metrics-web-stg-1", time: Time.now, value: 123 } + subject.add test: { source: "metrics-web-stg-1", time: Time.now, value: 123 } }.to raise_error(ArgumentError) expect { - subject.add foo: { tags: { hostname: "metrics-web-stg-1" }, measure_time: Time.now, value: 123 } + subject.add test: { tags: { hostname: "metrics-web-stg-1" }, measure_time: Time.now, value: 123 } + }.to raise_error(ArgumentError) + expect { + subject.add test: { type: "counter", tags: { hostname: "metrics-web-stg-1" }, value: 123 } }.to raise_error(ArgumentError) end end From d2e8c7578f1fdbc9054b821ef02153946c7f5325 Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Thu, 25 Aug 2016 10:48:23 -0700 Subject: [PATCH 15/55] add #get_measurement and #get_series --- lib/librato/metrics.rb | 2 +- lib/librato/metrics/client.rb | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/lib/librato/metrics.rb b/lib/librato/metrics.rb index 24cfac7..687e360 100644 --- a/lib/librato/metrics.rb +++ b/lib/librato/metrics.rb @@ -81,7 +81,7 @@ module Metrics :delete_metrics, :update_metric, :update_metrics, :submit, :sources, :get_source, :update_source, - :create_snapshot, :get_snapshot + :create_snapshot, :get_snapshot, :get_measurement, :get_series # The Librato::Metrics::Client being used by module-level # access. diff --git a/lib/librato/metrics/client.rb b/lib/librato/metrics/client.rb index bd91948..2051d9c 100644 --- a/lib/librato/metrics/client.rb +++ b/lib/librato/metrics/client.rb @@ -204,6 +204,28 @@ def get_metric(name, options = {}) parsed end + def get_measurement(name, options={}) + query = options.dup + if query[:start_time].respond_to?(:year) + query[:start_time] = query[:start_time].to_i + end + if query[:end_time].respond_to?(:year) + query[:end_time] = query[:end_time].to_i + end + query[:resolution] ||= 1 + unless query[:start_time] || query[:end_time] + query[:duration] ||= 3600 + end + url = connection.build_url("measurements/#{name}", query) + response = connection.get(url) + SmartJSON.read(response.body) + end + + def get_series(name, options={}) + raise ArgumentError, ":resolution and :duration or :start_time must be set" if options.empty? + get_measurement(name, options)["series"] + end + # Retrieve data points for a specific metric # # @example Get 20 most recent data points for metric From 48da1ec671463f5ef3615b82b35dc22b469b349f Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Thu, 25 Aug 2016 11:44:30 -0700 Subject: [PATCH 16/55] update Processor --- lib/librato/metrics/processor.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/librato/metrics/processor.rb b/lib/librato/metrics/processor.rb index c6b8688..9a4a933 100644 --- a/lib/librato/metrics/processor.rb +++ b/lib/librato/metrics/processor.rb @@ -95,7 +95,7 @@ def setup_common_options(options) @create_time = Time.now @clear_on_failure = options[:clear_failures] || false @prefix = options[:prefix] - @multidimensional = @client.has_tags? || @tags || @time + @multidimensional = @client.has_tags? || (@tags && !@tags.empty?) || !@time.nil? end def autosubmit_check From 8e4526a95d8465391115c1c62b9ff0fba84763ef Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Thu, 25 Aug 2016 13:28:10 -0700 Subject: [PATCH 17/55] use :time if :tags present --- lib/librato/metrics/queue.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/librato/metrics/queue.rb b/lib/librato/metrics/queue.rb index 3ece01a..5fd4fdf 100644 --- a/lib/librato/metrics/queue.rb +++ b/lib/librato/metrics/queue.rb @@ -42,7 +42,7 @@ def add(measurements) end type = :measurement if @multidimensional || metric[:tags] type = ("#{type}s").to_sym - time_key = @multidimensional ? :time : :measure_time + time_key = @multidimensional || metric[:tags] ? :time : :measure_time if metric[time_key] metric[time_key] = metric[time_key].to_i From 32367abafd17e009d0b018bc00f0e99ffda2f3b9 Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Thu, 25 Aug 2016 14:23:05 -0700 Subject: [PATCH 18/55] use /v1/measurements when individual measurement contains tags --- lib/librato/metrics/client.rb | 10 +++++----- lib/librato/metrics/persistence/direct.rb | 12 +++++++++--- lib/librato/metrics/queue.rb | 1 + 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/lib/librato/metrics/client.rb b/lib/librato/metrics/client.rb index 2051d9c..4ce2ce5 100644 --- a/lib/librato/metrics/client.rb +++ b/lib/librato/metrics/client.rb @@ -29,11 +29,6 @@ def add_tags(tags) self.tags.merge!(tags) end - def has_tags? - !self.tags.empty? - end - alias :tags? :has_tags? - # Provide agent identifier for the developer program. See: # http://support.metrics.librato.com/knowledgebase/articles/53548-developer-program # @@ -265,6 +260,11 @@ def flush_authentication @connection = nil end + def has_tags? + !self.tags.empty? + end + alias :tags? :has_tags? + # List currently existing metrics # # @example List all metrics diff --git a/lib/librato/metrics/persistence/direct.rb b/lib/librato/metrics/persistence/direct.rb index 20bdfa4..25f8277 100644 --- a/lib/librato/metrics/persistence/direct.rb +++ b/lib/librato/metrics/persistence/direct.rb @@ -16,9 +16,14 @@ def persist(client, queued, options={}) requests = [queued] end requests.each do |request| - if client.has_tags? + if client.has_tags? || queued[:tags] || queued[:multidimensional] resource = "measurements" - payload = SmartJSON.write(request.merge({ tags: client.tags })) + tags_payload = client.tags ? client.tags : queued[:tags] + if client.tags && queued[:tags] + tags_payload = client.tags.merge(queued[:tags]) + end + queued.delete(:multidimensional) if queued[:multidimensional] + payload = SmartJSON.write(request.merge({ tags: tags_payload })) else resource = "metrics" payload = SmartJSON.write(request) @@ -60,7 +65,8 @@ def fetch_globals(queued) end def queue_count(queued) - queued.inject(0) { |result, data| result + data.last.size } + queued.reject { |key| key == :multidimensional } + .inject(0) { |result, data| result + data.last.size } end end diff --git a/lib/librato/metrics/queue.rb b/lib/librato/metrics/queue.rb index 5fd4fdf..7ae921c 100644 --- a/lib/librato/metrics/queue.rb +++ b/lib/librato/metrics/queue.rb @@ -53,6 +53,7 @@ def add(measurements) @queued[type] ||= [] @queued[type] << metric + @queued[:multidimensional] = true if @multidimensional || metric[:tags] end submit_check self From 6dcd4cea5bb00adcc7ce0ad6266a600fffa4a386 Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Thu, 25 Aug 2016 15:02:43 -0700 Subject: [PATCH 19/55] add multidimensional bool to Aggregator when measurement contains tags --- lib/librato/metrics/aggregator.rb | 1 + spec/unit/metrics/aggregator_spec.rb | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/librato/metrics/aggregator.rb b/lib/librato/metrics/aggregator.rb index 8838c32..962dbf7 100644 --- a/lib/librato/metrics/aggregator.rb +++ b/lib/librato/metrics/aggregator.rb @@ -132,6 +132,7 @@ def queued req[:tags] = @tags if @tags req[:measure_time] = @measure_time if @measure_time req[:time] = @time if @time + req[:multidimensional] = true if @multidimensional || md_payload req end diff --git a/spec/unit/metrics/aggregator_spec.rb b/spec/unit/metrics/aggregator_spec.rb index f2e7070..0c9b8a0 100644 --- a/spec/unit/metrics/aggregator_spec.rb +++ b/spec/unit/metrics/aggregator_spec.rb @@ -148,7 +148,8 @@ module Metrics measurements: [ { name: "test", tags: { db: "rr1" }, count: 2, sum: 7.0, min: 1.0, max: 6.0 }, { name: "test", count: 2, sum: 15.0, min: 5.0, max: 10.0 } - ] + ], + multidimensional: true } expect(subject.queued).to equal_unordered(expected) From ce2a568c0e70106e2aebfe83046cf18a171b5d34 Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Thu, 25 Aug 2016 15:14:27 -0700 Subject: [PATCH 20/55] add delegators to Metrics --- lib/librato/metrics.rb | 1 + spec/unit/metrics/queue_spec.rb | 18 +++++++++--------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/lib/librato/metrics.rb b/lib/librato/metrics.rb index 687e360..350738a 100644 --- a/lib/librato/metrics.rb +++ b/lib/librato/metrics.rb @@ -73,6 +73,7 @@ module Metrics # Client. # def_delegators :client, :agent_identifier, :annotate, :api_endpoint, + :add_tags, :clear_tags, :has_tags?, :api_endpoint=, :authenticate, :connection, :proxy, :proxy=, :faraday_adapter, :faraday_adapter=, diff --git a/spec/unit/metrics/queue_spec.rb b/spec/unit/metrics/queue_spec.rb index ef2402f..5ee50b9 100644 --- a/spec/unit/metrics/queue_spec.rb +++ b/spec/unit/metrics/queue_spec.rb @@ -328,15 +328,15 @@ module Metrics end end - it "handles empty cases" do - q1 = Queue.new - q1.add foo: 123, users: {type: :counter, value: 1000} - q2 = Queue.new - q2.merge!(q1) - expected = {counters: [{name:"users", value:1000, measure_time: @time}], - gauges: [{name:"foo", value:123, measure_time: @time}]} - expect(q2.queued).to eq(expected) - end + it "handles empty cases" do + q1 = Queue.new + q1.add foo: 123, users: {type: :counter, value: 1000} + q2 = Queue.new + q2.merge!(q1) + expected = {counters: [{name:"users", value:1000, measure_time: @time}], + gauges: [{name:"foo", value:123, measure_time: @time}]} + expect(q2.queued).to eq(expected) + end context "with an aggregator" do it "merges" do From 8297f0a851bb8192c1a5578407a6be81dc06ef49 Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Thu, 25 Aug 2016 16:02:59 -0700 Subject: [PATCH 21/55] add seperator for hash key between tags k,v --- lib/librato/metrics/aggregator.rb | 2 +- lib/librato/metrics/queue.rb | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/librato/metrics/aggregator.rb b/lib/librato/metrics/aggregator.rb index 962dbf7..d553822 100644 --- a/lib/librato/metrics/aggregator.rb +++ b/lib/librato/metrics/aggregator.rb @@ -67,7 +67,7 @@ def add(measurements) if data[:tags] && data[:tags].respond_to?(:each) metric = metric.to_s data[:tags].each do |key, value| - metric = "#{metric}#{SEPARATOR}#{key}#{value}" + metric = "#{metric}#{SEPARATOR}#{key}#{SEPARATOR}#{value}" end entry[:tags] = data[:tags] end diff --git a/lib/librato/metrics/queue.rb b/lib/librato/metrics/queue.rb index 7ae921c..b20078a 100644 --- a/lib/librato/metrics/queue.rb +++ b/lib/librato/metrics/queue.rb @@ -42,13 +42,13 @@ def add(measurements) end type = :measurement if @multidimensional || metric[:tags] type = ("#{type}s").to_sym - time_key = @multidimensional || metric[:tags] ? :time : :measure_time + time_hsh_key = @multidimensional || metric[:tags] ? :time : :measure_time - if metric[time_key] - metric[time_key] = metric[time_key].to_i + if metric[time_hsh_key] + metric[time_hsh_key] = metric[time_hsh_key].to_i check_measure_time(metric) elsif !skip_measurement_times - metric[time_key] = epoch_time + metric[time_hsh_key] = epoch_time end @queued[type] ||= [] From 547c9cbfaf0532a31b7004a27411032ebf3cc4c7 Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Thu, 25 Aug 2016 16:44:09 -0700 Subject: [PATCH 22/55] rename var --- lib/librato/metrics/aggregator.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/librato/metrics/aggregator.rb b/lib/librato/metrics/aggregator.rb index d553822..11d2bbf 100644 --- a/lib/librato/metrics/aggregator.rb +++ b/lib/librato/metrics/aggregator.rb @@ -102,7 +102,7 @@ def clear # def queued entries = [] - md_payload = false + contains_measurements = false @aggregated.each_value do |data| entry = { @@ -116,14 +116,14 @@ def queued } entry[:source] = data[:source].to_s if data[:source] if data[:tags] - md_payload = true + contains_measurements = true entry[:tags] = data[:tags] end - md_payload = true if data[:time] + contains_measurements = true if data[:time] entries << entry end req = - if @multidimensional || md_payload + if @multidimensional || contains_measurements { measurements: entries } else { gauges: entries } @@ -132,7 +132,7 @@ def queued req[:tags] = @tags if @tags req[:measure_time] = @measure_time if @measure_time req[:time] = @time if @time - req[:multidimensional] = true if @multidimensional || md_payload + req[:multidimensional] = true if @multidimensional || contains_measurements req end From 495bc3a3082ae55371171dc21d0c3ffb586609cc Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Thu, 25 Aug 2016 17:55:40 -0700 Subject: [PATCH 23/55] rename method --- lib/librato/metrics/aggregator.rb | 2 +- lib/librato/metrics/processor.rb | 9 ++++----- lib/librato/metrics/queue.rb | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/lib/librato/metrics/aggregator.rb b/lib/librato/metrics/aggregator.rb index 11d2bbf..e0c16c9 100644 --- a/lib/librato/metrics/aggregator.rb +++ b/lib/librato/metrics/aggregator.rb @@ -58,7 +58,7 @@ def add(measurements) end entry[:name] = metric if data.respond_to?(:each) # hash form - validate_options(data) + validate_multidimensionality(data) value = data[:value] if data[:source] metric = "#{metric}#{SEPARATOR}#{data[:source]}" diff --git a/lib/librato/metrics/processor.rb b/lib/librato/metrics/processor.rb index 9a4a933..064e579 100644 --- a/lib/librato/metrics/processor.rb +++ b/lib/librato/metrics/processor.rb @@ -84,7 +84,7 @@ def epoch_time end def setup_common_options(options) - validate_options(options) + validate_multidimensionality(options) @autosubmit_interval = options[:autosubmit_interval] @client = options[:client] || Librato::Metrics.client @per_request = options[:per_request] || MEASUREMENTS_PER_REQUEST @@ -105,18 +105,17 @@ def autosubmit_check end end - def validate_options(options) - raise ArgumentError, ":options must be a Hash" unless options.is_a?(Hash) + def validate_multidimensionality(options) check_compatibility(options, [:source, :tags]) check_compatibility(options, [:measure_time, :time]) check_compatibility(options, [:source, :time]) check_compatibility(options, [:measure_time, :tags]) - check_compatibility(options, [:type]) if options[:tags] || options[:time] # multidimensional is typeless + check_compatibility(options, [:type]) if options[:tags] || options[:time] end def check_compatibility(options, incompatible_options) if incompatible_options.to_set.subset?(options.keys.to_set) - raise ArgumentError, "#{incompatible_options} cannot be simultaneously set" + raise ArgumentError, "#{incompatible_options} cannot be set" end end diff --git a/lib/librato/metrics/queue.rb b/lib/librato/metrics/queue.rb index b20078a..00aabb6 100644 --- a/lib/librato/metrics/queue.rb +++ b/lib/librato/metrics/queue.rb @@ -29,7 +29,7 @@ def initialize(opts={}) def add(measurements) measurements.each do |key, value| if value.respond_to?(:each) - validate_options(value) + validate_multidimensionality(value) metric = value metric[:name] = key.to_s type = metric.delete(:type) || metric.delete('type') || 'gauge' From 8d421f05705a6c9791d01600a3f2ecd9420cbc39 Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Thu, 25 Aug 2016 17:58:55 -0700 Subject: [PATCH 24/55] remove exceptions --- lib/librato/metrics/client.rb | 9 +-------- spec/unit/metrics/client_spec.rb | 18 ------------------ 2 files changed, 1 insertion(+), 26 deletions(-) diff --git a/lib/librato/metrics/client.rb b/lib/librato/metrics/client.rb index 4ce2ce5..fab95e8 100644 --- a/lib/librato/metrics/client.rb +++ b/lib/librato/metrics/client.rb @@ -8,10 +8,9 @@ class Client def_delegator :@tags, :clear, :clear_tags - attr_accessor :email, :api_key, :proxy + attr_accessor :email, :api_key, :proxy, :tags def initialize(options={}) - raise ArgumentError, ":tags must be a Hash" if options[:tags] && !options[:tags].is_a?(Hash) self.tags = options.fetch(:tags, {}) end @@ -19,13 +18,7 @@ def tags @tags ||= {} end - def tags=(tags) - raise ArgumentError, "tags must be a Hash" unless tags.is_a?(Hash) - @tags = tags - end - def add_tags(tags) - raise ArgumentError, "tags must be a Hash" unless tags.is_a?(Hash) self.tags.merge!(tags) end diff --git a/spec/unit/metrics/client_spec.rb b/spec/unit/metrics/client_spec.rb index 9ad5e12..abfe428 100644 --- a/spec/unit/metrics/client_spec.rb +++ b/spec/unit/metrics/client_spec.rb @@ -17,12 +17,6 @@ module Metrics expect(client.tags).to eq(expected_tags) end end - - context "when :tags are invalid" do - it "raises exception" do - expect { Client.new(tags: "invalid arg") }.to raise_error(ArgumentError) - end - end end context "when :tags are not present" do @@ -60,12 +54,6 @@ module Metrics expect(subject.tags).to be_a(Hash) expect(subject.tags).to eq(expected_tags) end - - context "when invalid argument" do - it "raises exception" do - expect { subject.tags = "invalid arg" }.to raise_error(ArgumentError) - end - end end describe "#add_tags" do @@ -94,12 +82,6 @@ module Metrics expect(subject.tags).to eq(expected_tags) end end - - context "when invalid argument" do - it "raises exception" do - expect { subject.add_tags "invalid arg" }.to raise_error(ArgumentError) - end - end end describe "#clear_tags" do From f4a15359c6ed7d2101c6540ccb6fe78d596e96ac Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Thu, 25 Aug 2016 18:02:36 -0700 Subject: [PATCH 25/55] sort delegators --- lib/librato/metrics.rb | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/lib/librato/metrics.rb b/lib/librato/metrics.rb index 350738a..25b5068 100644 --- a/lib/librato/metrics.rb +++ b/lib/librato/metrics.rb @@ -72,17 +72,15 @@ module Metrics # being called on a global Client instance. See further docs on # Client. # - def_delegators :client, :agent_identifier, :annotate, :api_endpoint, - :add_tags, :clear_tags, :has_tags?, - :api_endpoint=, :authenticate, :connection, - :proxy, :proxy=, - :faraday_adapter, :faraday_adapter=, - :persistence, :persistence=, :persister, - :get_composite, :get_metric, :get_measurements, :metrics, - :delete_metrics, :update_metric, :update_metrics, - :submit, - :sources, :get_source, :update_source, - :create_snapshot, :get_snapshot, :get_measurement, :get_series + def_delegators :client, :add_tags, :agent_identifier, :annotate, + :api_endpoint, :api_endpoint=, :authenticate, :clear_tags, + :connection, :create_snapshot, :delete_metrics, + :faraday_adapter, :faraday_adapter=, :get_composite, + :get_measurement, :get_measurements, :get_metric, + :get_series, :get_snapshot, :get_source, :metrics, + :persistence, :persistence=, :persister, :proxy, :proxy=, + :sources, :submit, :update_metric, :update_metrics, + :update_source # The Librato::Metrics::Client being used by module-level # access. From ad730fc86db82daae9ca2be683806947444f967d Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Thu, 25 Aug 2016 18:06:58 -0700 Subject: [PATCH 26/55] rename key --- lib/librato/metrics/queue.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/librato/metrics/queue.rb b/lib/librato/metrics/queue.rb index 00aabb6..986e64f 100644 --- a/lib/librato/metrics/queue.rb +++ b/lib/librato/metrics/queue.rb @@ -42,13 +42,13 @@ def add(measurements) end type = :measurement if @multidimensional || metric[:tags] type = ("#{type}s").to_sym - time_hsh_key = @multidimensional || metric[:tags] ? :time : :measure_time + time = @multidimensional || metric[:tags] ? :time : :measure_time - if metric[time_hsh_key] - metric[time_hsh_key] = metric[time_hsh_key].to_i + if metric[time] + metric[time] = metric[time].to_i check_measure_time(metric) elsif !skip_measurement_times - metric[time_hsh_key] = epoch_time + metric[time] = epoch_time end @queued[type] ||= [] From ee0bb7001f4630d1d2320254c1cc44f4ba8b224b Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Thu, 25 Aug 2016 20:35:58 -0700 Subject: [PATCH 27/55] do not check type. allow API to raise exception instead --- lib/librato/metrics/processor.rb | 3 +-- spec/unit/metrics/aggregator_spec.rb | 3 --- spec/unit/metrics/queue_spec.rb | 3 --- 3 files changed, 1 insertion(+), 8 deletions(-) diff --git a/lib/librato/metrics/processor.rb b/lib/librato/metrics/processor.rb index 064e579..2985b8f 100644 --- a/lib/librato/metrics/processor.rb +++ b/lib/librato/metrics/processor.rb @@ -110,12 +110,11 @@ def validate_multidimensionality(options) check_compatibility(options, [:measure_time, :time]) check_compatibility(options, [:source, :time]) check_compatibility(options, [:measure_time, :tags]) - check_compatibility(options, [:type]) if options[:tags] || options[:time] end def check_compatibility(options, incompatible_options) if incompatible_options.to_set.subset?(options.keys.to_set) - raise ArgumentError, "#{incompatible_options} cannot be set" + raise ArgumentError, "#{incompatible_options} cannot be simultaneously set" end end diff --git a/spec/unit/metrics/aggregator_spec.rb b/spec/unit/metrics/aggregator_spec.rb index 0c9b8a0..c027c31 100644 --- a/spec/unit/metrics/aggregator_spec.rb +++ b/spec/unit/metrics/aggregator_spec.rb @@ -86,9 +86,6 @@ module Metrics expect { subject.add test: { tags: { hostname: "metrics-web-stg-1" }, measure_time: Time.now, value: 123 } }.to raise_error(ArgumentError) - expect { - subject.add test: { type: "gauge", tags: { hostname: "metrics-web-stg-1" }, value: 123 } - }.to raise_error(ArgumentError) end end diff --git a/spec/unit/metrics/queue_spec.rb b/spec/unit/metrics/queue_spec.rb index 5ee50b9..dbbc806 100644 --- a/spec/unit/metrics/queue_spec.rb +++ b/spec/unit/metrics/queue_spec.rb @@ -74,9 +74,6 @@ module Metrics expect { subject.add test: { tags: { hostname: "metrics-web-stg-1" }, measure_time: Time.now, value: 123 } }.to raise_error(ArgumentError) - expect { - subject.add test: { type: "counter", tags: { hostname: "metrics-web-stg-1" }, value: 123 } - }.to raise_error(ArgumentError) end end From 72afd38d70ea664b3ceb063ed57ee31b8edb2033 Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Thu, 25 Aug 2016 20:54:13 -0700 Subject: [PATCH 28/55] refactor parameter validation --- lib/librato/metrics/aggregator.rb | 2 +- lib/librato/metrics/errors.rb | 1 + lib/librato/metrics/processor.rb | 24 +++++++++++++----------- lib/librato/metrics/queue.rb | 2 +- spec/unit/metrics/aggregator_spec.rb | 16 ++++++++-------- spec/unit/metrics/queue_spec.rb | 16 ++++++++-------- 6 files changed, 32 insertions(+), 29 deletions(-) diff --git a/lib/librato/metrics/aggregator.rb b/lib/librato/metrics/aggregator.rb index e0c16c9..6a5793c 100644 --- a/lib/librato/metrics/aggregator.rb +++ b/lib/librato/metrics/aggregator.rb @@ -58,7 +58,7 @@ def add(measurements) end entry[:name] = metric if data.respond_to?(:each) # hash form - validate_multidimensionality(data) + validate_parameters(data) value = data[:value] if data[:source] metric = "#{metric}#{SEPARATOR}#{data[:source]}" diff --git a/lib/librato/metrics/errors.rb b/lib/librato/metrics/errors.rb index aee95a6..55d52a3 100644 --- a/lib/librato/metrics/errors.rb +++ b/lib/librato/metrics/errors.rb @@ -9,6 +9,7 @@ class NoMetricsProvided < MetricsError; end class NoClientProvided < MetricsError; end class InvalidMeasureTime < MetricsError; end class NotMergeable < MetricsError; end + class InvalidParameters < MetricsError; end class NetworkError < StandardError attr_reader :response diff --git a/lib/librato/metrics/processor.rb b/lib/librato/metrics/processor.rb index 2985b8f..236229a 100644 --- a/lib/librato/metrics/processor.rb +++ b/lib/librato/metrics/processor.rb @@ -84,7 +84,7 @@ def epoch_time end def setup_common_options(options) - validate_multidimensionality(options) + validate_parameters(options) @autosubmit_interval = options[:autosubmit_interval] @client = options[:client] || Librato::Metrics.client @per_request = options[:per_request] || MEASUREMENTS_PER_REQUEST @@ -105,16 +105,18 @@ def autosubmit_check end end - def validate_multidimensionality(options) - check_compatibility(options, [:source, :tags]) - check_compatibility(options, [:measure_time, :time]) - check_compatibility(options, [:source, :time]) - check_compatibility(options, [:measure_time, :tags]) - end - - def check_compatibility(options, incompatible_options) - if incompatible_options.to_set.subset?(options.keys.to_set) - raise ArgumentError, "#{incompatible_options} cannot be simultaneously set" + def validate_parameters(options) + invalid_combinations = [ + [:source, :tags], + [:measure_time, :time], + [:source, :time], + [:measure_time, :tags] + ] + opts = options.keys.to_set + invalid_combinations.each do |combo| + if combo.to_set.subset?(opts) + raise InvalidParameters, "#{combo} cannot be simultaneously set" + end end end diff --git a/lib/librato/metrics/queue.rb b/lib/librato/metrics/queue.rb index 986e64f..230b05a 100644 --- a/lib/librato/metrics/queue.rb +++ b/lib/librato/metrics/queue.rb @@ -29,7 +29,7 @@ def initialize(opts={}) def add(measurements) measurements.each do |key, value| if value.respond_to?(:each) - validate_multidimensionality(value) + validate_parameters(value) metric = value metric[:name] = key.to_s type = metric.delete(:type) || metric.delete('type') || 'gauge' diff --git a/spec/unit/metrics/aggregator_spec.rb b/spec/unit/metrics/aggregator_spec.rb index c027c31..6e2cc24 100644 --- a/spec/unit/metrics/aggregator_spec.rb +++ b/spec/unit/metrics/aggregator_spec.rb @@ -54,15 +54,15 @@ module Metrics source: "metrics-web-stg-1", tags: { hostname: "metrics-web-stg-1" } ) - }.to raise_error(ArgumentError) - expect { Aggregator.new(measure_time: Time.now, time: Time.now) }.to raise_error(ArgumentError) - expect { Aggregator.new(source: "metrics-web-stg-1", time: Time.now) }.to raise_error(ArgumentError) + }.to raise_error(InvalidParameters) + expect { Aggregator.new(measure_time: Time.now, time: Time.now) }.to raise_error(InvalidParameters) + expect { Aggregator.new(source: "metrics-web-stg-1", time: Time.now) }.to raise_error(InvalidParameters) expect { Aggregator.new( measure_time: Time.now, tags: { hostname: "metrics-web-stg-1" } ) - }.to raise_error(ArgumentError) + }.to raise_error(InvalidParameters) end end end @@ -76,16 +76,16 @@ module Metrics it "raises exception" do expect { subject.add test: { source: "metrics-web-stg-1", tags: { hostname: "metrics-web-stg-1" }, value: 123 } - }.to raise_error(ArgumentError) + }.to raise_error(InvalidParameters) expect { subject.add test: { measure_time: Time.now, time: Time.now, value: 123 } - }.to raise_error(ArgumentError) + }.to raise_error(InvalidParameters) expect { subject.add test: { source: "metrics-web-stg-1", time: Time.now, value: 123 } - }.to raise_error(ArgumentError) + }.to raise_error(InvalidParameters) expect { subject.add test: { tags: { hostname: "metrics-web-stg-1" }, measure_time: Time.now, value: 123 } - }.to raise_error(ArgumentError) + }.to raise_error(InvalidParameters) end end diff --git a/spec/unit/metrics/queue_spec.rb b/spec/unit/metrics/queue_spec.rb index dbbc806..2783055 100644 --- a/spec/unit/metrics/queue_spec.rb +++ b/spec/unit/metrics/queue_spec.rb @@ -42,15 +42,15 @@ module Metrics source: "metrics-web-stg-1", tags: { hostname: "metrics-web-stg-1" } ) - }.to raise_error(ArgumentError) - expect { Queue.new(measure_time: Time.now, time: Time.now) }.to raise_error(ArgumentError) - expect { Queue.new(source: "metrics-web-stg-1", time: Time.now) }.to raise_error(ArgumentError) + }.to raise_error(InvalidParameters) + expect { Queue.new(measure_time: Time.now, time: Time.now) }.to raise_error(InvalidParameters) + expect { Queue.new(source: "metrics-web-stg-1", time: Time.now) }.to raise_error(InvalidParameters) expect { Queue.new( measure_time: Time.now, tags: { hostname: "metrics-web-stg-1" } ) - }.to raise_error(ArgumentError) + }.to raise_error(InvalidParameters) end end end @@ -64,16 +64,16 @@ module Metrics it "raises exception" do expect { subject.add test: { source: "metrics-web-stg-1", tags: { hostname: "metrics-web-stg-1" }, value: 123 } - }.to raise_error(ArgumentError) + }.to raise_error(InvalidParameters) expect { subject.add test: { measure_time: Time.now, time: Time.now, value: 123 } - }.to raise_error(ArgumentError) + }.to raise_error(InvalidParameters) expect { subject.add test: { source: "metrics-web-stg-1", time: Time.now, value: 123 } - }.to raise_error(ArgumentError) + }.to raise_error(InvalidParameters) expect { subject.add test: { tags: { hostname: "metrics-web-stg-1" }, measure_time: Time.now, value: 123 } - }.to raise_error(ArgumentError) + }.to raise_error(InvalidParameters) end end From d4d8e72353c632aa8a73871e6a578c0ccaa760d1 Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Thu, 25 Aug 2016 21:03:44 -0700 Subject: [PATCH 29/55] convert to string first --- lib/librato/metrics/aggregator.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/librato/metrics/aggregator.rb b/lib/librato/metrics/aggregator.rb index 6a5793c..b9cb82e 100644 --- a/lib/librato/metrics/aggregator.rb +++ b/lib/librato/metrics/aggregator.rb @@ -56,7 +56,7 @@ def add(measurements) if @prefix metric = "#{@prefix}.#{metric}" end - entry[:name] = metric + entry[:name] = metric.to_s if data.respond_to?(:each) # hash form validate_parameters(data) value = data[:value] @@ -106,7 +106,7 @@ def queued @aggregated.each_value do |data| entry = { - name: data[:name].to_s, + name: data[:name], count: data[:aggregate].count, sum: data[:aggregate].sum, # TODO: make float/non-float consistent in the gem From 8733031841770aa636c19f930882dad75652bb62 Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Fri, 26 Aug 2016 11:47:29 -0700 Subject: [PATCH 30/55] add Queue unit tests --- lib/librato/metrics/queue.rb | 2 +- spec/unit/metrics/queue_spec.rb | 75 ++++++++++++++++++++++++++++++++- 2 files changed, 74 insertions(+), 3 deletions(-) diff --git a/lib/librato/metrics/queue.rb b/lib/librato/metrics/queue.rb index 230b05a..72df0c4 100644 --- a/lib/librato/metrics/queue.rb +++ b/lib/librato/metrics/queue.rb @@ -143,7 +143,7 @@ def queued # # @return Integer def size - self.queued.inject(0) { |result, data| result + data.last.size } + self.queued.reject { |key| key == :multidimensional }.inject(0) { |result, data| result + data.last.size } end alias :length :size diff --git a/spec/unit/metrics/queue_spec.rb b/spec/unit/metrics/queue_spec.rb index 2783055..1cdd867 100644 --- a/spec/unit/metrics/queue_spec.rb +++ b/spec/unit/metrics/queue_spec.rb @@ -182,6 +182,69 @@ module Metrics }.to raise_error(InvalidMeasureTime) end end + + context "with tags" do + context "when Queue is initialized with tags" do + let(:queue) { Queue.new(tags: { region: "us-east-1" }) } + + it "applies top-level tags" do + expected = { name: "test", value: 1, time: @time } + queue.add test: 1 + + expect(queue.queued[:tags]).to eq({ region: "us-east-1" }) + expect(queue.queued[:measurements].first).to eq(expected) + end + end + + context "when tags are used as arguments" do + let(:queue) { Queue.new } + + it "applies per-measurement tags" do + expected = { name: "test", value: 2, tags: { db: "rr1" }, time: @time } + queue.add test: { value: 2, tags: { db: "rr1" } } + + expect(queue.queued[:tags]).to be_nil + expect(queue.queued[:measurements].first).to eq(expected) + end + end + + context "when Queue is initialized with tags and when tags are used as arguments" do + let(:queue) { Queue.new(tags: { region: "us-east-1" }) } + + it "applies top-level tags and per-measurement tags" do + expected = { name: "test", value: 3, tags: { db: "rr1" }, time: @time } + queue.add test: { value: 3, tags: { db: "rr1" } } + + expect(queue.queued[:tags]).to eq({ region: "us-east-1" }) + expect(queue.queued[:measurements].first).to eq(expected) + end + end + + context "when Queue is initialized with a Client with tags" do + let(:client) { Client.new(tags: { region: "us-east-1" }) } + let(:queue) { Queue.new(client: client) } + + it "applies Client top-level tags" do + expected = { name: "test", value: 4, time: @time } + queue.add test: 4 + + expect(queue.client.tags).to eq({ region: "us-east-1" }) # NOTE: add to queued[:tags] instead? + expect(queue.queued[:measurements].first).to eq(expected) + end + end + end + end + + describe "#measurements" do + it "returns currently queued measurements" do + subject.add test_1: { tags: { db: "rr1" }, value: 1 }, + test_2: { type: :counter, value: 2 } + expect(subject.measurements).to eq([{ name: "test_1", value: 1, tags: { db: "rr1" }, time: @time }]) + end + + it "returns [] when no queued measurements" do + expect(subject.measurements).to be_empty + end end describe "#counters" do @@ -264,7 +327,7 @@ module Metrics expect(q2.queued).to equal_unordered(expected) end - context "when multidimensional is true" do + context "when tags are present" do it "maintains specified tags" do q1 = Queue.new q1.add test: { tags: { db: "rr1" }, value: 123 } @@ -274,7 +337,7 @@ module Metrics expect(q2.queued[:measurements].first[:tags][:db]).to eq("rr1") end - it "does not change default tags" do + it "does not change top-level tags" do q1 = Queue.new(tags: { db: "rr1" }) q1.add test: 456 q2 = Queue.new(tags: { db: "rr2" }) @@ -413,6 +476,14 @@ module Metrics register_cents: {type: :gauge, value: 211101} expect(subject.size).to eq(4) end + + context "when measurement added" do + it "returns count of measurements" do + subject.add test_1: { tags: { db: "rr1" }, value: 1}, test_2: { tags: { db: "rr2" }, value: 2} + + expect(subject.size).to eq(2) + end + end end describe "#submit" do From a3d3160688d1596687e85c3f9bf2676fd3c01ee9 Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Fri, 26 Aug 2016 12:28:28 -0700 Subject: [PATCH 31/55] add Aggregator unit tests --- lib/librato/metrics/aggregator.rb | 4 +- spec/unit/metrics/aggregator_spec.rb | 60 ++++++++++++++++++++++++++++ spec/unit/metrics/queue_spec.rb | 2 +- 3 files changed, 63 insertions(+), 3 deletions(-) diff --git a/lib/librato/metrics/aggregator.rb b/lib/librato/metrics/aggregator.rb index b9cb82e..4bd4c28 100644 --- a/lib/librato/metrics/aggregator.rb +++ b/lib/librato/metrics/aggregator.rb @@ -62,7 +62,7 @@ def add(measurements) value = data[:value] if data[:source] metric = "#{metric}#{SEPARATOR}#{data[:source]}" - entry[:source] = data[:source] + entry[:source] = data[:source].to_s end if data[:tags] && data[:tags].respond_to?(:each) metric = metric.to_s @@ -114,7 +114,7 @@ def queued max: data[:aggregate].max.to_f # TODO: expose v.sum2 and include } - entry[:source] = data[:source].to_s if data[:source] + entry[:source] = data[:source] if data[:source] if data[:tags] contains_measurements = true entry[:tags] = data[:tags] diff --git a/spec/unit/metrics/aggregator_spec.rb b/spec/unit/metrics/aggregator_spec.rb index 6e2cc24..7e98b3e 100644 --- a/spec/unit/metrics/aggregator_spec.rb +++ b/spec/unit/metrics/aggregator_spec.rb @@ -222,6 +222,66 @@ module Metrics expect(subject.queued).to equal_unordered(expected) end end + + context "with tags" do + context "when Aggregator is initialized with tags" do + let(:aggregator) { Aggregator.new(tags: { region: "us-east-1" }) } + + it "applies top-level tags" do + expected = { name: "test", count: 2, sum: 3, min: 1, max: 2 } + aggregator.add test: 1 + aggregator.add test: 2 + + expect(aggregator.queued[:tags]).to eq({ region: "us-east-1" }) + expect(aggregator.queued[:measurements].first).to eq(expected) + end + end + + context "when tags are used as arguments" do + let(:aggregator) { Aggregator.new } + + it "applies per-measurement tags" do + expected = { name: "test", count: 2, sum: 3, min: 1, max: 2, tags: { db: "rr1" } } + aggregator.add test: { value: 1, tags: { db: "rr1" } } + aggregator.add test: { value: 2, tags: { db: "rr1" } } + + expect(aggregator.queued[:tags]).to be_nil + expect(aggregator.queued[:measurements].first).to eq(expected) + end + end + + context "when Aggregator is initialized with tags and when tags are used as arguments" do + let(:aggregator) { Aggregator.new(tags: { region: "us-east-1" }) } + + it "applies top-level tags and per-measurement tags" do + expected = { name: "test", count: 3, sum: 12, min: 3, max: 5, tags: { db: "rr1" } } + aggregator.add test: { value: 3, tags: { db: "rr1" } } + aggregator.add test: { value: 4, tags: { db: "rr1" } } + aggregator.add test: { value: 5, tags: { db: "rr1" } } + aggregator.add test: { value: 1, tags: { db: "rr2" } } + aggregator.add test: { value: 2, tags: { region: "us-tirefire-1" } } + + expect(aggregator.queued[:tags]).to eq({ region: "us-east-1" }) + expect(aggregator.queued[:measurements].first).to eq(expected) + end + end + + context "when Aggregator is initialized with a Client with tags" do + let(:client) { Client.new(tags: { region: "us-east-1" }) } + let(:aggregator) { Aggregator.new(client: client) } + + it "applies Client top-level tags" do + expected = { name: "test", count: 4, sum: 30.0, min: 6.0, max: 9.0 } + aggregator.add test: 6 + aggregator.add test: 7 + aggregator.add test: 8 + aggregator.add test: 9 + + expect(aggregator.client.tags).to eq({ region: "us-east-1" }) + expect(aggregator.queued[:measurements].first).to eq(expected) + end + end + end end describe "#queued" do diff --git a/spec/unit/metrics/queue_spec.rb b/spec/unit/metrics/queue_spec.rb index 1cdd867..3051d9a 100644 --- a/spec/unit/metrics/queue_spec.rb +++ b/spec/unit/metrics/queue_spec.rb @@ -228,7 +228,7 @@ module Metrics expected = { name: "test", value: 4, time: @time } queue.add test: 4 - expect(queue.client.tags).to eq({ region: "us-east-1" }) # NOTE: add to queued[:tags] instead? + expect(queue.client.tags).to eq({ region: "us-east-1" }) expect(queue.queued[:measurements].first).to eq(expected) end end From a7d0b6d146ee1a95b00728472407ad786ee101be Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Fri, 26 Aug 2016 14:00:53 -0700 Subject: [PATCH 32/55] add integration tests --- spec/integration/metrics/queue_spec.rb | 18 ++++++++++++++++++ spec/integration/metrics_spec.rb | 22 ++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/spec/integration/metrics/queue_spec.rb b/spec/integration/metrics/queue_spec.rb index 1ae0a0c..95a0b27 100644 --- a/spec/integration/metrics/queue_spec.rb +++ b/spec/integration/metrics/queue_spec.rb @@ -70,6 +70,24 @@ module Metrics expect(bar['barsource'][0]['value']).to eq(456) end + context "with tags" do + let(:queue) { Queue.new(tags: { a: "b" } ) } + + it "respects default and individual tags" do + queue.add test_1: 123 + queue.add test_2: { value: 456, tags: { c: "d" }} + queue.submit + + test_1 = Librato::Metrics.get_measurement :test_1, resolution: 1, duration: 3600 + expect(test_1["series"][0]["tags"]["a"]).to eq("b") + expect(test_1["series"][0]["measurements"][0]["value"]).to eq(123) + + test_2 = Metrics.get_measurement :test_2, resolution: 1, duration: 3600 + expect(test_2["series"][0]["tags"]["c"]).to eq("d") + expect(test_2["series"][0]["measurements"][0]["value"]).to eq(456) + end + end + end end diff --git a/spec/integration/metrics_spec.rb b/spec/integration/metrics_spec.rb index 590b565..39163fc 100644 --- a/spec/integration/metrics_spec.rb +++ b/spec/integration/metrics_spec.rb @@ -360,6 +360,28 @@ module Librato end + describe "#get_measurement" do + before { Metrics.submit test: { value: 123, tags: { foo: "bar" } } } + + it "gets measurement" do + measurement = Metrics.get_measurement :test, resolution: 1, duration: 3600 + + expect(measurement["series"][0]["tags"]["foo"]).to eq("bar") + expect(measurement["series"][0]["measurements"][0]["value"]).to eq(123) + end + end + + describe "#get_series" do + after { delete_all_metrics } + + it "gets series" do + series = Metrics.get_series :test, resolution: 1, duration: 3600 + + expect(series[0]["tags"]["foo"]).to eq("bar") + expect(series[0]["measurements"][0]["value"]).to eq(123) + end + end + # Note: These are challenging to test end-to-end, should probably # unit test instead. Disabling for now. # From 54f9cfe47770523e506bef27c115bdd0d80f1a90 Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Fri, 26 Aug 2016 16:04:41 -0700 Subject: [PATCH 33/55] merge client top-level tags --- lib/librato/metrics/aggregator.rb | 4 +-- lib/librato/metrics/persistence/direct.rb | 16 ++++++----- lib/librato/metrics/processor.rb | 12 ++++++--- lib/librato/metrics/queue.rb | 2 +- spec/integration/metrics_spec.rb | 2 -- spec/unit/metrics/aggregator_spec.rb | 33 ++++++++++++++++------- spec/unit/metrics/queue_spec.rb | 24 +++++++++++++---- 7 files changed, 64 insertions(+), 29 deletions(-) diff --git a/lib/librato/metrics/aggregator.rb b/lib/librato/metrics/aggregator.rb index 4bd4c28..8ed35e3 100644 --- a/lib/librato/metrics/aggregator.rb +++ b/lib/librato/metrics/aggregator.rb @@ -27,7 +27,7 @@ class Aggregator include Processor - attr_reader :source, :tags + attr_reader :source # @option opts [Integer] :autosubmit_interval If set the aggregator will auto-submit if the given number of seconds has passed when a new metric is added. # @option opts [Boolean] :clear_failures Should the aggregator remove all stored data if it runs into problems with a request? (default: false) @@ -129,7 +129,7 @@ def queued { gauges: entries } end req[:source] = @source if @source - req[:tags] = @tags if @tags + req[:tags] = @tags if !@tags.empty? req[:measure_time] = @measure_time if @measure_time req[:time] = @time if @time req[:multidimensional] = true if @multidimensional || contains_measurements diff --git a/lib/librato/metrics/persistence/direct.rb b/lib/librato/metrics/persistence/direct.rb index 25f8277..b733508 100644 --- a/lib/librato/metrics/persistence/direct.rb +++ b/lib/librato/metrics/persistence/direct.rb @@ -16,14 +16,16 @@ def persist(client, queued, options={}) requests = [queued] end requests.each do |request| - if client.has_tags? || queued[:tags] || queued[:multidimensional] + if queued[:tags] || queued[:multidimensional] resource = "measurements" - tags_payload = client.tags ? client.tags : queued[:tags] - if client.tags && queued[:tags] - tags_payload = client.tags.merge(queued[:tags]) - end - queued.delete(:multidimensional) if queued[:multidimensional] - payload = SmartJSON.write(request.merge({ tags: tags_payload })) + payload = + if queued[:multidimensional] + # request contains at least one measurement with per-measurement tags + queued.delete(:multidimensional) + SmartJSON.write(request) + else + SmartJSON.write(request.merge({ tags: tags_payload })) + end else resource = "metrics" payload = SmartJSON.write(request) diff --git a/lib/librato/metrics/processor.rb b/lib/librato/metrics/processor.rb index 236229a..4656e55 100644 --- a/lib/librato/metrics/processor.rb +++ b/lib/librato/metrics/processor.rb @@ -9,7 +9,12 @@ module Processor MEASUREMENTS_PER_REQUEST = 500 attr_reader :per_request, :last_submit_time - attr_accessor :prefix + attr_accessor :prefix, :tags + + def tags + @tags ||= {} + @tags.merge!(@client.tags) + end # The current Client instance this queue is using to authenticate # and connect to Librato Metrics. This will default to the primary @@ -89,13 +94,14 @@ def setup_common_options(options) @client = options[:client] || Librato::Metrics.client @per_request = options[:per_request] || MEASUREMENTS_PER_REQUEST @source = options[:source] - @tags = options[:tags] + @tags = options.fetch(:tags, {}) + @tags.merge!(@client.tags) @measure_time = options[:measure_time] && options[:measure_time].to_i @time = options[:time] && options[:time].to_i @create_time = Time.now @clear_on_failure = options[:clear_failures] || false @prefix = options[:prefix] - @multidimensional = @client.has_tags? || (@tags && !@tags.empty?) || !@time.nil? + @multidimensional = !@tags.empty? || !@time.nil? end def autosubmit_check diff --git a/lib/librato/metrics/queue.rb b/lib/librato/metrics/queue.rb index 72df0c4..eac63f5 100644 --- a/lib/librato/metrics/queue.rb +++ b/lib/librato/metrics/queue.rb @@ -133,7 +133,7 @@ def queued return {} if @queued.empty? globals = {} globals[:source] = @source if @source - globals[:tags] = @tags if @tags + globals[:tags] = @tags if !@tags.empty? globals[:measure_time] = @measure_time if @measure_time globals[:time] = @time if @time @queued.merge(globals) diff --git a/spec/integration/metrics_spec.rb b/spec/integration/metrics_spec.rb index 39163fc..051765c 100644 --- a/spec/integration/metrics_spec.rb +++ b/spec/integration/metrics_spec.rb @@ -372,8 +372,6 @@ module Librato end describe "#get_series" do - after { delete_all_metrics } - it "gets series" do series = Metrics.get_series :test, resolution: 1, duration: 3600 diff --git a/spec/unit/metrics/aggregator_spec.rb b/spec/unit/metrics/aggregator_spec.rb index 7e98b3e..82e8c34 100644 --- a/spec/unit/metrics/aggregator_spec.rb +++ b/spec/unit/metrics/aggregator_spec.rb @@ -267,19 +267,34 @@ module Metrics end context "when Aggregator is initialized with a Client with tags" do - let(:client) { Client.new(tags: { region: "us-east-1" }) } + let(:client) { Librato::Metrics::Client.new(tags: { region: "us-east-1" }) } let(:aggregator) { Aggregator.new(client: client) } - it "applies Client top-level tags" do - expected = { name: "test", count: 4, sum: 30.0, min: 6.0, max: 9.0 } - aggregator.add test: 6 - aggregator.add test: 7 - aggregator.add test: 8 - aggregator.add test: 9 + context "during initialization" do + it "applies Client top-level tags" do + expected = { name: "test", count: 4, sum: 30.0, min: 6.0, max: 9.0 } + aggregator.add test: 6 + aggregator.add test: 7 + aggregator.add test: 8 + aggregator.add test: 9 - expect(aggregator.client.tags).to eq({ region: "us-east-1" }) - expect(aggregator.queued[:measurements].first).to eq(expected) + expect(aggregator.queued[:tags]).to eq({ region: "us-east-1" }) + expect(aggregator.queued[:measurements].first).to eq(expected) + end end + + context "after initialization" do + it "applies Client top-level tags" do + expected = { name: "test", count: 2, sum: 3.0, min: 1.0, max: 2.0 } + client.add_tags foo: "bar" + aggregator.add test: 1 + aggregator.add test: 2 + + expect(aggregator.queued[:tags]).to eq({ region: "us-east-1", foo: "bar" }) + expect(aggregator.queued[:measurements].first).to eq(expected) + end + end + end end end diff --git a/spec/unit/metrics/queue_spec.rb b/spec/unit/metrics/queue_spec.rb index 3051d9a..b322e3a 100644 --- a/spec/unit/metrics/queue_spec.rb +++ b/spec/unit/metrics/queue_spec.rb @@ -15,6 +15,7 @@ module Metrics it "sets to client" do barney = Client allow(barney).to receive(:has_tags?).and_return(false) + allow(barney).to receive(:tags).and_return({}) queue = Queue.new(client: barney) expect(queue.client).to eq(barney) end @@ -224,12 +225,25 @@ module Metrics let(:client) { Client.new(tags: { region: "us-east-1" }) } let(:queue) { Queue.new(client: client) } - it "applies Client top-level tags" do - expected = { name: "test", value: 4, time: @time } - queue.add test: 4 + context "during initialization" do + it "applies Client top-level tags" do + expected = { name: "test", value: 4, time: @time } + queue.add test: 4 - expect(queue.client.tags).to eq({ region: "us-east-1" }) - expect(queue.queued[:measurements].first).to eq(expected) + expect(queue.queued[:tags]).to eq({ region: "us-east-1" }) + expect(queue.queued[:measurements].first).to eq(expected) + end + end + + context "after initialization" do + it "applies Client top-level tags" do + expected = { name: "test", value: 5, time: @time } + client.add_tags foo: "bar" + queue.add test: 5 + + expect(queue.queued[:tags]).to eq({ region: "us-east-1", foo: "bar" }) + expect(queue.queued[:measurements].first).to eq(expected) + end end end end From 98fb022f8d237964c35204699fb5f067fa24c828 Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Fri, 26 Aug 2016 16:13:43 -0700 Subject: [PATCH 34/55] change multidimensional to method --- lib/librato/metrics/aggregator.rb | 4 ++-- lib/librato/metrics/processor.rb | 5 ++++- lib/librato/metrics/queue.rb | 8 ++++---- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/lib/librato/metrics/aggregator.rb b/lib/librato/metrics/aggregator.rb index 8ed35e3..584abc8 100644 --- a/lib/librato/metrics/aggregator.rb +++ b/lib/librato/metrics/aggregator.rb @@ -123,7 +123,7 @@ def queued entries << entry end req = - if @multidimensional || contains_measurements + if multidimensional? || contains_measurements { measurements: entries } else { gauges: entries } @@ -132,7 +132,7 @@ def queued req[:tags] = @tags if !@tags.empty? req[:measure_time] = @measure_time if @measure_time req[:time] = @time if @time - req[:multidimensional] = true if @multidimensional || contains_measurements + req[:multidimensional] = true if multidimensional? || contains_measurements req end diff --git a/lib/librato/metrics/processor.rb b/lib/librato/metrics/processor.rb index 4656e55..5cc96c0 100644 --- a/lib/librato/metrics/processor.rb +++ b/lib/librato/metrics/processor.rb @@ -26,6 +26,10 @@ def client @client ||= Librato::Metrics.client end + def multidimensional? + !@tags.empty? || !@time.nil? + end + # The object this MetricSet will use to persist # def persister @@ -101,7 +105,6 @@ def setup_common_options(options) @create_time = Time.now @clear_on_failure = options[:clear_failures] || false @prefix = options[:prefix] - @multidimensional = !@tags.empty? || !@time.nil? end def autosubmit_check diff --git a/lib/librato/metrics/queue.rb b/lib/librato/metrics/queue.rb index eac63f5..57d6ab3 100644 --- a/lib/librato/metrics/queue.rb +++ b/lib/librato/metrics/queue.rb @@ -40,9 +40,9 @@ def add(measurements) if @prefix metric[:name] = "#{@prefix}.#{metric[:name]}" end - type = :measurement if @multidimensional || metric[:tags] + type = :measurement if multidimensional? || metric[:tags] type = ("#{type}s").to_sym - time = @multidimensional || metric[:tags] ? :time : :measure_time + time = multidimensional? || metric[:tags] ? :time : :measure_time if metric[time] metric[time] = metric[time].to_i @@ -53,7 +53,7 @@ def add(measurements) @queued[type] ||= [] @queued[type] << metric - @queued[:multidimensional] = true if @multidimensional || metric[:tags] + @queued[:multidimensional] = true if multidimensional? || metric[:tags] end submit_check self @@ -110,7 +110,7 @@ def merge!(mergeable) Metrics::PLURAL_TYPES.each do |type| if to_merge[type] measurements = - if @multidimensional + if multidimensional? reconcile(to_merge[type], to_merge[:tags]) else reconcile(to_merge[type], to_merge[:source]) From 48a2eaf60678b0c8162cb77558824948241e336a Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Fri, 26 Aug 2016 16:53:13 -0700 Subject: [PATCH 35/55] minor style change --- lib/librato/metrics/client.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/librato/metrics/client.rb b/lib/librato/metrics/client.rb index fab95e8..d439c18 100644 --- a/lib/librato/metrics/client.rb +++ b/lib/librato/metrics/client.rb @@ -11,7 +11,7 @@ class Client attr_accessor :email, :api_key, :proxy, :tags def initialize(options={}) - self.tags = options.fetch(:tags, {}) + @tags = options.fetch(:tags, {}) end def tags @@ -19,7 +19,7 @@ def tags end def add_tags(tags) - self.tags.merge!(tags) + @tags.merge!(tags) end # Provide agent identifier for the developer program. See: From 40997b7b326e9d26a0825e313962c26effdc9890 Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Fri, 26 Aug 2016 16:55:17 -0700 Subject: [PATCH 36/55] update has_tags --- lib/librato/metrics/client.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/librato/metrics/client.rb b/lib/librato/metrics/client.rb index d439c18..9589ce8 100644 --- a/lib/librato/metrics/client.rb +++ b/lib/librato/metrics/client.rb @@ -254,7 +254,7 @@ def flush_authentication end def has_tags? - !self.tags.empty? + !@tags.empty? end alias :tags? :has_tags? From b3a887acf71f0b63939132babf81b048f7c9b802 Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Fri, 26 Aug 2016 18:02:59 -0700 Subject: [PATCH 37/55] delegate tag management to Client --- lib/librato/metrics/aggregator.rb | 2 +- lib/librato/metrics/processor.rb | 16 +++++++--------- lib/librato/metrics/queue.rb | 2 +- spec/integration/metrics/queue_spec.rb | 5 ++++- spec/integration/metrics_spec.rb | 1 + spec/unit/metrics/aggregator_spec.rb | 2 ++ spec/unit/metrics/queue_spec.rb | 13 +++++++++---- 7 files changed, 25 insertions(+), 16 deletions(-) diff --git a/lib/librato/metrics/aggregator.rb b/lib/librato/metrics/aggregator.rb index 584abc8..425f4fc 100644 --- a/lib/librato/metrics/aggregator.rb +++ b/lib/librato/metrics/aggregator.rb @@ -129,7 +129,7 @@ def queued { gauges: entries } end req[:source] = @source if @source - req[:tags] = @tags if !@tags.empty? + req[:tags] = tags if has_tags? req[:measure_time] = @measure_time if @measure_time req[:time] = @time if @time req[:multidimensional] = true if multidimensional? || contains_measurements diff --git a/lib/librato/metrics/processor.rb b/lib/librato/metrics/processor.rb index 5cc96c0..4e0e2ee 100644 --- a/lib/librato/metrics/processor.rb +++ b/lib/librato/metrics/processor.rb @@ -6,15 +6,14 @@ module Metrics # Mixin which provides common logic between {Queue} and {Aggregator} # objects. module Processor + extend Forwardable + + def_delegators :@client, :add_tags, :clear_tags, :has_tags?, :tags + MEASUREMENTS_PER_REQUEST = 500 attr_reader :per_request, :last_submit_time - attr_accessor :prefix, :tags - - def tags - @tags ||= {} - @tags.merge!(@client.tags) - end + attr_accessor :prefix # The current Client instance this queue is using to authenticate # and connect to Librato Metrics. This will default to the primary @@ -27,7 +26,7 @@ def client end def multidimensional? - !@tags.empty? || !@time.nil? + has_tags? || !@time.nil? end # The object this MetricSet will use to persist @@ -98,8 +97,7 @@ def setup_common_options(options) @client = options[:client] || Librato::Metrics.client @per_request = options[:per_request] || MEASUREMENTS_PER_REQUEST @source = options[:source] - @tags = options.fetch(:tags, {}) - @tags.merge!(@client.tags) + @client.add_tags(options.fetch(:tags, {})) @measure_time = options[:measure_time] && options[:measure_time].to_i @time = options[:time] && options[:time].to_i @create_time = Time.now diff --git a/lib/librato/metrics/queue.rb b/lib/librato/metrics/queue.rb index 57d6ab3..0308cd8 100644 --- a/lib/librato/metrics/queue.rb +++ b/lib/librato/metrics/queue.rb @@ -133,7 +133,7 @@ def queued return {} if @queued.empty? globals = {} globals[:source] = @source if @source - globals[:tags] = @tags if !@tags.empty? + globals[:tags] = tags if has_tags? globals[:measure_time] = @measure_time if @measure_time globals[:time] = @time if @time @queued.merge(globals) diff --git a/spec/integration/metrics/queue_spec.rb b/spec/integration/metrics/queue_spec.rb index 95a0b27..48c9694 100644 --- a/spec/integration/metrics/queue_spec.rb +++ b/spec/integration/metrics/queue_spec.rb @@ -5,7 +5,10 @@ module Metrics describe Queue do before(:all) { prep_integration_tests } - before(:each) { delete_all_metrics } + before(:each) do + delete_all_metrics + Librato::Metrics.client.clear_tags + end context "with a large number of metrics" do it "submits them in multiple requests" do diff --git a/spec/integration/metrics_spec.rb b/spec/integration/metrics_spec.rb index 051765c..80ca7ec 100644 --- a/spec/integration/metrics_spec.rb +++ b/spec/integration/metrics_spec.rb @@ -3,6 +3,7 @@ module Librato describe Metrics do before(:all) { prep_integration_tests } + before(:each) { Librato::Metrics.client.clear_tags } describe "#annotate" do before(:all) { @annotator = Metrics::Annotator.new } diff --git a/spec/unit/metrics/aggregator_spec.rb b/spec/unit/metrics/aggregator_spec.rb index 82e8c34..dd7aaf8 100644 --- a/spec/unit/metrics/aggregator_spec.rb +++ b/spec/unit/metrics/aggregator_spec.rb @@ -9,6 +9,8 @@ module Metrics allow_any_instance_of(Aggregator).to receive(:epoch_time).and_return(@time) end + before(:each) { Librato::Metrics.client.clear_tags } + describe "initialization" do context "with specified client" do it "sets to client" do diff --git a/spec/unit/metrics/queue_spec.rb b/spec/unit/metrics/queue_spec.rb index b322e3a..48b945f 100644 --- a/spec/unit/metrics/queue_spec.rb +++ b/spec/unit/metrics/queue_spec.rb @@ -5,17 +5,20 @@ module Metrics describe Queue do - before(:each) do + before(:all) do @time = (Time.now.to_i - 1*60) allow_any_instance_of(Queue).to receive(:epoch_time).and_return(@time) end + before(:each) { Librato::Metrics.client.clear_tags } + describe "initialization" do context "with specified client" do it "sets to client" do barney = Client allow(barney).to receive(:has_tags?).and_return(false) allow(barney).to receive(:tags).and_return({}) + allow(barney).to receive(:add_tags).and_return({}) queue = Queue.new(client: barney) expect(queue.client).to eq(barney) end @@ -361,14 +364,16 @@ module Metrics end it "tracks previous default tags" do - q1 = Queue.new(tags: { db: "rr1" }) + q1 = Queue.new(tags: { instance_id: "i-1234567a" }) q1.add test_1: 123 - q2 = Queue.new(tags: { db: "rr2" }) + q2 = Queue.new(tags: { instance_type: "m3.medium" }) q2.add test_2: 456 q2.merge!(q1) metric = q2.measurements.find { |measurement| measurement[:name] == "test_1" } - expect(metric[:tags][:db]).to eq("rr1") + expect(metric[:tags][:instance_id]).to eq("i-1234567a") + expect(q2.queued[:tags]).to eq({ instance_id: "i-1234567a", instance_type: "m3.medium" }) + end end From 73b279f12112fa2a764655adf11402c29d4381d9 Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Fri, 26 Aug 2016 18:42:11 -0700 Subject: [PATCH 38/55] minor changes --- lib/librato/metrics/aggregator.rb | 2 +- lib/librato/metrics/persistence/direct.rb | 2 +- lib/librato/metrics/queue.rb | 8 +++-- spec/integration/metrics/queue_spec.rb | 8 ++--- spec/integration/metrics_spec.rb | 6 ++-- spec/unit/metrics/aggregator_spec.rb | 41 ++++++++++----------- spec/unit/metrics/client_spec.rb | 30 ++++++++-------- spec/unit/metrics/queue_spec.rb | 44 ++++++++++++----------- 8 files changed, 71 insertions(+), 70 deletions(-) diff --git a/lib/librato/metrics/aggregator.rb b/lib/librato/metrics/aggregator.rb index 425f4fc..6a658b7 100644 --- a/lib/librato/metrics/aggregator.rb +++ b/lib/librato/metrics/aggregator.rb @@ -132,7 +132,7 @@ def queued req[:tags] = tags if has_tags? req[:measure_time] = @measure_time if @measure_time req[:time] = @time if @time - req[:multidimensional] = true if multidimensional? || contains_measurements + req[:multidimensional] = true if req[:measurements] req end diff --git a/lib/librato/metrics/persistence/direct.rb b/lib/librato/metrics/persistence/direct.rb index b733508..7368532 100644 --- a/lib/librato/metrics/persistence/direct.rb +++ b/lib/librato/metrics/persistence/direct.rb @@ -20,7 +20,7 @@ def persist(client, queued, options={}) resource = "measurements" payload = if queued[:multidimensional] - # request contains at least one measurement with per-measurement tags + # request contains per-measurement tags queued.delete(:multidimensional) SmartJSON.write(request) else diff --git a/lib/librato/metrics/queue.rb b/lib/librato/metrics/queue.rb index 0308cd8..823431e 100644 --- a/lib/librato/metrics/queue.rb +++ b/lib/librato/metrics/queue.rb @@ -28,6 +28,7 @@ def initialize(opts={}) # @return [Queue] returns self def add(measurements) measurements.each do |key, value| + contains_measurements = multidimensional? if value.respond_to?(:each) validate_parameters(value) metric = value @@ -40,9 +41,10 @@ def add(measurements) if @prefix metric[:name] = "#{@prefix}.#{metric[:name]}" end - type = :measurement if multidimensional? || metric[:tags] + contains_measurements = contains_measurements || metric[:tags] + type = :measurement if contains_measurements type = ("#{type}s").to_sym - time = multidimensional? || metric[:tags] ? :time : :measure_time + time = contains_measurements ? :time : :measure_time if metric[time] metric[time] = metric[time].to_i @@ -53,7 +55,7 @@ def add(measurements) @queued[type] ||= [] @queued[type] << metric - @queued[:multidimensional] = true if multidimensional? || metric[:tags] + @queued[:multidimensional] = true if contains_measurements end submit_check self diff --git a/spec/integration/metrics/queue_spec.rb b/spec/integration/metrics/queue_spec.rb index 48c9694..e5ff8c4 100644 --- a/spec/integration/metrics/queue_spec.rb +++ b/spec/integration/metrics/queue_spec.rb @@ -74,19 +74,19 @@ module Metrics end context "with tags" do - let(:queue) { Queue.new(tags: { a: "b" } ) } + let(:queue) { Queue.new(tags: { hostname: "metrics-stg-1" } ) } it "respects default and individual tags" do queue.add test_1: 123 - queue.add test_2: { value: 456, tags: { c: "d" }} + queue.add test_2: { value: 456, tags: { hostname: "metrics-stg-2" }} queue.submit test_1 = Librato::Metrics.get_measurement :test_1, resolution: 1, duration: 3600 - expect(test_1["series"][0]["tags"]["a"]).to eq("b") + expect(test_1["series"][0]["tags"]["hostname"]).to eq("metrics-stg-1") expect(test_1["series"][0]["measurements"][0]["value"]).to eq(123) test_2 = Metrics.get_measurement :test_2, resolution: 1, duration: 3600 - expect(test_2["series"][0]["tags"]["c"]).to eq("d") + expect(test_2["series"][0]["tags"]["hostname"]).to eq("metrics-stg-2") expect(test_2["series"][0]["measurements"][0]["value"]).to eq(456) end end diff --git a/spec/integration/metrics_spec.rb b/spec/integration/metrics_spec.rb index 80ca7ec..57bd7ad 100644 --- a/spec/integration/metrics_spec.rb +++ b/spec/integration/metrics_spec.rb @@ -362,12 +362,12 @@ module Librato end describe "#get_measurement" do - before { Metrics.submit test: { value: 123, tags: { foo: "bar" } } } + before { Metrics.submit test: { value: 123, tags: { hostname: "metrics-stg-1" } } } it "gets measurement" do measurement = Metrics.get_measurement :test, resolution: 1, duration: 3600 - expect(measurement["series"][0]["tags"]["foo"]).to eq("bar") + expect(measurement["series"][0]["tags"]["hostname"]).to eq("metrics-stg-1") expect(measurement["series"][0]["measurements"][0]["value"]).to eq(123) end end @@ -376,7 +376,7 @@ module Librato it "gets series" do series = Metrics.get_series :test, resolution: 1, duration: 3600 - expect(series[0]["tags"]["foo"]).to eq("bar") + expect(series[0]["tags"]["hostname"]).to eq("metrics-stg-1") expect(series[0]["measurements"][0]["value"]).to eq(123) end end diff --git a/spec/unit/metrics/aggregator_spec.rb b/spec/unit/metrics/aggregator_spec.rb index dd7aaf8..8115fc4 100644 --- a/spec/unit/metrics/aggregator_spec.rb +++ b/spec/unit/metrics/aggregator_spec.rb @@ -137,21 +137,18 @@ module Metrics expect(subject.queued).to equal_unordered(expected) end - context "when multidimensional is true" do + context "when per-measurement tags" do it "maintains specified tags" do - subject.add test: { tags: { db: "rr1" }, value: 1 } + subject.add test: { tags: { hostname: "metrics-web-stg-1" }, value: 1 } subject.add test: 5 - subject.add test: { tags: { db: "rr1" }, value: 6 } + subject.add test: { tags: { hostname: "metrics-web-stg-1" }, value: 6 } subject.add test: 10 - expected = { - measurements: [ - { name: "test", tags: { db: "rr1" }, count: 2, sum: 7.0, min: 1.0, max: 6.0 }, - { name: "test", count: 2, sum: 15.0, min: 5.0, max: 10.0 } - ], - multidimensional: true - } + expected = [ + { name: "test", tags: { hostname: "metrics-web-stg-1" }, count: 2, sum: 7.0, min: 1.0, max: 6.0 }, + { name: "test", count: 2, sum: 15.0, min: 5.0, max: 10.0 } + ] - expect(subject.queued).to equal_unordered(expected) + expect(subject.queued[:measurements]).to equal_unordered(expected) end end @@ -243,9 +240,9 @@ module Metrics let(:aggregator) { Aggregator.new } it "applies per-measurement tags" do - expected = { name: "test", count: 2, sum: 3, min: 1, max: 2, tags: { db: "rr1" } } - aggregator.add test: { value: 1, tags: { db: "rr1" } } - aggregator.add test: { value: 2, tags: { db: "rr1" } } + expected = { name: "test", count: 2, sum: 3, min: 1, max: 2, tags: { hostname: "metrics-web-stg-1" } } + aggregator.add test: { value: 1, tags: { hostname: "metrics-web-stg-1" } } + aggregator.add test: { value: 2, tags: { hostname: "metrics-web-stg-1" } } expect(aggregator.queued[:tags]).to be_nil expect(aggregator.queued[:measurements].first).to eq(expected) @@ -256,12 +253,12 @@ module Metrics let(:aggregator) { Aggregator.new(tags: { region: "us-east-1" }) } it "applies top-level tags and per-measurement tags" do - expected = { name: "test", count: 3, sum: 12, min: 3, max: 5, tags: { db: "rr1" } } - aggregator.add test: { value: 3, tags: { db: "rr1" } } - aggregator.add test: { value: 4, tags: { db: "rr1" } } - aggregator.add test: { value: 5, tags: { db: "rr1" } } - aggregator.add test: { value: 1, tags: { db: "rr2" } } - aggregator.add test: { value: 2, tags: { region: "us-tirefire-1" } } + expected = { name: "test", count: 3, sum: 12, min: 3, max: 5, tags: { hostname: "metrics-web-stg-1" } } + aggregator.add test: { value: 3, tags: { hostname: "metrics-web-stg-1" } } + aggregator.add test: { value: 4, tags: { hostname: "metrics-web-stg-1" } } + aggregator.add test: { value: 5, tags: { hostname: "metrics-web-stg-1" } } + aggregator.add test: { value: 1, tags: { hostname: "metrics-web-stg-2" } } + aggregator.add test: { value: 2, tags: { region: "us-tirefire-1" } } expect(aggregator.queued[:tags]).to eq({ region: "us-east-1" }) expect(aggregator.queued[:measurements].first).to eq(expected) @@ -288,11 +285,11 @@ module Metrics context "after initialization" do it "applies Client top-level tags" do expected = { name: "test", count: 2, sum: 3.0, min: 1.0, max: 2.0 } - client.add_tags foo: "bar" + client.add_tags hostname: "metrics-web-stg-1" aggregator.add test: 1 aggregator.add test: 2 - expect(aggregator.queued[:tags]).to eq({ region: "us-east-1", foo: "bar" }) + expect(aggregator.queued[:tags]).to eq({ region: "us-east-1", hostname: "metrics-web-stg-1" }) expect(aggregator.queued[:measurements].first).to eq(expected) end end diff --git a/spec/unit/metrics/client_spec.rb b/spec/unit/metrics/client_spec.rb index abfe428..95b136f 100644 --- a/spec/unit/metrics/client_spec.rb +++ b/spec/unit/metrics/client_spec.rb @@ -8,14 +8,12 @@ module Metrics describe "#initialize" do context "when :tags are present" do after { Librato::Metrics.client.tags.clear } - context "when :tags are valid" do - it "sets @tags" do - expected_tags = { environment: "staging" } - client = Client.new(tags: expected_tags) + it "sets @tags" do + expected_tags = { environment: "staging" } + client = Client.new(tags: expected_tags) - expect(client.tags).not_to be_empty - expect(client.tags).to eq(expected_tags) - end + expect(client.tags).not_to be_empty + expect(client.tags).to eq(expected_tags) end end @@ -30,12 +28,12 @@ module Metrics describe "#tags" do context "when set" do - before { subject.tags = { instance: "i-1234567a" } } + before { subject.tags = { instance_id: "i-1234567a" } } after { Librato::Metrics.client.tags.clear } it "gets @tags" do expect(subject.tags).to be_a(Hash) - expect(subject.tags.keys).to include(:instance) - expect(subject.tags[:instance]).to eq("i-1234567a") + expect(subject.tags.keys).to include(:instance_id) + expect(subject.tags[:instance_id]).to eq("i-1234567a") end end @@ -49,7 +47,7 @@ module Metrics describe "#tags=" do after { Librato::Metrics.client.tags.clear } it "sets @tags" do - expected_tags = { instance: "i-1234567b" } + expected_tags = { instance_id: "i-1234567b" } expect{subject.tags = expected_tags}.to change{subject.tags}.from({}).to(expected_tags) expect(subject.tags).to be_a(Hash) expect(subject.tags).to eq(expected_tags) @@ -61,7 +59,7 @@ module Metrics context "when no existing tags" do it "adds top-level tags" do - expected_tags = { instance: "i-1234567c" } + expected_tags = { instance_id: "i-1234567c" } subject.add_tags expected_tags expect(subject.tags).to be_a(Hash) @@ -71,8 +69,8 @@ module Metrics context "when existing tags" do it "merges tags" do - tmp1 = { instance: "i-1234567c" } - tmp2 = { region: "us-east-1", elb: "metrics-stg" } + tmp1 = { instance_id: "i-1234567d" } + tmp2 = { region: "us-east-1", hostname: "metrics-web-stg-1" } expected_tags = tmp1.merge(tmp2) subject.add_tags tmp1 @@ -87,7 +85,7 @@ module Metrics describe "#clear_tags" do context "when tags are set" do it "empties Hash" do - expected_tags = { instance: "i-1234567d" } + expected_tags = { instance_id: "i-1234567e" } subject.add_tags expected_tags expect{subject.clear_tags}.to change{subject.tags}.from(expected_tags).to({}) @@ -99,7 +97,7 @@ module Metrics context "when tags are set" do after { Librato::Metrics.client.tags.clear } it "returns true" do - subject.add_tags instance: "i-1234567e" + subject.add_tags instance_id: "i-1234567f" expect(subject.has_tags?).to eq(true) end diff --git a/spec/unit/metrics/queue_spec.rb b/spec/unit/metrics/queue_spec.rb index 48b945f..2007f53 100644 --- a/spec/unit/metrics/queue_spec.rb +++ b/spec/unit/metrics/queue_spec.rb @@ -14,12 +14,15 @@ module Metrics describe "initialization" do context "with specified client" do - it "sets to client" do - barney = Client + let(:barney) { Client } + let(:queue) { Queue.new(client: barney) } + before do allow(barney).to receive(:has_tags?).and_return(false) allow(barney).to receive(:tags).and_return({}) allow(barney).to receive(:add_tags).and_return({}) - queue = Queue.new(client: barney) + end + + it "sets to client" do expect(queue.client).to eq(barney) end end @@ -204,8 +207,8 @@ module Metrics let(:queue) { Queue.new } it "applies per-measurement tags" do - expected = { name: "test", value: 2, tags: { db: "rr1" }, time: @time } - queue.add test: { value: 2, tags: { db: "rr1" } } + expected = { name: "test", value: 2, tags: { hostname: "metrics-web-stg-1" }, time: @time } + queue.add test: { value: 2, tags: { hostname: "metrics-web-stg-1" } } expect(queue.queued[:tags]).to be_nil expect(queue.queued[:measurements].first).to eq(expected) @@ -216,8 +219,8 @@ module Metrics let(:queue) { Queue.new(tags: { region: "us-east-1" }) } it "applies top-level tags and per-measurement tags" do - expected = { name: "test", value: 3, tags: { db: "rr1" }, time: @time } - queue.add test: { value: 3, tags: { db: "rr1" } } + expected = { name: "test", value: 3, tags: { hostname: "metrics-web-stg-1" }, time: @time } + queue.add test: { value: 3, tags: { hostname: "metrics-web-stg-1" } } expect(queue.queued[:tags]).to eq({ region: "us-east-1" }) expect(queue.queued[:measurements].first).to eq(expected) @@ -241,10 +244,10 @@ module Metrics context "after initialization" do it "applies Client top-level tags" do expected = { name: "test", value: 5, time: @time } - client.add_tags foo: "bar" + client.add_tags hostname: "metrics-web-stg-1" queue.add test: 5 - expect(queue.queued[:tags]).to eq({ region: "us-east-1", foo: "bar" }) + expect(queue.queued[:tags]).to eq({ region: "us-east-1", hostname: "metrics-web-stg-1" }) expect(queue.queued[:measurements].first).to eq(expected) end end @@ -254,9 +257,9 @@ module Metrics describe "#measurements" do it "returns currently queued measurements" do - subject.add test_1: { tags: { db: "rr1" }, value: 1 }, + subject.add test_1: { tags: { region: "us-east-1" }, value: 1 }, test_2: { type: :counter, value: 2 } - expect(subject.measurements).to eq([{ name: "test_1", value: 1, tags: { db: "rr1" }, time: @time }]) + expect(subject.measurements).to eq([{ name: "test_1", value: 1, tags: { region: "us-east-1" }, time: @time }]) end it "returns [] when no queued measurements" do @@ -344,23 +347,23 @@ module Metrics expect(q2.queued).to equal_unordered(expected) end - context "when tags are present" do + context "with tags" do it "maintains specified tags" do q1 = Queue.new - q1.add test: { tags: { db: "rr1" }, value: 123 } - q2 = Queue.new(tags: { db: "rr2" }) + q1.add test: { tags: { hostname: "metrics-web-stg-1" }, value: 123 } + q2 = Queue.new(tags: { hostname: "metrics-web-stg-2" }) q2.merge!(q1) - expect(q2.queued[:measurements].first[:tags][:db]).to eq("rr1") + expect(q2.queued[:measurements].first[:tags][:hostname]).to eq("metrics-web-stg-1") end it "does not change top-level tags" do - q1 = Queue.new(tags: { db: "rr1" }) + q1 = Queue.new(tags: { hostname: "metrics-web-stg-1" }) q1.add test: 456 - q2 = Queue.new(tags: { db: "rr2" }) + q2 = Queue.new(tags: { hostname: "metrics-web-stg-2" }) q2.merge!(q1) - expect(q2.queued[:tags][:db]).to eq("rr2") + expect(q2.queued[:tags][:hostname]).to eq("metrics-web-stg-2") end it "tracks previous default tags" do @@ -496,9 +499,10 @@ module Metrics expect(subject.size).to eq(4) end - context "when measurement added" do + context "when measurement present" do it "returns count of measurements" do - subject.add test_1: { tags: { db: "rr1" }, value: 1}, test_2: { tags: { db: "rr2" }, value: 2} + subject.add test_1: { tags: { hostname: "metrics-web-stg-1" }, value: 1 }, + test_2: { tags: { hostname: "metrics-web-stg-2" }, value: 2} expect(subject.size).to eq(2) end From 6f6f1e36a2cd8435465bb71670f2d0e94d045290 Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Fri, 26 Aug 2016 18:46:12 -0700 Subject: [PATCH 39/55] delegate has_tags --- lib/librato/metrics.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/librato/metrics.rb b/lib/librato/metrics.rb index 25b5068..fb0ec89 100644 --- a/lib/librato/metrics.rb +++ b/lib/librato/metrics.rb @@ -77,7 +77,7 @@ module Metrics :connection, :create_snapshot, :delete_metrics, :faraday_adapter, :faraday_adapter=, :get_composite, :get_measurement, :get_measurements, :get_metric, - :get_series, :get_snapshot, :get_source, :metrics, + :get_series, :get_snapshot, :get_source, :has_tags?, :metrics, :persistence, :persistence=, :persister, :proxy, :proxy=, :sources, :submit, :update_metric, :update_metrics, :update_source From 14d4b4f56ecab1c8cab47a2871946f2cd548f5ad Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Fri, 26 Aug 2016 19:00:08 -0700 Subject: [PATCH 40/55] simplify checking for measurements --- lib/librato/metrics/aggregator.rb | 4 ++-- spec/integration/metrics/queue_spec.rb | 10 +++++----- spec/integration/metrics_spec.rb | 6 +++--- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/librato/metrics/aggregator.rb b/lib/librato/metrics/aggregator.rb index 6a658b7..36819f7 100644 --- a/lib/librato/metrics/aggregator.rb +++ b/lib/librato/metrics/aggregator.rb @@ -102,7 +102,7 @@ def clear # def queued entries = [] - contains_measurements = false + contains_measurements = multidimensional? @aggregated.each_value do |data| entry = { @@ -123,7 +123,7 @@ def queued entries << entry end req = - if multidimensional? || contains_measurements + if contains_measurements { measurements: entries } else { gauges: entries } diff --git a/spec/integration/metrics/queue_spec.rb b/spec/integration/metrics/queue_spec.rb index e5ff8c4..a3d50d0 100644 --- a/spec/integration/metrics/queue_spec.rb +++ b/spec/integration/metrics/queue_spec.rb @@ -74,19 +74,19 @@ module Metrics end context "with tags" do - let(:queue) { Queue.new(tags: { hostname: "metrics-stg-1" } ) } + let(:queue) { Queue.new(tags: { hostname: "metrics-web-stg-1" } ) } it "respects default and individual tags" do queue.add test_1: 123 - queue.add test_2: { value: 456, tags: { hostname: "metrics-stg-2" }} + queue.add test_2: { value: 456, tags: { hostname: "metrics-web-stg-2" }} queue.submit test_1 = Librato::Metrics.get_measurement :test_1, resolution: 1, duration: 3600 - expect(test_1["series"][0]["tags"]["hostname"]).to eq("metrics-stg-1") + expect(test_1["series"][0]["tags"]["hostname"]).to eq("metrics-web-stg-1") expect(test_1["series"][0]["measurements"][0]["value"]).to eq(123) - test_2 = Metrics.get_measurement :test_2, resolution: 1, duration: 3600 - expect(test_2["series"][0]["tags"]["hostname"]).to eq("metrics-stg-2") + test_2 = Librato::Metrics.get_measurement :test_2, resolution: 1, duration: 3600 + expect(test_2["series"][0]["tags"]["hostname"]).to eq("metrics-web-stg-2") expect(test_2["series"][0]["measurements"][0]["value"]).to eq(456) end end diff --git a/spec/integration/metrics_spec.rb b/spec/integration/metrics_spec.rb index 57bd7ad..6ae57f0 100644 --- a/spec/integration/metrics_spec.rb +++ b/spec/integration/metrics_spec.rb @@ -362,12 +362,12 @@ module Librato end describe "#get_measurement" do - before { Metrics.submit test: { value: 123, tags: { hostname: "metrics-stg-1" } } } + before { Metrics.submit test: { value: 123, tags: { hostname: "metrics-web-stg-1" } } } it "gets measurement" do measurement = Metrics.get_measurement :test, resolution: 1, duration: 3600 - expect(measurement["series"][0]["tags"]["hostname"]).to eq("metrics-stg-1") + expect(measurement["series"][0]["tags"]["hostname"]).to eq("metrics-web-stg-1") expect(measurement["series"][0]["measurements"][0]["value"]).to eq(123) end end @@ -376,7 +376,7 @@ module Librato it "gets series" do series = Metrics.get_series :test, resolution: 1, duration: 3600 - expect(series[0]["tags"]["hostname"]).to eq("metrics-stg-1") + expect(series[0]["tags"]["hostname"]).to eq("metrics-web-stg-1") expect(series[0]["measurements"][0]["value"]).to eq(123) end end From 90bfa71c077bb1f9a29c61c2a09fd5390484512c Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Fri, 26 Aug 2016 19:15:00 -0700 Subject: [PATCH 41/55] refactor direct persistence --- lib/librato/metrics/persistence/direct.rb | 12 +++--------- lib/librato/metrics/queue.rb | 3 ++- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/lib/librato/metrics/persistence/direct.rb b/lib/librato/metrics/persistence/direct.rb index 7368532..d40fe6f 100644 --- a/lib/librato/metrics/persistence/direct.rb +++ b/lib/librato/metrics/persistence/direct.rb @@ -17,19 +17,13 @@ def persist(client, queued, options={}) end requests.each do |request| if queued[:tags] || queued[:multidimensional] + # request contains per-measurement tags + queued.delete(:multidimensional) if queued[:multidimensional] resource = "measurements" - payload = - if queued[:multidimensional] - # request contains per-measurement tags - queued.delete(:multidimensional) - SmartJSON.write(request) - else - SmartJSON.write(request.merge({ tags: tags_payload })) - end else resource = "metrics" - payload = SmartJSON.write(request) end + payload = SmartJSON.write(request) # expects 200 client.connection.post(resource, payload) end diff --git a/lib/librato/metrics/queue.rb b/lib/librato/metrics/queue.rb index 823431e..06c1a22 100644 --- a/lib/librato/metrics/queue.rb +++ b/lib/librato/metrics/queue.rb @@ -41,7 +41,7 @@ def add(measurements) if @prefix metric[:name] = "#{@prefix}.#{metric[:name]}" end - contains_measurements = contains_measurements || metric[:tags] + contains_measurements = true if metric[:tags] type = :measurement if contains_measurements type = ("#{type}s").to_sym time = contains_measurements ? :time : :measure_time @@ -52,6 +52,7 @@ def add(measurements) elsif !skip_measurement_times metric[time] = epoch_time end + contains_measurements = true if metric[:time] @queued[type] ||= [] @queued[type] << metric From f207868f131d0bdbc56e5eb858f54e81f81296f9 Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Fri, 26 Aug 2016 19:24:42 -0700 Subject: [PATCH 42/55] assign var from returned result --- lib/librato/metrics/aggregator.rb | 2 +- lib/librato/metrics/persistence/direct.rb | 15 ++++++++------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/lib/librato/metrics/aggregator.rb b/lib/librato/metrics/aggregator.rb index 36819f7..a373915 100644 --- a/lib/librato/metrics/aggregator.rb +++ b/lib/librato/metrics/aggregator.rb @@ -132,7 +132,7 @@ def queued req[:tags] = tags if has_tags? req[:measure_time] = @measure_time if @measure_time req[:time] = @time if @time - req[:multidimensional] = true if req[:measurements] + req[:multidimensional] = true if contains_measurements req end diff --git a/lib/librato/metrics/persistence/direct.rb b/lib/librato/metrics/persistence/direct.rb index d40fe6f..0f3a9e2 100644 --- a/lib/librato/metrics/persistence/direct.rb +++ b/lib/librato/metrics/persistence/direct.rb @@ -16,13 +16,14 @@ def persist(client, queued, options={}) requests = [queued] end requests.each do |request| - if queued[:tags] || queued[:multidimensional] - # request contains per-measurement tags - queued.delete(:multidimensional) if queued[:multidimensional] - resource = "measurements" - else - resource = "metrics" - end + resource = + if queued[:tags] || queued[:multidimensional] + # request contains per-measurement tags + queued.delete(:multidimensional) if queued[:multidimensional] + "measurements" + else + "metrics" + end payload = SmartJSON.write(request) # expects 200 client.connection.post(resource, payload) From 8529dacdeb0207a352b8e06bd5c7c5c058144c4d Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Tue, 30 Aug 2016 13:11:15 -0700 Subject: [PATCH 43/55] sort Aggregator tags hash key --- lib/librato/metrics/aggregator.rb | 2 +- spec/unit/metrics/aggregator_spec.rb | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/lib/librato/metrics/aggregator.rb b/lib/librato/metrics/aggregator.rb index a373915..f0df497 100644 --- a/lib/librato/metrics/aggregator.rb +++ b/lib/librato/metrics/aggregator.rb @@ -66,7 +66,7 @@ def add(measurements) end if data[:tags] && data[:tags].respond_to?(:each) metric = metric.to_s - data[:tags].each do |key, value| + data[:tags].sort.each do |key, value| metric = "#{metric}#{SEPARATOR}#{key}#{SEPARATOR}#{value}" end entry[:tags] = data[:tags] diff --git a/spec/unit/metrics/aggregator_spec.rb b/spec/unit/metrics/aggregator_spec.rb index 8115fc4..911a093 100644 --- a/spec/unit/metrics/aggregator_spec.rb +++ b/spec/unit/metrics/aggregator_spec.rb @@ -247,6 +247,19 @@ module Metrics expect(aggregator.queued[:tags]).to be_nil expect(aggregator.queued[:measurements].first).to eq(expected) end + + context "when tags arguments are not sorted" do + let(:aggregator) { Aggregator.new } + + it "uses sorted tags hash key" do + expected = { name: "test", count: 2, sum: 3, min: 1, max: 2, tags: { a: 1, b: 2, c: 3 } } + aggregator.add test: { value: 1, tags: { c: 3, b: 2, a: 1 } } + aggregator.add test: { value: 2, tags: { b: 2, a: 1, c: 3 } } + + expect(aggregator.queued[:tags]).to be_nil + expect(aggregator.queued[:measurements].first).to eq(expected) + end + end end context "when Aggregator is initialized with tags and when tags are used as arguments" do From a597b7f8f96eccd800722212b7a1cb0c657989fa Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Mon, 12 Sep 2016 14:44:15 -0700 Subject: [PATCH 44/55] move tag management from Client to Processor --- lib/librato/metrics.rb | 6 +- lib/librato/metrics/aggregator.rb | 2 +- lib/librato/metrics/client.rb | 21 +---- lib/librato/metrics/processor.rb | 19 ++++- lib/librato/metrics/queue.rb | 2 +- spec/integration/metrics/queue_spec.rb | 1 - spec/integration/metrics_spec.rb | 1 - spec/unit/metrics/aggregator_spec.rb | 114 ++++++++++++++++++------- spec/unit/metrics/client_spec.rb | 105 ----------------------- spec/unit/metrics/queue_spec.rb | 111 +++++++++++++++++------- 10 files changed, 185 insertions(+), 197 deletions(-) diff --git a/lib/librato/metrics.rb b/lib/librato/metrics.rb index fb0ec89..0181453 100644 --- a/lib/librato/metrics.rb +++ b/lib/librato/metrics.rb @@ -72,12 +72,12 @@ module Metrics # being called on a global Client instance. See further docs on # Client. # - def_delegators :client, :add_tags, :agent_identifier, :annotate, - :api_endpoint, :api_endpoint=, :authenticate, :clear_tags, + def_delegators :client, :agent_identifier, :annotate, + :api_endpoint, :api_endpoint=, :authenticate, :connection, :create_snapshot, :delete_metrics, :faraday_adapter, :faraday_adapter=, :get_composite, :get_measurement, :get_measurements, :get_metric, - :get_series, :get_snapshot, :get_source, :has_tags?, :metrics, + :get_series, :get_snapshot, :get_source, :metrics, :persistence, :persistence=, :persister, :proxy, :proxy=, :sources, :submit, :update_metric, :update_metrics, :update_source diff --git a/lib/librato/metrics/aggregator.rb b/lib/librato/metrics/aggregator.rb index f0df497..e6093fa 100644 --- a/lib/librato/metrics/aggregator.rb +++ b/lib/librato/metrics/aggregator.rb @@ -129,7 +129,7 @@ def queued { gauges: entries } end req[:source] = @source if @source - req[:tags] = tags if has_tags? + req[:tags] = @tags if has_tags? req[:measure_time] = @measure_time if @measure_time req[:time] = @time if @time req[:multidimensional] = true if contains_measurements diff --git a/lib/librato/metrics/client.rb b/lib/librato/metrics/client.rb index 9589ce8..3cf2870 100644 --- a/lib/librato/metrics/client.rb +++ b/lib/librato/metrics/client.rb @@ -6,21 +6,7 @@ class Client def_delegator :annotator, :add, :annotate - def_delegator :@tags, :clear, :clear_tags - - attr_accessor :email, :api_key, :proxy, :tags - - def initialize(options={}) - @tags = options.fetch(:tags, {}) - end - - def tags - @tags ||= {} - end - - def add_tags(tags) - @tags.merge!(tags) - end + attr_accessor :email, :api_key, :proxy # Provide agent identifier for the developer program. See: # http://support.metrics.librato.com/knowledgebase/articles/53548-developer-program @@ -253,11 +239,6 @@ def flush_authentication @connection = nil end - def has_tags? - !@tags.empty? - end - alias :tags? :has_tags? - # List currently existing metrics # # @example List all metrics diff --git a/lib/librato/metrics/processor.rb b/lib/librato/metrics/processor.rb index 4e0e2ee..9dd2986 100644 --- a/lib/librato/metrics/processor.rb +++ b/lib/librato/metrics/processor.rb @@ -8,12 +8,20 @@ module Metrics module Processor extend Forwardable - def_delegators :@client, :add_tags, :clear_tags, :has_tags?, :tags + def_delegator :@tags, :clear, :clear_tags MEASUREMENTS_PER_REQUEST = 500 attr_reader :per_request, :last_submit_time - attr_accessor :prefix + attr_accessor :prefix, :tags + + def tags + @tags ||= {} + end + + def add_tags(tags) + @tags.merge!(tags) + end # The current Client instance this queue is using to authenticate # and connect to Librato Metrics. This will default to the primary @@ -25,6 +33,11 @@ def client @client ||= Librato::Metrics.client end + def has_tags? + !@tags.empty? + end + alias :tags? :has_tags? + def multidimensional? has_tags? || !@time.nil? end @@ -97,7 +110,7 @@ def setup_common_options(options) @client = options[:client] || Librato::Metrics.client @per_request = options[:per_request] || MEASUREMENTS_PER_REQUEST @source = options[:source] - @client.add_tags(options.fetch(:tags, {})) + @tags = options.fetch(:tags, {}) @measure_time = options[:measure_time] && options[:measure_time].to_i @time = options[:time] && options[:time].to_i @create_time = Time.now diff --git a/lib/librato/metrics/queue.rb b/lib/librato/metrics/queue.rb index 06c1a22..28cd63f 100644 --- a/lib/librato/metrics/queue.rb +++ b/lib/librato/metrics/queue.rb @@ -136,7 +136,7 @@ def queued return {} if @queued.empty? globals = {} globals[:source] = @source if @source - globals[:tags] = tags if has_tags? + globals[:tags] = @tags if has_tags? globals[:measure_time] = @measure_time if @measure_time globals[:time] = @time if @time @queued.merge(globals) diff --git a/spec/integration/metrics/queue_spec.rb b/spec/integration/metrics/queue_spec.rb index a3d50d0..7d1c12a 100644 --- a/spec/integration/metrics/queue_spec.rb +++ b/spec/integration/metrics/queue_spec.rb @@ -7,7 +7,6 @@ module Metrics before(:all) { prep_integration_tests } before(:each) do delete_all_metrics - Librato::Metrics.client.clear_tags end context "with a large number of metrics" do diff --git a/spec/integration/metrics_spec.rb b/spec/integration/metrics_spec.rb index 6ae57f0..2c6c5bd 100644 --- a/spec/integration/metrics_spec.rb +++ b/spec/integration/metrics_spec.rb @@ -3,7 +3,6 @@ module Librato describe Metrics do before(:all) { prep_integration_tests } - before(:each) { Librato::Metrics.client.clear_tags } describe "#annotate" do before(:all) { @annotator = Metrics::Annotator.new } diff --git a/spec/unit/metrics/aggregator_spec.rb b/spec/unit/metrics/aggregator_spec.rb index 911a093..1381f3e 100644 --- a/spec/unit/metrics/aggregator_spec.rb +++ b/spec/unit/metrics/aggregator_spec.rb @@ -9,8 +9,6 @@ module Metrics allow_any_instance_of(Aggregator).to receive(:epoch_time).and_return(@time) end - before(:each) { Librato::Metrics.client.clear_tags } - describe "initialization" do context "with specified client" do it "sets to client" do @@ -69,6 +67,87 @@ module Metrics end end + describe "#tags" do + context "when set" do + let(:aggregator) { Aggregator.new(tags: { instance_id: "i-1234567a" }) } + it "gets @tags" do + expect(aggregator.tags).to be_a(Hash) + expect(aggregator.tags.keys).to include(:instance_id) + expect(aggregator.tags[:instance_id]).to eq("i-1234567a") + end + end + + context "when not set" do + let(:aggregator) { Aggregator.new } + it "defaults to empty hash" do + expect(aggregator.tags).to be_a(Hash) + expect(aggregator.tags).to be_empty + end + end + end + + describe "#tags=" do + it "sets @tags" do + expected_tags = { instance_id: "i-1234567b" } + expect{subject.tags = expected_tags}.to change{subject.tags}.from({}).to(expected_tags) + expect(subject.tags).to be_a(Hash) + expect(subject.tags).to eq(expected_tags) + end + end + + describe "#add_tags" do + context "when no existing tags" do + it "adds top-level tags" do + expected_tags = { instance_id: "i-1234567c" } + subject.add_tags expected_tags + + expect(subject.tags).to be_a(Hash) + expect(subject.tags).to eq(expected_tags) + end + end + + context "when existing tags" do + it "merges tags" do + tmp1 = { instance_id: "i-1234567d" } + tmp2 = { region: "us-east-1", hostname: "metrics-web-stg-1" } + expected_tags = tmp1.merge(tmp2) + + subject.add_tags tmp1 + subject.add_tags tmp2 + + expect(subject.tags).to be_a(Hash) + expect(subject.tags).to eq(expected_tags) + end + end + end + + describe "#clear_tags" do + context "when tags are set" do + it "empties Hash" do + expected_tags = { instance_id: "i-1234567e" } + subject.add_tags expected_tags + + expect{subject.clear_tags}.to change{subject.tags}.from(expected_tags).to({}) + end + end + end + + describe "#has_tags?" do + context "when tags are set" do + it "returns true" do + subject.add_tags instance_id: "i-1234567f" + + expect(subject.has_tags?).to eq(true) + end + end + + context "when tags are not set" do + it "returns false" do + expect(subject.has_tags?).to eq(false) + end + end + end + describe "#add" do it "allows chaining" do expect(subject.add(foo: 1234)).to eq(subject) @@ -277,37 +356,6 @@ module Metrics expect(aggregator.queued[:measurements].first).to eq(expected) end end - - context "when Aggregator is initialized with a Client with tags" do - let(:client) { Librato::Metrics::Client.new(tags: { region: "us-east-1" }) } - let(:aggregator) { Aggregator.new(client: client) } - - context "during initialization" do - it "applies Client top-level tags" do - expected = { name: "test", count: 4, sum: 30.0, min: 6.0, max: 9.0 } - aggregator.add test: 6 - aggregator.add test: 7 - aggregator.add test: 8 - aggregator.add test: 9 - - expect(aggregator.queued[:tags]).to eq({ region: "us-east-1" }) - expect(aggregator.queued[:measurements].first).to eq(expected) - end - end - - context "after initialization" do - it "applies Client top-level tags" do - expected = { name: "test", count: 2, sum: 3.0, min: 1.0, max: 2.0 } - client.add_tags hostname: "metrics-web-stg-1" - aggregator.add test: 1 - aggregator.add test: 2 - - expect(aggregator.queued[:tags]).to eq({ region: "us-east-1", hostname: "metrics-web-stg-1" }) - expect(aggregator.queued[:measurements].first).to eq(expected) - end - end - - end end end diff --git a/spec/unit/metrics/client_spec.rb b/spec/unit/metrics/client_spec.rb index 95b136f..c611e19 100644 --- a/spec/unit/metrics/client_spec.rb +++ b/spec/unit/metrics/client_spec.rb @@ -5,111 +5,6 @@ module Metrics describe Client do - describe "#initialize" do - context "when :tags are present" do - after { Librato::Metrics.client.tags.clear } - it "sets @tags" do - expected_tags = { environment: "staging" } - client = Client.new(tags: expected_tags) - - expect(client.tags).not_to be_empty - expect(client.tags).to eq(expected_tags) - end - end - - context "when :tags are not present" do - it "does not set @tags" do - client = Client.new - - expect(client.tags).to be_empty - end - end - end - - describe "#tags" do - context "when set" do - before { subject.tags = { instance_id: "i-1234567a" } } - after { Librato::Metrics.client.tags.clear } - it "gets @tags" do - expect(subject.tags).to be_a(Hash) - expect(subject.tags.keys).to include(:instance_id) - expect(subject.tags[:instance_id]).to eq("i-1234567a") - end - end - - context "when not set" do - it "defaults to empty hash" do - expect(subject.tags).to be_empty - end - end - end - - describe "#tags=" do - after { Librato::Metrics.client.tags.clear } - it "sets @tags" do - expected_tags = { instance_id: "i-1234567b" } - expect{subject.tags = expected_tags}.to change{subject.tags}.from({}).to(expected_tags) - expect(subject.tags).to be_a(Hash) - expect(subject.tags).to eq(expected_tags) - end - end - - describe "#add_tags" do - after { Librato::Metrics.client.tags.clear } - - context "when no existing tags" do - it "adds top-level tags" do - expected_tags = { instance_id: "i-1234567c" } - subject.add_tags expected_tags - - expect(subject.tags).to be_a(Hash) - expect(subject.tags).to eq(expected_tags) - end - end - - context "when existing tags" do - it "merges tags" do - tmp1 = { instance_id: "i-1234567d" } - tmp2 = { region: "us-east-1", hostname: "metrics-web-stg-1" } - expected_tags = tmp1.merge(tmp2) - - subject.add_tags tmp1 - subject.add_tags tmp2 - - expect(subject.tags).to be_a(Hash) - expect(subject.tags).to eq(expected_tags) - end - end - end - - describe "#clear_tags" do - context "when tags are set" do - it "empties Hash" do - expected_tags = { instance_id: "i-1234567e" } - subject.add_tags expected_tags - - expect{subject.clear_tags}.to change{subject.tags}.from(expected_tags).to({}) - end - end - end - - describe "#has_tags?" do - context "when tags are set" do - after { Librato::Metrics.client.tags.clear } - it "returns true" do - subject.add_tags instance_id: "i-1234567f" - - expect(subject.has_tags?).to eq(true) - end - end - - context "when tags are not set" do - it "returns false" do - expect(subject.has_tags?).to eq(false) - end - end - end - describe "#agent_identifier" do context "when given a single string argument" do it "sets agent_identifier" do diff --git a/spec/unit/metrics/queue_spec.rb b/spec/unit/metrics/queue_spec.rb index 2007f53..c72f605 100644 --- a/spec/unit/metrics/queue_spec.rb +++ b/spec/unit/metrics/queue_spec.rb @@ -10,8 +10,6 @@ module Metrics allow_any_instance_of(Queue).to receive(:epoch_time).and_return(@time) end - before(:each) { Librato::Metrics.client.clear_tags } - describe "initialization" do context "with specified client" do let(:barney) { Client } @@ -62,6 +60,87 @@ module Metrics end end + describe "#tags" do + context "when set" do + let(:queue) { Queue.new(tags: { instance_id: "i-1234567a" }) } + it "gets @tags" do + expect(queue.tags).to be_a(Hash) + expect(queue.tags.keys).to include(:instance_id) + expect(queue.tags[:instance_id]).to eq("i-1234567a") + end + end + + context "when not set" do + let(:queue) { Queue.new } + it "defaults to empty hash" do + expect(queue.tags).to be_a(Hash) + expect(queue.tags).to be_empty + end + end + end + + describe "#tags=" do + it "sets @tags" do + expected_tags = { instance_id: "i-1234567b" } + expect{subject.tags = expected_tags}.to change{subject.tags}.from({}).to(expected_tags) + expect(subject.tags).to be_a(Hash) + expect(subject.tags).to eq(expected_tags) + end + end + + describe "#add_tags" do + context "when no existing tags" do + it "adds top-level tags" do + expected_tags = { instance_id: "i-1234567c" } + subject.add_tags expected_tags + + expect(subject.tags).to be_a(Hash) + expect(subject.tags).to eq(expected_tags) + end + end + + context "when existing tags" do + it "merges tags" do + tmp1 = { instance_id: "i-1234567d" } + tmp2 = { region: "us-east-1", hostname: "metrics-web-stg-1" } + expected_tags = tmp1.merge(tmp2) + + subject.add_tags tmp1 + subject.add_tags tmp2 + + expect(subject.tags).to be_a(Hash) + expect(subject.tags).to eq(expected_tags) + end + end + end + + describe "#clear_tags" do + context "when tags are set" do + it "empties Hash" do + expected_tags = { instance_id: "i-1234567e" } + subject.add_tags expected_tags + + expect{subject.clear_tags}.to change{subject.tags}.from(expected_tags).to({}) + end + end + end + + describe "#has_tags?" do + context "when tags are set" do + it "returns true" do + subject.add_tags instance_id: "i-1234567f" + + expect(subject.has_tags?).to eq(true) + end + end + + context "when tags are not set" do + it "returns false" do + expect(subject.has_tags?).to eq(false) + end + end + end + describe "#add" do it "allows chaining" do expect(subject.add(foo: 123)).to eq(subject) @@ -226,32 +305,6 @@ module Metrics expect(queue.queued[:measurements].first).to eq(expected) end end - - context "when Queue is initialized with a Client with tags" do - let(:client) { Client.new(tags: { region: "us-east-1" }) } - let(:queue) { Queue.new(client: client) } - - context "during initialization" do - it "applies Client top-level tags" do - expected = { name: "test", value: 4, time: @time } - queue.add test: 4 - - expect(queue.queued[:tags]).to eq({ region: "us-east-1" }) - expect(queue.queued[:measurements].first).to eq(expected) - end - end - - context "after initialization" do - it "applies Client top-level tags" do - expected = { name: "test", value: 5, time: @time } - client.add_tags hostname: "metrics-web-stg-1" - queue.add test: 5 - - expect(queue.queued[:tags]).to eq({ region: "us-east-1", hostname: "metrics-web-stg-1" }) - expect(queue.queued[:measurements].first).to eq(expected) - end - end - end end end @@ -375,7 +428,7 @@ module Metrics metric = q2.measurements.find { |measurement| measurement[:name] == "test_1" } expect(metric[:tags][:instance_id]).to eq("i-1234567a") - expect(q2.queued[:tags]).to eq({ instance_id: "i-1234567a", instance_type: "m3.medium" }) + expect(q2.queued[:tags]).to eq({ instance_type: "m3.medium" }) end end From 5258c1f7ff390153c7d7d242b5ae5707219b9cec Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Fri, 30 Sep 2016 17:00:06 -0700 Subject: [PATCH 45/55] remove #get_measurement --- lib/librato/metrics.rb | 4 ++-- lib/librato/metrics/client.rb | 28 ++++++++++++++++++++-------- spec/integration/metrics_spec.rb | 15 +++------------ 3 files changed, 25 insertions(+), 22 deletions(-) diff --git a/lib/librato/metrics.rb b/lib/librato/metrics.rb index 0181453..35940ab 100644 --- a/lib/librato/metrics.rb +++ b/lib/librato/metrics.rb @@ -76,8 +76,8 @@ module Metrics :api_endpoint, :api_endpoint=, :authenticate, :connection, :create_snapshot, :delete_metrics, :faraday_adapter, :faraday_adapter=, :get_composite, - :get_measurement, :get_measurements, :get_metric, - :get_series, :get_snapshot, :get_source, :metrics, + :get_measurements, :get_metric, :get_series, + :get_snapshot, :get_source, :metrics, :persistence, :persistence=, :persister, :proxy, :proxy=, :sources, :submit, :update_metric, :update_metrics, :update_source diff --git a/lib/librato/metrics/client.rb b/lib/librato/metrics/client.rb index 3cf2870..0dd9511 100644 --- a/lib/librato/metrics/client.rb +++ b/lib/librato/metrics/client.rb @@ -178,7 +178,23 @@ def get_metric(name, options = {}) parsed end - def get_measurement(name, options={}) + # Retrieve series of measurements for a given metric + # + # @example Get series for metric + # series = Librato::Metrics.get_series :requests, resolution: 1, duration: 3600 + # + # @example Get series for metric grouped by tag + # query = { duration: 3600, resolution: 1, group_by: "environment", group_by_function: "sum" } + # series = Librato::Metrics.get_series :requests, query + # + # @example Get series for metric grouped by tag and negated by tag filter + # query = { duration: 3600, resolution: 1, group_by: "environment", group_by_function: "sum", tags_search: "environment=!staging" } + # series = Librato::Metrics.get_series :requests, query + # + # @param [Symbol|String] metric_name Metric name + # @param [Hash] options Query options + def get_series(metric_name, options={}) + raise ArgumentError, ":resolution and :duration or :start_time must be set" if options.empty? query = options.dup if query[:start_time].respond_to?(:year) query[:start_time] = query[:start_time].to_i @@ -190,14 +206,10 @@ def get_measurement(name, options={}) unless query[:start_time] || query[:end_time] query[:duration] ||= 3600 end - url = connection.build_url("measurements/#{name}", query) + url = connection.build_url("measurements/#{metric_name}", query) response = connection.get(url) - SmartJSON.read(response.body) - end - - def get_series(name, options={}) - raise ArgumentError, ":resolution and :duration or :start_time must be set" if options.empty? - get_measurement(name, options)["series"] + parsed = SmartJSON.read(response.body) + parsed["series"] end # Retrieve data points for a specific metric diff --git a/spec/integration/metrics_spec.rb b/spec/integration/metrics_spec.rb index 2c6c5bd..4029be4 100644 --- a/spec/integration/metrics_spec.rb +++ b/spec/integration/metrics_spec.rb @@ -360,20 +360,11 @@ module Librato end - describe "#get_measurement" do - before { Metrics.submit test: { value: 123, tags: { hostname: "metrics-web-stg-1" } } } - - it "gets measurement" do - measurement = Metrics.get_measurement :test, resolution: 1, duration: 3600 - - expect(measurement["series"][0]["tags"]["hostname"]).to eq("metrics-web-stg-1") - expect(measurement["series"][0]["measurements"][0]["value"]).to eq(123) - end - end - describe "#get_series" do + before { Metrics.submit test_series: { value: 123, tags: { hostname: "metrics-web-stg-1" } } } + it "gets series" do - series = Metrics.get_series :test, resolution: 1, duration: 3600 + series = Metrics.get_series :test_series, resolution: 1, duration: 3600 expect(series[0]["tags"]["hostname"]).to eq("metrics-web-stg-1") expect(series[0]["measurements"][0]["value"]).to eq(123) From 4638dbde0d3d8058ebf56907e6a2b4ce08849ff5 Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Fri, 30 Sep 2016 17:20:49 -0700 Subject: [PATCH 46/55] do not alter global tags state after initialization --- lib/librato/metrics/processor.rb | 6 ----- spec/unit/metrics/aggregator_spec.rb | 39 +--------------------------- spec/unit/metrics/queue_spec.rb | 39 +--------------------------- 3 files changed, 2 insertions(+), 82 deletions(-) diff --git a/lib/librato/metrics/processor.rb b/lib/librato/metrics/processor.rb index 9dd2986..23a4d4e 100644 --- a/lib/librato/metrics/processor.rb +++ b/lib/librato/metrics/processor.rb @@ -8,8 +8,6 @@ module Metrics module Processor extend Forwardable - def_delegator :@tags, :clear, :clear_tags - MEASUREMENTS_PER_REQUEST = 500 attr_reader :per_request, :last_submit_time @@ -19,10 +17,6 @@ def tags @tags ||= {} end - def add_tags(tags) - @tags.merge!(tags) - end - # The current Client instance this queue is using to authenticate # and connect to Librato Metrics. This will default to the primary # client used by the Librato::Metrics module unless it has been diff --git a/spec/unit/metrics/aggregator_spec.rb b/spec/unit/metrics/aggregator_spec.rb index 1381f3e..a549336 100644 --- a/spec/unit/metrics/aggregator_spec.rb +++ b/spec/unit/metrics/aggregator_spec.rb @@ -95,47 +95,10 @@ module Metrics end end - describe "#add_tags" do - context "when no existing tags" do - it "adds top-level tags" do - expected_tags = { instance_id: "i-1234567c" } - subject.add_tags expected_tags - - expect(subject.tags).to be_a(Hash) - expect(subject.tags).to eq(expected_tags) - end - end - - context "when existing tags" do - it "merges tags" do - tmp1 = { instance_id: "i-1234567d" } - tmp2 = { region: "us-east-1", hostname: "metrics-web-stg-1" } - expected_tags = tmp1.merge(tmp2) - - subject.add_tags tmp1 - subject.add_tags tmp2 - - expect(subject.tags).to be_a(Hash) - expect(subject.tags).to eq(expected_tags) - end - end - end - - describe "#clear_tags" do - context "when tags are set" do - it "empties Hash" do - expected_tags = { instance_id: "i-1234567e" } - subject.add_tags expected_tags - - expect{subject.clear_tags}.to change{subject.tags}.from(expected_tags).to({}) - end - end - end - describe "#has_tags?" do context "when tags are set" do it "returns true" do - subject.add_tags instance_id: "i-1234567f" + subject.tags = { instance_id: "i-1234567f" } expect(subject.has_tags?).to eq(true) end diff --git a/spec/unit/metrics/queue_spec.rb b/spec/unit/metrics/queue_spec.rb index c72f605..f598cd6 100644 --- a/spec/unit/metrics/queue_spec.rb +++ b/spec/unit/metrics/queue_spec.rb @@ -88,47 +88,10 @@ module Metrics end end - describe "#add_tags" do - context "when no existing tags" do - it "adds top-level tags" do - expected_tags = { instance_id: "i-1234567c" } - subject.add_tags expected_tags - - expect(subject.tags).to be_a(Hash) - expect(subject.tags).to eq(expected_tags) - end - end - - context "when existing tags" do - it "merges tags" do - tmp1 = { instance_id: "i-1234567d" } - tmp2 = { region: "us-east-1", hostname: "metrics-web-stg-1" } - expected_tags = tmp1.merge(tmp2) - - subject.add_tags tmp1 - subject.add_tags tmp2 - - expect(subject.tags).to be_a(Hash) - expect(subject.tags).to eq(expected_tags) - end - end - end - - describe "#clear_tags" do - context "when tags are set" do - it "empties Hash" do - expected_tags = { instance_id: "i-1234567e" } - subject.add_tags expected_tags - - expect{subject.clear_tags}.to change{subject.tags}.from(expected_tags).to({}) - end - end - end - describe "#has_tags?" do context "when tags are set" do it "returns true" do - subject.add_tags instance_id: "i-1234567f" + subject.tags = { instance_id: "i-1234567f" } expect(subject.has_tags?).to eq(true) end From 539d2a13daeec6c482f7cc68bd7e113b32070556 Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Fri, 30 Sep 2016 17:23:18 -0700 Subject: [PATCH 47/55] update conditional --- lib/librato/metrics/aggregator.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/librato/metrics/aggregator.rb b/lib/librato/metrics/aggregator.rb index e6093fa..dac669c 100644 --- a/lib/librato/metrics/aggregator.rb +++ b/lib/librato/metrics/aggregator.rb @@ -63,8 +63,7 @@ def add(measurements) if data[:source] metric = "#{metric}#{SEPARATOR}#{data[:source]}" entry[:source] = data[:source].to_s - end - if data[:tags] && data[:tags].respond_to?(:each) + elsif data[:tags] && data[:tags].respond_to?(:each) metric = metric.to_s data[:tags].sort.each do |key, value| metric = "#{metric}#{SEPARATOR}#{key}#{SEPARATOR}#{value}" From f1d4d3236aec7f54e13afabeb9a2859b0dcd709a Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Fri, 30 Sep 2016 17:28:59 -0700 Subject: [PATCH 48/55] update integration test --- spec/integration/metrics/queue_spec.rb | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/spec/integration/metrics/queue_spec.rb b/spec/integration/metrics/queue_spec.rb index 7d1c12a..86c3617 100644 --- a/spec/integration/metrics/queue_spec.rb +++ b/spec/integration/metrics/queue_spec.rb @@ -73,20 +73,20 @@ module Metrics end context "with tags" do - let(:queue) { Queue.new(tags: { hostname: "metrics-web-stg-1" } ) } + let(:queue) { Queue.new(tags: { hostname: "metrics-web-stg-1" }) } it "respects default and individual tags" do queue.add test_1: 123 queue.add test_2: { value: 456, tags: { hostname: "metrics-web-stg-2" }} queue.submit - test_1 = Librato::Metrics.get_measurement :test_1, resolution: 1, duration: 3600 - expect(test_1["series"][0]["tags"]["hostname"]).to eq("metrics-web-stg-1") - expect(test_1["series"][0]["measurements"][0]["value"]).to eq(123) + test_1 = Librato::Metrics.get_series :test_1, resolution: 1, duration: 3600 + expect(test_1[0]["tags"]["hostname"]).to eq("metrics-web-stg-1") + expect(test_1[0]["measurements"][0]["value"]).to eq(123) - test_2 = Librato::Metrics.get_measurement :test_2, resolution: 1, duration: 3600 - expect(test_2["series"][0]["tags"]["hostname"]).to eq("metrics-web-stg-2") - expect(test_2["series"][0]["measurements"][0]["value"]).to eq(456) + test_2 = Librato::Metrics.get_series :test_2, resolution: 1, duration: 3600 + expect(test_2[0]["tags"]["hostname"]).to eq("metrics-web-stg-2") + expect(test_2[0]["measurements"][0]["value"]).to eq(456) end end From 62a903ea2f0d8342902bc2757fe7f112b011a69a Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Fri, 30 Sep 2016 17:49:41 -0700 Subject: [PATCH 49/55] update hash key separator for k,v pairs --- lib/librato/metrics/aggregator.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/librato/metrics/aggregator.rb b/lib/librato/metrics/aggregator.rb index dac669c..efcc89a 100644 --- a/lib/librato/metrics/aggregator.rb +++ b/lib/librato/metrics/aggregator.rb @@ -66,7 +66,7 @@ def add(measurements) elsif data[:tags] && data[:tags].respond_to?(:each) metric = metric.to_s data[:tags].sort.each do |key, value| - metric = "#{metric}#{SEPARATOR}#{key}#{SEPARATOR}#{value}" + metric = "#{metric}#{SEPARATOR}#{key}=#{value}" end entry[:tags] = data[:tags] end From e84b1934c69cf382b206db242a7b86d3c861597d Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Wed, 5 Oct 2016 17:00:43 -0700 Subject: [PATCH 50/55] remove measurements from PLURAL_TYPES for clarity --- lib/librato/metrics.rb | 2 +- lib/librato/metrics/aggregator.rb | 15 ++++---- lib/librato/metrics/persistence/direct.rb | 8 +++-- lib/librato/metrics/processor.rb | 6 ---- lib/librato/metrics/queue.rb | 42 ++++++++++++++--------- 5 files changed, 40 insertions(+), 33 deletions(-) diff --git a/lib/librato/metrics.rb b/lib/librato/metrics.rb index 35940ab..b2a433e 100644 --- a/lib/librato/metrics.rb +++ b/lib/librato/metrics.rb @@ -64,7 +64,7 @@ module Librato module Metrics extend SingleForwardable - TYPES = [:counter, :gauge, :measurement] + TYPES = [:counter, :gauge] PLURAL_TYPES = TYPES.map { |type| "#{type}s".to_sym } MIN_MEASURE_TIME = (Time.now-(3600*24*365)).to_i diff --git a/lib/librato/metrics/aggregator.rb b/lib/librato/metrics/aggregator.rb index efcc89a..67d24d3 100644 --- a/lib/librato/metrics/aggregator.rb +++ b/lib/librato/metrics/aggregator.rb @@ -101,7 +101,7 @@ def clear # def queued entries = [] - contains_measurements = multidimensional? + multidimensional = has_tags? @aggregated.each_value do |data| entry = { @@ -113,16 +113,17 @@ def queued max: data[:aggregate].max.to_f # TODO: expose v.sum2 and include } - entry[:source] = data[:source] if data[:source] - if data[:tags] - contains_measurements = true + if data[:source] + entry[:source] = data[:source] + elsif data[:tags] + multidimensional = true entry[:tags] = data[:tags] end - contains_measurements = true if data[:time] + multidimensional = true if data[:time] entries << entry end req = - if contains_measurements + if multidimensional { measurements: entries } else { gauges: entries } @@ -131,7 +132,7 @@ def queued req[:tags] = @tags if has_tags? req[:measure_time] = @measure_time if @measure_time req[:time] = @time if @time - req[:multidimensional] = true if contains_measurements + req[:multidimensional] = true if multidimensional req end diff --git a/lib/librato/metrics/persistence/direct.rb b/lib/librato/metrics/persistence/direct.rb index 0f3a9e2..74a8cfe 100644 --- a/lib/librato/metrics/persistence/direct.rb +++ b/lib/librato/metrics/persistence/direct.rb @@ -37,7 +37,7 @@ def chunk_queued(queued, per_request) reqs = [] # separate metric-containing values from global values globals = fetch_globals(queued) - Librato::Metrics::PLURAL_TYPES.each do |metric_type| + top_level_keys.each do |metric_type| metrics = queued[metric_type] next unless metrics if metrics.size <= per_request @@ -57,8 +57,12 @@ def build_request(type, metrics, globals) {type => metrics}.merge(globals) end + def top_level_keys + [Librato::Metrics::PLURAL_TYPES, :measurements].flatten + end + def fetch_globals(queued) - queued.reject {|k, v| Librato::Metrics::PLURAL_TYPES.include?(k)} + queued.reject { |k, v| top_level_keys.include?(k) } end def queue_count(queued) diff --git a/lib/librato/metrics/processor.rb b/lib/librato/metrics/processor.rb index 23a4d4e..b2825e0 100644 --- a/lib/librato/metrics/processor.rb +++ b/lib/librato/metrics/processor.rb @@ -6,8 +6,6 @@ module Metrics # Mixin which provides common logic between {Queue} and {Aggregator} # objects. module Processor - extend Forwardable - MEASUREMENTS_PER_REQUEST = 500 attr_reader :per_request, :last_submit_time @@ -32,10 +30,6 @@ def has_tags? end alias :tags? :has_tags? - def multidimensional? - has_tags? || !@time.nil? - end - # The object this MetricSet will use to persist # def persister diff --git a/lib/librato/metrics/queue.rb b/lib/librato/metrics/queue.rb index 28cd63f..9e55d14 100644 --- a/lib/librato/metrics/queue.rb +++ b/lib/librato/metrics/queue.rb @@ -28,7 +28,7 @@ def initialize(opts={}) # @return [Queue] returns self def add(measurements) measurements.each do |key, value| - contains_measurements = multidimensional? + multidimensional = has_tags? if value.respond_to?(:each) validate_parameters(value) metric = value @@ -41,10 +41,9 @@ def add(measurements) if @prefix metric[:name] = "#{@prefix}.#{metric[:name]}" end - contains_measurements = true if metric[:tags] - type = :measurement if contains_measurements + multidimensional = true if metric[:tags] type = ("#{type}s").to_sym - time = contains_measurements ? :time : :measure_time + time = multidimensional ? :time : :measure_time if metric[time] metric[time] = metric[time].to_i @@ -52,11 +51,15 @@ def add(measurements) elsif !skip_measurement_times metric[time] = epoch_time end - contains_measurements = true if metric[:time] - - @queued[type] ||= [] - @queued[type] << metric - @queued[:multidimensional] = true if contains_measurements + multidimensional = true if metric[:time] + if multidimensional + @queued[:measurements] ||= [] + @queued[:measurements] << metric + else + @queued[type] ||= [] + @queued[type] << metric + end + @queued[:multidimensional] = true if multidimensional end submit_check self @@ -112,19 +115,24 @@ def merge!(mergeable) end Metrics::PLURAL_TYPES.each do |type| if to_merge[type] - measurements = - if multidimensional? - reconcile(to_merge[type], to_merge[:tags]) - else - reconcile(to_merge[type], to_merge[:source]) - end + payload = reconcile(to_merge[type], to_merge[:source]) if @queued[type] - @queued[type] += measurements + @queued[type] += payload else - @queued[type] = measurements + @queued[type] = payload end end end + + if to_merge[:measurements] + payload = reconcile(to_merge[:measurements], to_merge[:tags]) + if @queued[:measurements] + @queued[:measurements] += payload + else + @queued[:measurements] = payload + end + end + submit_check self end From b611c8e691e91e42257db2eecb8470bfac4e6b61 Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Thu, 6 Oct 2016 12:57:07 -0700 Subject: [PATCH 51/55] add Util --- lib/librato/metrics.rb | 1 + lib/librato/metrics/aggregator.rb | 5 +---- lib/librato/metrics/util.rb | 25 +++++++++++++++++++++++++ spec/unit/metrics/util_spec.rb | 23 +++++++++++++++++++++++ 4 files changed, 50 insertions(+), 4 deletions(-) create mode 100644 lib/librato/metrics/util.rb create mode 100644 spec/unit/metrics/util_spec.rb diff --git a/lib/librato/metrics.rb b/lib/librato/metrics.rb index b2a433e..fc37e39 100644 --- a/lib/librato/metrics.rb +++ b/lib/librato/metrics.rb @@ -13,6 +13,7 @@ require 'metrics/persistence' require 'metrics/queue' require 'metrics/smart_json' +require 'metrics/util' require 'metrics/version' module Librato diff --git a/lib/librato/metrics/aggregator.rb b/lib/librato/metrics/aggregator.rb index 67d24d3..b677bbd 100644 --- a/lib/librato/metrics/aggregator.rb +++ b/lib/librato/metrics/aggregator.rb @@ -64,10 +64,7 @@ def add(measurements) metric = "#{metric}#{SEPARATOR}#{data[:source]}" entry[:source] = data[:source].to_s elsif data[:tags] && data[:tags].respond_to?(:each) - metric = metric.to_s - data[:tags].sort.each do |key, value| - metric = "#{metric}#{SEPARATOR}#{key}=#{value}" - end + metric = Librato::Metrics::Util.build_key_for(metric.to_s, data[:tags]) entry[:tags] = data[:tags] end else diff --git a/lib/librato/metrics/util.rb b/lib/librato/metrics/util.rb new file mode 100644 index 0000000..8488758 --- /dev/null +++ b/lib/librato/metrics/util.rb @@ -0,0 +1,25 @@ +module Librato + module Metrics + + class Util + SEPARATOR = "%%" + + # Builds a Hash key from metric name and tags. + # + # @param metric_name [String] The unique identifying metric name of the property being tracked. + # @param tags [Hash] A set of name=value tag pairs that describe the particular data stream. + # @return [String] the Hash key + def self.build_key_for(metric_name, tags) + key_name = metric_name + tags.sort.each do |key, value| + k = key.to_s.downcase + v = value.is_a?(String) ? value.downcase : value + key_name = "#{key_name}#{SEPARATOR}#{k}=#{v}" + end + key_name + end + + end + + end +end diff --git a/spec/unit/metrics/util_spec.rb b/spec/unit/metrics/util_spec.rb new file mode 100644 index 0000000..c19bb23 --- /dev/null +++ b/spec/unit/metrics/util_spec.rb @@ -0,0 +1,23 @@ +require "spec_helper" + +module Librato + module Metrics + + describe Util do + + describe "#build_key_for" do + it "builds a Hash key" do + metric_name = "requests" + tags = { status: 200, MeThoD: "GET", controller: "users", ACTION: "show" } + expected = "requests%%action=show%%method=get%%controller=users%%status=200" + actual = Util.build_key_for(metric_name, tags) + + expect(expected).to eq(actual) + end + + end + + end + + end +end From 7673919e663c088b99345ba710aecbae7173f47b Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Thu, 6 Oct 2016 14:58:50 -0700 Subject: [PATCH 52/55] rename block parameter --- lib/librato/metrics/persistence/direct.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/librato/metrics/persistence/direct.rb b/lib/librato/metrics/persistence/direct.rb index 74a8cfe..ec2e845 100644 --- a/lib/librato/metrics/persistence/direct.rb +++ b/lib/librato/metrics/persistence/direct.rb @@ -37,16 +37,16 @@ def chunk_queued(queued, per_request) reqs = [] # separate metric-containing values from global values globals = fetch_globals(queued) - top_level_keys.each do |metric_type| - metrics = queued[metric_type] + top_level_keys.each do |key| + metrics = queued[key] next unless metrics if metrics.size <= per_request # we can fit all of this metric type in a single request - reqs << build_request(metric_type, metrics, globals) + reqs << build_request(key, metrics, globals) else # going to have to split things up metrics.each_slice(per_request) do |elements| - reqs << build_request(metric_type, elements, globals) + reqs << build_request(key, elements, globals) end end end From 78f4e1f86bc30065526484b6f5adce937757079b Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Fri, 7 Oct 2016 10:11:14 -0700 Subject: [PATCH 53/55] internally accept time or measure_time --- lib/librato/metrics/aggregator.rb | 5 ++--- lib/librato/metrics/persistence/direct.rb | 11 ++++------- lib/librato/metrics/processor.rb | 6 +----- lib/librato/metrics/queue.rb | 7 +++---- spec/unit/metrics/aggregator_spec.rb | 21 ++------------------- spec/unit/metrics/queue_spec.rb | 21 ++------------------- 6 files changed, 14 insertions(+), 57 deletions(-) diff --git a/lib/librato/metrics/aggregator.rb b/lib/librato/metrics/aggregator.rb index b677bbd..66fee12 100644 --- a/lib/librato/metrics/aggregator.rb +++ b/lib/librato/metrics/aggregator.rb @@ -119,6 +119,7 @@ def queued multidimensional = true if data[:time] entries << entry end + time = multidimensional ? :time : :measure_time req = if multidimensional { measurements: entries } @@ -127,9 +128,7 @@ def queued end req[:source] = @source if @source req[:tags] = @tags if has_tags? - req[:measure_time] = @measure_time if @measure_time - req[:time] = @time if @time - req[:multidimensional] = true if multidimensional + req[time] = @time if @time req end diff --git a/lib/librato/metrics/persistence/direct.rb b/lib/librato/metrics/persistence/direct.rb index ec2e845..cfb4fef 100644 --- a/lib/librato/metrics/persistence/direct.rb +++ b/lib/librato/metrics/persistence/direct.rb @@ -17,12 +17,10 @@ def persist(client, queued, options={}) end requests.each do |request| resource = - if queued[:tags] || queued[:multidimensional] - # request contains per-measurement tags - queued.delete(:multidimensional) if queued[:multidimensional] - "measurements" - else + if queued[:gauges] || queued[:counters] "metrics" + else + "measurements" end payload = SmartJSON.write(request) # expects 200 @@ -66,8 +64,7 @@ def fetch_globals(queued) end def queue_count(queued) - queued.reject { |key| key == :multidimensional } - .inject(0) { |result, data| result + data.last.size } + queued.inject(0) { |result, data| result + data.last.size } end end diff --git a/lib/librato/metrics/processor.rb b/lib/librato/metrics/processor.rb index b2825e0..5524b7a 100644 --- a/lib/librato/metrics/processor.rb +++ b/lib/librato/metrics/processor.rb @@ -99,8 +99,7 @@ def setup_common_options(options) @per_request = options[:per_request] || MEASUREMENTS_PER_REQUEST @source = options[:source] @tags = options.fetch(:tags, {}) - @measure_time = options[:measure_time] && options[:measure_time].to_i - @time = options[:time] && options[:time].to_i + @time = (options[:time] && options[:time].to_i || options[:measure_time] && options[:measure_time].to_i) @create_time = Time.now @clear_on_failure = options[:clear_failures] || false @prefix = options[:prefix] @@ -116,9 +115,6 @@ def autosubmit_check def validate_parameters(options) invalid_combinations = [ [:source, :tags], - [:measure_time, :time], - [:source, :time], - [:measure_time, :tags] ] opts = options.keys.to_set invalid_combinations.each do |combo| diff --git a/lib/librato/metrics/queue.rb b/lib/librato/metrics/queue.rb index 9e55d14..f3980de 100644 --- a/lib/librato/metrics/queue.rb +++ b/lib/librato/metrics/queue.rb @@ -59,7 +59,6 @@ def add(measurements) @queued[type] ||= [] @queued[type] << metric end - @queued[:multidimensional] = true if multidimensional end submit_check self @@ -143,10 +142,10 @@ def merge!(mergeable) def queued return {} if @queued.empty? globals = {} + time = has_tags? ? :time : :measure_time + globals[time] = @time if @time globals[:source] = @source if @source globals[:tags] = @tags if has_tags? - globals[:measure_time] = @measure_time if @measure_time - globals[:time] = @time if @time @queued.merge(globals) end @@ -154,7 +153,7 @@ def queued # # @return Integer def size - self.queued.reject { |key| key == :multidimensional }.inject(0) { |result, data| result + data.last.size } + self.queued.inject(0) { |result, data| result + data.last.size } end alias :length :size diff --git a/spec/unit/metrics/aggregator_spec.rb b/spec/unit/metrics/aggregator_spec.rb index a549336..381de9f 100644 --- a/spec/unit/metrics/aggregator_spec.rb +++ b/spec/unit/metrics/aggregator_spec.rb @@ -55,14 +55,6 @@ module Metrics tags: { hostname: "metrics-web-stg-1" } ) }.to raise_error(InvalidParameters) - expect { Aggregator.new(measure_time: Time.now, time: Time.now) }.to raise_error(InvalidParameters) - expect { Aggregator.new(source: "metrics-web-stg-1", time: Time.now) }.to raise_error(InvalidParameters) - expect { - Aggregator.new( - measure_time: Time.now, - tags: { hostname: "metrics-web-stg-1" } - ) - }.to raise_error(InvalidParameters) end end end @@ -121,15 +113,6 @@ module Metrics expect { subject.add test: { source: "metrics-web-stg-1", tags: { hostname: "metrics-web-stg-1" }, value: 123 } }.to raise_error(InvalidParameters) - expect { - subject.add test: { measure_time: Time.now, time: Time.now, value: 123 } - }.to raise_error(InvalidParameters) - expect { - subject.add test: { source: "metrics-web-stg-1", time: Time.now, value: 123 } - }.to raise_error(InvalidParameters) - expect { - subject.add test: { tags: { hostname: "metrics-web-stg-1" }, measure_time: Time.now, value: 123 } - }.to raise_error(InvalidParameters) end end @@ -331,7 +314,7 @@ module Metrics it "includes global measure_time if set" do measure_time = (Time.now-1000).to_i - a = Aggregator.new(measure_time: measure_time) + a = Aggregator.new(source: "foo", measure_time: measure_time) a.add foo: 12 expect(a.queued[:measure_time]).to eq(measure_time) end @@ -349,7 +332,7 @@ module Metrics context "when time is set" do it "includes global time" do expected_time = (Time.now-1000).to_i - subject = Aggregator.new(time: expected_time) + subject = Aggregator.new(tags: { foo: "bar" }, time: expected_time) subject.add test: 10 expect(subject.queued[:time]).to eq(expected_time) diff --git a/spec/unit/metrics/queue_spec.rb b/spec/unit/metrics/queue_spec.rb index f598cd6..6138475 100644 --- a/spec/unit/metrics/queue_spec.rb +++ b/spec/unit/metrics/queue_spec.rb @@ -48,14 +48,6 @@ module Metrics tags: { hostname: "metrics-web-stg-1" } ) }.to raise_error(InvalidParameters) - expect { Queue.new(measure_time: Time.now, time: Time.now) }.to raise_error(InvalidParameters) - expect { Queue.new(source: "metrics-web-stg-1", time: Time.now) }.to raise_error(InvalidParameters) - expect { - Queue.new( - measure_time: Time.now, - tags: { hostname: "metrics-web-stg-1" } - ) - }.to raise_error(InvalidParameters) end end end @@ -114,15 +106,6 @@ module Metrics expect { subject.add test: { source: "metrics-web-stg-1", tags: { hostname: "metrics-web-stg-1" }, value: 123 } }.to raise_error(InvalidParameters) - expect { - subject.add test: { measure_time: Time.now, time: Time.now, value: 123 } - }.to raise_error(InvalidParameters) - expect { - subject.add test: { source: "metrics-web-stg-1", time: Time.now, value: 123 } - }.to raise_error(InvalidParameters) - expect { - subject.add test: { tags: { hostname: "metrics-web-stg-1" }, measure_time: Time.now, value: 123 } - }.to raise_error(InvalidParameters) end end @@ -478,7 +461,7 @@ module Metrics it "includes global measure_time if set" do measure_time = (Time.now-1000).to_i - q = Queue.new(measure_time: measure_time) + q = Queue.new(source: "foo", measure_time: measure_time) q.add foo: 12 expect(q.queued[:measure_time]).to eq(measure_time) end @@ -495,7 +478,7 @@ module Metrics context "when time is set" do it "includes global time" do expected_time = (Time.now-1000).to_i - queue = Queue.new(time: expected_time) + queue = Queue.new(tags: { foo: "bar" }, time: expected_time) queue.add test: 10 expect(queue.queued[:time]).to eq(expected_time) end From 8d748b35649e8674d388e662ff03bc3d26e46f90 Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Fri, 7 Oct 2016 10:22:10 -0700 Subject: [PATCH 54/55] update #check_measure_time --- lib/librato/metrics/queue.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/librato/metrics/queue.rb b/lib/librato/metrics/queue.rb index f3980de..21ee6d2 100644 --- a/lib/librato/metrics/queue.rb +++ b/lib/librato/metrics/queue.rb @@ -160,11 +160,11 @@ def size private def check_measure_time(data) - invalid_time = - data[:measure_time] && data[:measure_time] < Metrics::MIN_MEASURE_TIME || - data[:time] && data[:time] < Metrics::MIN_MEASURE_TIME + time_keys = [:measure_time, :time] - raise InvalidMeasureTime, "Measure time for submitted metric (#{data}) is invalid." if invalid_time + if time_keys.any? { |key| data[key] && data[key] < Metrics::MIN_MEASURE_TIME } + raise InvalidMeasureTime, "Measure time for submitted metric (#{data}) is invalid." + end end def reconcile(measurements, val) From 9c048389fce490840bbbbfd67c9accbf238f54ab Mon Sep 17 00:00:00 2001 From: Chance Feick Date: Thu, 20 Oct 2016 12:02:06 -0700 Subject: [PATCH 55/55] convert legacy measure_time to time --- lib/librato/metrics/queue.rb | 12 ++++++------ spec/unit/metrics/queue_spec.rb | 14 ++++++++++++++ 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/lib/librato/metrics/queue.rb b/lib/librato/metrics/queue.rb index 21ee6d2..78c9d54 100644 --- a/lib/librato/metrics/queue.rb +++ b/lib/librato/metrics/queue.rb @@ -41,17 +41,17 @@ def add(measurements) if @prefix metric[:name] = "#{@prefix}.#{metric[:name]}" end - multidimensional = true if metric[:tags] + multidimensional = true if metric[:tags] || metric[:time] type = ("#{type}s").to_sym - time = multidimensional ? :time : :measure_time + time_key = multidimensional ? :time : :measure_time + metric[:time] = metric.delete(:measure_time) if multidimensional && metric[:measure_time] - if metric[time] - metric[time] = metric[time].to_i + if metric[time_key] + metric[time_key] = metric[time_key].to_i check_measure_time(metric) elsif !skip_measurement_times - metric[time] = epoch_time + metric[time_key] = epoch_time end - multidimensional = true if metric[:time] if multidimensional @queued[:measurements] ||= [] @queued[:measurements] << metric diff --git a/spec/unit/metrics/queue_spec.rb b/spec/unit/metrics/queue_spec.rb index 6138475..163f34f 100644 --- a/spec/unit/metrics/queue_spec.rb +++ b/spec/unit/metrics/queue_spec.rb @@ -238,6 +238,20 @@ module Metrics expect(queue.queued[:tags]).to be_nil expect(queue.queued[:measurements].first).to eq(expected) end + + it "converts legacy measure_time to time" do + expected_time = Time.now.to_i + expected_tags = { foo: "bar" } + expected = { + measurements: [{ + name: "test", value: 1, tags: expected_tags, time: expected_time + }] + } + + subject.add test: { value: 1, tags: expected_tags, measure_time: expected_time } + + expect(subject.queued).to equal_unordered(expected) + end end context "when Queue is initialized with tags and when tags are used as arguments" do