Dependency inject tracer instance into profiler

As discussed during PR review of #1568, it's problematic to just cache
whatever's in `Datadog.tracer` because we can observe the old value
during component initialization.

To break the loop, let's instead directly initialize the
`TraceIdentifiers::Ddtrace` class with the correct tracer during
component initialization.

IMHO this has a further advantage: it makes it really explict where
there is a tracer-to-profiler dependency whereas previously there was
just a call to `Datadog.tracer` deep in the bowels of the profiler that
could be called at any point.

docs/ProfilingDevelopment.md

@@ -42,22 +42,14 @@ flow:
 4. The `Setup` task activates our extensions
     * `Datadog::Profiling::Ext::Forking`
     * `Datadog::Profiling::Ext::CPU`
-5. Still inside `Datadog::Components`, the `build_profiler` method then creates and wires up the Profiler:
-    ```ruby
-          recorder = build_profiler_recorder(settings)
-          collectors = build_profiler_collectors(settings, recorder)
-          exporters = build_profiler_exporters(settings)
-          scheduler = build_profiler_scheduler(settings, recorder, exporters)
-
-          Datadog::Profiler.new(collectors, scheduler)
-    ```
+5. Still inside `Datadog::Components`, the `build_profiler` method then creates and wires up the Profiler as such:
     ```asciiflow
             +------------+
             |  Profiler  |
-            +-+--------+-+
-              |        |
-              v        v
-    +---------+--+  +--+--------+
+            +-+-------+--+
+              |       |
+              v       v
+    +---------+--+  +-+---------+
     | Collectors |  | Scheduler |
     +---------+--+  +-+-------+-+
               |       |       |

lib/ddtrace/configuration/components.rb

@@ -70,7 +70,7 @@ def build_tracer(settings, agent_settings)
           tracer
         end
 
-        def build_profiler(settings, agent_settings)
+        def build_profiler(settings, agent_settings, tracer)
           return unless Datadog::Profiling.supported? && settings.profiling.enabled
 
           unless defined?(Datadog::Profiling::Tasks::Setup)
@@ -107,8 +107,10 @@ def build_profiler(settings, agent_settings)
 
           # NOTE: Please update the Initialization section of ProfilingDevelopment.md with any changes to this method
 
+          trace_identifiers_helper = Datadog::Profiling::TraceIdentifiers::Helper.new(tracer: tracer)
+
           recorder = build_profiler_recorder(settings)
-          collectors = build_profiler_collectors(settings, recorder)
+          collectors = build_profiler_collectors(settings, recorder, trace_identifiers_helper)
           exporters = build_profiler_exporters(settings, agent_settings)
           scheduler = build_profiler_scheduler(settings, recorder, exporters)
 
@@ -164,10 +166,11 @@ def build_profiler_recorder(settings)
           Datadog::Profiling::Recorder.new(event_classes, settings.profiling.max_events)
         end
 
-        def build_profiler_collectors(settings, recorder)
+        def build_profiler_collectors(settings, recorder, trace_identifiers_helper)
           [
             Datadog::Profiling::Collectors::Stack.new(
               recorder,
+              trace_identifiers_helper: trace_identifiers_helper,
               max_frames: settings.profiling.max_frames
               # TODO: Provide proc that identifies Datadog worker threads?
               # ignore_thread: settings.profiling.ignore_profiler
@@ -209,7 +212,7 @@ def initialize(settings)
         @tracer = self.class.build_tracer(settings, agent_settings)
 
         # Profiler
-        @profiler = self.class.build_profiler(settings, agent_settings)
+        @profiler = self.class.build_profiler(settings, agent_settings, @tracer)
 
         # Runtime metrics
         @runtime_metrics = self.class.build_runtime_metrics_worker(settings)

lib/ddtrace/profiling.rb

@@ -126,6 +126,7 @@ def self.load_profiling
       require 'ddtrace/profiling/transport/http'
       require 'ddtrace/profiling/profiler'
       require 'ddtrace/profiling/native_extension'
+      require 'ddtrace/profiling/trace_identifiers/helper'
       require 'ddtrace/profiling/pprof/pprof_pb'
 
       true

lib/ddtrace/profiling/collectors/stack.rb

@@ -1,5 +1,4 @@
 require 'ddtrace/profiling/backtrace_location'
-require 'ddtrace/profiling/trace_identifiers/helper'
 require 'ddtrace/profiling/events/stack'
 require 'ddtrace/utils/only_once'
 require 'ddtrace/utils/time'
@@ -12,7 +11,7 @@ module Collectors
       # Collects stack trace samples from Ruby threads for both CPU-time (if available) and wall-clock.
       # Runs on its own background thread.
       #
-      class Stack < Worker
+      class Stack < Worker # rubocop:disable Metrics/ClassLength
         include Workers::Polling
 
         DEFAULT_MAX_TIME_USAGE_PCT = 2.0
@@ -22,27 +21,28 @@ class Stack < Worker
         attr_reader \
           :recorder,
           :max_frames,
+          :trace_identifiers_helper,
           :ignore_thread,
           :max_time_usage_pct,
           :thread_api
 
         def initialize(
           recorder,
           max_frames:,
+          trace_identifiers_helper:, # Usually an instance of Datadog::Profiling::TraceIdentifiers::Helper
           ignore_thread: nil,
           max_time_usage_pct: DEFAULT_MAX_TIME_USAGE_PCT,
           thread_api: Thread,
-          trace_identifiers_helper: Datadog::Profiling::TraceIdentifiers::Helper.new,
           fork_policy: Workers::Async::Thread::FORK_POLICY_RESTART, # Restart in forks by default
           interval: MIN_INTERVAL,
           enabled: true
         )
           @recorder = recorder
           @max_frames = max_frames
+          @trace_identifiers_helper = trace_identifiers_helper
           @ignore_thread = ignore_thread
           @max_time_usage_pct = max_time_usage_pct
           @thread_api = thread_api
-          @trace_identifiers_helper = trace_identifiers_helper
 
           # Workers::Async::Thread settings
           self.fork_policy = fork_policy
@@ -125,7 +125,7 @@ def collect_thread_event(thread, wall_time_interval_ns)
           locations = convert_backtrace_locations(locations)
 
           thread_id = thread.respond_to?(:pthread_thread_id) ? thread.pthread_thread_id : thread.object_id
-          trace_id, span_id = @trace_identifiers_helper.trace_identifiers_for(thread)
+          trace_id, span_id = trace_identifiers_helper.trace_identifiers_for(thread)
           cpu_time = get_cpu_time_interval!(thread)
 
           Events::StackSample.new(

lib/ddtrace/profiling/trace_identifiers/ddtrace.rb

@@ -7,34 +7,18 @@ module TraceIdentifiers
       # given thread, if there is an active trace for that thread in Datadog.tracer.
       class Ddtrace
         def initialize(tracer: nil)
-          @tracer = tracer
+          @tracer = (tracer if tracer.respond_to?(:active_correlation))
         end
 
         def trace_identifiers_for(thread)
-          current_tracer = tracer
-          return unless current_tracer
+          return unless @tracer
 
-          correlation = current_tracer.active_correlation(thread)
+          correlation = @tracer.active_correlation(thread)
           trace_id = correlation.trace_id
           span_id = correlation.span_id
 
           [trace_id, span_id] if trace_id && trace_id != 0 && span_id && span_id != 0
         end
-
-        private
-
-        def tracer
-          return @tracer if @tracer
-
-          # NOTE: Because the profiler may start working concurrently with tracer initialization,
-          # we need to be defensive here.
-          return unless Datadog.respond_to?(:tracer)
-
-          tracer = Datadog.tracer
-          return unless tracer.respond_to?(:active_correlation)
-
-          @tracer = tracer
-        end
       end
     end
   end

lib/ddtrace/profiling/trace_identifiers/helper.rb

@@ -16,7 +16,7 @@ class Helper
         ].freeze
         private_constant :DEFAULT_SUPPORTED_APIS
 
-        def initialize(supported_apis: DEFAULT_SUPPORTED_APIS.map(&:new))
+        def initialize(tracer:, supported_apis: DEFAULT_SUPPORTED_APIS.map { |api| api.new(tracer: tracer) })
           @supported_apis = supported_apis
         end
 

spec/ddtrace/configuration/components_spec.rb

@@ -35,7 +35,7 @@
         .and_return(tracer)
 
       expect(described_class).to receive(:build_profiler)
-        .with(settings, instance_of(Datadog::Configuration::AgentSettingsResolver::AgentSettings))
+        .with(settings, instance_of(Datadog::Configuration::AgentSettingsResolver::AgentSettings), tracer)
         .and_return(profiler)
 
       expect(described_class).to receive(:build_runtime_metrics_worker)
@@ -666,8 +666,9 @@
   describe '::build_profiler' do
     let(:agent_settings) { Datadog::Configuration::AgentSettingsResolver.call(settings, logger: nil) }
     let(:profiler) { build_profiler }
+    let(:tracer) { instance_double(Datadog::Tracer) }
 
-    subject(:build_profiler) { described_class.build_profiler(settings, agent_settings) }
+    subject(:build_profiler) { described_class.build_profiler(settings, agent_settings, tracer) }
 
     context 'when profiling is not supported' do
       before { allow(Datadog::Profiling).to receive(:supported?).and_return(false) }

spec/ddtrace/profiling/integration_spec.rb

@@ -15,6 +15,8 @@
     skip 'Profiling is not supported.' unless Datadog::Profiling.supported?
   end
 
+  let(:tracer) { instance_double(Datadog::Tracer) }
+
   shared_context 'StackSample events' do
     # NOTE: Please do not convert stack_one or stack_two to let, because
     # we want the method names on the resulting stacks to be stack_one or
@@ -55,7 +57,7 @@ def stack_two
     let(:collector) do
       Datadog::Profiling::Collectors::Stack.new(
         recorder,
-        enabled: true,
+        trace_identifiers_helper: Datadog::Profiling::TraceIdentifiers::Helper.new(tracer: tracer),
         max_frames: 400
       )
     end
@@ -121,10 +123,11 @@ def stack_two
           @current_span = span
           example.run
         end
-
         Datadog.tracer.shutdown!
       end
 
+      let(:tracer) { Datadog.tracer }
+
       before do
         expect(recorder)
           .to receive(:flush)

spec/ddtrace/profiling/trace_identifiers/ddtrace_spec.rb

@@ -46,22 +46,16 @@
     context 'when no tracer instance is available' do
       let(:tracer) { nil }
 
-      context 'because the tracer is unavailable' do
-        let(:datadog) { Module.new { const_set('Utils', Datadog::Utils) } }
-
-        before { stub_const('Datadog', datadog, transfer_nested_constant: true) }
-
-        it do
-          expect(trace_identifiers_for).to be nil
-        end
+      it do
+        expect(trace_identifiers_for).to be nil
       end
+    end
 
-      context 'because correlations are unavailable' do
-        before { allow(Datadog).to receive(:tracer).and_return(instance_double(Datadog::Tracer)) }
+    context 'when tracer does not support #active_correlation' do
+      let(:tracer) { double('Tracer') } # rubocop:disable RSpec/VerifiedDoubles
 
-        it do
-          expect(trace_identifiers_for).to be nil
-        end
+      it do
+        expect(trace_identifiers_for).to be nil
       end
     end
   end

spec/ddtrace/profiling/trace_identifiers/helper_spec.rb

@@ -6,7 +6,7 @@
   let(:api1) { instance_double(Datadog::Profiling::TraceIdentifiers::Ddtrace, 'api1') }
   let(:api2) { instance_double(Datadog::Profiling::TraceIdentifiers::Ddtrace, 'api2') }
 
-  subject(:trace_identifiers_helper) { described_class.new(supported_apis: [api1, api2]) }
+  subject(:trace_identifiers_helper) { described_class.new(tracer: nil, supported_apis: [api1, api2]) }
 
   describe '::DEFAULT_SUPPORTED_APIS' do
     it 'contains the Datadog trace identifiers' do