Skip to content

Commit

Permalink
exception boundaries at trace points
Browse files Browse the repository at this point in the history
  • Loading branch information
p committed Oct 25, 2024
1 parent 15d1f90 commit 3e59e5a
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 42 deletions.
81 changes: 47 additions & 34 deletions lib/datadog/di/code_tracker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 <definition-file>:<line>)" 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 <definition-file>:<line>)" 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
Expand Down
20 changes: 13 additions & 7 deletions lib/datadog/di/instrumenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
8 changes: 7 additions & 1 deletion lib/datadog/di/probe_manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit 3e59e5a

Please sign in to comment.