diff --git a/benchmarks/di_instrument.rb b/benchmarks/di_instrument.rb index d5a027e9131..935a90b0e20 100644 --- a/benchmarks/di_instrument.rb +++ b/benchmarks/di_instrument.rb @@ -73,7 +73,11 @@ def run_benchmark settings = Datadog.configuration # We benchmark untargeted and targeted trace points; untargeted ones # are prohibited by default, permit them. - settings.dynamic_instrumentation.untargeted_trace_points = true + begin + settings.dynamic_instrumentation.internal.untargeted_trace_points = true + rescue NoMethodError + settings.dynamic_instrumentation.untargeted_trace_points = true + end redactor = Datadog::DI::Redactor.new(settings) serializer = Datadog::DI::Serializer.new(settings, redactor) logger = Logger.new(STDERR) diff --git a/lib/datadog/di/code_tracker.rb b/lib/datadog/di/code_tracker.rb index 33aba799852..dbf3d394c45 100644 --- a/lib/datadog/di/code_tracker.rb +++ b/lib/datadog/di/code_tracker.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +# rubocop:disable Lint/AssignmentInCondition + module Datadog module DI # Tracks loaded Ruby code by source file and maintains a map from @@ -77,6 +79,23 @@ def start registry[path] = tp.instruction_sequence end end + # Since this method normally is called from customer applications, + # 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.component # steep:ignore + 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") + # TODO test this path + else + # If we don't have a component, we cannot log anything properly. + # Do not just print a warning to avoid spamming customer logs. + # Don't reraise the exception either. + # TODO test this path + end end end end @@ -112,15 +131,18 @@ def active? def iseqs_for_path_suffix(suffix) registry_lock.synchronize do exact = registry[suffix] - return [exact] if exact + return [suffix, exact] if exact inexact = [] registry.each do |path, iseq| if Utils.path_matches_suffix?(path, suffix) - inexact << iseq + inexact << [path, iseq] end end - inexact + if inexact.length > 1 + raise Error::MultiplePathsMatch, "Multiple paths matched requested suffix" + end + inexact.first end end @@ -164,3 +186,5 @@ def clear end end end + +# rubocop:enable Lint/AssignmentInCondition diff --git a/lib/datadog/di/configuration/settings.rb b/lib/datadog/di/configuration/settings.rb index 5bf6f83d441..55413613494 100644 --- a/lib/datadog/di/configuration/settings.rb +++ b/lib/datadog/di/configuration/settings.rb @@ -12,8 +12,6 @@ def self.extended(base) def self.add_settings!(base) base.class_eval do - # The setting has "internal" prefix to prevent it from being - # prematurely turned on by customers. settings :dynamic_instrumentation do option :enabled do |o| o.type :bool @@ -26,48 +24,6 @@ def self.add_settings!(base) o.default false end - # This option instructs dynamic instrumentation to use - # untargeted trace points when installing line probes and - # code tracking is not active. - # WARNING: untargeted trace points carry a massive performance - # penalty for the entire file in which a line probe is placed. - # - # If this option is set to false, which is the default, - # dynamic instrumentation will add probes that reference - # unknown files to the list of pending probes, and when - # the respective files are loaded, the line probes will be - # installed using targeted trace points. If the file in - # question is already loaded when the probe is received - # (for example, it is in a third-party library loaded during - # application boot), and code tracking was not active when - # the file was loaded, such files will not be instrumentable - # via line probes. - # - # If this option is set to true - # - # activated, DI will in - # activated or because the files being targeted have beenIf true and code tracking is not enabled, dynamic instrumentation - # will use untargeted trace points. - # If false and code tracking is not enabled, dynamic - # instrumentation will not instrument any files loaded - # WARNING: these trace points will greatly degrade performance - # of all code in the instrumented files. - option :untargeted_trace_points do |o| - o.type :bool - o.default false - end - - # If true, all of the catch-all rescue blocks in DI - # will propagate the exceptions onward. - # WARNING: for internal Datadog use only - this will break - # the DI product and potentially the library in general in - # a multitude of ways, cause resource leakage, permanent - # performance decreases, etc. - option :propagate_all_exceptions do |o| - o.type :bool - o.default false - end - # An array of variable and key names to redact in addition to # the built-in list of identifiers. # @@ -154,6 +110,75 @@ def self.add_settings!(base) o.type :int o.default 20 end + + # Settings in the 'internal' group are for internal Datadog + # use only, and are needed to test dynamic instrumentation or + # experiment with features not released to customers. + settings :internal do + # This option instructs dynamic instrumentation to use + # untargeted trace points when installing line probes and + # code tracking is not active. + # WARNING: untargeted trace points carry a massive performance + # penalty for the entire file in which a line probe is placed. + # + # If this option is set to false, which is the default, + # dynamic instrumentation will add probes that reference + # unknown files to the list of pending probes, and when + # the respective files are loaded, the line probes will be + # installed using targeted trace points. If the file in + # question is already loaded when the probe is received + # (for example, it is in a third-party library loaded during + # application boot), and code tracking was not active when + # the file was loaded, such files will not be instrumentable + # via line probes. + # + # If this option is set to true, dynamic instrumentation will + # install untargeted trace points for all line probes, + # regardless of whether the referenced file is loaded. + # This permits instrumenting code which was loaded prior to + # code tracking being activated and instrumenting lines when + # code tracking is not activated at all. However, untargeted + # trace points are extremely slow and will greatly degrade + # performance of *all* code executed while they are installed, + # not just the instrumentation target. + option :untargeted_trace_points do |o| + o.type :bool + o.default false + end + + # If true, all of the catch-all rescue blocks in DI + # will propagate the exceptions onward. + # WARNING: for internal Datadog use only - this will break + # the DI product and potentially the library in general in + # a multitude of ways, cause resource leakage, permanent + # performance decreases, etc. + option :propagate_all_exceptions do |o| + o.type :bool + o.default false + end + + # Minimum interval, in seconds, between probe status and + # snapshot submissions to the agent. Probe notifier worker will + # batch together payloads submitted during each interval. + # A longer interval reduces the overhead imposed by dynamic + # instrumentation on the application, but also increases the + # time when application code cannot run (when the batches are + # being sent out by the probe notifier worker) and creates a + # possibility of dropping payloads if the queue gets too long. + option :min_send_interval do |o| + o.type :int + o.default 3 + end + + # Enable dynamic instrumentation in development environments. + # Currently DI does not fully implement support for code + # unloading and reloading, and is not supported in + # non-production environments. + option :development do |o| + o.type :bool + o.default false + end + end end end end diff --git a/lib/datadog/di/error.rb b/lib/datadog/di/error.rb index 5a90d259959..d5f305617cd 100644 --- a/lib/datadog/di/error.rb +++ b/lib/datadog/di/error.rb @@ -26,6 +26,23 @@ class AgentCommunicationError < Error # that does not in fact exist anywhere (e.g. due to a misspelling). class DITargetNotDefined < Error end + + # Raised when trying to install a probe whose installation failed + # earlier in the same process. This exception should contain the + # original exception report from initial installation attempt. + class ProbePreviouslyFailed < Error + end + + # Raised when installing a line probe and multiple files match the + # specified path suffix. + # A probe must be installed into one file only, since UI only + # supports one instrumented location for a probe. + # If multiple files match, UI cannot properly render the data from + # all of them, and arbitrarily choosing one file may be not what the + # user intended. Instrumentation will fail when multiple files match + # and the user will need to make their suffix more precise. + class MultiplePathsMatch < Error + end end end end diff --git a/lib/datadog/di/instrumenter.rb b/lib/datadog/di/instrumenter.rb index 540a6c0db7e..068a5205e9c 100644 --- a/lib/datadog/di/instrumenter.rb +++ b/lib/datadog/di/instrumenter.rb @@ -54,10 +54,11 @@ module DI # # @api private class Instrumenter - def initialize(settings, serializer, logger, code_tracker: nil) + def initialize(settings, serializer, logger, code_tracker: nil, telemetry: nil) @settings = settings @serializer = serializer @logger = logger + @telemetry = telemetry @code_tracker = code_tracker @lock = Mutex.new @@ -66,6 +67,7 @@ def initialize(settings, serializer, logger, code_tracker: nil) attr_reader :settings attr_reader :serializer attr_reader :logger + attr_reader :telemetry attr_reader :code_tracker # This is a substitute for Thread::Backtrace::Location @@ -172,12 +174,12 @@ def hook_line(probe, &block) # we use mock objects and the methods may be mocked with # individual invocations, yielding different return values on # different calls to the same method. - permit_untargeted_trace_points = settings.dynamic_instrumentation.untargeted_trace_points + permit_untargeted_trace_points = settings.dynamic_instrumentation.internal.untargeted_trace_points iseq = nil if code_tracker - iseq = code_tracker.iseqs_for_path_suffix(probe.file).first # steep:ignore - unless iseq + ret = code_tracker.iseqs_for_path_suffix(probe.file) # steep:ignore + unless ret if permit_untargeted_trace_points # Continue withoout targeting the trace point. # This is going to cause a serious performance penalty for @@ -204,6 +206,10 @@ def hook_line(probe, &block) raise Error::DITargetNotDefined, "File not in code tracker registry: #{probe.file}" end + if ret + actual_path, iseq = ret + end + # If trace point is not targeted, we only need one trace point per file. # Creating a trace point for each probe does work but the performance # penalty will be taken for each trace point defined in the file. @@ -217,18 +223,26 @@ def hook_line(probe, &block) # this optimization just yet and create a trace point for each probe. tp = TracePoint.new(:line) do |tp| - # If trace point is not targeted, we must verify that the invocation - # is the file & line that we want, because untargeted trace points - # are invoked for *each* line of Ruby executed. - if iseq || tp.lineno == probe.line_no && probe.file_matches?(tp.path) - if rate_limiter.nil? || rate_limiter.allow? - # & is to stop steep complaints, block is always present here. - block&.call(probe: probe, trace_point: tp, caller_locations: caller_locations) + begin + # If trace point is not targeted, we must verify that the invocation + # is the file & line that we want, because untargeted trace points + # are invoked for *each* line of Ruby executed. + if iseq || tp.lineno == probe.line_no && probe.file_matches?(tp.path) + if rate_limiter.nil? || rate_limiter.allow? + # & is to stop steep complaints, block is always present here. + block&.call(probe: probe, trace_point: tp, caller_locations: caller_locations) + end end + rescue => exc + raise if settings.dynamic_instrumentation.internal.propagate_all_exceptions + logger.warn("Unhandled exception in line trace point: #{exc.class}: #{exc}") + telemetry&.report(exc, description: "Unhandled exception in line trace point") + # TODO test this path end rescue => exc raise if settings.dynamic_instrumentation.propagate_all_exceptions logger.warn("Unhandled exception in line trace point: #{exc.class}: #{exc}") + telemetry&.report(exc, description: "Unhandled exception in line trace point") # TODO test this path end @@ -244,6 +258,8 @@ def hook_line(probe, &block) end probe.instrumentation_trace_point = tp + # actual_path could be nil if we don't use targeted trace points. + probe.instrumented_path = actual_path if iseq tp.enable(target: iseq, target_line: line_no) diff --git a/lib/datadog/di/probe.rb b/lib/datadog/di/probe.rb index 877c3ade846..5dd5cf88a72 100644 --- a/lib/datadog/di/probe.rb +++ b/lib/datadog/di/probe.rb @@ -71,6 +71,8 @@ def initialize(id:, type:, @rate_limit = rate_limit || (@capture_snapshot ? 1 : 5000) @rate_limiter = Datadog::Core::TokenBucket.new(@rate_limit) + + @emitting_notified = false end attr_reader :id @@ -157,6 +159,16 @@ def file_matches?(path) # Line trace point for line probes. Normally this would be a targeted # trace point. attr_accessor :instrumentation_trace_point + + # Actual path to the file instrumented by the probe, for line probes, + # when code tracking is available and line trace point is targeted. + # For untargeted line trace points instrumented path will be nil. + attr_accessor :instrumented_path + + attr_writer :emitting_notified + def emitting_notified? + !!@emitting_notified + end end end end diff --git a/lib/datadog/di/probe_notifier_worker.rb b/lib/datadog/di/probe_notifier_worker.rb index 98e49e5f0f7..6ce2ccdf340 100644 --- a/lib/datadog/di/probe_notifier_worker.rb +++ b/lib/datadog/di/probe_notifier_worker.rb @@ -23,12 +23,9 @@ module DI # # @api private class ProbeNotifierWorker - # Minimum interval between submissions. - # TODO make this into an internal setting and increase default to 2 or 3. - MIN_SEND_INTERVAL = 1 - - def initialize(settings, transport, logger) + def initialize(settings, transport, logger, telemetry: nil) @settings = settings + @telemetry = telemetry @status_queue = [] @snapshot_queue = [] @transport = transport @@ -39,10 +36,12 @@ def initialize(settings, transport, logger) @sleep_remaining = nil @wake_scheduled = false @thread = nil + @flush = 0 end attr_reader :settings attr_reader :logger + attr_reader :telemetry def start return if @thread @@ -53,33 +52,38 @@ def start # and then quit? break if @stop_requested - sleep_remaining = @lock.synchronize do - if sleep_remaining && sleep_remaining > 0 - # Recalculate how much sleep time is remaining, then sleep that long. - set_sleep_remaining - else - 0 + # If a flush was requested, send immediately and do not + # wait for the cooldown period. + if @lock.synchronize { @flush } == 0 + sleep_remaining = @lock.synchronize do + if sleep_remaining && sleep_remaining > 0 + # Recalculate how much sleep time is remaining, then sleep that long. + set_sleep_remaining + else + 0 + end end - end - if sleep_remaining > 0 - # Do not need to update @wake_scheduled here because - # wake-up is already scheduled for the earliest possible time. - wake.wait(sleep_remaining) - next + if sleep_remaining > 0 + # Do not need to update @wake_scheduled here because + # wake-up is already scheduled for the earliest possible time. + wake.wait(sleep_remaining) + next + end end begin more = maybe_send rescue => exc - raise if settings.dynamic_instrumentation.propagate_all_exceptions + raise if settings.dynamic_instrumentation.internal.propagate_all_exceptions logger.warn("Error in probe notifier worker: #{exc.class}: #{exc} (at #{exc.backtrace.first})") + telemetry&.report(exc, description: "Error in probe notifier worker") end @lock.synchronize do @wake_scheduled = more end - wake.wait(more ? MIN_SEND_INTERVAL : nil) + wake.wait(more ? min_send_interval : nil) end end end @@ -106,26 +110,40 @@ def stop(timeout = 1) # therefore, it should only be called when there is no parallel # activity (in another thread) that causes more notifications # to be generated. + # + # This method is used by the test suite to wait until notifications have + # been sent out, and could be used for graceful stopping of the + # worker thread. def flush - loop do - if @thread.nil? || !@thread.alive? - return - end + @lock.synchronize do + @flush += 1 + end + begin + loop do + if @thread.nil? || !@thread.alive? + return + end - io_in_progress, queues_empty = @lock.synchronize do - [io_in_progress?, status_queue.empty? && snapshot_queue.empty?] - end + io_in_progress, queues_empty = @lock.synchronize do + [io_in_progress?, status_queue.empty? && snapshot_queue.empty?] + end - if io_in_progress - # If we just call Thread.pass we could be in a busy loop - - # add a sleep. - sleep 0.25 - next - elsif queues_empty - break - else - sleep 0.25 - next + if io_in_progress + # If we just call Thread.pass we could be in a busy loop - + # add a sleep. + sleep 0.25 + next + elsif queues_empty + break + else + wake.signal + sleep 0.25 + next + end + end + ensure + @lock.synchronize do + @flush -= 1 end end end @@ -136,6 +154,11 @@ def flush attr_reader :wake attr_reader :thread + # Convenience method to keep line length reasonable in the rest of the file. + def min_send_interval + settings.dynamic_instrumentation.internal.min_send_interval + end + # This method should be called while @lock is held. def io_in_progress? @io_in_progress @@ -181,14 +204,14 @@ def io_in_progress? end # Determine how much longer the worker thread should sleep - # so as not to send in less than MIN_SEND_INTERVAL since the last send. + # so as not to send in less than min send interval since the last send. # Important: this method must be called when @lock is held. # # Returns the time remaining to sleep. def set_sleep_remaining now = Core::Utils::Time.get_time @sleep_remaining = if last_sent - [last_sent + MIN_SEND_INTERVAL - now, 0].max + [last_sent + min_send_interval - now, 0].max else 0 end @@ -218,16 +241,20 @@ def set_sleep_remaining @last_sent = time end rescue => exc - raise if settings.dynamic_instrumentation.propagate_all_exceptions + raise if settings.dynamic_instrumentation.internal.propagate_all_exceptions logger.warn("failed to send #{event_name}: #{exc.class}: #{exc} (at #{exc.backtrace.first})") + # Should we report this error to telemetry? Most likely failure + # to send is due to a network issue, and trying to send a + # telemetry message would also fail. end end batch.any? # steep:ignore - rescue ThreadError + rescue ThreadError => exc # Normally the queue should only be consumed in this method, # however if anyone consumes it elsewhere we don't want to block # while consuming it here. Rescue ThreadError and return. - logger.warn("unexpected #{event_name} queue underflow - consumed elsewhere?") + logger.warn("Unexpected #{event_name} queue underflow - consumed elsewhere?") + telemetry&.report(exc, description: "Unexpected #{event_name} queue underflow") ensure @lock.synchronize do @io_in_progress = false diff --git a/sig/datadog/di/code_tracker.rbs b/sig/datadog/di/code_tracker.rbs index c3aa0fae9e0..f09b14b8086 100644 --- a/sig/datadog/di/code_tracker.rbs +++ b/sig/datadog/di/code_tracker.rbs @@ -11,7 +11,7 @@ module Datadog def start: () -> void def active?: () -> bool - def iseqs_for_path_suffix: (String suffix) -> (::Array[RubyVM::InstructionSequence]) + def iseqs_for_path_suffix: (String suffix) -> untyped def stop: () -> void def clear: () -> void diff --git a/sig/datadog/di/error.rbs b/sig/datadog/di/error.rbs index 3f3dddc7780..351fa7bb85a 100644 --- a/sig/datadog/di/error.rbs +++ b/sig/datadog/di/error.rbs @@ -7,6 +7,10 @@ module Datadog end class DITargetNotDefined < Error end + class ProbePreviouslyFailed < Error + end + class MultiplePathsMatch < Error + end end end end diff --git a/sig/datadog/di/instrumenter.rbs b/sig/datadog/di/instrumenter.rbs index f11b82d1a82..cfaa56cac64 100644 --- a/sig/datadog/di/instrumenter.rbs +++ b/sig/datadog/di/instrumenter.rbs @@ -16,10 +16,12 @@ module Datadog @code_tracker: CodeTracker @logger: Core::Logger + + @telemetry: Core::Telemetry::Component? @lock: Mutex - def initialize: (untyped settings, Serializer serializer, Core::Logger logger, ?code_tracker: CodeTracker?) -> void + def initialize: (untyped settings, Serializer serializer, Core::Logger logger, ?code_tracker: CodeTracker?, ?telemetry: Core::Telemetry::Component) -> void attr_reader settings: untyped @@ -28,6 +30,8 @@ module Datadog attr_reader code_tracker: CodeTracker attr_reader logger: Core::Logger + + attr_reader telemetry: Core::Telemetry::Component? def hook_method: (Probe probe) ?{ (?) -> untyped } -> void diff --git a/sig/datadog/di/probe.rbs b/sig/datadog/di/probe.rbs index dd63ef4bc56..d492b650414 100644 --- a/sig/datadog/di/probe.rbs +++ b/sig/datadog/di/probe.rbs @@ -48,6 +48,7 @@ module Datadog attr_accessor instrumentation_module: Module? attr_accessor instrumentation_trace_point: TracePoint? + attr_accessor instrumented_path: String? end end end diff --git a/sig/datadog/di/probe_notifier_worker.rbs b/sig/datadog/di/probe_notifier_worker.rbs index e490ff036b2..1886aff2735 100644 --- a/sig/datadog/di/probe_notifier_worker.rbs +++ b/sig/datadog/di/probe_notifier_worker.rbs @@ -23,12 +23,16 @@ module Datadog @stop_requested: bool @logger: Core::Logger + + @telemetry: Core::Telemetry::Component? - def initialize: (untyped settings, Transport transport, Core::Logger logger) -> void + def initialize: (untyped settings, Transport transport, Core::Logger logger, ?telemetry: Core::Telemetry::Component) -> void attr_reader settings: untyped attr_reader logger: Core::Logger + + attr_reader telemetry: Core::Telemetry::Component? def start: () -> void def stop: (?::Integer timeout) -> void @@ -56,6 +60,8 @@ module Datadog def maybe_send_status: () -> bool def maybe_send_snapshot: () -> bool + + def min_send_interval: () -> Float end end end diff --git a/spec/datadog/di/code_tracker_spec.rb b/spec/datadog/di/code_tracker_spec.rb index 1bdc3644cd3..3f4b921e7be 100644 --- a/spec/datadog/di/code_tracker_spec.rb +++ b/spec/datadog/di/code_tracker_spec.rb @@ -142,11 +142,11 @@ end it "returns the exact match only" do - expect(tracker.iseqs_for_path_suffix(path)).to eq([path]) + expect(tracker.iseqs_for_path_suffix(path)).to eq([path, path]) end end - context "basename match" do + context "basename matches two paths" do let(:expected) do [ File.join(File.dirname(__FILE__), "code_tracker_test_class_1.rb"), @@ -154,14 +154,16 @@ ] end - it "returns the exact match only" do - expect(tracker.iseqs_for_path_suffix("code_tracker_test_class_1.rb")).to eq(expected) + it "raises exception" do + expect do + tracker.iseqs_for_path_suffix("code_tracker_test_class_1.rb") + end.to raise_error(Datadog::DI::Error::MultiplePathsMatch) end end context "match not on path component boundary" do - it "returns no matches" do - expect(tracker.iseqs_for_path_suffix("1.rb")).to eq([]) + it "returns nil" do + expect(tracker.iseqs_for_path_suffix("1.rb")).to be nil end end end diff --git a/spec/datadog/di/configuration/settings_spec.rb b/spec/datadog/di/configuration/settings_spec.rb index e0f5b75e3fb..54dc019c2f6 100644 --- a/spec/datadog/di/configuration/settings_spec.rb +++ b/spec/datadog/di/configuration/settings_spec.rb @@ -6,33 +6,45 @@ describe "dynamic_instrumentation" do context "programmatic configuration" do [ - ["enabled", true], - ["enabled", false], - ["untargeted_trace_points", true], - ["untargeted_trace_points", false], - ["propagate_all_exceptions", true], - ["propagate_all_exceptions", false], - ["redacted_identifiers", ["foo"]], - ["redacted_identifiers", []], - ["redacted_type_names", ["foo*", "bar"]], - ["redacted_type_names", []], - ["max_capture_depth", 5], - ["max_capture_collection_size", 10], - ["max_capture_string_length", 20], - ["max_capture_attribute_count", 4], - ].each do |(name_, value_)| + [nil, "enabled", true], + [nil, "enabled", false], + ["internal", "untargeted_trace_points", true], + ["internal", "untargeted_trace_points", false], + ["internal", "propagate_all_exceptions", true], + ["internal", "propagate_all_exceptions", false], + ['internal', 'min_send_interval', 5], + ['internal', 'development', true], + ['internal', 'development', false], + [nil, "redacted_identifiers", ["foo"]], + [nil, "redacted_identifiers", []], + [nil, "redacted_type_names", ["foo*", "bar"]], + [nil, "redacted_type_names", []], + [nil, "max_capture_depth", 5], + [nil, "max_capture_collection_size", 10], + [nil, "max_capture_string_length", 20], + [nil, "max_capture_attribute_count", 4], + ].each do |(scope_name_, name_, value_)| name = name_ + scope_name = scope_name_ value = value_.freeze context "when #{name} set to #{value}" do let(:value) { _value } + let(:scope) do + if scope_name + settings.dynamic_instrumentation.public_send(scope_name) + else + settings.dynamic_instrumentation + end + end + before do - settings.dynamic_instrumentation.public_send("#{name}=", value) + scope.public_send("#{name}=", value) end it "returns the value back" do - expect(settings.dynamic_instrumentation.public_send(name)).to eq(value) + expect(scope.public_send(name)).to eq(value) end end end diff --git a/spec/datadog/di/instrumenter_spec.rb b/spec/datadog/di/instrumenter_spec.rb index 0f03dcd7126..f34c397f1e3 100644 --- a/spec/datadog/di/instrumenter_spec.rb +++ b/spec/datadog/di/instrumenter_spec.rb @@ -8,20 +8,12 @@ let(:observed_calls) { [] } - let(:settings) do - double('settings').tap do |settings| - allow(settings).to receive(:dynamic_instrumentation).and_return(di_settings) - end - end - - let(:di_settings) do - double('di settings').tap do |settings| - allow(settings).to receive(:enabled).and_return(true) - allow(settings).to receive(:untargeted_trace_points).and_return(false) - allow(settings).to receive(:max_capture_depth).and_return(2) - allow(settings).to receive(:redacted_type_names).and_return([]) - allow(settings).to receive(:redacted_identifiers).and_return([]) - end + mock_settings_for_di do |settings| + allow(settings.dynamic_instrumentation).to receive(:enabled).and_return(true) + allow(settings.dynamic_instrumentation.internal).to receive(:untargeted_trace_points).and_return(false) + allow(settings.dynamic_instrumentation).to receive(:max_capture_depth).and_return(2) + allow(settings.dynamic_instrumentation).to receive(:redacted_type_names).and_return([]) + allow(settings.dynamic_instrumentation).to receive(:redacted_identifiers).and_return([]) end let(:redactor) do @@ -321,7 +313,7 @@ before do # We need untargeted trace points for this test since the line # being instrumented has already been loaded. - expect(di_settings).to receive(:untargeted_trace_points).and_return(true) + expect(di_internal_settings).to receive(:untargeted_trace_points).and_return(true) end let(:code_tracker) { nil } @@ -359,7 +351,7 @@ let(:code_tracker) { Datadog::DI.code_tracker } before do - expect(di_settings).to receive(:untargeted_trace_points).and_return(false) + expect(di_internal_settings).to receive(:untargeted_trace_points).and_return(false) Datadog::DI.activate_tracking! code_tracker.clear end @@ -389,7 +381,7 @@ before do # We need untargeted trace points for this test since the line # being instrumented has already been loaded. - expect(di_settings).to receive(:untargeted_trace_points).and_return(true) + expect(di_internal_settings).to receive(:untargeted_trace_points).and_return(true) end let(:probe) do @@ -416,7 +408,7 @@ before do # We need untargeted trace points for this test since the line # being instrumented has already been loaded. - expect(di_settings).to receive(:untargeted_trace_points).and_return(true) + expect(di_internal_settings).to receive(:untargeted_trace_points).and_return(true) end let(:probe) do @@ -472,7 +464,7 @@ let(:code_tracker) { nil } before do - expect(di_settings).to receive(:untargeted_trace_points).at_least(:once).and_return(true) + expect(di_internal_settings).to receive(:untargeted_trace_points).at_least(:once).and_return(true) end # We do not currently de-duplicate. diff --git a/spec/datadog/di/probe_notifier_worker_spec.rb b/spec/datadog/di/probe_notifier_worker_spec.rb index bdf6735eaef..0c595d330cb 100644 --- a/spec/datadog/di/probe_notifier_worker_spec.rb +++ b/spec/datadog/di/probe_notifier_worker_spec.rb @@ -4,16 +4,11 @@ RSpec.describe Datadog::DI::ProbeNotifierWorker do di_test - let(:settings) do - double('settings').tap do |settings| - allow(settings).to receive(:dynamic_instrumentation).and_return(di_settings) - end - end - - let(:di_settings) do - double('di settings').tap do |settings| - allow(settings).to receive(:propagate_all_exceptions).and_return(false) - end + mock_settings_for_di do |settings| + allow(settings.dynamic_instrumentation).to receive(:enabled).and_return(true) + allow(settings.dynamic_instrumentation.internal).to receive(:propagate_all_exceptions).and_return(false) + # Reduce to 1 to have the test run faster + allow(settings.dynamic_instrumentation.internal).to receive(:min_send_interval).and_return(1) end let(:transport) do @@ -81,22 +76,20 @@ {hello: 'world'} end - xit 'sends the snapshot' do + it 'sends the snapshot' do expect(worker.send(:snapshot_queue)).to be_empty expect(transport).to receive(:send_snapshot).once.with([snapshot]) worker.add_snapshot(snapshot) - # Since sending is asynchronous, we need to relinquish execution - # for the sending thread to run. - sleep(0.1) + worker.flush expect(worker.send(:snapshot_queue)).to eq([]) end context 'when three snapshots are added in quick succession' do - xit 'sends two batches' do + it 'sends two batches' do expect(worker.send(:snapshot_queue)).to be_empty expect(transport).to receive(:send_snapshot).once.with([snapshot]) @@ -106,22 +99,15 @@ worker.add_snapshot(snapshot) sleep 0.1 worker.add_snapshot(snapshot) - - # Since sending is asynchronous, we need to relinquish execution - # for the sending thread to run. sleep(0.1) # At this point the first snapshot should have been sent, # with the remaining two in the queue expect(worker.send(:snapshot_queue)).to eq([snapshot, snapshot]) - sleep 0.4 - # Still within the cooldown period - expect(worker.send(:snapshot_queue)).to eq([snapshot, snapshot]) - expect(transport).to receive(:send_snapshot).once.with([snapshot, snapshot]) - sleep 0.5 + worker.flush expect(worker.send(:snapshot_queue)).to eq([]) end end diff --git a/spec/datadog/di/spec_helper.rb b/spec/datadog/di/spec_helper.rb index 5fe045e4cea..3ce872c2943 100644 --- a/spec/datadog/di/spec_helper.rb +++ b/spec/datadog/di/spec_helper.rb @@ -12,9 +12,34 @@ def di_test end end end + + def mock_settings_for_di(&block) + let(:settings) do + double('settings').tap do |settings| + allow(settings).to receive(:dynamic_instrumentation).and_return(di_settings) + if block + instance_exec(settings, &block) + end + end + end + + let(:di_settings) do + double('di settings').tap do |settings| + allow(settings).to receive(:internal).and_return(di_internal_settings) + end + end + + let(:di_internal_settings) do + double('di internal settings') + end + end + end + + module InstanceMethods end end RSpec.configure do |config| config.extend DIHelpers::ClassMethods + config.include DIHelpers::InstanceMethods end