feat(debugger): add special handling for very large collections/objects

watson · watson · commit eb9dfa605c53 · 2025-11-14T11:27:18.000+01:00
Handle edge cases where an object has a lot of properties, or where
collections (arrays/sets/maps) has a lot of elements.

In those cases, a single Chrome DevTools Protocol
`Debugger.getProperties` request will take a long time to serialize
their properties/elements, which can siginificantly overshoot the time
budget (as ongoing CDP requests are allowed to complete before aborting)

This is dealt with in the following way:

- Skip enumerating large collections (arrays/maps/sets) by parsing size from
  descriptions; mark with notCapturedReason: "collectionSize" and record size.
- Large objects are still collected the first time, but objects with
  &gt;500 properties set a fatalSnapshotError to prevent future snapshot
  attempts for that probe and emits error to the diagnostics endpoint.
diff --git a/integration-tests/debugger/snapshot-time-budget.spec.js b/integration-tests/debugger/snapshot-time-budget.spec.js
@@ -3,63 +3,162 @@
 const assert = require('node:assert')
 const { setup } = require('./utils')
 
-const tolerance = 5
+const DEFAULT_MAX_COLLECTION_SIZE = 100
+const COLLECTION_SIZE_THRESHOLD = 500
 
 describe('Dynamic Instrumentation', function () {
   describe('input messages', function () {
     describe('with snapshot under tight time budget', function () {
       context('1ms time budget', function () {
         // Force a very small time budget in ms to trigger partial snapshots
         const target = 1
+        const tolerance = 5
         const t = setup({
           dependencies: ['fastify'],
           env: { DD_DYNAMIC_INSTRUMENTATION_CAPTURE_TIMEOUT_MS: String(target) }
         })
 
         it(
           'should include partial snapshot marked with notCapturedReason: timeout',
-          test({ t, maxPausedTime: target + tolerance, breakpointIndex: 0, maxReferenceDepth: 5 })
+          test({ t, maxPausedTime: target + tolerance, breakpointIndex: 0, maxReferenceDepth: 5 }, (locals) => {
+            assert.strictEqual(
+              containsTimeBudget(locals),
+              true,
+              'expected at least one field/element to be marked with notCapturedReason: "timeout"'
+            )
+          })
         )
       })
 
       context('default time budget', function () {
         const target = 10 // default time budget in ms
+        const tolerance = 5
         const t = setup({ dependencies: ['fastify'] })
 
-        // TODO: Make this pass
-        it.skip(
-          'should keep budget when state includes an object with 1 million properties',
-          test({ t, maxPausedTime: target + tolerance, breakpointIndex: 1, maxReferenceDepth: 1 })
+        it(
+          'should timeout first, then disable subsequent snapshots and emit error diagnostics',
+          async function () {
+            const breakpoint = t.breakpoints[1]
+
+            // Listen for the first snapshot payload (should contain notCapturedReason: "timeout")
+            const firstPayloadReceived = new Promise((resolve) => {
+              t.agent.once('debugger-input', ({ payload: [{ debugger: { snapshot: { captures } } }] }) => {
+                const { locals } = captures.lines[breakpoint.line]
+                resolve(locals)
+              })
+            })
+
+            // Prepare to assert that an ERROR diagnostics event with exception details is emitted
+            const errorDiagnosticsReceived = new Promise((/** @type {(value?: unknown) => void} */ resolve, reject) => {
+              const handler = ({ payload }) => {
+                payload.forEach(({ debugger: { diagnostics } }) => {
+                  if (diagnostics.status !== 'ERROR') return
+                  try {
+                    assert.strictEqual(
+                      diagnostics.exception.message,
+                      'An object with more than 500 properties was detected while collecting a snapshot. Future ' +
+                      'snapshots for exising probes in this location will be skipped until the Node.js process is ' +
+                      'restarted'
+                    )
+                    resolve()
+                  } catch (e) {
+                    reject(e)
+                  } finally {
+                    t.agent.off('debugger-diagnostics', handler)
+                  }
+                })
+              }
+              t.agent.on('debugger-diagnostics', handler)
+            })
+
+            // Install probe with snapshot capture enabled
+            t.agent.addRemoteConfig(breakpoint.generateRemoteConfig({
+              captureSnapshot: true,
+              capture: { maxReferenceDepth: 1 }
+            }))
+
+            // Trigger once; this run is expected to be slow and mark fields with "timeout"
+            const result1 = await breakpoint.triggerBreakpoint()
+            assert.ok(
+              result1.data.paused >= 1_000,
+              `expected thread to be paused for at least 1 second, but was paused for ~${result1.data.paused}ms`
+            )
+            const locals = await firstPayloadReceived
+            assert.strictEqual(
+              containsTimeBudget(locals),
+              true,
+              'expected at least one field/element to be marked with notCapturedReason: "timeout"'
+            )
+            await errorDiagnosticsReceived
+
+            // Prepare to assert that no snapshot is produced on a subsequent trigger
+            const noSnapshotAfterSecondTrigger = new Promise((/** @type {(value?: unknown) => void} */ resolve) => {
+              t.agent.once('debugger-input', ({ payload: [{ debugger: { snapshot: { captures } } }] }) => {
+                assert.strictEqual(captures, undefined)
+                resolve()
+              })
+            })
+
+            // Trigger the same breakpoint again directly
+            const result2 = await t.axios.get(breakpoint.url)
+            assert.ok(
+              result2.data.paused <= 5,
+              `expected thread to be paused <=5ms, but was paused for ~${result2.data.paused}ms`
+            )
+
+            await noSnapshotAfterSecondTrigger
+          }
         )
 
-        // TODO: Make this pass
-        it.skip(
-          'should keep budget when state includes an array of 1 million primitives',
-          test({ t, maxPausedTime: target + tolerance, breakpointIndex: 2, maxReferenceDepth: 1 })
+        it(
+          'should keep budget when state includes collections with 1 million elements',
+          test({ t, maxPausedTime: target + tolerance, breakpointIndex: 2, maxReferenceDepth: 1 }, (locals) => {
+            assert.strictEqual(locals.arrOfPrimitives.notCapturedReason, 'collectionSize')
+            assert.strictEqual(locals.arrOfPrimitives.size, 1_000_000)
+            assert.strictEqual(locals.arrOfPrimitives.elements.length, 0)
+            assert.strictEqual(locals.arrOfObjects.notCapturedReason, 'collectionSize')
+            assert.strictEqual(locals.arrOfObjects.size, 1_000_000)
+            assert.strictEqual(locals.arrOfObjects.elements.length, 0)
+            assert.strictEqual(locals.map.notCapturedReason, 'collectionSize')
+            assert.strictEqual(locals.map.size, 1_000_000)
+            assert.strictEqual(locals.map.entries.length, 0)
+            assert.strictEqual(locals.set.notCapturedReason, 'collectionSize')
+            assert.strictEqual(locals.set.size, 1_000_000)
+            assert.strictEqual(locals.set.elements.length, 0)
+          })
         )
 
-        // TODO: Make this pass
-        it.skip(
-          'should keep budget when state includes an array of 1 million objects',
-          test({ t, maxPausedTime: target + tolerance, breakpointIndex: 3, maxReferenceDepth: 1 })
+        it(
+          'should keep budget when state includes collections with less than the size threshold',
+          // Use a slightly larger tolerance to avoid flakiness
+          test({ t, maxPausedTime: target + tolerance * 3, breakpointIndex: 3, maxReferenceDepth: 1 }, (locals) => {
+            assert.strictEqual(locals.arrOfPrimitives.notCapturedReason, 'collectionSize')
+            assert.strictEqual(locals.arrOfPrimitives.size, COLLECTION_SIZE_THRESHOLD - 1)
+            assert.strictEqual(locals.arrOfPrimitives.elements.length, DEFAULT_MAX_COLLECTION_SIZE)
+            assert.strictEqual(locals.arrOfObjects.notCapturedReason, 'collectionSize')
+            assert.strictEqual(locals.arrOfObjects.size, COLLECTION_SIZE_THRESHOLD - 1)
+            assert.strictEqual(locals.arrOfObjects.elements.length, DEFAULT_MAX_COLLECTION_SIZE)
+            assert.strictEqual(locals.map.notCapturedReason, 'collectionSize')
+            assert.strictEqual(locals.map.size, COLLECTION_SIZE_THRESHOLD - 1)
+            assert.strictEqual(locals.map.entries.length, DEFAULT_MAX_COLLECTION_SIZE)
+            assert.strictEqual(locals.set.notCapturedReason, 'collectionSize')
+            assert.strictEqual(locals.set.size, COLLECTION_SIZE_THRESHOLD - 1)
+            assert.strictEqual(locals.set.elements.length, DEFAULT_MAX_COLLECTION_SIZE)
+          })
         )
       })
     })
   })
 })
 
-function test ({ t, maxPausedTime, breakpointIndex, maxReferenceDepth }) {
+function test ({ t, maxPausedTime = 0, breakpointIndex, maxReferenceDepth }, assertFn) {
   const breakpoint = t.breakpoints[breakpointIndex]
 
   return async function () {
     const payloadReceived = new Promise((/** @type {(value?: unknown) => void} */ resolve) => {
       t.agent.on('debugger-input', ({ payload: [{ debugger: { snapshot: { captures } } }] }) => {
         const { locals } = captures.lines[breakpoint.line]
-        assert.strictEqual(
-          containsTimeBudget(locals),
-          true,
-          'expected at least one field/element to be marked with notCapturedReason: "timeout"'
-        )
+        assertFn?.(locals)
         resolve()
       })
     })
diff --git a/integration-tests/debugger/target-app/snapshot-time-budget.js b/integration-tests/debugger/target-app/snapshot-time-budget.js
@@ -6,6 +6,8 @@ const Fastify = require('fastify')
 
 const fastify = Fastify({ logger: { level: 'error' } })
 
+const COLLECTION_SIZE_THRESHOLD = 500
+
 fastify.get('/deeply-nested-large-object', function handler () {
   // The size of `obj` generated is carefully tuned to never result in a snapshot larger than the 1MB size limit, while
   // still being large enough to trigger the time budget limit. However, the size of `fastify` and `request` is not
@@ -16,24 +18,31 @@ fastify.get('/deeply-nested-large-object', function handler () {
   return { paused: Number(diff) / 1_000_000 }
 })
 
-fastify.get('/large-object-of-primitives', function handler () {
+fastify.get('/object-with-many-properties', function handler () {
   const obj = generateObject(0, 1_000_000) // eslint-disable-line no-unused-vars
   const start = process.hrtime.bigint()
-  const diff = process.hrtime.bigint() - start // BREAKPOINT: /large-object-of-primitives
+  const diff = process.hrtime.bigint() - start // BREAKPOINT: /object-with-many-properties
   return { paused: Number(diff) / 1_000_000 }
 })
 
-fastify.get('/large-array-of-primitives', function handler () {
-  const arr = Array.from({ length: 1_000_000 }, (_, i) => i) // eslint-disable-line no-unused-vars
+fastify.get('/large-collections', function handler () {
+  const arrOfPrimitives = Array.from({ length: 1_000_000 }, (_, i) => i) // eslint-disable-line no-unused-vars
+  const arrOfObjects = Array.from({ length: 1_000_000 }, (_, i) => ({ i }))
+  const map = new Map(arrOfObjects.map((obj, i) => [i, obj])) // eslint-disable-line no-unused-vars
+  const set = new Set(arrOfObjects) // eslint-disable-line no-unused-vars
   const start = process.hrtime.bigint()
-  const diff = process.hrtime.bigint() - start // BREAKPOINT: /large-array-of-primitives
+  const diff = process.hrtime.bigint() - start // BREAKPOINT: /large-collections
   return { paused: Number(diff) / 1_000_000 }
 })
 
-fastify.get('/large-array-of-objects', function handler () {
-  const arr = Array.from({ length: 1_000_000 }, (_, i) => ({ i })) // eslint-disable-line no-unused-vars
+fastify.get('/large-collections-below-size-threshold', function handler () {
+  // eslint-disable-next-line no-unused-vars
+  const arrOfPrimitives = Array.from({ length: COLLECTION_SIZE_THRESHOLD - 1 }, (_, i) => i)
+  const arrOfObjects = Array.from({ length: COLLECTION_SIZE_THRESHOLD - 1 }, (_, i) => ({ i }))
+  const map = new Map(arrOfObjects.map((obj, i) => [i, obj])) // eslint-disable-line no-unused-vars
+  const set = new Set(arrOfObjects) // eslint-disable-line no-unused-vars
   const start = process.hrtime.bigint()
-  const diff = process.hrtime.bigint() - start // BREAKPOINT: /large-array-of-objects
+  const diff = process.hrtime.bigint() - start // BREAKPOINT: /large-collections-below-size-threshold
   return { paused: Number(diff) / 1_000_000 }
 })
 
diff --git a/packages/dd-trace/src/debugger/devtools_client/index.js b/packages/dd-trace/src/debugger/devtools_client/index.js
@@ -6,7 +6,7 @@ const session = require('./session')
 const { getLocalStateForCallFrame } = require('./snapshot')
 const send = require('./send')
 const { getStackFromCallFrames } = require('./state')
-const { ackEmitting } = require('./status')
+const { ackEmitting, ackError } = require('./status')
 const config = require('./config')
 const { MAX_SNAPSHOTS_PER_SECOND_GLOBALLY } = require('./defaults')
 const log = require('./log')
@@ -165,16 +165,17 @@ session.on('Debugger.paused', async ({ params }) => {
   }
 
   // TODO: Create unique states for each affected probe based on that probes unique `capture` settings (DEBUG-2863)
-  const processLocalState = numberOfProbesWithSnapshots !== 0 && await getLocalStateForCallFrame(
-    params.callFrames[0],
-    {
+  let processLocalState, fatalSnapshotError
+  if (numberOfProbesWithSnapshots !== 0) {
+    const opts = {
       maxReferenceDepth,
       maxCollectionSize,
       maxFieldCount,
       maxLength,
       deadlineNs: start + BigInt(config.dynamicInstrumentation.captureTimeoutMs ?? 10) * 1_000_000n
     }
-  )
+    ;({ processLocalState, fatalSnapshotError } = await getLocalStateForCallFrame(params.callFrames[0], opts))
+  }
 
   await session.post('Debugger.resume')
   const diff = process.hrtime.bigint() - start // TODO: Recored as telemetry (DEBUG-2858)
@@ -215,6 +216,11 @@ session.on('Debugger.paused', async ({ params }) => {
 
     if (probe.captureSnapshot) {
       const state = processLocalState()
+      if (fatalSnapshotError) {
+        // There was an error collecting the snapshot for this probe, let's not try again
+        probe.captureSnapshot = false
+        ackError(fatalSnapshotError, probe)
+      }
       if (state instanceof Error) {
         snapshot.captureError = state.message
       } else if (state) {
diff --git a/packages/dd-trace/src/debugger/devtools_client/snapshot/collector.js b/packages/dd-trace/src/debugger/devtools_client/snapshot/collector.js
@@ -5,6 +5,8 @@ const session = require('../session')
 
 const LEAF_SUBTYPES = new Set(['date', 'regexp'])
 const ITERABLE_SUBTYPES = new Set(['map', 'set', 'weakmap', 'weakset'])
+const SIZE_IN_DESCRIPTION_SUBTYPES = new Set(['array', 'typedarray', 'arraybuffer', 'dataview', 'map', 'set'])
+const COLLECTION_SIZE_THRESHOLD = 500
 
 module.exports = {
   getRuntimeObject: getObject // TODO: Called once per stack frame, but doesn't retain the `deadlineReached` flag.
@@ -50,6 +52,12 @@ async function getObject (objectId, opts, depth = 0, collection = false) {
   } else if (result.length > opts.maxFieldCount) {
     // Trim the number of properties on the object if there's too many.
     const size = result.length
+    if (size > COLLECTION_SIZE_THRESHOLD) {
+      opts.fatalSnapshotError = new Error(
+        `An object with more than ${COLLECTION_SIZE_THRESHOLD} properties was detected while collecting a snapshot. ` +
+        'Future snapshots for exising probes in this location will be skipped until the Node.js process is restarted'
+      )
+    }
     result.length = opts.maxFieldCount
     result[fieldCountSym] = size
   } else if (privateProperties) {
@@ -69,10 +77,17 @@ async function traverseGetPropertiesResult (props, opts, depth) {
 
   for (const prop of props) {
     if (prop.value === undefined) continue
-    const { value: { type, objectId, subtype } } = prop
+    const { value: { type, objectId, subtype, description } } = prop
     if (type === 'object') {
       if (objectId === undefined) continue // if `subtype` is "null"
       if (LEAF_SUBTYPES.has(subtype)) continue // don't waste time with these subtypes
+      const size = parseLengthFromDescription(description, subtype)
+      if (size !== null && size >= COLLECTION_SIZE_THRESHOLD) {
+        const empty = []
+        empty[collectionSizeSym] = size
+        prop.value.properties = empty
+        continue
+      }
       work.push([
         prop.value,
         () => getObjectProperties(subtype, objectId, opts, depth).then((properties) => {
@@ -234,6 +249,26 @@ function removeNonEnumerableProperties (props) {
   }
 }
 
+function parseLengthFromDescription (description, subtype) {
+  if (typeof description !== 'string') return null
+  if (!SIZE_IN_DESCRIPTION_SUBTYPES.has(subtype)) return null
+
+  const open = description.lastIndexOf('(')
+  if (open === -1) return null
+
+  const close = description.indexOf(')', open + 1)
+  if (close === -1) return null
+
+  const s = description.slice(open + 1, close)
+  if (s === '') return null
+
+  const n = Number(s)
+  if (!Number.isSafeInteger(n) || n < 0) return null
+  if (String(n) !== s) return null
+
+  return n
+}
+
 function overBudget (opts) {
   if (opts.deadlineReached) return true
   opts.deadlineReached = process.hrtime.bigint() >= opts.deadlineNs
diff --git a/packages/dd-trace/src/debugger/devtools_client/snapshot/index.js b/packages/dd-trace/src/debugger/devtools_client/snapshot/index.js
@@ -65,8 +65,11 @@ async function getLocalStateForCallFrame (callFrame, opts = {}) {
   }
 
   // Delay calling `processRawState` so the caller gets a chance to resume the main thread before processing `rawState`
-  return () => {
-    processedState = processedState ?? processRawState(rawState, opts.maxLength)
-    return processedState
+  return {
+    processLocalState () {
+      processedState = processedState ?? processRawState(rawState, opts.maxLength)
+      return processedState
+    },
+    fatalSnapshotError: opts.fatalSnapshotError
   }
 }

Original file line number	Diff line number	Diff line change
`@@ -65,8 +65,11 @@ async function getLocalStateForCallFrame (callFrame, opts = {}) {`
`65`	`65`	`}`
`66`	`66`
`67`	`67`	// Delay calling `processRawState` so the caller gets a chance to resume the main thread before processing `rawState`
`68`		`- return () => {`
`69`		`- processedState = processedState ?? processRawState(rawState, opts.maxLength)`
`70`		`- return processedState`
	`68`	`+ return {`
	`69`	`+ processLocalState () {`
	`70`	`+ processedState = processedState ?? processRawState(rawState, opts.maxLength)`
	`71`	`+ return processedState`
	`72`	`+ },`
	`73`	`+ fatalSnapshotError: opts.fatalSnapshotError`
`71`	`74`	`}`
`72`	`75`	`}`