Skip to content

Commit 3f15a95

Browse files
authored
⚗️ Add Live Debugger lifetime event budgets (#4510)
Co-authored-by: thomas.watson <thomas.watson@datadoghq.com>
1 parent 042e529 commit 3f15a95

4 files changed

Lines changed: 330 additions & 7 deletions

File tree

packages/debugger/src/domain/api.spec.ts

Lines changed: 220 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -567,6 +567,226 @@ describe('api', () => {
567567
})
568568
})
569569

570+
describe('probe lifetime budgets', () => {
571+
it('should stop sending snapshot events after maxSnapshotsPerProbeLifetime', () => {
572+
initTransport({ maxSnapshotsPerProbeLifetime: 1 })
573+
574+
const probe: Probe = {
575+
id: 'snapshot-lifetime-probe',
576+
version: 0,
577+
type: 'LOG_PROBE',
578+
where: { typeName: 'TestClass', methodName: 'snapshotLifetime' },
579+
template: 'Test',
580+
captureSnapshot: true,
581+
capture: { maxReferenceDepth: 1 },
582+
// Disable per-probe rate limiting so the second invocation exercises the
583+
// lifetime cap rather than the per-second cap.
584+
sampling: { snapshotsPerSecond: Infinity },
585+
evaluateAt: 'ENTRY',
586+
}
587+
addProbe(probe)
588+
589+
// First invocation: probe sends its single allowed event.
590+
const probes = getProbes('TestClass;snapshotLifetime')!
591+
onEntry(probes, {}, {})
592+
onReturn(probes, null, {}, {}, {})
593+
expect(mockBatchAdd).toHaveBeenCalledTimes(1)
594+
595+
// Second invocation: the lifetime budget is now exhausted. No new event should
596+
// be queued, and the probe should be auto-unregistered.
597+
onEntry(probes, {}, {})
598+
onReturn(probes, null, {}, {}, {})
599+
expect(mockBatchAdd).toHaveBeenCalledTimes(1)
600+
expect(getProbes('TestClass;snapshotLifetime')).toBeUndefined()
601+
})
602+
603+
it('should skip snapshot collection once the lifetime budget is exhausted', () => {
604+
initTransport({ maxSnapshotsPerProbeLifetime: 1 })
605+
606+
const getterSpy = jasmine.createSpy('argGetter').and.returnValue('value')
607+
const args = {}
608+
Object.defineProperty(args, 'arg', {
609+
enumerable: true,
610+
get: getterSpy,
611+
})
612+
const probe: Probe = {
613+
id: 'snapshot-lifetime-collection-probe',
614+
version: 0,
615+
type: 'LOG_PROBE',
616+
where: { typeName: 'TestClass', methodName: 'snapshotLifetimeCollection' },
617+
template: 'Test',
618+
captureSnapshot: true,
619+
capture: { maxReferenceDepth: 1 },
620+
// Disable per-probe rate limiting so the second invocation isn't sampled out
621+
// by it — we want to exercise the lifetime cap, not the rate cap.
622+
sampling: { snapshotsPerSecond: Infinity },
623+
evaluateAt: 'ENTRY',
624+
}
625+
addProbe(probe)
626+
627+
// First invocation does the full pipeline: 2 reads from entry capture
628+
// (context spread + captureFields) + 1 read from return capture = 3 reads.
629+
// This exhausts the lifetime budget.
630+
const probes = getProbes('TestClass;snapshotLifetimeCollection')!
631+
onEntry(probes, {}, args)
632+
onReturn(probes, null, {}, args, {})
633+
634+
// Second invocation: both onEntry and onReturn detect the exhausted budget
635+
// up front and skip all capture work — no further reads from args.
636+
onEntry(probes, {}, args)
637+
onReturn(probes, null, {}, args, {})
638+
639+
expect(getterSpy).toHaveBeenCalledTimes(3)
640+
})
641+
642+
it('should stop sending non-snapshot events after maxNonSnapshotsPerProbeLifetime', () => {
643+
initTransport({ maxNonSnapshotsPerProbeLifetime: 1 })
644+
645+
const probe: Probe = {
646+
id: 'non-snapshot-lifetime-probe',
647+
version: 0,
648+
type: 'LOG_PROBE',
649+
where: { typeName: 'TestClass', methodName: 'nonSnapshotLifetime' },
650+
template: 'Test',
651+
captureSnapshot: false,
652+
capture: {},
653+
// Disable per-probe rate limiting so the second invocation exercises the
654+
// lifetime cap rather than the per-second cap.
655+
sampling: { snapshotsPerSecond: Infinity },
656+
evaluateAt: 'ENTRY',
657+
}
658+
addProbe(probe)
659+
660+
// First invocation: probe sends its single allowed event.
661+
const probes = getProbes('TestClass;nonSnapshotLifetime')!
662+
onEntry(probes, {}, {})
663+
onReturn(probes, null, {}, {}, {})
664+
expect(mockBatchAdd).toHaveBeenCalledTimes(1)
665+
666+
// Second invocation: the lifetime budget is now exhausted. No new event should
667+
// be queued, and the probe should be auto-unregistered.
668+
onEntry(probes, {}, {})
669+
onReturn(probes, null, {}, {}, {})
670+
expect(mockBatchAdd).toHaveBeenCalledTimes(1)
671+
expect(getProbes('TestClass;nonSnapshotLifetime')).toBeUndefined()
672+
})
673+
674+
it('should reset the lifetime budget when a new probe version is delivered', () => {
675+
initTransport({ maxSnapshotsPerProbeLifetime: 1 })
676+
677+
const probe: Probe = {
678+
id: 'versioned-lifetime-probe',
679+
version: 0,
680+
type: 'LOG_PROBE',
681+
where: { typeName: 'TestClass', methodName: 'versionedLifetime' },
682+
template: 'Test',
683+
captureSnapshot: true,
684+
capture: { maxReferenceDepth: 1 },
685+
sampling: { snapshotsPerSecond: 5000 },
686+
evaluateAt: 'ENTRY',
687+
}
688+
addProbe(probe)
689+
690+
let probes = getProbes('TestClass;versionedLifetime')!
691+
onEntry(probes, {}, {})
692+
onReturn(probes, null, {}, {}, {})
693+
expect(mockBatchAdd).toHaveBeenCalledTimes(1)
694+
695+
// A Remote Config delivery for an existing probe id replaces the old probe with
696+
// the new version. After re-add, the new version should have a fresh budget.
697+
removeProbe(probe.id)
698+
addProbe({ ...probe, version: 1 })
699+
700+
probes = getProbes('TestClass;versionedLifetime')!
701+
onEntry(probes, {}, {})
702+
onReturn(probes, null, {}, {}, {})
703+
expect(mockBatchAdd).toHaveBeenCalledTimes(2)
704+
})
705+
706+
it('should not emit any event when the lifetime budget is zero', () => {
707+
initTransport({ maxSnapshotsPerProbeLifetime: 0 })
708+
709+
const probe: Probe = {
710+
id: 'zero-budget-probe',
711+
version: 0,
712+
type: 'LOG_PROBE',
713+
where: { typeName: 'TestClass', methodName: 'zeroBudget' },
714+
template: 'Test',
715+
captureSnapshot: true,
716+
capture: { maxReferenceDepth: 1 },
717+
sampling: { snapshotsPerSecond: 5000 },
718+
evaluateAt: 'ENTRY',
719+
}
720+
addProbe(probe)
721+
722+
const probes = getProbes('TestClass;zeroBudget')!
723+
onEntry(probes, {}, {})
724+
onReturn(probes, null, {}, {}, {})
725+
726+
expect(mockBatchAdd).not.toHaveBeenCalled()
727+
expect(getProbes('TestClass;zeroBudget')).toBeUndefined()
728+
})
729+
730+
it('should still process sibling probes when one is removed mid-iteration', () => {
731+
// Use distinct snapshot/non-snapshot lifetime caps so probeA hits its cap after
732+
// one event while probeB still has plenty of budget. On the second invocation,
733+
// probeA's pre-call budget check fails and it gets removed from the shared
734+
// probes array. This exposes the array mutation hazard: removing probeA
735+
// mid-iteration must not cause probeB to be skipped.
736+
initTransport({ maxSnapshotsPerProbeLifetime: 1, maxNonSnapshotsPerProbeLifetime: 1000 })
737+
738+
// Disable per-probe rate limiting on both probes so the second invocation
739+
// exercises the lifetime cap rather than the per-second cap.
740+
const probeA: Probe = {
741+
id: 'sibling-probe-a',
742+
version: 0,
743+
type: 'LOG_PROBE',
744+
where: { typeName: 'TestClass', methodName: 'sibling' },
745+
template: 'A',
746+
captureSnapshot: true,
747+
capture: { maxReferenceDepth: 1 },
748+
sampling: { snapshotsPerSecond: Infinity },
749+
evaluateAt: 'ENTRY',
750+
}
751+
const probeB: Probe = {
752+
id: 'sibling-probe-b',
753+
version: 0,
754+
type: 'LOG_PROBE',
755+
where: { typeName: 'TestClass', methodName: 'sibling' },
756+
template: 'B',
757+
captureSnapshot: false,
758+
capture: {},
759+
sampling: { snapshotsPerSecond: Infinity },
760+
evaluateAt: 'ENTRY',
761+
}
762+
addProbe(probeA)
763+
addProbe(probeB)
764+
765+
// First invocation: both probes emit one event. probeA hits its cap (eventsSent=1,
766+
// max=1) but is not removed yet — the pre-call budget check still passed.
767+
const probes = getProbes('TestClass;sibling')!
768+
onEntry(probes, {}, {})
769+
onReturn(probes, null, {}, {}, {})
770+
expect(mockBatchAdd).toHaveBeenCalledTimes(2)
771+
772+
// Second invocation: probeA's pre-call check now fails and it is queued for
773+
// removal. probeB must still be processed in the same iteration even though
774+
// probeA gets spliced out of the probes array.
775+
mockBatchAdd.calls.reset()
776+
const probesAfterFirst = getProbes('TestClass;sibling')!
777+
onEntry(probesAfterFirst, {}, {})
778+
onReturn(probesAfterFirst, null, {}, {}, {})
779+
expect(mockBatchAdd).toHaveBeenCalledTimes(1)
780+
expect(getProbes('TestClass;sibling')).toEqual([jasmine.objectContaining({ id: 'sibling-probe-b' })])
781+
782+
// probeB's stack entry must not leak: a third onReturn without onEntry is a no-op.
783+
mockBatchAdd.calls.reset()
784+
const remainingProbes = getProbes('TestClass;sibling')!
785+
onReturn(remainingProbes, null, {}, {}, {})
786+
expect(mockBatchAdd).not.toHaveBeenCalled()
787+
})
788+
})
789+
570790
describe('active entries cleanup', () => {
571791
function createProbe(id: string, methodName: string): Probe {
572792
return {

packages/debugger/src/domain/api.ts

Lines changed: 40 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,13 @@ import type { BrowserWindow, DebuggerInitConfiguration } from '../entries/main'
55
import { capture, captureFields } from './capture'
66
import type { CaptureContext } from './capture'
77
import type { InitializedProbe } from './probes'
8-
import { checkGlobalSnapshotBudget, resetProbeBudgetConfiguration, setProbeBudgetConfiguration } from './probes'
8+
import {
9+
checkGlobalSnapshotBudget,
10+
hasProbeLifetimeBudgetRemaining,
11+
removeProbe,
12+
resetProbeBudgetConfiguration,
13+
setProbeBudgetConfiguration,
14+
} from './probes'
915
import type { ActiveEntry } from './activeEntries'
1016
import { active } from './activeEntries'
1117
import { captureStackTrace, parseStackTrace } from './stacktrace'
@@ -50,6 +56,10 @@ export function onEntry(probes: InitializedProbe[], self: any, args: Record<stri
5056

5157
// TODO: A lot of repeated work performed for each probe that could be shared between probes
5258
for (const probe of probes) {
59+
if (!hasProbeLifetimeBudgetRemaining(probe)) {
60+
continue
61+
}
62+
5363
let stack = active.get(probe.id) // TODO: Should we use the functionId instead?
5464
if (!stack) {
5565
stack = []
@@ -130,9 +140,15 @@ export function onReturn(
130140
): any {
131141
const end = performance.now()
132142
const captureCtx: CaptureContext = { deadline: performance.now() + SNAPSHOT_TIMEOUT_MS, timedOut: false }
143+
let exhaustedProbeIds: string[] | undefined
133144

134145
// TODO: A lot of repeated work performed for each probe that could be shared between probes
135146
for (const probe of probes) {
147+
if (!hasProbeLifetimeBudgetRemaining(probe)) {
148+
;(exhaustedProbeIds ??= []).push(probe.id)
149+
continue
150+
}
151+
136152
const stack = active.get(probe.id) // TODO: Should we use the functionId instead?
137153
if (!stack) {
138154
continue // TODO: This shouldn't be possible, do we need it? Should we warn?
@@ -181,7 +197,13 @@ export function onReturn(
181197
}
182198
}
183199

184-
sendDebuggerSnapshot(probe, result)
200+
queueDebuggerSnapshot(probe, result)
201+
}
202+
203+
if (exhaustedProbeIds) {
204+
for (const id of exhaustedProbeIds) {
205+
removeProbe(id)
206+
}
185207
}
186208

187209
return value
@@ -198,9 +220,15 @@ export function onReturn(
198220
export function onThrow(probes: InitializedProbe[], error: Error, self: any, args: Record<string, any> = {}): void {
199221
const end = performance.now()
200222
const captureCtx: CaptureContext = { deadline: performance.now() + SNAPSHOT_TIMEOUT_MS, timedOut: false }
223+
let exhaustedProbeIds: string[] | undefined
201224

202225
// TODO: A lot of repeated work performed for each probe that could be shared between probes
203226
for (const probe of probes) {
227+
if (!hasProbeLifetimeBudgetRemaining(probe)) {
228+
;(exhaustedProbeIds ??= []).push(probe.id)
229+
continue
230+
}
231+
204232
const stack = active.get(probe.id) // TODO: Should we use the functionId instead?
205233
if (!stack) {
206234
continue // TODO: This shouldn't be possible, do we need it? Should we warn?
@@ -252,17 +280,23 @@ export function onThrow(probes: InitializedProbe[], error: Error, self: any, arg
252280
},
253281
}
254282

255-
sendDebuggerSnapshot(probe, result)
283+
queueDebuggerSnapshot(probe, result)
284+
}
285+
286+
if (exhaustedProbeIds) {
287+
for (const id of exhaustedProbeIds) {
288+
removeProbe(id)
289+
}
256290
}
257291
}
258292

259293
/**
260-
* Send a debugger snapshot to Datadog via the debugger's own transport.
294+
* Queue a debugger snapshot for delivery via the debugger's own transport.
261295
*
262296
* @param probe - The probe that was executed
263297
* @param result - The result of the probe execution
264298
*/
265-
function sendDebuggerSnapshot(probe: InitializedProbe, result: ActiveEntry): void {
299+
function queueDebuggerSnapshot(probe: InitializedProbe, result: ActiveEntry): void {
266300
if (!debuggerBatch || !debuggerConfig) {
267301
display.warn('Debugger transport is not initialized. Make sure DD_DEBUGGER.init() has been called.')
268302
return
@@ -310,6 +344,7 @@ function sendDebuggerSnapshot(probe: InitializedProbe, result: ActiveEntry): voi
310344
}
311345

312346
debuggerBatch.add(payload)
347+
probe.eventsSentInLifetime++
313348
}
314349

315350
function getDebuggerDDtags(debuggerVersion: string): string {

0 commit comments

Comments
 (0)