Use dogstatsd single threaded mode (#1576)

Author: marcotc

Gemfile

@@ -30,7 +30,7 @@ gem 'redcarpet', '~> 3.4' if RUBY_PLATFORM != 'java'
 gem 'rspec', '~> 3.10'
 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 'rspec_n', '~> 1.3' if RUBY_VERSION >= '2.4.0'
 gem 'ruby-prof', '~> 1.4' if RUBY_PLATFORM != 'java' && RUBY_VERSION >= '2.4.0'
 if RUBY_VERSION >= '2.5.0'
   # Merging branch coverage results does not work for old, unsupported rubies.
@@ -58,7 +58,9 @@ end
 
 # Optional extensions
 # TODO: Move this to Appraisals?
-gem 'dogstatsd-ruby', '< 5.0'
+# dogstatsd v5, but lower than 5.2, has possible memory leak with ddtrace.
+# @see https://github.com/DataDog/dogstatsd-ruby/issues/182
+gem 'dogstatsd-ruby', '>= 3.3.0', '!= 5.0.0', '!= 5.0.1', '!= 5.1.0'
 gem 'opentracing', '>= 0.4.1'
 
 # Profiler optional dependencies

docs/GettingStarted.md

@@ -2410,7 +2410,7 @@ The tracer and its integrations can produce some additional metrics that can pro
 To configure your application for metrics collection:
 
 1. [Configure your Datadog agent for StatsD](https://docs.datadoghq.com/developers/dogstatsd/#setup)
-2. Add `gem 'dogstatsd-ruby', '~> 4.0'` to your Gemfile (note that it is advised to pin v4.x [until this issue is solved](https://github.com/DataDog/dogstatsd-ruby/issues/182))
+2. Add `gem 'dogstatsd-ruby', '~> 5.2'` to your Gemfile
 
 #### For application runtime
 

lib/ddtrace/metrics.rb

@@ -25,16 +25,12 @@ def initialize(statsd: nil, enabled: true, **_)
     end
 
     def supported?
-      version = (
-          defined?(Datadog::Statsd::VERSION) &&
-          Datadog::Statsd::VERSION &&
-          Gem::Version.new(Datadog::Statsd::VERSION)
-        ) || (
-          Gem.loaded_specs['dogstatsd-ruby'] &&
-          Gem.loaded_specs['dogstatsd-ruby'].version
-        )
+      version = dogstatsd_version
 
-      !version.nil? && (version >= Gem::Version.new('3.3.0'))
+      !version.nil? && version >= Gem::Version.new('3.3.0') &&
+        # dogstatsd-ruby >= 5.0 & < 5.2.0 has known issues with process forks
+        # and do not support the single thread mode we use to avoid this problem.
+        !(version >= Gem::Version.new('5.0') && version < Gem::Version.new('5.2'))
     end
 
     def enabled?
@@ -56,10 +52,23 @@ def default_port
     def default_statsd_client
       require 'datadog/statsd'
 
-      incompatible_statsd_warning
-
       # Create a StatsD client that points to the agent.
-      Datadog::Statsd.new(default_hostname, default_port)
+      #
+      # We use `single_thread: true`, as dogstatsd-ruby >= 5.0 creates a background thread
+      # by default, but does not handle forks correctly, causing resource leaks.
+      #
+      # Using dogstatsd-ruby >= 5.0 is still valuable, as it supports
+      # transparent batch metric submission, which reduces submission
+      # overhead.
+      #
+      # Versions < 5.0 are always single-threaded, but do not have the kwarg option.
+      options = if dogstatsd_version >= Gem::Version.new('5.2')
+                  { single_thread: true }
+                else
+                  {}
+                end
+
+      Datadog::Statsd.new(default_hostname, default_port, **options)
     end
 
     def configure(options = {})
@@ -244,21 +253,17 @@ def gauge(stat, value, options = nil)
 
     private
 
-    INCOMPATIBLE_STATSD_ONLY_ONCE = Datadog::Utils::OnlyOnce.new
-    private_constant :INCOMPATIBLE_STATSD_ONLY_ONCE
-
-    # IMPORTANT: Once you delete this, you probably want to update the `#ignored_statsd_warning` below where we
-    # recommend that customers add `gem 'dogstatsd-ruby', '~> 4.0'` to their Gemfile to perhaps state something
-    # different.
-    def incompatible_statsd_warning
-      return if Gem.loaded_specs['dogstatsd-ruby'].version < Gem::Version.new('5.0')
+    def dogstatsd_version
+      return @dogstatsd_version if instance_variable_defined?(:@dogstatsd_version)
 
-      INCOMPATIBLE_STATSD_ONLY_ONCE.run do
-        Datadog.logger.warn(
-          'This version of `ddtrace` is incompatible with `dogstastd-ruby` version >= 5.0 and can ' \
-          'cause unbounded memory usage. Please use `dogstastd-ruby` version < 5.0 instead.'
-        )
-      end
+      @dogstatsd_version = (
+        defined?(Datadog::Statsd::VERSION) &&
+          Datadog::Statsd::VERSION &&
+          Gem::Version.new(Datadog::Statsd::VERSION)
+      ) || (
+        Gem.loaded_specs['dogstatsd-ruby'] &&
+          Gem.loaded_specs['dogstatsd-ruby'].version
+      )
     end
 
     IGNORED_STATSD_ONLY_ONCE = Datadog::Utils::OnlyOnce.new
@@ -268,7 +273,7 @@ def ignored_statsd_warning
       IGNORED_STATSD_ONLY_ONCE.run do
         Datadog.logger.warn(
           'Ignoring user-supplied statsd instance as currently-installed version of dogstastd-ruby is incompatible. ' \
-          "To fix this, ensure that you have `gem 'dogstatsd-ruby', '~> 4.0'` on your Gemfile or gems.rb file."
+          "To fix this, ensure that you have `gem 'dogstatsd-ruby', '~> 5.2'` on your Gemfile or gems.rb file."
         )
       end
     end

spec/ddtrace/metrics_spec.rb

@@ -118,6 +118,18 @@
 
           it { is_expected.to be true }
         end
+
+        context 'with incompatible 5.x version' do
+          let(:version) { Gem::Version.new('5.0.0') }
+
+          it { is_expected.to be false }
+        end
+
+        context 'with compatible 5.x version' do
+          let(:version) { Gem::Version.new('5.2.0') }
+
+          it { is_expected.to be true }
+        end
       end
 
       context 'is loaded but ruby is not using rubygems' do
@@ -138,6 +150,18 @@
 
           it { is_expected.to be true }
         end
+
+        context 'with incompatible 5.x version' do
+          let(:gem_version_number) { '5.0.0' }
+
+          it { is_expected.to be false }
+        end
+
+        context 'with compatible 5.x version' do
+          let(:gem_version_number) { '5.2.0' }
+
+          it { is_expected.to be true }
+        end
       end
     end
   end
@@ -246,10 +270,18 @@
     subject(:default_statsd_client) { metrics.default_statsd_client }
 
     let(:statsd_client) { instance_double(Datadog::Statsd) }
+    let(:options) do
+      # This tests is run with both ~> 4.0 and latest dogstatsd-ruby.
+      if Gem::Version.new(Datadog::Statsd::VERSION) >= Gem::Version.new('5.2.0')
+        { single_thread: true }
+      else
+        {}
+      end
+    end
 
     before do
       expect(Datadog::Statsd).to receive(:new)
-        .with(metrics.default_hostname, metrics.default_port)
+        .with(metrics.default_hostname, metrics.default_port, **options)
         .and_return(statsd_client)
     end
 
@@ -786,33 +818,6 @@
       end
     end
   end
-
-  describe '#incompatible_statsd_warning' do
-    let(:options) { {} }
-
-    before { described_class.const_get('INCOMPATIBLE_STATSD_ONLY_ONCE').send(:reset_ran_once_state_for_tests) }
-
-    context 'with an incompatible dogstastd-ruby version' do
-      before { skip unless Gem.loaded_specs['dogstatsd-ruby'].version >= Gem::Version.new('5.0') }
-
-      it 'emits deprecation warning once' do
-        expect(Datadog.logger).to receive(:warn)
-          .with(/This version of `ddtrace` is incompatible with `dogstastd-ruby`/).once
-
-        metrics
-      end
-    end
-
-    context 'with a compatible dogstastd-ruby version' do
-      before { skip unless Gem.loaded_specs['dogstatsd-ruby'].version < Gem::Version.new('5.0') }
-
-      it 'emits no warnings' do
-        expect(Datadog.logger).to_not receive(:warn)
-
-        metrics
-      end
-    end
-  end
 end
 
 RSpec.describe Datadog::Metrics::Options do

spec/ddtrace_integration_spec.rb

@@ -113,12 +113,8 @@ def open_file_descriptors
         expect(Datadog.configuration.runtime_metrics.enabled).to be(true).or be(false)
       end
 
-      it 'does not error on reporting health metrics', if: Datadog::Statsd::VERSION >= '5.0.0' do
-        expect(Datadog.health_metrics.queue_accepted(1)).to be_truthy
-      end
-
-      it 'does not error on reporting health metrics', if: Datadog::Statsd::VERSION < '5.0.0' do
-        expect(Datadog.health_metrics.queue_accepted(1)).to be_a(Integer)
+      it 'does not error on reporting health metrics' do
+        expect { Datadog.health_metrics.queue_accepted(1) }.to_not raise_error
       end
 
       context 'with OpenTracer' do

spec/support/configuration_helpers.rb

@@ -1,4 +1,4 @@
-require 'bundler/stub_specification'
+require 'bundler'
 
 module ConfigurationHelpers
   shared_context 'loaded gems' do |gems = {}|