Skip to content

Commit

Permalink
DEBUG-3176 DI: report an error when instrumenting a loaded file that …
Browse files Browse the repository at this point in the history
…is not in registry (#4208)

Co-authored-by: Oleg Pudeyev <code@olegp.name>
  • Loading branch information
p-datadog and p authored Dec 12, 2024
1 parent 3c0de7d commit 65fb81d
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 1 deletion.
5 changes: 5 additions & 0 deletions lib/datadog/di/error.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ class AgentCommunicationError < Error
class DITargetNotDefined < Error
end

# Attempting to instrument a line and the file containing the line
# was loaded prior to code tracking being enabled.
class DITargetNotInRegistry < 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.
Expand Down
24 changes: 23 additions & 1 deletion lib/datadog/di/instrumenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -246,18 +246,20 @@ def hook_line(probe, &block)
#
# If the requested file is not in code tracker's registry,
# or the code tracker does not exist at all,
# do not attempt to instrumnet now.
# do not attempt to instrument now.
# The caller should add the line to the list of pending lines
# to instrument and install the hook when the file in
# question is loaded (and hopefully, by then code tracking
# is active, otherwise the line will never be instrumented.)
raise_if_probe_in_loaded_features(probe)
raise Error::DITargetNotDefined, "File not in code tracker registry: #{probe.file}"
end
end
elsif !permit_untargeted_trace_points
# Same as previous comment, if untargeted trace points are not
# explicitly defined, and we do not have code tracking, do not
# instrument the method.
raise_if_probe_in_loaded_features(probe)
raise Error::DITargetNotDefined, "File not in code tracker registry: #{probe.file}"
end

Expand Down Expand Up @@ -374,6 +376,26 @@ def unhook(probe)

attr_reader :lock

def raise_if_probe_in_loaded_features(probe)
return unless probe.file

# If the probe file is in the list of loaded files
# (as per $LOADED_FEATURES, using either exact or suffix match),
# raise an error indicating that
# code tracker is missing the loaded file because the file
# won't be loaded again (DI only works in production environments
# that do not normally reload code).
if $LOADED_FEATURES.include?(probe.file)
raise Error::DITargetNotInRegistry, "File loaded but is not in code tracker registry: #{probe.file}"
end
# Ths is an expensive check
$LOADED_FEATURES.each do |path|
if Utils.path_matches_suffix?(path, probe.file)
raise Error::DITargetNotInRegistry, "File matching probe path (#{probe.file}) was loaded and is not in code tracker registry: #{path}"
end
end
end

# TODO test that this resolves qualified names e.g. A::B
def symbolize_class_name(cls_name)
Object.const_get(cls_name)
Expand Down
2 changes: 2 additions & 0 deletions sig/datadog/di/error.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ module Datadog
end
class DITargetNotDefined < Error
end
class DITargetNotInRegistry < Error
end
class ProbePreviouslyFailed < Error
end
class MultiplePathsMatch < Error
Expand Down
1 change: 1 addition & 0 deletions sig/datadog/di/instrumenter.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ module Datadog

attr_reader lock: untyped
def symbolize_class_name: (untyped cls_name) -> untyped
def raise_if_probe_in_loaded_features: (Probe probe) -> void
end
end
end
14 changes: 14 additions & 0 deletions spec/datadog/di/instrumenter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -983,6 +983,20 @@
expect(observed_calls.length).to eq 1
expect(observed_calls.first).to be_a(Hash)
end

context 'when instrumenting a line in loaded but not tracked file' do
let(:probe) do
Datadog::DI::Probe.new(file: 'hook_line.rb', line_no: 3,
id: 1, type: :log)
end

it 'raises DITargetNotInRegistry' do
expect do
instrumenter.hook_line(probe) do |payload|
end
end.to raise_error(Datadog::DI::Error::DITargetNotInRegistry, /File matching probe path.*was loaded and is not in code tracker registry/)
end
end
end

context 'when method is recursive' do
Expand Down

0 comments on commit 65fb81d

Please sign in to comment.