From ba25f37d127bd2a8bbec7275f91af163484d6159 Mon Sep 17 00:00:00 2001 From: Ivo Anjo Date: Fri, 5 Feb 2021 12:10:31 +0000 Subject: [PATCH 1/3] Add `rspec_n` gem for development This gem is quite nifty for exercising flaky tests, so let's keep it around. --- Gemfile | 1 + 1 file changed, 1 insertion(+) diff --git a/Gemfile b/Gemfile index 6164b796d08..7a843229661 100644 --- a/Gemfile +++ b/Gemfile @@ -24,6 +24,7 @@ gem 'redcarpet', '~> 3.4' if RUBY_PLATFORM != 'java' gem 'rspec', '~> 3.0' gem 'rspec-collection_matchers', '~> 1.1' gem 'rspec_junit_formatter', '>= 0.4.1' +gem 'rspec_n', '~> 1.3' if RUBY_VERSION >= '2.3.0' gem 'rubocop', '= 0.50.0' if RUBY_VERSION >= '2.1.0' gem 'ruby-prof', '~> 1.4' if RUBY_PLATFORM != 'java' && RUBY_VERSION >= '2.4.0' gem 'simplecov', '~> 0.17' From cb34e2bc339788d119890c2125248618a2bdcc41 Mon Sep 17 00:00:00 2001 From: Ivo Anjo Date: Fri, 5 Feb 2021 12:12:44 +0000 Subject: [PATCH 2/3] [ART-380] Fix tracer leaving background writer threads This was causing a race in `bundle exec rspec --seed 50224 spec/ddtrace/tracer_integration_spec.rb` exposing the issue in ART-380 where multiple writer background threads would trigger initialization of metrics. This does not fix the underlying issue causing `ArgumentError: wrong number of arguments (given 2, expected 0)` (see next commit for that). --- spec/ddtrace/tracer_integration_spec.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/spec/ddtrace/tracer_integration_spec.rb b/spec/ddtrace/tracer_integration_spec.rb index d70e7b9690d..7057ccf201f 100644 --- a/spec/ddtrace/tracer_integration_spec.rb +++ b/spec/ddtrace/tracer_integration_spec.rb @@ -9,6 +9,10 @@ subject(:tracer) { described_class.new(writer: FauxWriter.new) } let(:spans) { tracer.writer.spans(:keep) } + after do + tracer.shutdown! # Ensure no state gets left behind + end + def sampling_priority_metric(span) span.get_metric(Datadog::Ext::DistributedTracing::SAMPLING_PRIORITY_KEY) end From 4863e652af18e1dbf40701399b65667582e7655e Mon Sep 17 00:00:00 2001 From: Ivo Anjo Date: Fri, 5 Feb 2021 12:19:07 +0000 Subject: [PATCH 3/3] [ART-380] Fix ArgumentError due to concurrent require and class use If for some reason multiple tracers are initializing concurrently (see customer issue in #1306 or the flaky test in `tracer_integration_spec.rb` in the previous commit), multiple threads can try to run `default_statsd_client` concurrently. Because Ruby `require`s are not atomic (e.g. you can observe classes that are half-loaded), our conditional ```ruby require 'datadog/statsd' unless defined?(::Datadog::Statsd) Datadog::Statsd.new(default_hostname, default_port) ``` could try to create a `Statsd` instance BEFORE the class was fully defined, leading to `ArgumentError: wrong number of arguments (given 2, expected 0)` when calling `initialize`. This error stems from the custom `#initialize` method not being available yet, so Ruby falls back to `Object#initialize`, which does not take any argument. See also: * * The fix in this case is to always call `require`. Calling `require` repeatedly has little impact -- a given file is only loaded by `require` the first time AND concurrent `require`s are protected -- only one thread will get to execute the require, will all others wait until it finishes. The follow example demonstrates this: * `lazy-require.rb`: ```ruby def try_require puts "#{Thread.current} Going to require" require_relative './lazy-require-sleep-1' puts "#{Thread.current} After require" end 2.times.map { Thread.new { try_require } }.map(&:join) try_require ``` * `lazy-require-sleep-1.rb`: ```ruby puts "#{Thread.current} Running require" sleep 1 puts "#{Thread.current} Require finished" ``` * Output: ``` $ ruby lazy-require.rb # Going to require # Going to require # Running require # Require finished # After require # After require # Going to require # After require ``` Notice how two threads tried to require, but only one actually ran the code on the other file, while the other was blocked and only woke up after the `require` finished. Fixes #1306 --- lib/ddtrace/metrics.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ddtrace/metrics.rb b/lib/ddtrace/metrics.rb index 7c785067496..dde8482ec71 100644 --- a/lib/ddtrace/metrics.rb +++ b/lib/ddtrace/metrics.rb @@ -41,7 +41,7 @@ def default_port end def default_statsd_client - require 'datadog/statsd' unless defined?(::Datadog::Statsd) + require 'datadog/statsd' # Create a StatsD client that points to the agent. Datadog::Statsd.new(default_hostname, default_port)