From f96286e6b0fb5fcebc554d6677259b3b5bb59d74 Mon Sep 17 00:00:00 2001 From: nakedsushi Date: Mon, 11 May 2020 15:51:47 -0700 Subject: [PATCH 01/14] PLAT-425 #time 5m add concurrent-ruby and update version --- CHANGELOG.md | 4 ++++ Gemfile | 1 + lib/datadog/statsd/version.rb | 2 +- spec/statsd/version_spec.rb | 4 ++-- 4 files changed, 8 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1cdc4060..08962cdd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # CHANGELOG +## Unreleased + + * Try to make this thread safe + [//]: # (comment: Don't forget to update lib/datadog/statsd.rb:DogStatsd::Statsd::VERSION when releasing a new version) ## 4.8.0 / 2020.04.20 diff --git a/Gemfile b/Gemfile index 3dccdfed..eb0e8901 100644 --- a/Gemfile +++ b/Gemfile @@ -7,6 +7,7 @@ gem 'minitest-matchers' gem 'yard', '~> 0.9.20' gem 'single_cov' gem 'climate_control' +gem 'concurrent-ruby' if RUBY_VERSION >= '2.0.0' gem 'rubocop', '~> 0.50.0' # bump this and TargetRubyVersion once we drop ruby 2.0 diff --git a/lib/datadog/statsd/version.rb b/lib/datadog/statsd/version.rb index 95d4e31d..fc729720 100644 --- a/lib/datadog/statsd/version.rb +++ b/lib/datadog/statsd/version.rb @@ -4,6 +4,6 @@ module Datadog class Statsd - VERSION = '4.8.0' + VERSION = '4.8.1' end end diff --git a/spec/statsd/version_spec.rb b/spec/statsd/version_spec.rb index 455ecab4..9f4a0b4c 100644 --- a/spec/statsd/version_spec.rb +++ b/spec/statsd/version_spec.rb @@ -3,7 +3,7 @@ describe Datadog::Statsd do describe 'VERSION' do it 'has a version' do - expect(Datadog::Statsd::VERSION).to eq '4.8.0' + expect(Datadog::Statsd::VERSION).to eq '4.8.1' end end -end \ No newline at end of file +end From 3afadb43593f9d6b5237ea1bc85f0c8cbbd983a0 Mon Sep 17 00:00:00 2001 From: nakedsushi Date: Tue, 12 May 2020 10:20:04 -0700 Subject: [PATCH 02/14] PLAT-425 #time 2h redo thread safety --- lib/datadog/statsd.rb | 6 ++- lib/datadog/statsd/batch.rb | 36 ++++++++++------ lib/datadog/statsd/connection.rb | 64 ++++++++++++++++------------ lib/datadog/statsd/udp_connection.rb | 6 ++- spec/statsd_spec.rb | 26 +++++------ 5 files changed, 81 insertions(+), 57 deletions(-) diff --git a/lib/datadog/statsd.rb b/lib/datadog/statsd.rb index 4cb97751..45bfaa5d 100644 --- a/lib/datadog/statsd.rb +++ b/lib/datadog/statsd.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + require 'socket' require_relative 'statsd/version' @@ -42,6 +43,7 @@ class Statsd DISTRIBUTION_TYPE = 'd' TIMING_TYPE = 'ms' SET_TYPE = 's' + UNIT_MS = ['un:ms'].freeze # A namespace to prepend to all statsd calls. Defaults to no namespace. attr_reader :namespace @@ -110,6 +112,8 @@ def initialize( @sample_rate = sample_rate + @buffer = Concurrent::Array.new + # we reduce max_buffer_bytes by a the rough estimate of the telemetry payload @batch = Batch.new(connection, (max_buffer_bytes - telemetry.estimate_max_size)) end @@ -220,7 +224,7 @@ def distribution(stat, value, opts = EMPTY_OPTIONS) # @option opts [Array] :tags An array of tags def timing(stat, ms, opts = EMPTY_OPTIONS) opts = { sample_rate: opts } if opts.is_a?(Numeric) - send_stats(stat, ms, TIMING_TYPE, opts) + send_stats stat, ms, TIMING_TYPE, opts.merge(tags: tags + UNIT_MS) end # Reports execution time of the provided block using {#timing}. diff --git a/lib/datadog/statsd/batch.rb b/lib/datadog/statsd/batch.rb index 84e12a66..f41f0c94 100644 --- a/lib/datadog/statsd/batch.rb +++ b/lib/datadog/statsd/batch.rb @@ -1,9 +1,16 @@ # frozen_string_literal: true +require 'concurrent' +require 'monitor' + module Datadog class Statsd class Batch + include MonitorMixin + def initialize(connection, max_buffer_bytes) + super() + @connection = connection @max_buffer_bytes = max_buffer_bytes @depth = 0 @@ -11,11 +18,10 @@ def initialize(connection, max_buffer_bytes) end def open - @depth += 1 - + synchronize { @depth += 1 } yield ensure - @depth -= 1 + synchronize { @depth -= 1 } flush if !open? end @@ -24,19 +30,21 @@ def open? end def add(message) - message_bytes = message.bytesize - - unless @buffer_bytes == 0 - if @buffer_bytes + 1 + message_bytes >= @max_buffer_bytes - flush - else - @buffer << "\n" - @buffer_bytes += 1 + synchronize do + message_bytes = message.bytesize + + unless @buffer_bytes == 0 + if @buffer_bytes + 1 + message_bytes >= @max_buffer_bytes + flush + else + @buffer << "\n" + @buffer_bytes += 1 + end end - end - @buffer << message - @buffer_bytes += message_bytes + @buffer << message + @buffer_bytes += message_bytes + end end def flush diff --git a/lib/datadog/statsd/connection.rb b/lib/datadog/statsd/connection.rb index a98bd88d..87e3126d 100644 --- a/lib/datadog/statsd/connection.rb +++ b/lib/datadog/statsd/connection.rb @@ -1,9 +1,15 @@ # frozen_string_literal: true +require 'concurrent' +require 'monitor' + module Datadog class Statsd class Connection + include MonitorMixin + def initialize(telemetry) + super() @telemetry = telemetry end @@ -18,36 +24,38 @@ def close end def write(payload) - logger.debug { "Statsd: #{payload}" } if logger - - flush_telemetry = telemetry.flush? - - payload += telemetry.flush if flush_telemetry - - send_message(payload) - - telemetry.reset if flush_telemetry - - telemetry.sent(packets: 1, bytes: payload.length) - rescue StandardError => boom - # Try once to reconnect if the socket has been closed - retries ||= 1 - if retries <= 1 && - (boom.is_a?(Errno::ENOTCONN) or - boom.is_a?(Errno::ECONNREFUSED) or - boom.is_a?(IOError) && boom.message =~ /closed stream/i) - retries += 1 - begin - close - retry - rescue StandardError => e - boom = e + synchronize do + logger.debug { "Statsd: #{payload}" } if logger + + flush_telemetry = telemetry.flush? + + payload += telemetry.flush if flush_telemetry + + send_message(payload) + + telemetry.reset if flush_telemetry + + telemetry.sent(packets: 1, bytes: payload.length) + rescue StandardError => boom + # Try once to reconnect if the socket has been closed + retries ||= 1 + if retries <= 1 && + (boom.is_a?(Errno::ENOTCONN) or + boom.is_a?(Errno::ECONNREFUSED) or + boom.is_a?(IOError) && boom.message =~ /closed stream/i) + retries += 1 + begin + close + retry + rescue StandardError => e + boom = e + end end - end - telemetry.dropped(packets: 1, bytes: payload.length) - logger.error { "Statsd: #{boom.class} #{boom}" } if logger - nil + telemetry.dropped(packets: 1, bytes: payload.length) + logger.error { "Statsd: #{boom.class} #{boom}" } if logger + nil + end end private diff --git a/lib/datadog/statsd/udp_connection.rb b/lib/datadog/statsd/udp_connection.rb index 166b24e4..28cf411c 100644 --- a/lib/datadog/statsd/udp_connection.rb +++ b/lib/datadog/statsd/udp_connection.rb @@ -1,10 +1,14 @@ # frozen_string_literal: true +require 'concurrent' +require 'monitor' require_relative 'connection' module Datadog class Statsd class UDPConnection < Connection + include MonitorMixin + DEFAULT_HOST = '127.0.0.1' DEFAULT_PORT = 8125 @@ -25,7 +29,7 @@ def initialize(host, port, logger, telemetry) def connect UDPSocket.new.tap do |socket| - socket.connect(host, port) + synchronize { socket.connect(host, port) } end end diff --git a/spec/statsd_spec.rb b/spec/statsd_spec.rb index 1ccbf53b..df463d38 100644 --- a/spec/statsd_spec.rb +++ b/spec/statsd_spec.rb @@ -464,7 +464,7 @@ let(:sample_rate) { nil } let(:tags) { nil } - it_behaves_like 'a metrics method', 'foobar:500|ms' do + it_behaves_like 'a metrics method', 'foobar:500|ms|#un:ms' do let(:basic_action) do subject.timing('foobar', 500, tags: action_tags) end @@ -472,7 +472,7 @@ it 'sends the timing' do subject.timing('foobar', 500) - expect(socket.recv[0]).to eq_with_telemetry 'foobar:500|ms' + expect(socket.recv[0]).to eq_with_telemetry 'foobar:500|ms|#un:ms' end context 'with a sample rate' do @@ -482,7 +482,7 @@ it 'sends the timing with the sample rate' do subject.timing('foobar', 500, sample_rate: 0.5) - expect(socket.recv[0]).to eq_with_telemetry 'foobar:500|ms|@0.5' + expect(socket.recv[0]).to eq_with_telemetry 'foobar:500|ms|@0.5|#un:ms' end end @@ -493,7 +493,7 @@ it 'sends the timing with the sample rate' do subject.timing('foobar', 500, 0.5) - expect(socket.recv[0]).to eq_with_telemetry 'foobar:500|ms|@0.5' + expect(socket.recv[0]).to eq_with_telemetry 'foobar:500|ms|@0.5|#un:ms' end end end @@ -516,7 +516,7 @@ allow(Process).to receive(:clock_gettime).and_return(0) if Datadog::Statsd::PROCESS_TIME_SUPPORTED end - it_behaves_like 'a metrics method', 'foobar:1000|ms' do + it_behaves_like 'a metrics method', 'foobar:1000|ms|#un:ms' do let(:basic_action) do subject.time('foobar', tags: action_tags) do Timecop.travel(after_date) @@ -532,7 +532,7 @@ allow(Process).to receive(:clock_gettime).and_return(1) if Datadog::Statsd::PROCESS_TIME_SUPPORTED end - expect(socket.recv[0]).to eq_with_telemetry 'foobar:1000|ms' + expect(socket.recv[0]).to eq_with_telemetry 'foobar:1000|ms|#un:ms' end it 'ensures the timing is sent' do @@ -544,7 +544,7 @@ end rescue nil # rubocop:enable Lint/RescueWithoutErrorClass - expect(socket.recv[0]).to eq_with_telemetry 'foobar:1000|ms' + expect(socket.recv[0]).to eq_with_telemetry 'foobar:1000|ms|#un:ms' end end @@ -579,7 +579,7 @@ allow(Process).to receive(:clock_gettime).and_return(1) if Datadog::Statsd::PROCESS_TIME_SUPPORTED end - expect(socket.recv[0]).to eq_with_telemetry 'foobar:1000|ms|@0.5' + expect(socket.recv[0]).to eq_with_telemetry 'foobar:1000|ms|@0.5|#un:ms' end end @@ -594,7 +594,7 @@ allow(Process).to receive(:clock_gettime).and_return(1) if Datadog::Statsd::PROCESS_TIME_SUPPORTED end - expect(socket.recv[0]).to eq_with_telemetry 'foobar:1000|ms|@0.5' + expect(socket.recv[0]).to eq_with_telemetry 'foobar:1000|ms|@0.5|#un:ms' end end end @@ -1029,13 +1029,13 @@ def subject.telemetry s.event('ev', 'text') end - expect(socket.recv[0]).to eq_with_telemetry("test:1|c\ntest:-1|c\ntest:21|c\ntest:21|g\ntest:21|h\ntest:21|ms\ntest:21|s\n_sc|sc|0\n_e{2,4}:ev|text", + expect(socket.recv[0]).to eq_with_telemetry("test:1|c\ntest:-1|c\ntest:21|c\ntest:21|g\ntest:21|h\ntest:21|ms|#un:ms\ntest:21|s\n_sc|sc|0\n_e{2,4}:ev|text", metrics: 7, service_checks: 1, events: 1 ) - expect(subject.telemetry.flush).to eq_with_telemetry('', metrics: 0, service_checks: 0, events: 0, packets_sent: 1, bytes_sent: 766) + expect(subject.telemetry.flush).to eq_with_telemetry('', metrics: 0, service_checks: 0, events: 0, packets_sent: 1, bytes_sent: 773) end end @@ -1065,7 +1065,7 @@ def subject.telemetry subject.gauge('test', 21) expect(socket.recv[0]).to eq_with_telemetry('test:21|g', metrics: 3, service_checks: 0, events: 0, packets_dropped: 2, bytes_dropped: 1364) - expect(subject.telemetry.flush).to eq_with_telemetry('', metrics: 0, service_checks: 0, events: 0, packets_sent: 1, bytes_sent: 684) + expect(subject.telemetry.flush).to eq_with_telemetry('', metrics: 0, service_checks: 0, events: 0, packets_sent: 1, bytes_sent: 691) end end end @@ -1131,4 +1131,4 @@ class Datadog::Statsd::SomeClass; end expect(socket.recv[0]).to eq_with_telemetry 'stat:1|c|#yolo' end end -end \ No newline at end of file +end From 45bfbba81f28f1d618ffa7f2536896d8751cd6b3 Mon Sep 17 00:00:00 2001 From: nakedsushi Date: Tue, 12 May 2020 13:13:37 -0700 Subject: [PATCH 03/14] PLAT-425 #time 1h add un:ms to tags in specs --- lib/datadog/statsd.rb | 6 +++++- spec/shared/namespaceable_method.rb | 2 +- spec/shared/taggable_method.rb | 6 +++--- spec/statsd_spec.rb | 27 +++++++++++++++++++++++---- 4 files changed, 32 insertions(+), 9 deletions(-) diff --git a/lib/datadog/statsd.rb b/lib/datadog/statsd.rb index 45bfaa5d..52c24e29 100644 --- a/lib/datadog/statsd.rb +++ b/lib/datadog/statsd.rb @@ -224,7 +224,11 @@ def distribution(stat, value, opts = EMPTY_OPTIONS) # @option opts [Array] :tags An array of tags def timing(stat, ms, opts = EMPTY_OPTIONS) opts = { sample_rate: opts } if opts.is_a?(Numeric) - send_stats stat, ms, TIMING_TYPE, opts.merge(tags: tags + UNIT_MS) + tags = opts[:tags] || [] + if tags.is_a?(Array) + tags = tags + UNIT_MS + end + send_stats stat, ms, TIMING_TYPE, opts.merge(tags: tags) end # Reports execution time of the provided block using {#timing}. diff --git a/spec/shared/namespaceable_method.rb b/spec/shared/namespaceable_method.rb index 56e9b0d3..6950fb4a 100644 --- a/spec/shared/namespaceable_method.rb +++ b/spec/shared/namespaceable_method.rb @@ -19,4 +19,4 @@ expect(socket.recv[0]).to eq_with_telemetry("yolo.#{normal_expected_result}") end end -end \ No newline at end of file +end diff --git a/spec/shared/taggable_method.rb b/spec/shared/taggable_method.rb index bb79b0e1..a9ed1307 100644 --- a/spec/shared/taggable_method.rb +++ b/spec/shared/taggable_method.rb @@ -4,13 +4,13 @@ context 'when tags are an array of strings' do let(:action_tags) do - %w[country:usa state:ny other] + %w[country:usa state:ny other un:ms] end it 'supports tags' do basic_action - expect(socket.recv[0]).to eq_with_telemetry "#{normal_expected_result}|#country:usa,state:ny,other", telemetry_options + expect(socket.recv[0]).to eq_with_telemetry "#{normal_expected_result}|#country:usa,state:ny,other,un:ms", telemetry_options end context 'when there is global tags' do @@ -21,7 +21,7 @@ it 'merges global and provided tags' do basic_action - expect(socket.recv[0]).to match(/^#{normal_expected_result}|#global_tag,country:usa,state:ny,other/) + expect(socket.recv[0]).to match(/^#{normal_expected_result}|#global_tag,country:usa,state:ny,other,un:ms/) end end end diff --git a/spec/statsd_spec.rb b/spec/statsd_spec.rb index df463d38..d67266e5 100644 --- a/spec/statsd_spec.rb +++ b/spec/statsd_spec.rb @@ -463,8 +463,27 @@ let(:namespace) { nil } let(:sample_rate) { nil } let(:tags) { nil } + let(:action_tags) { [] } - it_behaves_like 'a metrics method', 'foobar:500|ms|#un:ms' do + # it_behaves_like 'a metrics method', 'foobar:500|ms|#un:ms' do + # let(:basic_action) do + # subject.timing('foobar', 500, tags: action_tags) + # end + # end + + it_behaves_like 'a namespaceable method', 'foobar:500|ms|#un:ms' do + let(:basic_action) do + subject.timing('foobar', 500, tags: action_tags) + end + end + + it_behaves_like 'a log debuggable method', 'foobar:500|ms|#un:ms' do + let(:basic_action) do + subject.timing('foobar', 500, tags: action_tags) + end + end + + it_behaves_like 'a taggable method', 'foobar:500|ms' do let(:basic_action) do subject.timing('foobar', 500, tags: action_tags) end @@ -996,10 +1015,10 @@ def subject.telemetry expect(socket.recv[0]).to eq_with_telemetry('test:21|h', metrics: 1, packets_sent: 1, bytes_sent: 683) subject.timing('test', 21) - expect(socket.recv[0]).to eq_with_telemetry('test:21|ms', metrics: 1, packets_sent: 1, bytes_sent: 683) + expect(socket.recv[0]).to eq_with_telemetry('test:21|ms|#un:ms', metrics: 1, packets_sent: 1, bytes_sent: 683) subject.set('test', 21) - expect(socket.recv[0]).to eq_with_telemetry('test:21|s', metrics: 1, packets_sent: 1, bytes_sent: 684) + expect(socket.recv[0]).to eq_with_telemetry('test:21|s', metrics: 1, packets_sent: 1, bytes_sent: 691) subject.service_check('sc', 0) expect(socket.recv[0]).to eq_with_telemetry('_sc|sc|0', metrics: 0, service_checks: 1, packets_sent: 1, bytes_sent: 683) @@ -1065,7 +1084,7 @@ def subject.telemetry subject.gauge('test', 21) expect(socket.recv[0]).to eq_with_telemetry('test:21|g', metrics: 3, service_checks: 0, events: 0, packets_dropped: 2, bytes_dropped: 1364) - expect(subject.telemetry.flush).to eq_with_telemetry('', metrics: 0, service_checks: 0, events: 0, packets_sent: 1, bytes_sent: 691) + expect(subject.telemetry.flush).to eq_with_telemetry('', metrics: 0, service_checks: 0, events: 0, packets_sent: 1, bytes_sent: 684) end end end From d5362a7fc29329da1636cbee8b5d6d5fffd027c7 Mon Sep 17 00:00:00 2001 From: nakedsushi Date: Tue, 12 May 2020 14:22:44 -0700 Subject: [PATCH 04/14] PLAT-425 #time 1m add ruby 2.6 at least --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 0949dc6a..79d3b26d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -8,6 +8,7 @@ rvm: - 2.3 - 2.4 - 2.5 + - 2.6 script: bundle exec rake $TASK env: - TASK=spec From 6d4edaf1b2c574188a86390e5d73d474678ee477 Mon Sep 17 00:00:00 2001 From: nakedsushi Date: Tue, 12 May 2020 15:21:39 -0700 Subject: [PATCH 05/14] PLAT-425 #time 1m don't care about rubies below 2.6 --- .travis.yml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/.travis.yml b/.travis.yml index 79d3b26d..0f239b7d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,12 +2,6 @@ sudo: false language: ruby rvm: - - 2.0 - - 2.1 - - 2.2 - - 2.3 - - 2.4 - - 2.5 - 2.6 script: bundle exec rake $TASK env: From 16d3dfa9cb67d3c59928d61eb417a3e70c370e7c Mon Sep 17 00:00:00 2001 From: nakedsushi Date: Tue, 12 May 2020 15:47:32 -0700 Subject: [PATCH 06/14] PLAT-425 #time 1m don't care about rubies below 2.6 --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 0f239b7d..bbe89085 100644 --- a/.travis.yml +++ b/.travis.yml @@ -9,5 +9,5 @@ env: matrix: include: env: TASK=rubocop - rvm: 2.5 + rvm: 2.6 bundler_args: --without=localdev From 76d0bac2010b831c621c4b2296e9b4c6303f3fc1 Mon Sep 17 00:00:00 2001 From: nakedsushi Date: Tue, 12 May 2020 15:57:45 -0700 Subject: [PATCH 07/14] PLAT-425 #time 5m fix missing begin --- lib/datadog/statsd/connection.rb | 48 +++++++++++++++++--------------- 1 file changed, 25 insertions(+), 23 deletions(-) diff --git a/lib/datadog/statsd/connection.rb b/lib/datadog/statsd/connection.rb index 87e3126d..dc2d3df5 100644 --- a/lib/datadog/statsd/connection.rb +++ b/lib/datadog/statsd/connection.rb @@ -25,36 +25,38 @@ def close def write(payload) synchronize do - logger.debug { "Statsd: #{payload}" } if logger + begin + logger.debug { "Statsd: #{payload}" } if logger - flush_telemetry = telemetry.flush? + flush_telemetry = telemetry.flush? - payload += telemetry.flush if flush_telemetry + payload += telemetry.flush if flush_telemetry - send_message(payload) + send_message(payload) - telemetry.reset if flush_telemetry + telemetry.reset if flush_telemetry - telemetry.sent(packets: 1, bytes: payload.length) - rescue StandardError => boom - # Try once to reconnect if the socket has been closed - retries ||= 1 - if retries <= 1 && - (boom.is_a?(Errno::ENOTCONN) or - boom.is_a?(Errno::ECONNREFUSED) or - boom.is_a?(IOError) && boom.message =~ /closed stream/i) - retries += 1 - begin - close - retry - rescue StandardError => e - boom = e + telemetry.sent(packets: 1, bytes: payload.length) + rescue StandardError => boom + # Try once to reconnect if the socket has been closed + retries ||= 1 + if retries <= 1 && + (boom.is_a?(Errno::ENOTCONN) or + boom.is_a?(Errno::ECONNREFUSED) or + boom.is_a?(IOError) && boom.message =~ /closed stream/i) + retries += 1 + begin + close + retry + rescue StandardError => e + boom = e + end end - end - telemetry.dropped(packets: 1, bytes: payload.length) - logger.error { "Statsd: #{boom.class} #{boom}" } if logger - nil + telemetry.dropped(packets: 1, bytes: payload.length) + logger.error { "Statsd: #{boom.class} #{boom}" } if logger + nil + end end end From 4ec2fef001b805490c082868b39e795f40a4a60b Mon Sep 17 00:00:00 2001 From: nakedsushi Date: Tue, 12 May 2020 16:06:38 -0700 Subject: [PATCH 08/14] PLAT-425 #time 5m run the actual specs, travis --- .travis.yml | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/.travis.yml b/.travis.yml index bbe89085..f69bf48c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,11 +3,9 @@ sudo: false language: ruby rvm: - 2.6 -script: bundle exec rake $TASK +script: + - bundle exec rubocop + - bundle exec rake $TASK env: - TASK=spec -matrix: - include: - env: TASK=rubocop - rvm: 2.6 bundler_args: --without=localdev From b785d20d368ad80b15468636f927c6d6ac3766ae Mon Sep 17 00:00:00 2001 From: nakedsushi Date: Tue, 12 May 2020 16:53:37 -0700 Subject: [PATCH 09/14] PLAT-425 #time 40m fix stats_spec --- .travis.yml | 4 +--- lib/datadog/statsd.rb | 10 ++++++---- spec/shared/metrics_method.rb | 10 ++++++++++ spec/shared/taggable_method.rb | 10 +++++----- spec/statsd_spec.rb | 22 ++-------------------- 5 files changed, 24 insertions(+), 32 deletions(-) diff --git a/.travis.yml b/.travis.yml index f69bf48c..8b8d405e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,7 +5,5 @@ rvm: - 2.6 script: - bundle exec rubocop - - bundle exec rake $TASK -env: - - TASK=spec + - bundle exec rake spec bundler_args: --without=localdev diff --git a/lib/datadog/statsd.rb b/lib/datadog/statsd.rb index 52c24e29..9bd81bbb 100644 --- a/lib/datadog/statsd.rb +++ b/lib/datadog/statsd.rb @@ -224,11 +224,13 @@ def distribution(stat, value, opts = EMPTY_OPTIONS) # @option opts [Array] :tags An array of tags def timing(stat, ms, opts = EMPTY_OPTIONS) opts = { sample_rate: opts } if opts.is_a?(Numeric) - tags = opts[:tags] || [] - if tags.is_a?(Array) - tags = tags + UNIT_MS + safe_tags = opts[:tags] || [] + if safe_tags.is_a?(Array) + safe_tags += UNIT_MS + elsif safe_tags.is_a?(Hash) + safe_tags[:un] = 'ms' end - send_stats stat, ms, TIMING_TYPE, opts.merge(tags: tags) + send_stats stat, ms, TIMING_TYPE, opts.merge(tags: safe_tags) end # Reports execution time of the provided block using {#timing}. diff --git a/spec/shared/metrics_method.rb b/spec/shared/metrics_method.rb index aa672181..66a969b4 100644 --- a/spec/shared/metrics_method.rb +++ b/spec/shared/metrics_method.rb @@ -8,3 +8,13 @@ it_behaves_like 'a log debuggable method', normal_expected_result it_behaves_like 'a taggable method', normal_expected_result, telemetry_options end + +RSpec.shared_examples 'a metrics method with timing' do |normal_expected_result, telemetry_options| + let(:action_tags) { [] } + + it_behaves_like 'a namespaceable method', "#{normal_expected_result}|#un:ms" + + it_behaves_like 'a log debuggable method', "#{normal_expected_result}|#un:ms" + + it_behaves_like 'a taggable method', normal_expected_result, telemetry_options, ',un:ms' +end diff --git a/spec/shared/taggable_method.rb b/spec/shared/taggable_method.rb index a9ed1307..4d19c17e 100644 --- a/spec/shared/taggable_method.rb +++ b/spec/shared/taggable_method.rb @@ -1,16 +1,16 @@ -RSpec.shared_examples 'a taggable method' do |normal_expected_result, telemetry_options| +RSpec.shared_examples 'a taggable method' do |normal_expected_result, telemetry_options,timing| telemetry_options ||= {} context 'when tags are an array of strings' do let(:action_tags) do - %w[country:usa state:ny other un:ms] + %w[country:usa state:ny other] end it 'supports tags' do basic_action - expect(socket.recv[0]).to eq_with_telemetry "#{normal_expected_result}|#country:usa,state:ny,other,un:ms", telemetry_options + expect(socket.recv[0]).to eq_with_telemetry "#{normal_expected_result}|#country:usa,state:ny,other#{timing}", telemetry_options end context 'when there is global tags' do @@ -21,7 +21,7 @@ it 'merges global and provided tags' do basic_action - expect(socket.recv[0]).to match(/^#{normal_expected_result}|#global_tag,country:usa,state:ny,other,un:ms/) + expect(socket.recv[0]).to match(/^#{normal_expected_result}|#global_tag,country:usa,state:ny,other#{timing}/) end end end @@ -37,7 +37,7 @@ it 'supports tags' do basic_action - expect(socket.recv[0]).to eq_with_telemetry "#{normal_expected_result}|#country:usa,state:ny", telemetry_options + expect(socket.recv[0]).to eq_with_telemetry "#{normal_expected_result}|#country:usa,state:ny#{timing}", telemetry_options end context 'when there is global tags' do diff --git a/spec/statsd_spec.rb b/spec/statsd_spec.rb index d67266e5..c4adebe5 100644 --- a/spec/statsd_spec.rb +++ b/spec/statsd_spec.rb @@ -465,25 +465,7 @@ let(:tags) { nil } let(:action_tags) { [] } - # it_behaves_like 'a metrics method', 'foobar:500|ms|#un:ms' do - # let(:basic_action) do - # subject.timing('foobar', 500, tags: action_tags) - # end - # end - - it_behaves_like 'a namespaceable method', 'foobar:500|ms|#un:ms' do - let(:basic_action) do - subject.timing('foobar', 500, tags: action_tags) - end - end - - it_behaves_like 'a log debuggable method', 'foobar:500|ms|#un:ms' do - let(:basic_action) do - subject.timing('foobar', 500, tags: action_tags) - end - end - - it_behaves_like 'a taggable method', 'foobar:500|ms' do + it_behaves_like 'a metrics method with timing', 'foobar:500|ms' do let(:basic_action) do subject.timing('foobar', 500, tags: action_tags) end @@ -535,7 +517,7 @@ allow(Process).to receive(:clock_gettime).and_return(0) if Datadog::Statsd::PROCESS_TIME_SUPPORTED end - it_behaves_like 'a metrics method', 'foobar:1000|ms|#un:ms' do + it_behaves_like 'a metrics method with timing', 'foobar:1000|ms' do let(:basic_action) do subject.time('foobar', tags: action_tags) do Timecop.travel(after_date) From 7b43e5a40e40e63b6a0b3d343681e022fef73a03 Mon Sep 17 00:00:00 2001 From: nakedsushi Date: Wed, 13 May 2020 09:59:53 -0700 Subject: [PATCH 10/14] PLAT-425 #time 5m add case for ruby 2.6 in expected_allocations --- spec/integrations/allocation_spec.rb | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/spec/integrations/allocation_spec.rb b/spec/integrations/allocation_spec.rb index 223d3472..2ff5c21a 100644 --- a/spec/integrations/allocation_spec.rb +++ b/spec/integrations/allocation_spec.rb @@ -46,6 +46,8 @@ 18 elsif RUBY_VERSION >= '2.4.0' && RUBY_VERSION < '2.5.0' 17 + elsif RUBY_VERSION >= '2.6.0' + 15 else 16 end @@ -73,6 +75,8 @@ 9 elsif RUBY_VERSION >= '2.4.0' && RUBY_VERSION < '2.5.0' 8 + elsif RUBY_VERSION >= '2.6.0' + 6 else 7 end @@ -91,6 +95,8 @@ 26 elsif RUBY_VERSION >= '2.4.0' && RUBY_VERSION < '2.5.0' 25 + elsif RUBY_VERSION >= '2.6.0' + 23 else 24 end @@ -115,6 +121,8 @@ 18 elsif RUBY_VERSION >= '2.4.0' && RUBY_VERSION < '2.5.0' 17 + elsif RUBY_VERSION >= '2.6.0' + 22 else 16 end @@ -142,6 +150,8 @@ 9 elsif RUBY_VERSION >= '2.4.0' && RUBY_VERSION < '2.5.0' 8 + elsif RUBY_VERSION >= '2.6.0' + 13 else 7 end @@ -160,6 +170,8 @@ 26 elsif RUBY_VERSION >= '2.4.0' && RUBY_VERSION < '2.5.0' 25 + elsif RUBY_VERSION >= '2.6.0' + 29 else 24 end @@ -184,6 +196,8 @@ 19 elsif RUBY_VERSION >= '2.4.0' && RUBY_VERSION < '2.5.0' 18 + elsif RUBY_VERSION >= '2.6.0' + 16 else 17 end @@ -211,6 +225,8 @@ 10 elsif RUBY_VERSION >= '2.4.0' && RUBY_VERSION < '2.5.0' 9 + elsif RUBY_VERSION >= '2.6.0' + 7 else 8 end @@ -229,6 +245,8 @@ 28 elsif RUBY_VERSION >= '2.4.0' && RUBY_VERSION < '2.5.0' 27 + elsif RUBY_VERSION >= '2.6.0' + 25 else 26 end @@ -253,6 +271,8 @@ 15 elsif RUBY_VERSION >= '2.4.0' && RUBY_VERSION < '2.5.0' 14 + elsif RUBY_VERSION >= '2.6.0' + 12 else 13 end @@ -280,6 +300,8 @@ 6 elsif RUBY_VERSION >= '2.4.0' && RUBY_VERSION < '2.5.0' 5 + elsif RUBY_VERSION >= '2.6.0' + 3 else 4 end @@ -298,6 +320,8 @@ 24 elsif RUBY_VERSION >= '2.4.0' && RUBY_VERSION < '2.5.0' 23 + elsif RUBY_VERSION >= '2.6.0' + 21 else 22 end @@ -310,4 +334,4 @@ end end end -end \ No newline at end of file +end From bfcafe2259f8d25ba6e4bd2375393f063ed7ebb8 Mon Sep 17 00:00:00 2001 From: nakedsushi Date: Wed, 13 May 2020 10:00:56 -0700 Subject: [PATCH 11/14] PLAT-425 #time 1m add ruby 2.7 to test matrix --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 8b8d405e..c6d565ff 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,6 +3,7 @@ sudo: false language: ruby rvm: - 2.6 + - 2.7 script: - bundle exec rubocop - bundle exec rake spec From 5133857fea4b1afb0f3abdce9799ccb8a9eae770 Mon Sep 17 00:00:00 2001 From: nakedsushi Date: Wed, 13 May 2020 10:23:09 -0700 Subject: [PATCH 12/14] PLAT-425 #time 15m clean up allocation specs to just use ruby 2.6 and above --- spec/integrations/allocation_spec.rb | 80 ++++++---------------------- 1 file changed, 16 insertions(+), 64 deletions(-) diff --git a/spec/integrations/allocation_spec.rb b/spec/integrations/allocation_spec.rb index 2ff5c21a..09fc8243 100644 --- a/spec/integrations/allocation_spec.rb +++ b/spec/integrations/allocation_spec.rb @@ -4,7 +4,7 @@ describe 'Allocations and garbage collection' do before do - skip 'Ruby too old' if RUBY_VERSION < '2.3.0' + skip 'Ruby too old' if RUBY_VERSION < '2.6.0' end let(:socket) { FakeUDPSocket.new } @@ -42,12 +42,8 @@ end let(:expected_allocations) do - if RUBY_VERSION < '2.4.0' - 18 - elsif RUBY_VERSION >= '2.4.0' && RUBY_VERSION < '2.5.0' + if RUBY_VERSION >= '2.6.6' && RUBY_VERSION < '2.7.0' 17 - elsif RUBY_VERSION >= '2.6.0' - 15 else 16 end @@ -71,12 +67,8 @@ end let(:expected_allocations) do - if RUBY_VERSION < '2.4.0' - 9 - elsif RUBY_VERSION >= '2.4.0' && RUBY_VERSION < '2.5.0' + if RUBY_VERSION >= '2.6.6' && RUBY_VERSION < '2.7.0' 8 - elsif RUBY_VERSION >= '2.6.0' - 6 else 7 end @@ -91,12 +83,8 @@ context 'with tags' do let(:expected_allocations) do - if RUBY_VERSION < '2.4.0' - 26 - elsif RUBY_VERSION >= '2.4.0' && RUBY_VERSION < '2.5.0' + if RUBY_VERSION >= '2.6.6' && RUBY_VERSION < '2.7.0' 25 - elsif RUBY_VERSION >= '2.6.0' - 23 else 24 end @@ -117,12 +105,8 @@ end let(:expected_allocations) do - if RUBY_VERSION < '2.4.0' - 18 - elsif RUBY_VERSION >= '2.4.0' && RUBY_VERSION < '2.5.0' - 17 - elsif RUBY_VERSION >= '2.6.0' - 22 + if RUBY_VERSION >= '2.6.6' && RUBY_VERSION < '2.7.0' + 24 else 16 end @@ -146,12 +130,8 @@ end let(:expected_allocations) do - if RUBY_VERSION < '2.4.0' - 9 - elsif RUBY_VERSION >= '2.4.0' && RUBY_VERSION < '2.5.0' - 8 - elsif RUBY_VERSION >= '2.6.0' - 13 + if RUBY_VERSION >= '2.6.6' && RUBY_VERSION < '2.7.0' + 15 else 7 end @@ -166,12 +146,8 @@ context 'with tags' do let(:expected_allocations) do - if RUBY_VERSION < '2.4.0' - 26 - elsif RUBY_VERSION >= '2.4.0' && RUBY_VERSION < '2.5.0' - 25 - elsif RUBY_VERSION >= '2.6.0' - 29 + if RUBY_VERSION >= '2.6.6' && RUBY_VERSION < '2.7.0' + 31 else 24 end @@ -192,12 +168,8 @@ end let(:expected_allocations) do - if RUBY_VERSION < '2.4.0' - 19 - elsif RUBY_VERSION >= '2.4.0' && RUBY_VERSION < '2.5.0' + if RUBY_VERSION >= '2.6.6' && RUBY_VERSION < '2.7.0' 18 - elsif RUBY_VERSION >= '2.6.0' - 16 else 17 end @@ -221,12 +193,8 @@ end let(:expected_allocations) do - if RUBY_VERSION < '2.4.0' - 10 - elsif RUBY_VERSION >= '2.4.0' && RUBY_VERSION < '2.5.0' + if RUBY_VERSION >= '2.6.6' && RUBY_VERSION < '2.7.0' 9 - elsif RUBY_VERSION >= '2.6.0' - 7 else 8 end @@ -241,12 +209,8 @@ context 'with tags' do let(:expected_allocations) do - if RUBY_VERSION < '2.4.0' - 28 - elsif RUBY_VERSION >= '2.4.0' && RUBY_VERSION < '2.5.0' + if RUBY_VERSION >= '2.6.6' && RUBY_VERSION < '2.7.0' 27 - elsif RUBY_VERSION >= '2.6.0' - 25 else 26 end @@ -267,12 +231,8 @@ end let(:expected_allocations) do - if RUBY_VERSION < '2.4.0' - 15 - elsif RUBY_VERSION >= '2.4.0' && RUBY_VERSION < '2.5.0' + if RUBY_VERSION >= '2.6.6' && RUBY_VERSION < '2.7.0' 14 - elsif RUBY_VERSION >= '2.6.0' - 12 else 13 end @@ -296,12 +256,8 @@ end let(:expected_allocations) do - if RUBY_VERSION < '2.4.0' - 6 - elsif RUBY_VERSION >= '2.4.0' && RUBY_VERSION < '2.5.0' + if RUBY_VERSION >= '2.6.6' && RUBY_VERSION < '2.7.0' 5 - elsif RUBY_VERSION >= '2.6.0' - 3 else 4 end @@ -316,12 +272,8 @@ context 'with tags' do let(:expected_allocations) do - if RUBY_VERSION < '2.4.0' - 24 - elsif RUBY_VERSION >= '2.4.0' && RUBY_VERSION < '2.5.0' + if RUBY_VERSION >= '2.6.6' && RUBY_VERSION < '2.7.0' 23 - elsif RUBY_VERSION >= '2.6.0' - 21 else 22 end From d0ea9becf06f3f6901c960b8bc56196e829056d3 Mon Sep 17 00:00:00 2001 From: nakedsushi Date: Wed, 13 May 2020 10:30:37 -0700 Subject: [PATCH 13/14] PLAT-425 #time 5m fix allocations for ruby 2.7.1 --- spec/integrations/allocation_spec.rb | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/spec/integrations/allocation_spec.rb b/spec/integrations/allocation_spec.rb index 09fc8243..e3ce988f 100644 --- a/spec/integrations/allocation_spec.rb +++ b/spec/integrations/allocation_spec.rb @@ -45,7 +45,7 @@ if RUBY_VERSION >= '2.6.6' && RUBY_VERSION < '2.7.0' 17 else - 16 + 15 end end @@ -70,7 +70,7 @@ if RUBY_VERSION >= '2.6.6' && RUBY_VERSION < '2.7.0' 8 else - 7 + 6 end end @@ -86,7 +86,7 @@ if RUBY_VERSION >= '2.6.6' && RUBY_VERSION < '2.7.0' 25 else - 24 + 23 end end @@ -108,7 +108,7 @@ if RUBY_VERSION >= '2.6.6' && RUBY_VERSION < '2.7.0' 24 else - 16 + 22 end end @@ -133,7 +133,7 @@ if RUBY_VERSION >= '2.6.6' && RUBY_VERSION < '2.7.0' 15 else - 7 + 13 end end @@ -149,7 +149,7 @@ if RUBY_VERSION >= '2.6.6' && RUBY_VERSION < '2.7.0' 31 else - 24 + 29 end end @@ -171,7 +171,7 @@ if RUBY_VERSION >= '2.6.6' && RUBY_VERSION < '2.7.0' 18 else - 17 + 16 end end @@ -196,7 +196,7 @@ if RUBY_VERSION >= '2.6.6' && RUBY_VERSION < '2.7.0' 9 else - 8 + 7 end end @@ -212,7 +212,7 @@ if RUBY_VERSION >= '2.6.6' && RUBY_VERSION < '2.7.0' 27 else - 26 + 25 end end @@ -234,7 +234,7 @@ if RUBY_VERSION >= '2.6.6' && RUBY_VERSION < '2.7.0' 14 else - 13 + 12 end end @@ -259,7 +259,7 @@ if RUBY_VERSION >= '2.6.6' && RUBY_VERSION < '2.7.0' 5 else - 4 + 3 end end @@ -275,7 +275,7 @@ if RUBY_VERSION >= '2.6.6' && RUBY_VERSION < '2.7.0' 23 else - 22 + 21 end end From 7f0775712d90bde2678d32275c1aa32043573282 Mon Sep 17 00:00:00 2001 From: nakedsushi Date: Wed, 13 May 2020 10:42:46 -0700 Subject: [PATCH 14/14] PLAT-425 #time 1m lock concurrent-ruby to version 1.1.6 --- Gemfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile b/Gemfile index eb0e8901..542b50b6 100644 --- a/Gemfile +++ b/Gemfile @@ -7,7 +7,7 @@ gem 'minitest-matchers' gem 'yard', '~> 0.9.20' gem 'single_cov' gem 'climate_control' -gem 'concurrent-ruby' +gem 'concurrent-ruby', '~> 1.1.6' if RUBY_VERSION >= '2.0.0' gem 'rubocop', '~> 0.50.0' # bump this and TargetRubyVersion once we drop ruby 2.0