Skip to content

Commit d8677ee

Browse files
authored
[DI] Add support for probe condition (#5488)
Add support for having a condition on a probe. A condition should return either `true` or `false`. It can however also throw an error. The error can be thrown in two places: 1. When compiling the AST down to the JavaScript source code (an issue was detected at compilation-time) 2. When running the condition in the context of the breakpoint (an issue was detected at run-time) Compilation-time exceptions are reported as an error event on the probe to the DI diagnostics backend. Run-time exceptions are currently just swallowed and the result of the condition is going to be the same as if it evaluated `false` (i.e. it's not going to tigger the breakpoint). To be 100% compatible with how conditions work in the other tracers, we should technically record the run-time errors and expose those to the user (as a help in debugging the condition). However, due to how conditions are implemented in this PR (by using the `condition` property on the `Debugger.setBreakpoint` Chrome DevTools Protocol API), it's not possible to know if a condition failed, or just returned `false`. For now, this is an ok compromise, due to the increased performance gained by using this API.
1 parent 658800a commit d8677ee

6 files changed

Lines changed: 1109 additions & 1 deletion

File tree

integration-tests/debugger/basic.spec.js

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,54 @@ describe('Dynamic Instrumentation', function () {
391391
})
392392
})
393393

394+
describe('condition', function () {
395+
beforeEach(t.triggerBreakpoint)
396+
397+
it('should trigger when condition is met', function (done) {
398+
t.agent.on('debugger-input', (x) => {
399+
done()
400+
})
401+
402+
t.agent.addRemoteConfig(t.generateRemoteConfig({
403+
when: { json: { eq: [{ getmember: [{ getmember: [{ ref: 'request' }, 'params'] }, 'name'] }, 'bar'] } }
404+
}))
405+
})
406+
407+
it('should not trigger when condition is not met', function (done) {
408+
t.agent.on('debugger-diagnostics', ({ payload }) => {
409+
payload.forEach((event) => {
410+
if (event.debugger.diagnostics.status === 'INSTALLED') {
411+
// Can't know if the probe didn't trigger, so just wait a bit and see if the test fails in the mean time
412+
setTimeout(done, 2000)
413+
}
414+
})
415+
})
416+
417+
t.agent.on('debugger-input', () => {
418+
assert.fail('Should not trigger when condition is not met')
419+
})
420+
421+
t.agent.addRemoteConfig(t.generateRemoteConfig({
422+
when: { json: { eq: [{ getmember: [{ getmember: [{ ref: 'request' }, 'params'] }, 'name'] }, 'invalid'] } }
423+
}))
424+
})
425+
426+
it('should report error if condition is invalid', function (done) {
427+
t.agent.on('debugger-diagnostics', ({ payload }) => {
428+
payload.forEach(({ debugger: { diagnostics } }) => {
429+
if (diagnostics.status === 'ERROR') {
430+
assert.strictEqual(diagnostics.exception.message, 'Cannot compile expression: original dsl')
431+
done()
432+
}
433+
})
434+
})
435+
436+
t.agent.addRemoteConfig(t.generateRemoteConfig({
437+
when: { dsl: 'original dsl', json: { ref: 'this is not a valid ref' } }
438+
}))
439+
})
440+
})
441+
394442
describe('race conditions', function () {
395443
it('should remove the last breakpoint completely before trying to add a new one', function (done) {
396444
const rcConfig2 = t.generateRemoteConfig()

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
const { getGeneratedPosition } = require('./source-maps')
44
const session = require('./session')
5+
const compileCondition = require('./condition')
56
const { MAX_SNAPSHOTS_PER_SECOND_PER_PROBE, MAX_NON_SNAPSHOTS_PER_SECOND_PER_PROBE } = require('./defaults')
67
const { findScriptFromPartialPath, probes, breakpoints } = require('./state')
78
const log = require('../../log')
@@ -47,12 +48,20 @@ async function addBreakpoint (probe) {
4748
url, lineNumber, columnNumber, probe.id, probe.version
4849
)
4950

51+
let condition
52+
try {
53+
condition = probe.when?.json && compileCondition(probe.when.json)
54+
} catch (err) {
55+
throw new Error(`Cannot compile expression: ${probe.when.dsl}`, { cause: err })
56+
}
57+
5058
const { breakpointId } = await session.post('Debugger.setBreakpoint', {
5159
location: {
5260
scriptId,
5361
lineNumber: lineNumber - 1, // Beware! lineNumber is zero-indexed
5462
columnNumber
55-
}
63+
},
64+
condition
5665
})
5766

5867
probes.set(probe.id, breakpointId)
Lines changed: 244 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,244 @@
1+
'use strict'
2+
3+
module.exports = compile
4+
5+
const identifierRegex = /^[@a-zA-Z_$][\w$]*$/
6+
7+
// The following identifiers have purposefully not been included in this list:
8+
// - The reserved words `this` and `super` as they can have valid use cases as `ref` values
9+
// - The literals `undefined` and `Infinity` as they can be useful as `ref` values, especially to check if a
10+
// variable is `undefined`.
11+
// - The following future reserved words in older standards, as they can now be used safely:
12+
// `abstract`, `boolean`, `byte`, `char`, `double`, `final`, `float`, `goto`, `int`, `long`, `native`, `short`,
13+
// `synchronized`, `throws`, `transient`, `volatile`.
14+
const reservedWords = new Set([
15+
// Reserved words
16+
'break', 'case', 'catch', 'class', 'const', 'continue', 'debugger', 'default', 'delete', 'do', 'else', 'export',
17+
'extends', 'false', 'finally', 'for', 'function', 'if', 'import', 'in', 'instanceof', 'new', 'null', 'return',
18+
'switch', 'throw', 'true', 'try', 'typeof', 'var', 'void', 'while', 'with',
19+
20+
// Reserved in strict mode
21+
'let', 'static', 'yield',
22+
23+
// Reserved in module code or async function bodies:
24+
'await',
25+
26+
// Future reserved words
27+
'enum',
28+
29+
// Future reserved words in strict mode
30+
'implements', 'interface', 'package', 'private', 'protected', 'public',
31+
32+
// Litterals
33+
'NaN'
34+
])
35+
36+
// TODO: Consider storing some of these functions on `process` so they can be reused across probes
37+
function compile (node) {
38+
if (node === null || typeof node === 'number' || typeof node === 'boolean' || typeof node === 'string') {
39+
return JSON.stringify(node)
40+
}
41+
42+
const [type, value] = Object.entries(node)[0]
43+
44+
if (type === 'not') {
45+
return `!(${compile(value)})`
46+
} else if (type === 'len' || type === 'count') {
47+
return getSize(compile(value))
48+
} else if (type === 'isEmpty') {
49+
return `${getSize(compile(value))} === 0`
50+
} else if (type === 'isDefined') {
51+
return `(() => {
52+
try {
53+
${value}
54+
return true
55+
} catch {
56+
return false
57+
}
58+
})()`
59+
} else if (type === 'instanceof') {
60+
return `Function.prototype[Symbol.hasInstance].call(${value[1]}, ${compile(value[0])})`
61+
} else if (type === 'ref') {
62+
if (value === '@it') {
63+
return '$dd_it'
64+
} else if (value === '@key') {
65+
return '$dd_key'
66+
} else if (value === '@value') {
67+
return '$dd_value'
68+
} else {
69+
return assertIdentifier(value)
70+
}
71+
} else if (Array.isArray(value)) {
72+
const args = value.map(compile)
73+
switch (type) {
74+
case 'eq': return `(${args[0]}) === (${args[1]})`
75+
case 'ne': return `(${args[0]}) !== (${args[1]})`
76+
case 'gt': return `${guardAgainstCoercionSideEffects(args[0])} > ${guardAgainstCoercionSideEffects(args[1])}`
77+
case 'ge': return `${guardAgainstCoercionSideEffects(args[0])} >= ${guardAgainstCoercionSideEffects(args[1])}`
78+
case 'lt': return `${guardAgainstCoercionSideEffects(args[0])} < ${guardAgainstCoercionSideEffects(args[1])}`
79+
case 'le': return `${guardAgainstCoercionSideEffects(args[0])} <= ${guardAgainstCoercionSideEffects(args[1])}`
80+
case 'any': return iterateOn('some', ...args)
81+
case 'all': return iterateOn('every', ...args)
82+
case 'and': return `(${args.join(') && (')})`
83+
case 'or': return `(${args.join(') || (')})`
84+
case 'startsWith': return `String.prototype.startsWith.call(${assertString(args[0])}, ${assertString(args[1])})`
85+
case 'endsWith': return `String.prototype.endsWith.call(${assertString(args[0])}, ${assertString(args[1])})`
86+
case 'contains': return `((obj, elm) => {
87+
if (${isString('obj')}) {
88+
return String.prototype.includes.call(obj, elm)
89+
} else if (Array.isArray(obj)) {
90+
return Array.prototype.includes.call(obj, elm)
91+
} else if (${isTypedArray('obj')}) {
92+
return Object.getPrototypeOf(Int8Array.prototype).includes.call(obj, elm)
93+
} else if (${isInstanceOfCoreType('Set', 'obj')}) {
94+
return Set.prototype.has.call(obj, elm)
95+
} else if (${isInstanceOfCoreType('WeakSet', 'obj')}) {
96+
return WeakSet.prototype.has.call(obj, elm)
97+
} else if (${isInstanceOfCoreType('Map', 'obj')}) {
98+
return Map.prototype.has.call(obj, elm)
99+
} else if (${isInstanceOfCoreType('WeakMap', 'obj')}) {
100+
return WeakMap.prototype.has.call(obj, elm)
101+
} else {
102+
throw new TypeError('Variable does not support contains')
103+
}
104+
})(${args[0]}, ${args[1]})`
105+
case 'matches': return `((str, regex) => {
106+
if (${isString('str')}) {
107+
const regexIsString = ${isString('regex')}
108+
if (regexIsString || Object.getPrototypeOf(regex) === RegExp.prototype) {
109+
return RegExp.prototype.test.call(regexIsString ? new RegExp(regex) : regex, str)
110+
} else {
111+
throw new TypeError('Regular expression must be either a string or an instance of RegExp')
112+
}
113+
} else {
114+
throw new TypeError('Variable is not a string')
115+
}
116+
})(${args[0]}, ${args[1]})`
117+
case 'filter': return `(($dd_var) => {
118+
return ${isIterableCollection('$dd_var')}
119+
? Array.from($dd_var).filter(($dd_it) => ${args[1]})
120+
: Object.entries($dd_var).reduce((acc, [$dd_key, $dd_value]) => {
121+
if (${args[1]}) acc[$dd_key] = $dd_value
122+
return acc
123+
}, {})
124+
})(${args[0]})`
125+
case 'substring': return `((str) => {
126+
if (${isString('str')}) {
127+
return String.prototype.substring.call(str, ${args[1]}, ${args[2]})
128+
} else {
129+
throw new TypeError('Variable is not a string')
130+
}
131+
})(${args[0]})`
132+
case 'getmember': return accessProperty(args[0], args[1], false)
133+
case 'index': return accessProperty(args[0], args[1], true)
134+
}
135+
}
136+
137+
throw new TypeError(`Unknown AST node type: ${type}`)
138+
}
139+
140+
function iterateOn (fnName, variable, callbackCode) {
141+
return `(($dd_val) => {
142+
return ${isIterableCollection('$dd_val')}
143+
? Array.from($dd_val).${fnName}(($dd_it) => ${callbackCode})
144+
: Object.entries($dd_val).${fnName}(([$dd_key, $dd_value]) => ${callbackCode})
145+
})(${variable})`
146+
}
147+
148+
function isString (variable) {
149+
return `(typeof ${variable} === 'string' || ${variable} instanceof String)`
150+
}
151+
152+
function isIterableCollection (variable) {
153+
return `(${isArrayOrTypedArray(variable)} || ${isInstanceOfCoreType('Set', variable)} || ` +
154+
`${isInstanceOfCoreType('WeakSet', variable)})`
155+
}
156+
157+
function isArrayOrTypedArray (variable) {
158+
return `(Array.isArray(${variable}) || ${isTypedArray(variable)})`
159+
}
160+
161+
function isTypedArray (variable) {
162+
return isInstanceOfCoreType('TypedArray', variable, `${variable} instanceof Object.getPrototypeOf(Int8Array)`)
163+
}
164+
165+
function isInstanceOfCoreType (type, variable, fallback = `${variable} instanceof ${type}`) {
166+
return `(process[Symbol.for('datadog:node:util:types')]?.is${type}?.(${variable}) ?? ${fallback})`
167+
}
168+
169+
function getSize (variable) {
170+
return `((val) => {
171+
if (${isString('val')} || ${isArrayOrTypedArray('val')}) {
172+
return ${guardAgainstPropertyAccessSideEffects('val', '"length"')}
173+
} else if (${isInstanceOfCoreType('Set', 'val')} || ${isInstanceOfCoreType('Map', 'val')}) {
174+
return ${guardAgainstPropertyAccessSideEffects('val', '"size"')}
175+
} else {
176+
throw new TypeError('Cannot get length or size of string/collection')
177+
}
178+
})(${variable})`
179+
}
180+
181+
function accessProperty (variable, keyOrIndex, allowMapAccess) {
182+
return `((val, key) => {
183+
if (${isInstanceOfCoreType('Map', 'val')}) {
184+
${allowMapAccess
185+
? 'return Map.prototype.get.call(val, key)'
186+
: 'throw new Error(\'Accessing a Map is not allowed\')'}
187+
} else if (${isInstanceOfCoreType('WeakMap', 'val')}) {
188+
${allowMapAccess
189+
? 'return WeakMap.prototype.get.call(val, key)'
190+
: 'throw new Error(\'Accessing a WeakMap is not allowed\')'}
191+
} else if (${isInstanceOfCoreType('Set', 'val')} || ${isInstanceOfCoreType('WeakSet', 'val')}) {
192+
throw new Error('Accessing a Set or WeakSet is not allowed')
193+
} else {
194+
return ${guardAgainstPropertyAccessSideEffects('val', 'key')}
195+
}
196+
})(${variable}, ${keyOrIndex})`
197+
}
198+
199+
function guardAgainstPropertyAccessSideEffects (variable, propertyName) {
200+
return `((val, key) => {
201+
if (
202+
${isInstanceOfCoreType('Proxy', 'val', 'true')} ||
203+
Object.getOwnPropertyDescriptor(val, key)?.get !== undefined
204+
) {
205+
throw new Error('Possibility of side effect')
206+
} else {
207+
return val[key]
208+
}
209+
})(${variable}, ${propertyName})`
210+
}
211+
212+
function guardAgainstCoercionSideEffects (variable) {
213+
return `((val) => {
214+
if (
215+
typeof val === 'object' && val !== null && (
216+
${isInstanceOfCoreType('Proxy', 'val', 'true')} ||
217+
val[Symbol.toPrimitive] !== undefined ||
218+
val.valueOf !== Object.prototype.valueOf ||
219+
val.toString !== Object.prototype.toString
220+
)
221+
) {
222+
throw new Error('Possibility of side effect due to coercion methods')
223+
} else {
224+
return val
225+
}
226+
})(${variable})`
227+
}
228+
229+
function assertString (variable) {
230+
return `((val) => {
231+
if (typeof val === 'string' || val instanceof String) {
232+
return val
233+
} else {
234+
throw new TypeError('Variable is not a string')
235+
}
236+
})(${variable})`
237+
}
238+
239+
function assertIdentifier (value) {
240+
if (!identifierRegex.test(value) || reservedWords.has(value)) {
241+
throw new SyntaxError(`Illegal identifier: ${value}`)
242+
}
243+
return value
244+
}

packages/dd-trace/src/debugger/index.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
'use strict'
22

3+
const { types } = require('util')
34
const { join } = require('path')
45
const { Worker, MessageChannel, threadId: parentThreadId } = require('worker_threads')
56
const log = require('../log')
@@ -24,6 +25,8 @@ function start (config, rc) {
2425
const rcChannel = new MessageChannel()
2526
configChannel = new MessageChannel()
2627

28+
process[Symbol.for('datadog:node:util:types')] = types
29+
2730
rc.setProductHandler('LIVE_DEBUGGING', (action, conf, id, ack) => {
2831
rcAckCallbacks.set(++ackId, ack)
2932
rcChannel.port2.postMessage({ action, conf, ackId })

0 commit comments

Comments
 (0)