Skip to content

Commit dfe0a13

Browse files
watsonwconti27
watson
authored and
wconti27
committed
[DI] Support multiple probes in the same location (#5535)
1 parent 1effe89 commit dfe0a13

File tree

7 files changed

+776
-68
lines changed

7 files changed

+776
-68
lines changed

‎integration-tests/debugger/basic.spec.js‎

Lines changed: 149 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,154 @@ describe('Dynamic Instrumentation', function () {
240240
}
241241
})
242242
}
243+
244+
describe('multiple probes at the same location', function () {
245+
it('should support adding multiple probes at the same location', function (done) {
246+
const rcConfig1 = t.generateRemoteConfig()
247+
const rcConfig2 = t.generateRemoteConfig()
248+
const expectedPayloads = [{
249+
ddsource: 'dd_debugger',
250+
service: 'node',
251+
debugger: { diagnostics: { probeId: rcConfig1.config.id, probeVersion: 0, status: 'RECEIVED' } }
252+
}, {
253+
ddsource: 'dd_debugger',
254+
service: 'node',
255+
debugger: { diagnostics: { probeId: rcConfig2.config.id, probeVersion: 0, status: 'RECEIVED' } }
256+
}, {
257+
ddsource: 'dd_debugger',
258+
service: 'node',
259+
debugger: { diagnostics: { probeId: rcConfig1.config.id, probeVersion: 0, status: 'INSTALLED' } }
260+
}, {
261+
ddsource: 'dd_debugger',
262+
service: 'node',
263+
debugger: { diagnostics: { probeId: rcConfig2.config.id, probeVersion: 0, status: 'INSTALLED' } }
264+
}]
265+
266+
t.agent.on('debugger-diagnostics', ({ payload }) => {
267+
payload.forEach((event) => {
268+
const expected = expectedPayloads.shift()
269+
assertObjectContains(event, expected)
270+
})
271+
endIfDone()
272+
})
273+
274+
t.agent.addRemoteConfig(rcConfig1)
275+
t.agent.addRemoteConfig(rcConfig2)
276+
277+
function endIfDone () {
278+
if (expectedPayloads.length === 0) done()
279+
}
280+
})
281+
282+
it('should support triggering multiple probes added at the same location', function (done) {
283+
let installed = 0
284+
const rcConfig1 = t.generateRemoteConfig()
285+
const rcConfig2 = t.generateRemoteConfig()
286+
const expectedPayloads = new Map([
287+
[rcConfig1.config.id, {
288+
ddsource: 'dd_debugger',
289+
service: 'node',
290+
debugger: { diagnostics: { probeId: rcConfig1.config.id, probeVersion: 0, status: 'EMITTING' } }
291+
}],
292+
[rcConfig2.config.id, {
293+
ddsource: 'dd_debugger',
294+
service: 'node',
295+
debugger: { diagnostics: { probeId: rcConfig2.config.id, probeVersion: 0, status: 'EMITTING' } }
296+
}]
297+
])
298+
299+
t.agent.on('debugger-diagnostics', ({ payload }) => {
300+
payload.forEach((event) => {
301+
const { diagnostics } = event.debugger
302+
if (diagnostics.status === 'INSTALLED') {
303+
if (++installed === 2) {
304+
t.axios.get(t.breakpoint.url).catch(done)
305+
}
306+
} else if (diagnostics.status === 'EMITTING') {
307+
const expected = expectedPayloads.get(diagnostics.probeId)
308+
expectedPayloads.delete(diagnostics.probeId)
309+
assertObjectContains(event, expected)
310+
}
311+
})
312+
endIfDone()
313+
})
314+
315+
t.agent.addRemoteConfig(rcConfig1)
316+
t.agent.addRemoteConfig(rcConfig2)
317+
318+
function endIfDone () {
319+
if (expectedPayloads.size === 0) done()
320+
}
321+
})
322+
323+
it('should support not triggering any probes when all conditions are not met', function (done) {
324+
let installed = 0
325+
const rcConfig1 = t.generateRemoteConfig({ when: { json: { eq: [{ ref: 'foo' }, 'bar'] } } })
326+
const rcConfig2 = t.generateRemoteConfig({ when: { json: { eq: [{ ref: 'foo' }, 'baz'] } } })
327+
328+
t.agent.on('debugger-diagnostics', ({ payload }) => {
329+
payload.forEach((event) => {
330+
const { diagnostics } = event.debugger
331+
if (diagnostics.status === 'INSTALLED') {
332+
if (++installed === 2) {
333+
t.axios.get(t.breakpoint.url).catch(done)
334+
setTimeout(done, 2000)
335+
}
336+
} else if (diagnostics.status === 'EMITTING') {
337+
assert.fail('should not trigger any probes when all conditions are not met')
338+
}
339+
})
340+
})
341+
342+
t.agent.addRemoteConfig(rcConfig1)
343+
t.agent.addRemoteConfig(rcConfig2)
344+
})
345+
346+
it('should support only triggering the probes whos conditions are met', function (done) {
347+
let installed = 0
348+
const rcConfig1 = t.generateRemoteConfig({ when: { json: { eq: [{ ref: 'foo' }, 'bar'] } } })
349+
const rcConfig2 = t.generateRemoteConfig({
350+
when: { json: { eq: [{ getmember: [{ getmember: [{ ref: 'request' }, 'params'] }, 'name'] }, 'bar'] } }
351+
})
352+
const rcConfig3 = t.generateRemoteConfig()
353+
const expectedPayloads = new Map([
354+
[rcConfig2.config.id, {
355+
ddsource: 'dd_debugger',
356+
service: 'node',
357+
debugger: { diagnostics: { probeId: rcConfig2.config.id, probeVersion: 0, status: 'EMITTING' } }
358+
}],
359+
[rcConfig3.config.id, {
360+
ddsource: 'dd_debugger',
361+
service: 'node',
362+
debugger: { diagnostics: { probeId: rcConfig3.config.id, probeVersion: 0, status: 'EMITTING' } }
363+
}]
364+
])
365+
366+
t.agent.on('debugger-diagnostics', ({ payload }) => {
367+
payload.forEach((event) => {
368+
const { diagnostics } = event.debugger
369+
if (diagnostics.status === 'INSTALLED') {
370+
if (++installed === 3) {
371+
t.axios.get(t.breakpoint.url).catch(done)
372+
}
373+
} else if (diagnostics.status === 'EMITTING') {
374+
const expected = expectedPayloads.get(diagnostics.probeId)
375+
expectedPayloads.delete(diagnostics.probeId)
376+
assertObjectContains(event, expected)
377+
}
378+
})
379+
endIfDone()
380+
})
381+
382+
t.agent.addRemoteConfig(rcConfig1)
383+
t.agent.addRemoteConfig(rcConfig2)
384+
t.agent.addRemoteConfig(rcConfig3)
385+
386+
function endIfDone () {
387+
if (expectedPayloads.size === 0) done()
388+
}
389+
})
390+
})
243391
})
244392

245393
describe('input messages', function () {
@@ -395,7 +543,7 @@ describe('Dynamic Instrumentation', function () {
395543
beforeEach(t.triggerBreakpoint)
396544

397545
it('should trigger when condition is met', function (done) {
398-
t.agent.on('debugger-input', (x) => {
546+
t.agent.on('debugger-input', () => {
399547
done()
400548
})
401549

‎packages/dd-trace/src/debugger/devtools_client/breakpoints.js‎

Lines changed: 98 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
'use strict'
22

33
const { getGeneratedPosition } = require('./source-maps')
4+
const lock = require('./lock')()
45
const session = require('./session')
56
const compileCondition = require('./condition')
67
const { MAX_SNAPSHOTS_PER_SECOND_PER_PROBE, MAX_NON_SNAPSHOTS_PER_SECOND_PER_PROBE } = require('./defaults')
7-
const { findScriptFromPartialPath, probes, breakpoints } = require('./state')
8+
const { findScriptFromPartialPath, locationToBreakpoint, breakpointToProbes, probeToLocation } = require('./state')
89
const log = require('../../log')
910

1011
let sessionStarted = false
@@ -43,54 +44,124 @@ async function addBreakpoint (probe) {
4344
({ line: lineNumber, column: columnNumber } = await getGeneratedPosition(url, source, lineNumber, sourceMapURL))
4445
}
4546

46-
log.debug(
47-
'[debugger:devtools_client] Adding breakpoint at %s:%d:%d (probe: %s, version: %d)',
48-
url, lineNumber, columnNumber, probe.id, probe.version
49-
)
50-
51-
let condition
5247
try {
53-
condition = probe.when?.json && compileCondition(probe.when.json)
48+
probe.condition = probe.when?.json && compileCondition(probe.when.json)
5449
} catch (err) {
5550
throw new Error(`Cannot compile expression: ${probe.when.dsl}`, { cause: err })
5651
}
5752

58-
const { breakpointId } = await session.post('Debugger.setBreakpoint', {
59-
location: {
60-
scriptId,
61-
lineNumber: lineNumber - 1, // Beware! lineNumber is zero-indexed
62-
columnNumber
63-
},
64-
condition
65-
})
66-
67-
probes.set(probe.id, breakpointId)
68-
breakpoints.set(breakpointId, probe)
53+
const release = await lock()
54+
55+
try {
56+
log.debug(
57+
'[debugger:devtools_client] Adding breakpoint at %s:%d:%d (probe: %s, version: %d)',
58+
url, lineNumber, columnNumber, probe.id, probe.version
59+
)
60+
61+
const locationKey = generateLocationKey(scriptId, lineNumber, columnNumber)
62+
const breakpoint = locationToBreakpoint.get(locationKey)
63+
64+
if (breakpoint) {
65+
// A breakpoint already exists at this location, so we need to add the probe to the existing breakpoint
66+
await updateBreakpoint(breakpoint, probe)
67+
} else {
68+
// No breakpoint exists at this location, so we need to create a new one
69+
const location = {
70+
scriptId,
71+
lineNumber: lineNumber - 1, // Beware! lineNumber is zero-indexed
72+
columnNumber
73+
}
74+
const result = await session.post('Debugger.setBreakpoint', {
75+
location,
76+
condition: probe.condition
77+
})
78+
probeToLocation.set(probe.id, locationKey)
79+
locationToBreakpoint.set(locationKey, { id: result.breakpointId, location, locationKey })
80+
breakpointToProbes.set(result.breakpointId, new Map([[probe.id, probe]]))
81+
}
82+
} finally {
83+
release()
84+
}
6985
}
7086

7187
async function removeBreakpoint ({ id }) {
7288
if (!sessionStarted) {
7389
// We should not get in this state, but abort if we do, so the code doesn't fail unexpected
7490
throw Error(`Cannot remove probe ${id}: Debugger not started`)
7591
}
76-
if (!probes.has(id)) {
92+
if (!probeToLocation.has(id)) {
7793
throw Error(`Unknown probe id: ${id}`)
7894
}
7995

80-
const breakpointId = probes.get(id)
81-
await session.post('Debugger.removeBreakpoint', { breakpointId })
82-
probes.delete(id)
83-
breakpoints.delete(breakpointId)
96+
const release = await lock()
97+
98+
try {
99+
const locationKey = probeToLocation.get(id)
100+
const breakpoint = locationToBreakpoint.get(locationKey)
101+
const probesAtLocation = breakpointToProbes.get(breakpoint.id)
102+
103+
probesAtLocation.delete(id)
104+
probeToLocation.delete(id)
105+
106+
if (probesAtLocation.size === 0) {
107+
locationToBreakpoint.delete(locationKey)
108+
breakpointToProbes.delete(breakpoint.id)
109+
if (breakpointToProbes.size === 0) {
110+
await stop() // TODO: Will this actually delete the breakpoint?
111+
} else {
112+
await session.post('Debugger.removeBreakpoint', { breakpointId: breakpoint.id })
113+
}
114+
} else {
115+
await updateBreakpoint(breakpoint)
116+
}
117+
} finally {
118+
release()
119+
}
120+
}
121+
122+
async function updateBreakpoint (breakpoint, probe) {
123+
const probesAtLocation = breakpointToProbes.get(breakpoint.id)
124+
const conditionBeforeNewProbe = compileCompoundCondition(Array.from(probesAtLocation.values()))
125+
126+
// If a probe is provided, add it to the breakpoint. If not, it's because we're removing a probe, but potentially
127+
// need to update the condtion of the breakpoint.
128+
if (probe) {
129+
probesAtLocation.set(probe.id, probe)
130+
probeToLocation.set(probe.id, breakpoint.locationKey)
131+
}
84132

85-
if (breakpoints.size === 0) return stop() // return instead of await to reduce number of promises created
133+
const condition = compileCompoundCondition(Array.from(probesAtLocation.values()))
134+
135+
if (condition || conditionBeforeNewProbe !== condition) {
136+
await session.post('Debugger.removeBreakpoint', { breakpointId: breakpoint.id })
137+
breakpointToProbes.delete(breakpoint.id)
138+
const result = await session.post('Debugger.setBreakpoint', {
139+
location: breakpoint.location,
140+
condition
141+
})
142+
breakpoint.id = result.breakpointId
143+
breakpointToProbes.set(result.breakpointId, probesAtLocation)
144+
}
86145
}
87146

88147
function start () {
89148
sessionStarted = true
90-
return session.post('Debugger.enable') // return instead of await to reduce number of promises created
149+
return session.post('Debugger.enable')
91150
}
92151

93152
function stop () {
94153
sessionStarted = false
95-
return session.post('Debugger.disable') // return instead of await to reduce number of promises created
154+
return session.post('Debugger.disable')
155+
}
156+
157+
// Only if all probes have a condition can we use a compound condition.
158+
// Otherwise, we need to evaluate each probe individually once the breakpoint is hit.
159+
function compileCompoundCondition (probes) {
160+
return probes.every(p => p.condition)
161+
? probes.map(p => p.condition).filter(Boolean).join(' || ')
162+
: undefined
163+
}
164+
165+
function generateLocationKey (scriptId, lineNumber, columnNumber) {
166+
return `${scriptId}:${lineNumber}:${columnNumber}`
96167
}

0 commit comments

Comments
 (0)