From 3e59e5aab23ae076e2116cf512cf8b7f6b28dad2 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Fri, 25 Oct 2024 14:00:45 -0400 Subject: [PATCH] exception boundaries at trace points --- lib/datadog/di/code_tracker.rb | 81 +++++++++++++++++++-------------- lib/datadog/di/instrumenter.rb | 20 +++++--- lib/datadog/di/probe_manager.rb | 8 +++- 3 files changed, 67 insertions(+), 42 deletions(-) diff --git a/lib/datadog/di/code_tracker.rb b/lib/datadog/di/code_tracker.rb index c01f6d2445e..98bc8f90d30 100644 --- a/lib/datadog/di/code_tracker.rb +++ b/lib/datadog/di/code_tracker.rb @@ -44,42 +44,55 @@ def start # Note: .trace enables the trace point. @compiled_trace_point = TracePoint.trace(:script_compiled) do |tp| - # Useful attributes of the trace point object here: - # .instruction_sequence - # .instruction_sequence.path (either absolute file path for - # loaded or required code, or for eval'd code, if filename - # is specified as argument to eval, then this is the provided - # filename, otherwise this is a synthesized - # "(eval at :)" string) - # .instruction_sequence.absolute_path (absolute file path when - # load or require are used to load code, nil for eval'd code - # regardless of whether filename was specified as an argument - # to eval on ruby 3.1+, same as path for eval'd code on ruby 3.0 - # and lower) - # .method_id - # .path (refers to the code location that called the require/eval/etc., - # not where the loaded code is; use .path on the instruction sequence - # to obtain the location of the compiled code) - # .eval_script - # - # For now just map the path to the instruction sequence. - path = tp.instruction_sequence.absolute_path - # Do not store mapping for eval'd code, since there is no way - # to target such code from dynamic instrumentation UI. - # eval'd code always sets tp.eval_script. - # When tp.eval_script is nil, code is either 'load'ed or 'require'd. - # steep, of course, complains about indexing with +path+ - # without checking that it is not nil, so here, maybe there is - # some situation where path would in fact be nil and - # steep would end up saving the day. - if path && !tp.eval_script - registry_lock.synchronize do - registry[path] = tp.instruction_sequence + begin + # Useful attributes of the trace point object here: + # .instruction_sequence + # .instruction_sequence.path (either absolute file path for + # loaded or required code, or for eval'd code, if filename + # is specified as argument to eval, then this is the provided + # filename, otherwise this is a synthesized + # "(eval at :)" string) + # .instruction_sequence.absolute_path (absolute file path when + # load or require are used to load code, nil for eval'd code + # regardless of whether filename was specified as an argument + # to eval on ruby 3.1+, same as path for eval'd code on ruby 3.0 + # and lower) + # .method_id + # .path (refers to the code location that called the require/eval/etc., + # not where the loaded code is; use .path on the instruction sequence + # to obtain the location of the compiled code) + # .eval_script + # + # For now just map the path to the instruction sequence. + path = tp.instruction_sequence.absolute_path + # Do not store mapping for eval'd code, since there is no way + # to target such code from dynamic instrumentation UI. + # eval'd code always sets tp.eval_script. + # When tp.eval_script is nil, code is either 'load'ed or 'require'd. + # steep, of course, complains about indexing with +path+ + # without checking that it is not nil, so here, maybe there is + # some situation where path would in fact be nil and + # steep would end up saving the day. + if path && !tp.eval_script + registry_lock.synchronize do + registry[path] = tp.instruction_sequence + end end - end - # TODO this line is not being exercised - DI.component&.probe_manager&.install_pending_line_probes(path) + # TODO this line is not being exercised + DI.component&.probe_manager&.install_pending_line_probes(path) + rescue => exc + if component = DI.component + raise if component.settings.dynamic_instrumentation.propagate_all_exceptions + component.logger.warn("Unhandled exception in script_compiled trace point: #{exc.class}: #{exc}") + # 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 end diff --git a/lib/datadog/di/instrumenter.rb b/lib/datadog/di/instrumenter.rb index bfdb83d25e0..0bb38b56a57 100644 --- a/lib/datadog/di/instrumenter.rb +++ b/lib/datadog/di/instrumenter.rb @@ -217,14 +217,20 @@ 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.propagate_all_exceptions + logger.warn("Unhandled exception in line trace point: #{exc.class}: #{exc}") + # TODO test this path end end diff --git a/lib/datadog/di/probe_manager.rb b/lib/datadog/di/probe_manager.rb index 8c65373cb34..bfd7b698766 100644 --- a/lib/datadog/di/probe_manager.rb +++ b/lib/datadog/di/probe_manager.rb @@ -22,7 +22,13 @@ def initialize(settings, instrumenter, probe_notification_builder, @pending_probes = {} @definition_trace_point = TracePoint.trace(:end) do |tp| - install_pending_method_probes(tp.self) + begin + install_pending_method_probes(tp.self) + rescue => exc + raise if settings.dynamic_instrumentation.propagate_all_exceptions + logger.warn("Unhandled exception in definition trace point: #{exc.class}: #{exc}") + # TODO test this path + end end end