Refactored: Extract #health_metrics to Components

lib/ddtrace/buffer.rb

@@ -96,16 +96,16 @@ def measure_drop(trace)
 
     def measure_pop(traces)
       # Accepted
-      Diagnostics::Health.metrics.queue_accepted(@buffer_accepted)
-      Diagnostics::Health.metrics.queue_accepted_lengths(@buffer_accepted_lengths)
+      Datadog.health_metrics.queue_accepted(@buffer_accepted)
+      Datadog.health_metrics.queue_accepted_lengths(@buffer_accepted_lengths)
 
       # Dropped
-      Diagnostics::Health.metrics.queue_dropped(@buffer_dropped)
+      Datadog.health_metrics.queue_dropped(@buffer_dropped)
 
       # Queue gauges
-      Diagnostics::Health.metrics.queue_max_length(@max_size)
-      Diagnostics::Health.metrics.queue_spans(@buffer_spans)
-      Diagnostics::Health.metrics.queue_length(traces.length)
+      Datadog.health_metrics.queue_max_length(@max_size)
+      Datadog.health_metrics.queue_spans(@buffer_spans)
+      Datadog.health_metrics.queue_length(traces.length)
 
       # Reset aggregated metrics
       @buffer_accepted = 0

lib/ddtrace/configuration.rb

@@ -30,6 +30,7 @@ def configure(target = configuration, opts = {})
 
     def_delegators \
       :components,
+      :health_metrics,
       :runtime_metrics,
       :tracer
 
@@ -57,7 +58,10 @@ def teardown_components!(old, current)
 
       # Shutdown the old metrics, unless they are still being used.
       # (e.g. custom Statsd instances.)
-      old.runtime_metrics.statsd.close unless old.runtime_metrics.statsd == current.runtime_metrics.statsd
+      old_statsd = [old.runtime_metrics.statsd, old.health_metrics.statsd].uniq
+      new_statsd = [current.runtime_metrics.statsd, current.health_metrics.statsd].uniq
+      unused_statsd = (old_statsd - (old_statsd & new_statsd))
+      unused_statsd.each(&:close)
     end
   end
 end

lib/ddtrace/configuration/components.rb

@@ -11,9 +11,13 @@ def initialize(settings)
 
         # Runtime metrics
         build_runtime_metrics(settings)
+
+        # Health metrics
+        @health_metrics = build_health_metrics(settings)
       end
 
       attr_reader \
+        :health_metrics,
         :tracer
 
       def runtime_metrics
@@ -77,6 +81,14 @@ def build_runtime_metrics(settings)
         #       runtime metrics are extracted from tracer/writer.
         runtime_metrics.configure(options)
       end
+
+      def build_health_metrics(settings)
+        settings = settings.diagnostics.health_metrics
+        options = { enabled: settings.enabled }
+        options[:statsd] = settings.statsd unless settings.statsd.nil?
+
+        Datadog::Diagnostics::Health::Metrics.new(options)
+      end
     end
   end
 end

lib/ddtrace/configuration/settings.rb

@@ -34,14 +34,13 @@ class Settings
       end
 
       settings :diagnostics do
-        option :health_metrics do |o|
-          o.default do
-            Datadog::Diagnostics::Health::Metrics.new(
-              enabled: env_to_bool(Datadog::Ext::Diagnostics::Health::Metrics::ENV_ENABLED, false)
-            )
+        settings :health_metrics do
+          option :enabled do |o|
+            o.default { env_to_bool(Datadog::Ext::Diagnostics::Health::Metrics::ENV_ENABLED, false) }
+            o.lazy
           end
 
-          o.lazy
+          option :statsd
         end
       end
 

lib/ddtrace/context.rb

@@ -100,7 +100,7 @@ def add_span(span)
           # If overflow has already occurred, don't send this metric.
           # Prevents metrics spam if buffer repeatedly overflows for the same trace.
           unless @overflow
-            Diagnostics::Health.metrics.error_context_overflow(1, tags: ["max_length:#{@max_length}"])
+            Datadog.health_metrics.error_context_overflow(1, tags: ["max_length:#{@max_length}"])
             @overflow = true
           end
 
@@ -131,7 +131,7 @@ def close_span(span)
 
           @trace.reject(&:finished?).group_by(&:name).each do |unfinished_span_name, unfinished_spans|
             Datadog::Logger.log.debug("unfinished span: #{unfinished_spans.first}") if Datadog::Logger.debug_logging
-            Diagnostics::Health.metrics.error_unfinished_spans(
+            Datadog.health_metrics.error_unfinished_spans(
               unfinished_spans.length,
               tags: ["name:#{unfinished_span_name}"]
             )

lib/ddtrace/contrib/patcher.rb

@@ -28,7 +28,7 @@ def patch
             begin
               super.tap do
                 # Emit a metric
-                Diagnostics::Health.metrics.instrumentation_patched(1, tags: default_tags)
+                Datadog.health_metrics.instrumentation_patched(1, tags: default_tags)
               end
             rescue StandardError => e
               # Log the error
@@ -38,7 +38,7 @@ def patch
               tags = default_tags
               tags << "error:#{e.class.name}"
 
-              Diagnostics::Health.metrics.error_instrumentation_patch(1, tags: tags)
+              Datadog.health_metrics.error_instrumentation_patch(1, tags: tags)
             end
           end
         end

lib/ddtrace/diagnostics/health.rb

@@ -26,12 +26,6 @@ class Metrics < ::Datadog::Metrics
         gauge :queue_spans, Ext::Diagnostics::Health::Metrics::METRIC_QUEUE_SPANS
         gauge :sampling_service_cache_length, Ext::Diagnostics::Health::Metrics::METRIC_SAMPLING_SERVICE_CACHE_LENGTH
       end
-
-      module_function
-
-      def metrics
-        Datadog.configuration.diagnostics.health_metrics
-      end
     end
   end
 end

lib/ddtrace/sampler.rb

@@ -176,7 +176,7 @@ def update(rate_by_service)
       update_all(rate_by_service)
 
       # Emit metric for service cache size
-      Diagnostics::Health.metrics.sampling_service_cache_length(length)
+      Datadog.health_metrics.sampling_service_cache_length(length)
     end
 
     private

lib/ddtrace/span.rb

@@ -180,7 +180,7 @@ def finish(finish_time = nil)
         @tracer.record(self)
       rescue StandardError => e
         Datadog::Logger.log.debug("error recording finished trace: #{e}")
-        Diagnostics::Health.metrics.error_span_finish(1, tags: ["error:#{e.class.name}"])
+        Datadog.health_metrics.error_span_finish(1, tags: ["error:#{e.class.name}"])
       end
       self
     end

lib/ddtrace/transport/statistics.rb

@@ -20,7 +20,7 @@ def update_stats_from_response!(response)
         end
 
         # Send health metrics
-        Diagnostics::Health.metrics.send_metrics(
+        Datadog.health_metrics.send_metrics(
           metrics_for_response(response).values
         )
       end
@@ -37,7 +37,7 @@ def update_stats_from_exception!(exception)
         stats.consecutive_errors += 1
 
         # Send health metrics
-        Diagnostics::Health.metrics.send_metrics(
+        Datadog.health_metrics.send_metrics(
           metrics_for_exception(exception).values
         )
       end

spec/ddtrace/configuration/components_spec.rb

@@ -119,6 +119,15 @@
     end
 
     context 'given some health metrics settings' do
+      let(:statsd) { instance_double('statsd') }
+
+      before do
+        settings.diagnostics.health_metrics.enabled = true
+        settings.diagnostics.health_metrics.statsd = statsd
+      end
+
+      it { expect(components.health_metrics.enabled?).to be true }
+      it { expect(components.health_metrics.statsd).to be statsd }
     end
   end
 end

spec/ddtrace/configuration/settings_spec.rb

@@ -61,11 +61,52 @@
 
   describe '#diagnostics' do
     describe '#health_metrics' do
-      # TODO
-    end
+      describe '#enabled' do
+        subject(:enabled) { settings.diagnostics.health_metrics.enabled }
+
+        context "when #{Datadog::Ext::Diagnostics::Health::Metrics::ENV_ENABLED}" do
+          around do |example|
+            ClimateControl.modify(Datadog::Ext::Diagnostics::Health::Metrics::ENV_ENABLED => environment) do
+              example.run
+            end
+          end
 
-    describe '#health_metrics=' do
-      # TODO
+          context 'is not defined' do
+            let(:environment) { nil }
+            it { is_expected.to be false }
+          end
+
+          context 'is defined' do
+            let(:environment) { 'true' }
+            it { is_expected.to be true }
+          end
+        end
+      end
+
+      describe '#enabled=' do
+        it 'changes the #enabled setting' do
+          expect { settings.diagnostics.health_metrics.enabled = true }
+            .to change { settings.diagnostics.health_metrics.enabled }
+            .from(false)
+            .to(true)
+        end
+      end
+
+      describe '#statsd' do
+        subject(:statsd) { settings.diagnostics.health_metrics.statsd }
+        it { is_expected.to be nil }
+      end
+
+      describe '#statsd=' do
+        let(:statsd) { double('statsd') }
+
+        it 'changes the #statsd setting' do
+          expect { settings.diagnostics.health_metrics.statsd = statsd }
+            .to change { settings.diagnostics.health_metrics.statsd }
+            .from(nil)
+            .to(statsd)
+        end
+      end
     end
   end
 

spec/ddtrace/configuration_spec.rb

@@ -62,14 +62,45 @@
         let(:new_statsd) { instance_double(Datadog::Statsd) }
 
         before do
-          expect(old_statsd).to receive(:close)
+          expect(old_statsd).to receive(:close).once
 
-          test_class.configure { |c| c.runtime_metrics.statsd = old_statsd }
-          test_class.configure { |c| c.runtime_metrics.statsd = new_statsd }
+          test_class.configure do |c|
+            c.runtime_metrics.statsd = old_statsd
+            c.diagnostics.health_metrics.statsd = old_statsd
+          end
+
+          test_class.configure do |c|
+            c.runtime_metrics.statsd = new_statsd
+            c.diagnostics.health_metrics.statsd = new_statsd
+          end
+        end
+
+        it 'replaces the old Statsd and closes it' do
+          expect(test_class.runtime_metrics.statsd).to be new_statsd
+          expect(test_class.health_metrics.statsd).to be new_statsd
+        end
+      end
+
+      context 'when replacing one of the metrics' do
+        let(:old_statsd) { instance_double(Datadog::Statsd) }
+        let(:new_statsd) { instance_double(Datadog::Statsd) }
+
+        before do
+          expect(old_statsd).to_not receive(:close)
+
+          test_class.configure do |c|
+            c.runtime_metrics.statsd = old_statsd
+            c.diagnostics.health_metrics.statsd = old_statsd
+          end
+
+          test_class.configure do |c|
+            c.runtime_metrics.statsd = new_statsd
+          end
         end
 
         it 'replaces the old Statsd and closes it' do
           expect(test_class.runtime_metrics.statsd).to be new_statsd
+          expect(test_class.health_metrics.statsd).to be old_statsd
         end
       end
 
@@ -79,8 +110,15 @@
         before do
           expect(statsd).to_not receive(:close)
 
-          test_class.configure { |c| c.runtime_metrics.statsd = statsd }
-          test_class.configure { |c| c.runtime_metrics.statsd = statsd }
+          test_class.configure do |c|
+            c.runtime_metrics.statsd = statsd
+            c.diagnostics.health_metrics.statsd = statsd
+          end
+
+          test_class.configure do |c|
+            c.runtime_metrics.statsd = statsd
+            c.diagnostics.health_metrics.statsd = statsd
+          end
         end
 
         it 'replaces the old Statsd and closes it' do
@@ -94,7 +132,11 @@
         before do
           expect(statsd).to_not receive(:close)
 
-          test_class.configure { |c| c.runtime_metrics.statsd = statsd }
+          test_class.configure do |c|
+            c.runtime_metrics.statsd = statsd
+            c.diagnostics.health_metrics.statsd = statsd
+          end
+
           test_class.configure { |_c| }
         end
 

spec/ddtrace/transport/http/client_spec.rb

@@ -41,7 +41,7 @@
     let(:response_class) { stub_const('TestResponse', Class.new { include Datadog::Transport::HTTP::Response }) }
     let(:response) { instance_double(response_class, code: double('status code')) }
 
-    before { allow(Datadog::Diagnostics::Health.metrics).to receive(:send_metrics) }
+    before { allow(Datadog.health_metrics).to receive(:send_metrics) }
 
     context 'given a block' do
       let(:handler) { double }