Skip to content

Commit 3d0ea3c

Browse files
p-datadogp-datadog
authored
DEBUG-4343 DI: add debugger.type to snapshot payloads (#5094)
1 parent 18e70e4 commit 3d0ea3c

File tree

5 files changed

+184
-140
lines changed

5 files changed

+184
-140
lines changed

‎lib/datadog/di/probe_notification_builder.rb‎

Lines changed: 35 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,12 @@ def build_snapshot_base(context, evaluation_errors: [], captures: nil, message:
116116
location = if probe.line?
117117
{
118118
file: context.path,
119-
lines: [probe.line_no],
119+
# Line numbers are required to be strings by the
120+
# system tests schema.
121+
# Backend I think accepts them also as integers, but some
122+
# other languages send strings and we decided to require
123+
# strings for everyone.
124+
lines: [probe.line_no.to_s],
120125
}
121126
elsif probe.method?
122127
{
@@ -131,19 +136,36 @@ def build_snapshot_base(context, evaluation_errors: [], captures: nil, message:
131136

132137
{
133138
service: settings.service,
134-
"debugger.snapshot": {
135-
id: SecureRandom.uuid,
136-
timestamp: timestamp,
137-
evaluationErrors: evaluation_errors,
138-
probe: {
139-
id: probe.id,
140-
version: 0,
141-
location: location,
139+
debugger: {
140+
type: 'snapshot',
141+
# Product can have three values: di, ld, er.
142+
# We do not currently implement exception replay.
143+
# There is currently no specification, and no consensus, for
144+
# when product should be di (dynamic instrumentation) and when
145+
# it should be ld (live debugger). I thought the backend was
146+
# supposed to provide this in probe specification via remote
147+
# config, but apparently this is not the case and the expectation
148+
# is that the library figures out the product via heuristics,
149+
# except there is currently no consensus on said heuristics.
150+
# .NET always sends ld, other languages send nothing at the moment.
151+
# Don't send anything for the time being.
152+
#product: 'di/ld',
153+
snapshot: {
154+
id: SecureRandom.uuid,
155+
timestamp: timestamp,
156+
evaluationErrors: evaluation_errors,
157+
probe: {
158+
id: probe.id,
159+
version: 0,
160+
location: location,
161+
},
162+
language: 'ruby',
163+
# TODO add test coverage for callers being nil
164+
stack: stack,
165+
# System tests schema validation requires captures to
166+
# always be present
167+
captures: captures || {},
142168
},
143-
language: 'ruby',
144-
# TODO add test coverage for callers being nil
145-
stack: stack,
146-
captures: captures,
147169
},
148170
# In python tracer duration is under debugger.snapshot,
149171
# but UI appears to expect it here at top level.

‎spec/datadog/di/integration/everything_from_remote_config_spec.rb‎

Lines changed: 70 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ def target_method
1111

1212
RSpec.describe 'DI integration from remote config' do
1313
di_test
14+
skip_unless_integration_testing_enabled
1415

1516
let(:remote) { Datadog::DI::Remote }
1617
let(:path) { 'datadog/2/LIVE_DEBUGGING/logProbe_uuid/hash' }
@@ -197,21 +198,24 @@ def target_method
197198
# We do not have active span/trace in the test.
198199
"dd.span_id": nil,
199200
"dd.trace_id": nil,
200-
"debugger.snapshot": {
201-
captures: nil,
202-
evaluationErrors: [],
203-
id: LOWERCASE_UUID_REGEXP,
204-
language: 'ruby',
205-
probe: {
206-
id: '11',
207-
location: {
208-
method: 'target_method',
209-
type: 'EverythingFromRemoteConfigSpecTestClass',
201+
debugger: {
202+
type: 'snapshot',
203+
snapshot: {
204+
captures: {},
205+
evaluationErrors: [],
206+
id: LOWERCASE_UUID_REGEXP,
207+
language: 'ruby',
208+
probe: {
209+
id: '11',
210+
location: {
211+
method: 'target_method',
212+
type: 'EverythingFromRemoteConfigSpecTestClass',
213+
},
214+
version: 0,
210215
},
211-
version: 0,
216+
stack: Array,
217+
timestamp: Integer,
212218
},
213-
stack: Array,
214-
timestamp: Integer,
215219
},
216220
ddsource: 'dd_debugger',
217221
duration: Integer,
@@ -459,23 +463,26 @@ def assert_received_and_errored
459463
# We do not have active span/trace in the test.
460464
"dd.span_id": nil,
461465
"dd.trace_id": nil,
462-
"debugger.snapshot": {
463-
captures: nil,
464-
evaluationErrors: [
465-
{'expr' => '(failing expression)', 'message' => 'Datadog::DI::Error::ExpressionEvaluationError: Bad collection type for filter: NilClass'},
466-
],
467-
id: LOWERCASE_UUID_REGEXP,
468-
language: 'ruby',
469-
probe: {
470-
id: '11',
471-
location: {
472-
method: 'target_method',
473-
type: 'EverythingFromRemoteConfigSpecTestClass',
466+
debugger: {
467+
type: 'snapshot',
468+
snapshot: {
469+
captures: {},
470+
evaluationErrors: [
471+
{'expr' => '(failing expression)', 'message' => 'Datadog::DI::Error::ExpressionEvaluationError: Bad collection type for filter: NilClass'},
472+
],
473+
id: LOWERCASE_UUID_REGEXP,
474+
language: 'ruby',
475+
probe: {
476+
id: '11',
477+
location: {
478+
method: 'target_method',
479+
type: 'EverythingFromRemoteConfigSpecTestClass',
480+
},
481+
version: 0,
474482
},
475-
version: 0,
483+
stack: Array,
484+
timestamp: Integer,
476485
},
477-
stack: Array,
478-
timestamp: Integer,
479486
},
480487
ddsource: 'dd_debugger',
481488
duration: Integer,
@@ -633,23 +640,26 @@ def assert_received_and_errored
633640
# We do not have active span/trace in the test.
634641
"dd.span_id": nil,
635642
"dd.trace_id": nil,
636-
"debugger.snapshot": {
637-
captures: nil,
638-
evaluationErrors: [
639-
{'expr' => '(expression)', 'message' => evaluation_error_message},
640-
],
641-
id: LOWERCASE_UUID_REGEXP,
642-
language: 'ruby',
643-
probe: {
644-
id: '11',
645-
location: {
646-
file: String,
647-
lines: [Integer],
643+
debugger: {
644+
type: 'snapshot',
645+
snapshot: {
646+
captures: {},
647+
evaluationErrors: [
648+
{'expr' => '(expression)', 'message' => evaluation_error_message},
649+
],
650+
id: LOWERCASE_UUID_REGEXP,
651+
language: 'ruby',
652+
probe: {
653+
id: '11',
654+
location: {
655+
file: String,
656+
lines: [String],
657+
},
658+
version: 0,
648659
},
649-
version: 0,
660+
stack: Array,
661+
timestamp: Integer,
650662
},
651-
stack: Array,
652-
timestamp: Integer,
653663
},
654664
ddsource: 'dd_debugger',
655665
duration: Integer,
@@ -709,29 +719,32 @@ def assert_received_and_errored
709719
'Datadog::DI::Error::ExpressionEvaluationError: Invalid arguments for contains: false, baz'
710720
end
711721

712-
let(:expected_captures) { nil }
722+
let(:expected_captures) { {} }
713723

714724
let(:expected_second_snapshot_payload) do
715725
{
716726
path: '/debugger/v1/input',
717727
# We do not have active span/trace in the test.
718728
"dd.span_id": nil,
719729
"dd.trace_id": nil,
720-
"debugger.snapshot": {
721-
captures: expected_captures,
722-
evaluationErrors: [],
723-
id: LOWERCASE_UUID_REGEXP,
724-
language: 'ruby',
725-
probe: {
726-
id: '11',
727-
location: {
728-
file: File.join(File.dirname(__FILE__), 'instrumentation_integration_test_class.rb'),
729-
lines: [67],
730+
debugger: {
731+
type: 'snapshot',
732+
snapshot: {
733+
captures: expected_captures,
734+
evaluationErrors: [],
735+
id: LOWERCASE_UUID_REGEXP,
736+
language: 'ruby',
737+
probe: {
738+
id: '11',
739+
location: {
740+
file: File.join(File.dirname(__FILE__), 'instrumentation_integration_test_class.rb'),
741+
lines: ['67'],
742+
},
743+
version: 0,
730744
},
731-
version: 0,
745+
stack: Array,
746+
timestamp: Integer,
732747
},
733-
stack: Array,
734-
timestamp: Integer,
735748
},
736749
ddsource: 'dd_debugger',
737750
duration: Integer,

‎spec/datadog/di/integration/instrumentation_spec.rb‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -178,9 +178,9 @@ def run_test
178178
component.probe_notifier_worker.flush
179179

180180
expect(payload).to be_a(Hash)
181-
expect(payload).to include(:"debugger.snapshot")
182-
snapshot = payload.fetch(:"debugger.snapshot")
183-
expect(snapshot[:captures]).to be nil
181+
expect(payload).to include(:debugger)
182+
snapshot = payload.fetch(:debugger).fetch(:snapshot)
183+
expect(snapshot.fetch(:captures)).to eq({})
184184
end
185185

186186
it 'assembles expected notification payload which does not include captures' do
@@ -215,7 +215,7 @@ def test_method
215215
expect(InstrumentationDelayedTestClass.new.test_method).to eq(43)
216216
component.probe_notifier_worker.flush
217217

218-
snapshot = payload.fetch(:"debugger.snapshot")
218+
snapshot = payload.fetch(:debugger).fetch(:snapshot)
219219
expect(snapshot).to match(
220220
id: String,
221221
timestamp: Integer,
@@ -225,7 +225,7 @@ def test_method
225225
}},
226226
language: 'ruby',
227227
stack: Array,
228-
captures: nil,
228+
captures: {},
229229
)
230230
end
231231
end
@@ -260,7 +260,7 @@ def test_method
260260
expect(InstrumentationDelayedPartialTestClass.new.test_method).to eq(43)
261261
component.probe_notifier_worker.flush
262262

263-
snapshot = payload.fetch(:"debugger.snapshot")
263+
snapshot = payload.fetch(:debugger).fetch(:snapshot)
264264
expect(snapshot).to match(
265265
id: String,
266266
timestamp: Integer,
@@ -272,7 +272,7 @@ def test_method
272272
# TODO the stack trace here does not contain the target method
273273
# as the first frame - see the comment in Instrumenter.
274274
stack: Array,
275-
captures: nil,
275+
captures: {},
276276
)
277277
end
278278
end
@@ -303,7 +303,7 @@ def method_missing(name) # rubocop:disable Style/MissingRespondToMissing
303303
expect(InstrumentationVirtualTestClass.new.test_method).to eq(:test_method)
304304
component.probe_notifier_worker.flush
305305

306-
snapshot = payload.fetch(:"debugger.snapshot")
306+
snapshot = payload.fetch(:debugger).fetch(:snapshot)
307307
expect(snapshot).to match(
308308
id: String,
309309
timestamp: Integer,
@@ -315,7 +315,7 @@ def method_missing(name) # rubocop:disable Style/MissingRespondToMissing
315315
# TODO the stack trace here does not contain the target method
316316
# as the first frame - see the comment in Instrumenter.
317317
stack: Array,
318-
captures: nil,
318+
captures: {},
319319
)
320320
end
321321
end
@@ -373,7 +373,7 @@ def run_test
373373
component.probe_notifier_worker.flush
374374

375375
expect(payload).to be_a(Hash)
376-
captures = payload.fetch(:"debugger.snapshot").fetch(:captures)
376+
captures = payload.fetch(:debugger).fetch(:snapshot).fetch(:captures)
377377
expect(captures).to eq(expected_captures)
378378
end
379379

@@ -671,13 +671,13 @@ def run_test
671671
end
672672

673673
let(:snapshot) do
674-
payload.fetch(:"debugger.snapshot")
674+
payload.fetch(:debugger).fetch(:snapshot)
675675
end
676676

677677
it 'does not have captures' do
678678
expect(diagnostics_transport).to receive(:send_diagnostics)
679679
# add_snapshot expectation replaces assertion on send_input
680-
expect(snapshot.fetch(:captures)).to be nil
680+
expect(snapshot.fetch(:captures)).to eq({})
681681
end
682682

683683
let(:stack) do
@@ -740,13 +740,13 @@ def run_test
740740
end
741741

742742
let(:snapshot) do
743-
payload.fetch(:"debugger.snapshot")
743+
payload.fetch(:debugger).fetch(:snapshot)
744744
end
745745

746746
it 'does not have captures' do
747747
expect(diagnostics_transport).to receive(:send_diagnostics)
748748
# add_snapshot expectation replaces assertion on send_input
749-
expect(snapshot.fetch(:captures)).to be nil
749+
expect(snapshot.fetch(:captures)).to eq({})
750750
end
751751

752752
let(:stack) do
@@ -909,7 +909,7 @@ def run_test
909909
component.probe_notifier_worker.flush
910910

911911
expect(payload).to be_a(Hash)
912-
captures = payload.fetch(:"debugger.snapshot").fetch(:captures)
912+
captures = payload.fetch(:debugger).fetch(:snapshot).fetch(:captures)
913913
expect(captures).to eq(expected_captures)
914914
end
915915
end

0 commit comments

Comments
 (0)