Skip to content

Commit

Permalink
DEBUG-2334 require file presence for line probe (#4105)
Browse files Browse the repository at this point in the history
While testing manually I encountered an exception due to
a line probe missing the file. This should not happen
and such a configuration is not usable since we don't know
which file to instrument. Add extra guards and a unit test
to handle this case.

Co-authored-by: Oleg Pudeyev <[email protected]>
  • Loading branch information
p-datadog and p authored Nov 12, 2024
1 parent 7489e77 commit 7d88137
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 3 deletions.
9 changes: 8 additions & 1 deletion lib/datadog/di/probe.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ def initialize(id:, type:,
raise ArgumentError, "Probe contains both line number and method name: #{id}"
end

if line_no && !file
raise ArgumentError, "Probe contains line number but not file: #{id}"
end

if type_name && !method_name || method_name && !type_name
raise ArgumentError, "Partial method probe definition: #{id}"
end
Expand Down Expand Up @@ -103,7 +107,10 @@ def capture_snapshot?
# method or for stack traversal purposes?), therefore we do not check
# for file name/path presence here and just consider the line number.
def line?
!line_no.nil?
# Constructor checks that file is given if line number is given,
# but for safety, check again here since we somehow got a probe with
# a line number but no file in the wild.
!!(file && line_no)
end

# Returns whether the probe is a method probe.
Expand Down
16 changes: 14 additions & 2 deletions spec/datadog/di/probe_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,18 @@
end
end

context 'line number given but file is not' do
let(:probe) do
described_class.new(id: "42", type: :log, line_no: 5)
end

it "raises ArgumentError" do
expect do
probe
end.to raise_error(ArgumentError, /Probe contains line number but not file/)
end
end

context 'unsupported type' do
let(:probe) do
# LOG_PROBE is a valid type in RC probe specification but not
Expand Down Expand Up @@ -151,7 +163,7 @@

describe "#line_no" do
context "one line number" do
let(:probe) { described_class.new(id: "x", type: :log, line_no: 5) }
let(:probe) { described_class.new(id: "x", type: :log, file: 'x', line_no: 5) }

it "returns the line number" do
expect(probe.line_no).to eq 5
Expand All @@ -169,7 +181,7 @@

describe "#line_no!" do
context "one line number" do
let(:probe) { described_class.new(id: "x", type: :log, line_no: 5) }
let(:probe) { described_class.new(id: "x", type: :log, file: 'x', line_no: 5) }

it "returns the line number" do
expect(probe.line_no!).to eq 5
Expand Down

0 comments on commit 7d88137

Please sign in to comment.