Skip to content

Commit 5abf807

Browse files
authored
fix(debugger): initialize lastCaptureNs to ensure first probe hit is captured (#7042)
When a probe is added with any sampling rate, the first probe hit could be incorrectly skipped if it occurred within the sampling interval after process start. This happened because `lastCaptureNs` was initialized to `0n`, causing the sampling check `start - probe.lastCaptureNs < probe.nsBetweenSampling` to evaluate to true when the process had been running for less time than the sampling interval. For example: - At 10 samples/sec: first hit skipped if within first 100ms - At 5 samples/sec: first hit skipped if within first 200ms - At 1 sample/sec: first hit skipped if within first 1 second - At 0.5 samples/sec: first hit skipped if within first 2 seconds Fix by initializing `lastCaptureNs` to `BigInt(Number.MIN_SAFE_INTEGER)`, ensuring the first probe hit always passes the sampling check regardless of when it occurs.
1 parent d23ef75 commit 5abf807

File tree

2 files changed

+28
-3
lines changed

2 files changed

+28
-3
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,8 @@ async function addBreakpoint (probe) {
6565
? MAX_SNAPSHOTS_PER_SECOND_PER_PROBE
6666
: MAX_NON_SNAPSHOTS_PER_SECOND_PER_PROBE)
6767
probe.nsBetweenSampling = BigInt(Math.trunc(1 / snapshotsPerSecond * 1e9))
68-
probe.lastCaptureNs = 0n
68+
// Initialize to a large negative value to ensure first probe hit is always captured
69+
probe.lastCaptureNs = BigInt(Number.MIN_SAFE_INTEGER)
6970

7071
// Warning: The code below relies on undocumented behavior of the inspector!
7172
// It expects that `await session.post('Debugger.enable')` will wait for all loaded scripts to be emitted as

packages/dd-trace/test/debugger/devtools_client/breakpoints.spec.js

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,29 @@ describe('breakpoints', function () {
111111
sinon.assert.calledTwice(stateMock.findScriptFromPartialPath)
112112
})
113113

114+
it('should initialize lastCaptureNs to ensure first probe hit is always captured', async function () {
115+
await addProbe({ sampling: { snapshotsPerSecond: 0.5 } })
116+
117+
// Verify the probe was stored in the breakpointToProbes map
118+
const breakpointId = 'bp-script-1:9:0'
119+
const probesAtLocation = stateMock.breakpointToProbes.get(breakpointId)
120+
assert(probesAtLocation, 'Probes should be stored at breakpoint location')
121+
122+
const probe = probesAtLocation.get('probe-1')
123+
assert(probe, 'Probe should be stored in map')
124+
125+
// Verify lastCaptureNs is initialized to -(2^53 - 1) to ensure first hit is always captured
126+
assert.strictEqual(probe.lastCaptureNs, BigInt(Number.MIN_SAFE_INTEGER),
127+
'lastCaptureNs should be initialized to -(2^53 - 1) to ensure first probe hit is always captured')
128+
129+
// Verify nsBetweenSampling is calculated correctly
130+
assert.strictEqual(
131+
probe.nsBetweenSampling,
132+
2000000000n,
133+
'nsBetweenSampling should be 2 seconds for 0.5 samples/second'
134+
)
135+
})
136+
114137
describe('add multiple probes to the same location', function () {
115138
it('no conditions', async function () {
116139
await addProbe()
@@ -486,11 +509,12 @@ describe('breakpoints', function () {
486509
* { json: { eq: [{ ref: 'foo' }, 42] }, dsl: 'foo = 42' } by default.
487510
* @returns {{ id: string; version: number; where: object; when: object; }}
488511
*/
489-
function genProbeConfig ({ id, version, where, when } = {}) {
512+
function genProbeConfig ({ id, version, where, when, ...rest } = {}) {
490513
return {
491514
id: id || 'probe-1',
492515
version: version || 1,
493516
where: where || { sourceFile: 'test.js', lines: ['10'] },
494-
when
517+
when,
518+
...rest
495519
}
496520
}

0 commit comments

Comments
 (0)