From fa252bc404a07d69e859139f4d884b55f0f66f45 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Mon, 16 Dec 2024 16:49:48 -0500 Subject: [PATCH 01/15] Rework DI loading --- lib/datadog/di.rb | 59 +--------------------------- lib/datadog/di/base.rb | 72 ++++++++++++++++++++++++++++++++++ lib/datadog/di/code_tracker.rb | 8 ++-- lib/datadog/di/init.rb | 2 +- 4 files changed, 79 insertions(+), 62 deletions(-) create mode 100644 lib/datadog/di/base.rb diff --git a/lib/datadog/di.rb b/lib/datadog/di.rb index 700fbce100f..debf2a1f986 100644 --- a/lib/datadog/di.rb +++ b/lib/datadog/di.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +require_relative 'di/base' require_relative 'di/error' require_relative 'di/code_tracker' require_relative 'di/component' @@ -49,64 +50,6 @@ def enabled? LOCK = Mutex.new class << self - attr_reader :code_tracker - - # Activates code tracking. Normally this method should be called - # when the application starts. If instrumenting third-party code, - # code tracking needs to be enabled before the third-party libraries - # are loaded. If you definitely will not be instrumenting - # third-party libraries, activating tracking after third-party libraries - # have been loaded may improve lookup performance. - # - # TODO test that activating tracker multiple times preserves - # existing mappings in the registry - def activate_tracking! - (@code_tracker ||= CodeTracker.new).start - end - - # Activates code tracking if possible. - # - # This method does nothing if invoked in an environment that does not - # implement required trace points for code tracking (MRI Ruby < 2.6, - # JRuby) and rescues any exceptions that may be raised by downstream - # DI code. - def activate_tracking - # :script_compiled trace point was added in Ruby 2.6. - return unless RUBY_VERSION >= '2.6' - - begin - # Activate code tracking by default because line trace points will not work - # without it. - Datadog::DI.activate_tracking! - rescue => exc - if defined?(Datadog.logger) - Datadog.logger.warn("Failed to activate code tracking for DI: #{exc.class}: #{exc}") - else - # We do not have Datadog logger potentially because DI code tracker is - # being loaded early in application boot process and the rest of datadog - # wasn't loaded yet. Output to standard error. - warn("Failed to activate code tracking for DI: #{exc.class}: #{exc}") - end - end - end - - # Deactivates code tracking. In normal usage of DI this method should - # never be called, however it is used by DI's test suite to reset - # state for individual tests. - # - # Note that deactivating tracking clears out the registry, losing - # the ability to look up files that have been loaded into the process - # already. - def deactivate_tracking! - code_tracker&.stop - end - - # Returns whether code tracking is available. - # This method should be used instead of querying #code_tracker - # because the latter one may be nil. - def code_tracking_active? - code_tracker&.active? || false - end # This method is called from DI Remote handler to issue DI operations # to the probe manager (add or remove probes). diff --git a/lib/datadog/di/base.rb b/lib/datadog/di/base.rb new file mode 100644 index 00000000000..73c82e20ac1 --- /dev/null +++ b/lib/datadog/di/base.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +require_relative 'code_tracker' + +module Datadog + # Namespace for Datadog dynamic instrumentation. + # + # @api private + module DI + + class << self + attr_reader :code_tracker + + # Activates code tracking. Normally this method should be called + # when the application starts. If instrumenting third-party code, + # code tracking needs to be enabled before the third-party libraries + # are loaded. If you definitely will not be instrumenting + # third-party libraries, activating tracking after third-party libraries + # have been loaded may improve lookup performance. + # + # TODO test that activating tracker multiple times preserves + # existing mappings in the registry + def activate_tracking! + (@code_tracker ||= CodeTracker.new).start + end + + # Activates code tracking if possible. + # + # This method does nothing if invoked in an environment that does not + # implement required trace points for code tracking (MRI Ruby < 2.6, + # JRuby) and rescues any exceptions that may be raised by downstream + # DI code. + def activate_tracking + # :script_compiled trace point was added in Ruby 2.6. + return unless RUBY_VERSION >= '2.6' + + begin + # Activate code tracking by default because line trace points will not work + # without it. + Datadog::DI.activate_tracking! + rescue => exc + if defined?(Datadog.logger) + Datadog.logger.warn("Failed to activate code tracking for DI: #{exc.class}: #{exc}") + else + # We do not have Datadog logger potentially because DI code tracker is + # being loaded early in application boot process and the rest of datadog + # wasn't loaded yet. Output to standard error. + warn("Failed to activate code tracking for DI: #{exc.class}: #{exc}") + end + end + end + + # Deactivates code tracking. In normal usage of DI this method should + # never be called, however it is used by DI's test suite to reset + # state for individual tests. + # + # Note that deactivating tracking clears out the registry, losing + # the ability to look up files that have been loaded into the process + # already. + def deactivate_tracking! + code_tracker&.stop + end + + # Returns whether code tracking is available. + # This method should be used instead of querying #code_tracker + # because the latter one may be nil. + def code_tracking_active? + code_tracker&.active? || false + end + end + end +end diff --git a/lib/datadog/di/code_tracker.rb b/lib/datadog/di/code_tracker.rb index 5ace92a17d2..23aa896067a 100644 --- a/lib/datadog/di/code_tracker.rb +++ b/lib/datadog/di/code_tracker.rb @@ -2,6 +2,8 @@ # rubocop:disable Lint/AssignmentInCondition +require_relative 'error' + module Datadog module DI # Tracks loaded Ruby code by source file and maintains a map from @@ -87,9 +89,9 @@ def start # rescue any exceptions that might not be handled to not break said # customer applications. rescue => exc - # TODO we do not have DI.component defined yet, remove steep:ignore - # before release. - if component = DI.current_component # steep:ignore + # Code tracker may be loaded without the rest of DI, + # in which case DI.current_component won't be defined. + if component = DI.respond_to?(:current_component) && DI.current_component raise if component.settings.dynamic_instrumentation.internal.propagate_all_exceptions component.logger.warn("Unhandled exception in script_compiled trace point: #{exc.class}: #{exc}") component.telemetry&.report(exc, description: "Unhandled exception in script_compiled trace point") diff --git a/lib/datadog/di/init.rb b/lib/datadog/di/init.rb index 0b6a9217c2d..2af4c29c325 100644 --- a/lib/datadog/di/init.rb +++ b/lib/datadog/di/init.rb @@ -4,7 +4,7 @@ # enable dynamic instrumentation for third-party libraries used by the # application. -require_relative '../di' +require_relative 'base' # Code tracking is required for line probes to work; see the comments # on the activate_tracking methods in di.rb for further details. From 3579a0c190cc9e71f410da1ab6a94e26d81e1705 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Mon, 16 Dec 2024 16:55:09 -0500 Subject: [PATCH 02/15] add note --- lib/datadog/di/base.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/datadog/di/base.rb b/lib/datadog/di/base.rb index 73c82e20ac1..62a9b8fdc75 100644 --- a/lib/datadog/di/base.rb +++ b/lib/datadog/di/base.rb @@ -1,5 +1,13 @@ # frozen_string_literal: true +# This file is loaded by datadog/di/init.rb. +# It contains just the global DI reference to the (normally one and only) +# code tracker for the current process. +# This file should not require the rest of DI, specifically none of the +# contrib code that is meant to be loaded after third-party libraries +# are loaded, and also none of the rest of datadog library which also +# has contrib code in other products. + require_relative 'code_tracker' module Datadog From ad35c7d4b82eaa0ea5d9eb6a7524f9235b399677 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Mon, 16 Dec 2024 16:58:01 -0500 Subject: [PATCH 03/15] note purpose of current_component --- lib/datadog/di.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/datadog/di.rb b/lib/datadog/di.rb index debf2a1f986..844388a8267 100644 --- a/lib/datadog/di.rb +++ b/lib/datadog/di.rb @@ -72,6 +72,8 @@ def component # which contains a reference to the most recently instantiated # DI::Component. This way, if a DI component hasn't been instantiated, # we do not try to reference Datadog.components. + # In other words, this method exists so that we never attempt to call + # Datadog.components from the code tracker. def current_component LOCK.synchronize do @current_components&.last From d15e60a02363ca65bc02fe9b56937f8511108997 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Mon, 16 Dec 2024 16:58:46 -0500 Subject: [PATCH 04/15] move current_component --- lib/datadog/di.rb | 37 ------------------------------------- lib/datadog/di/base.rb | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 37 deletions(-) diff --git a/lib/datadog/di.rb b/lib/datadog/di.rb index 844388a8267..221c32864f6 100644 --- a/lib/datadog/di.rb +++ b/lib/datadog/di.rb @@ -47,8 +47,6 @@ def enabled? # Expose DI to global shared objects Extensions.activate! - LOCK = Mutex.new - class << self # This method is called from DI Remote handler to issue DI operations @@ -63,41 +61,6 @@ class << self def component Datadog.send(:components).dynamic_instrumentation end - - # DI code tracker is instantiated globally before the regular set of - # components is created, but the code tracker needs to call out to the - # "current" DI component to perform instrumentation when application - # code is loaded. Because this call may happen prior to Datadog - # components having been initialized, we maintain the "current component" - # which contains a reference to the most recently instantiated - # DI::Component. This way, if a DI component hasn't been instantiated, - # we do not try to reference Datadog.components. - # In other words, this method exists so that we never attempt to call - # Datadog.components from the code tracker. - def current_component - LOCK.synchronize do - @current_components&.last - end - end - - # To avoid potential races with DI::Component being added and removed, - # we maintain a list of the components. Normally the list should contain - # either zero or one component depending on whether DI is enabled in - # Datadog configuration. However, if a new instance of DI::Component - # is created while the previous instance is still running, we are - # guaranteed to not end up with no component when one is running. - def add_current_component(component) - LOCK.synchronize do - @current_components ||= [] - @current_components << component - end - end - - def remove_current_component(component) - LOCK.synchronize do - @current_components&.delete(component) - end - end end end end diff --git a/lib/datadog/di/base.rb b/lib/datadog/di/base.rb index 62a9b8fdc75..9f8fece15ce 100644 --- a/lib/datadog/di/base.rb +++ b/lib/datadog/di/base.rb @@ -16,6 +16,8 @@ module Datadog # @api private module DI + LOCK = Mutex.new + class << self attr_reader :code_tracker @@ -75,6 +77,41 @@ def deactivate_tracking! def code_tracking_active? code_tracker&.active? || false end + + # DI code tracker is instantiated globally before the regular set of + # components is created, but the code tracker needs to call out to the + # "current" DI component to perform instrumentation when application + # code is loaded. Because this call may happen prior to Datadog + # components having been initialized, we maintain the "current component" + # which contains a reference to the most recently instantiated + # DI::Component. This way, if a DI component hasn't been instantiated, + # we do not try to reference Datadog.components. + # In other words, this method exists so that we never attempt to call + # Datadog.components from the code tracker. + def current_component + LOCK.synchronize do + @current_components&.last + end + end + + # To avoid potential races with DI::Component being added and removed, + # we maintain a list of the components. Normally the list should contain + # either zero or one component depending on whether DI is enabled in + # Datadog configuration. However, if a new instance of DI::Component + # is created while the previous instance is still running, we are + # guaranteed to not end up with no component when one is running. + def add_current_component(component) + LOCK.synchronize do + @current_components ||= [] + @current_components << component + end + end + + def remove_current_component(component) + LOCK.synchronize do + @current_components&.delete(component) + end + end end end end From f87e9b12cfe735d51fc05fd6faeebe7d91af0cf5 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Tue, 17 Dec 2024 09:05:47 -0500 Subject: [PATCH 05/15] current_component will always be defined --- lib/datadog/di/code_tracker.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/datadog/di/code_tracker.rb b/lib/datadog/di/code_tracker.rb index 23aa896067a..dcd875e9731 100644 --- a/lib/datadog/di/code_tracker.rb +++ b/lib/datadog/di/code_tracker.rb @@ -90,8 +90,9 @@ def start # customer applications. rescue => exc # Code tracker may be loaded without the rest of DI, - # in which case DI.current_component won't be defined. - if component = DI.respond_to?(:current_component) && DI.current_component + # in which case DI.component will not yet be defined, + # but we will have DI.current_component (set to nil). + if component = DI.current_component raise if component.settings.dynamic_instrumentation.internal.propagate_all_exceptions component.logger.warn("Unhandled exception in script_compiled trace point: #{exc.class}: #{exc}") component.telemetry&.report(exc, description: "Unhandled exception in script_compiled trace point") From 31d7a9bd5d0f5628b5b7e5bf96ce622c73f86ca8 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Thu, 19 Dec 2024 09:53:25 -0500 Subject: [PATCH 06/15] standard --- lib/datadog/di/base.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/datadog/di/base.rb b/lib/datadog/di/base.rb index 9f8fece15ce..057a83eb82c 100644 --- a/lib/datadog/di/base.rb +++ b/lib/datadog/di/base.rb @@ -15,7 +15,6 @@ module Datadog # # @api private module DI - LOCK = Mutex.new class << self From 4eeb858ac39c089b3fc671c7ffd27ca016a1f2f6 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Tue, 24 Dec 2024 10:17:40 -0500 Subject: [PATCH 07/15] types --- sig/datadog/di.rbs | 15 --------------- sig/datadog/di/base.rbs | 23 +++++++++++++++++++++++ 2 files changed, 23 insertions(+), 15 deletions(-) create mode 100644 sig/datadog/di/base.rbs diff --git a/sig/datadog/di.rbs b/sig/datadog/di.rbs index a8c7f3a4a97..375da13af21 100644 --- a/sig/datadog/di.rbs +++ b/sig/datadog/di.rbs @@ -1,21 +1,6 @@ module Datadog module DI - def self.code_tracker: () -> CodeTracker? - def self.component: () -> Component? - def self.current_component: () -> Component? - - def self.add_current_component: (Component) -> void - - def self.remove_current_component: (Component) -> void - - def self.activate_tracking: () -> void - - def self.activate_tracking!: () -> void - - def self.deactivate_tracking!: () -> void - - LOCK: Mutex end end diff --git a/sig/datadog/di/base.rbs b/sig/datadog/di/base.rbs new file mode 100644 index 00000000000..82d6d74e5dd --- /dev/null +++ b/sig/datadog/di/base.rbs @@ -0,0 +1,23 @@ +module Datadog + module DI + self.@code_tracker: CodeTracker? + + attr_reader self.code_tracker: CodeTracker? + + def self.activate_tracking: () -> void + + def self.activate_tracking!: () -> void + + def self.deactivate_tracking!: () -> void + + def self.code_tracking_active?: () -> bool + + def self.current_component: () -> Component? + + def self.add_current_component: (Component) -> void + + def self.remove_current_component: (Component) -> void + + LOCK: Mutex + end +end From 49a172878f82972f6ae3968cd3e89fc9600ebb4a Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Tue, 24 Dec 2024 11:48:32 -0500 Subject: [PATCH 08/15] integration test for DI loading --- spec/datadog/di/init_spec.rb | 49 ++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 spec/datadog/di/init_spec.rb diff --git a/spec/datadog/di/init_spec.rb b/spec/datadog/di/init_spec.rb new file mode 100644 index 00000000000..f0c6478d95e --- /dev/null +++ b/spec/datadog/di/init_spec.rb @@ -0,0 +1,49 @@ +require "datadog/di/spec_helper" +require 'open3' + +RSpec.describe 'DI initializer' do + di_test + + context 'when loaded initially into a clean process' do + it 'loads only DI code tracker' do + script = <<-SCRIPT + if defined?(Datadog) && Datadog.constants != %i(VERSION) + raise "Datadog code loaded too early" + end + + require 'datadog/di/init' + + if Datadog.constants.sort != %i(DI VERSION) + raise "Too many datadog components loaded: \#{Datadog.constants}" + end + + unless Datadog::DI.code_tracker + raise "Code tracker not instantiated" + end + + unless Datadog::DI.code_tracker.send(:registry).empty? + raise "Code tracker registry is not empty" + end + + # Test load something + require 'open3' + + if Datadog::DI.code_tracker.send(:registry).empty? + raise "Code tracker did not add loaded file to registry" + end + + unless Datadog::DI.code_tracker.send(:registry).detect { |key, value| key =~ /open3.rb\\z/ } + raise "Loaded script not found in code tracker registry" + end + + if Datadog.constants.sort != %i(DI VERSION) + raise "Too many datadog components loaded at the end of execution: \#{Datadog.constants}" + end + SCRIPT + out, status = Open3.capture2e('ruby', stdin_data: script) + unless status.exitstatus == 0 + fail("Test script failed with exist status #{status.exitstatus}:\n#{out}") + end + end + end +end From f62fcb22f5a46743b7fe859e4fd5c67cc8d8f90b Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Tue, 24 Dec 2024 11:30:51 -0500 Subject: [PATCH 09/15] add exit status assertions to execution spec --- spec/datadog/core/environment/execution_spec.rb | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/spec/datadog/core/environment/execution_spec.rb b/spec/datadog/core/environment/execution_spec.rb index ba709c90601..be3fae47139 100644 --- a/spec/datadog/core/environment/execution_spec.rb +++ b/spec/datadog/core/environment/execution_spec.rb @@ -59,10 +59,11 @@ context 'when in an IRB session' do it 'returns true' do # Ruby 2.6 does not have irb by default in a bundle, but has it outside of it. - _, err, = Bundler.with_unbundled_env do + _, err, status = Bundler.with_unbundled_env do Open3.capture3('irb', '--noprompt', '--noverbose', '--noecho', stdin_data: repl_script) end expect(err).to end_with('ACTUAL:true') + expect(status.exitstatus).to eq(0) end end @@ -203,11 +204,12 @@ def test_it_does_something_useful # Add our script to `env.rb`, which is always run before any feature is executed. File.write('features/support/env.rb', repl_script) - _, err, = Bundler.with_unbundled_env do + _, err, status = Bundler.with_unbundled_env do Open3.capture3('ruby', stdin_data: script) end expect(err).to include('ACTUAL:true') + expect(status.exitstatus).to eq(0) end end end @@ -270,7 +272,7 @@ def test_it_does_something_useful context 'when given WebMock', skip: Gem::Version.new(Bundler::VERSION) < Gem::Version.new('2') do it do - out, = Bundler.with_unbundled_env do + out, err, status = Bundler.with_unbundled_env do Open3.capture3('ruby', stdin_data: <<-RUBY require 'bundler/inline' @@ -292,6 +294,7 @@ def test_it_does_something_useful end expect(out).to end_with('ACTUAL:true') + expect(status.exitstatus).to eq(0) end end end From 5b1a9e01f220e83f2d5f10e3e535e5e436720871 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Tue, 24 Dec 2024 12:35:42 -0500 Subject: [PATCH 10/15] typo fix --- spec/datadog/di/init_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/datadog/di/init_spec.rb b/spec/datadog/di/init_spec.rb index f0c6478d95e..d31aaf8a8bd 100644 --- a/spec/datadog/di/init_spec.rb +++ b/spec/datadog/di/init_spec.rb @@ -42,7 +42,7 @@ SCRIPT out, status = Open3.capture2e('ruby', stdin_data: script) unless status.exitstatus == 0 - fail("Test script failed with exist status #{status.exitstatus}:\n#{out}") + fail("Test script failed with exit status #{status.exitstatus}:\n#{out}") end end end From 4cc2d795cd053c5af97df634ed142c47f680ed20 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Tue, 24 Dec 2024 12:39:49 -0500 Subject: [PATCH 11/15] rubocop --- spec/datadog/core/environment/execution_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/datadog/core/environment/execution_spec.rb b/spec/datadog/core/environment/execution_spec.rb index be3fae47139..0cda18a9b00 100644 --- a/spec/datadog/core/environment/execution_spec.rb +++ b/spec/datadog/core/environment/execution_spec.rb @@ -272,7 +272,7 @@ def test_it_does_something_useful context 'when given WebMock', skip: Gem::Version.new(Bundler::VERSION) < Gem::Version.new('2') do it do - out, err, status = Bundler.with_unbundled_env do + out, _err, status = Bundler.with_unbundled_env do Open3.capture3('ruby', stdin_data: <<-RUBY require 'bundler/inline' From d3b854c4d899fc6083aac23aff2782e5b6d86992 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Wed, 25 Dec 2024 12:09:18 -0500 Subject: [PATCH 12/15] test that code tracker maintains mappings after datadog is loaded --- spec/datadog/di/init_spec.rb | 70 ++++++++++++++++++++++++------------ 1 file changed, 47 insertions(+), 23 deletions(-) diff --git a/spec/datadog/di/init_spec.rb b/spec/datadog/di/init_spec.rb index d31aaf8a8bd..f5a0a0106be 100644 --- a/spec/datadog/di/init_spec.rb +++ b/spec/datadog/di/init_spec.rb @@ -4,40 +4,64 @@ RSpec.describe 'DI initializer' do di_test - context 'when loaded initially into a clean process' do - it 'loads only DI code tracker' do - script = <<-SCRIPT - if defined?(Datadog) && Datadog.constants != %i(VERSION) - raise "Datadog code loaded too early" - end + BOOTSTRAP_SCRIPT = <<-SCRIPT + if defined?(Datadog) && Datadog.constants != %i(VERSION) + raise "Datadog code loaded too early" + end - require 'datadog/di/init' + require 'datadog/di/init' - if Datadog.constants.sort != %i(DI VERSION) - raise "Too many datadog components loaded: \#{Datadog.constants}" - end + if Datadog.constants.sort != %i(DI VERSION) + raise "Too many datadog components loaded: \#{Datadog.constants}" + end - unless Datadog::DI.code_tracker - raise "Code tracker not instantiated" - end + unless Datadog::DI.code_tracker + raise "Code tracker not instantiated" + end - unless Datadog::DI.code_tracker.send(:registry).empty? - raise "Code tracker registry is not empty" - end + unless Datadog::DI.code_tracker.send(:registry).empty? + raise "Code tracker registry is not empty" + end - # Test load something - require 'open3' + # Test load something + require 'open3' - if Datadog::DI.code_tracker.send(:registry).empty? - raise "Code tracker did not add loaded file to registry" - end + if Datadog::DI.code_tracker.send(:registry).empty? + raise "Code tracker did not add loaded file to registry" + end + + unless Datadog::DI.code_tracker.send(:registry).detect { |key, value| key =~ /open3.rb\\z/ } + raise "Loaded script not found in code tracker registry" + end + + if Datadog.constants.sort != %i(DI VERSION) + raise "Too many datadog components loaded at the end of execution: \#{Datadog.constants}" + end + SCRIPT + + context 'when loaded initially into a clean process' do + it 'loads only DI code tracker' do + out, status = Open3.capture2e('ruby', stdin_data: BOOTSTRAP_SCRIPT) + unless status.exitstatus == 0 + fail("Test script failed with exit status #{status.exitstatus}:\n#{out}") + end + end + end + + context 'when entire library is loaded after di bootstrapper' do + it 'keeps the mappings in code tracker prior to datadog load' do + script = <<-SCRIPT + #{BOOTSTRAP_SCRIPT} + + require 'datadog' + # Should still have the open3 entry in code tracker unless Datadog::DI.code_tracker.send(:registry).detect { |key, value| key =~ /open3.rb\\z/ } raise "Loaded script not found in code tracker registry" end - if Datadog.constants.sort != %i(DI VERSION) - raise "Too many datadog components loaded at the end of execution: \#{Datadog.constants}" + unless defined?(Datadog::Tracing) + raise "Expected Datadog::Tracing to be defined after datadog was loaded" end SCRIPT out, status = Open3.capture2e('ruby', stdin_data: script) From a1bde4fa85dff60974161fa25f87fa37f594885b Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Wed, 8 Jan 2025 23:13:10 -0500 Subject: [PATCH 13/15] add prefixes to logging calls --- lib/datadog/di/base.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/datadog/di/base.rb b/lib/datadog/di/base.rb index 057a83eb82c..6d4e957778c 100644 --- a/lib/datadog/di/base.rb +++ b/lib/datadog/di/base.rb @@ -49,12 +49,12 @@ def activate_tracking Datadog::DI.activate_tracking! rescue => exc if defined?(Datadog.logger) - Datadog.logger.warn("Failed to activate code tracking for DI: #{exc.class}: #{exc}") + Datadog.logger.warn { "di: Failed to activate code tracking for DI: #{exc.class}: #{exc}" } else # We do not have Datadog logger potentially because DI code tracker is # being loaded early in application boot process and the rest of datadog # wasn't loaded yet. Output to standard error. - warn("Failed to activate code tracking for DI: #{exc.class}: #{exc}") + warn("datadog: di: Failed to activate code tracking for DI: #{exc.class}: #{exc}") end end end From dfe7473d2ca5234caf147dd55562549215eeded2 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Wed, 8 Jan 2025 23:14:00 -0500 Subject: [PATCH 14/15] revise comment --- lib/datadog/di/base.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/datadog/di/base.rb b/lib/datadog/di/base.rb index 6d4e957778c..afe8c38e602 100644 --- a/lib/datadog/di/base.rb +++ b/lib/datadog/di/base.rb @@ -23,9 +23,8 @@ class << self # Activates code tracking. Normally this method should be called # when the application starts. If instrumenting third-party code, # code tracking needs to be enabled before the third-party libraries - # are loaded. If you definitely will not be instrumenting - # third-party libraries, activating tracking after third-party libraries - # have been loaded may improve lookup performance. + # are loaded. Any third-party code loaded before code tracking is + # activated will NOT be instrumentable using dynamic instrumentation. # # TODO test that activating tracker multiple times preserves # existing mappings in the registry From a482c0f727d72ce23d1870447f99dcfc1007e87e Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Thu, 9 Jan 2025 01:35:28 -0500 Subject: [PATCH 15/15] standard --- spec/datadog/di/init_spec.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/datadog/di/init_spec.rb b/spec/datadog/di/init_spec.rb index f5a0a0106be..45a5f650fc4 100644 --- a/spec/datadog/di/init_spec.rb +++ b/spec/datadog/di/init_spec.rb @@ -4,6 +4,7 @@ RSpec.describe 'DI initializer' do di_test + # rubocop:disable Lint/ConstantDefinitionInBlock BOOTSTRAP_SCRIPT = <<-SCRIPT if defined?(Datadog) && Datadog.constants != %i(VERSION) raise "Datadog code loaded too early" @@ -38,6 +39,7 @@ raise "Too many datadog components loaded at the end of execution: \#{Datadog.constants}" end SCRIPT + # rubocop:enable Lint/ConstantDefinitionInBlock context 'when loaded initially into a clean process' do it 'loads only DI code tracker' do