diff --git a/integration-tests/debugger/index.spec.js b/integration-tests/debugger/index.spec.js index 613c4eeb695..8670ba82b47 100644 --- a/integration-tests/debugger/index.spec.js +++ b/integration-tests/debugger/index.spec.js @@ -436,7 +436,9 @@ describe('Dynamic Instrumentation', function () { elements: [ { type: 'number', value: '1' }, { type: 'number', value: '2' }, - { type: 'number', value: '3' } + { type: 'number', value: '3' }, + { type: 'number', value: '4' }, + { type: 'number', value: '5' } ] }, obj: { @@ -556,6 +558,27 @@ describe('Dynamic Instrumentation', function () { agent.addRemoteConfig(generateRemoteConfig({ captureSnapshot: true, capture: { maxLength: 10 } })) }) + + it('should respect maxCollectionSize', (done) => { + agent.on('debugger-input', ({ payload: { 'debugger.snapshot': { captures } } }) => { + const { locals } = captures.lines[probeLineNo] + + assert.deepEqual(locals.arr, { + type: 'Array', + elements: [ + { type: 'number', value: '1' }, + { type: 'number', value: '2' }, + { type: 'number', value: '3' } + ], + notCapturedReason: 'collectionSize', + size: 5 + }) + + done() + }) + + agent.addRemoteConfig(generateRemoteConfig({ captureSnapshot: true, capture: { maxCollectionSize: 3 } })) + }) }) }) @@ -612,7 +635,9 @@ function generateRemoteConfig (overrides = {}) { } } -function generateProbeConfig (overrides) { +function generateProbeConfig (overrides = {}) { + overrides.capture = { maxReferenceDepth: 3, ...overrides.capture } + overrides.sampling = { snapshotsPerSecond: 5000, ...overrides.sampling } return { id: randomUUID(), version: 0, @@ -623,8 +648,6 @@ function generateProbeConfig (overrides) { template: 'Hello World!', segments: [{ str: 'Hello World!' }], captureSnapshot: false, - capture: { maxReferenceDepth: 3 }, - sampling: { snapshotsPerSecond: 5000 }, evaluateAt: 'EXIT', ...overrides } diff --git a/integration-tests/debugger/target-app/index.js b/integration-tests/debugger/target-app/index.js index dd7f5e6328a..75b8f551a7a 100644 --- a/integration-tests/debugger/target-app/index.js +++ b/integration-tests/debugger/target-app/index.js @@ -36,7 +36,7 @@ function getSomeData () { lstr: 'Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.', sym: Symbol('foo'), regex: /bar/i, - arr: [1, 2, 3], + arr: [1, 2, 3, 4, 5], obj: { foo: { baz: 42, diff --git a/packages/dd-trace/src/debugger/devtools_client/index.js b/packages/dd-trace/src/debugger/devtools_client/index.js index aa19c14ef64..4675b61d725 100644 --- a/packages/dd-trace/src/debugger/devtools_client/index.js +++ b/packages/dd-trace/src/debugger/devtools_client/index.js @@ -23,12 +23,13 @@ session.on('Debugger.paused', async ({ params }) => { const timestamp = Date.now() let captureSnapshotForProbe = null - let maxReferenceDepth, maxLength + let maxReferenceDepth, maxCollectionSize, maxLength const probes = params.hitBreakpoints.map((id) => { const probe = breakpoints.get(id) if (probe.captureSnapshot) { captureSnapshotForProbe = probe maxReferenceDepth = highestOrUndefined(probe.capture.maxReferenceDepth, maxReferenceDepth) + maxCollectionSize = highestOrUndefined(probe.capture.maxCollectionSize, maxCollectionSize) maxLength = highestOrUndefined(probe.capture.maxLength, maxLength) } return probe @@ -38,7 +39,10 @@ session.on('Debugger.paused', async ({ params }) => { if (captureSnapshotForProbe !== null) { try { // TODO: Create unique states for each affected probe based on that probes unique `capture` settings (DEBUG-2863) - processLocalState = await getLocalStateForCallFrame(params.callFrames[0], { maxReferenceDepth, maxLength }) + processLocalState = await getLocalStateForCallFrame( + params.callFrames[0], + { maxReferenceDepth, maxCollectionSize, maxLength } + ) } catch (err) { // TODO: This error is not tied to a specific probe, but to all probes with `captureSnapshot: true`. // However, in 99,99% of cases, there will be just a single probe, so I guess this simplification is ok? diff --git a/packages/dd-trace/src/debugger/devtools_client/snapshot/collector.js b/packages/dd-trace/src/debugger/devtools_client/snapshot/collector.js index 0a8848ce5e5..14f6db9727f 100644 --- a/packages/dd-trace/src/debugger/devtools_client/snapshot/collector.js +++ b/packages/dd-trace/src/debugger/devtools_client/snapshot/collector.js @@ -1,5 +1,6 @@ 'use strict' +const { collectionSizeSym } = require('./symbols') const session = require('../session') const LEAF_SUBTYPES = new Set(['date', 'regexp']) @@ -14,22 +15,33 @@ module.exports = { // each lookup will just finish in its own time and traverse the child nodes when the event loop allows it. // Alternatively, use `Promise.all` or something like that, but the code would probably be more complex. -async function getObject (objectId, maxDepth, depth = 0) { +async function getObject (objectId, opts, depth = 0, collection = false) { const { result, privateProperties } = await session.post('Runtime.getProperties', { objectId, ownProperties: true // exclude inherited properties }) - if (privateProperties) result.push(...privateProperties) + if (collection) { + // Trim the collection if it's too large. + // Collections doesn't contain private properties, so the code in this block doesn't have to deal with it. + removeNonEnumerableProperties(result) // remove the `length` property + const size = result.length + if (size > opts.maxCollectionSize) { + result.splice(opts.maxCollectionSize) + result[collectionSizeSym] = size + } + } else if (privateProperties) { + result.push(...privateProperties) + } - return traverseGetPropertiesResult(result, maxDepth, depth) + return traverseGetPropertiesResult(result, opts, depth) } -async function traverseGetPropertiesResult (props, maxDepth, depth) { +async function traverseGetPropertiesResult (props, opts, depth) { // TODO: Decide if we should filter out non-enumerable properties or not: // props = props.filter((e) => e.enumerable) - if (depth >= maxDepth) return props + if (depth >= opts.maxReferenceDepth) return props for (const prop of props) { if (prop.value === undefined) continue @@ -37,33 +49,33 @@ async function traverseGetPropertiesResult (props, maxDepth, depth) { if (type === 'object') { if (objectId === undefined) continue // if `subtype` is "null" if (LEAF_SUBTYPES.has(subtype)) continue // don't waste time with these subtypes - prop.value.properties = await getObjectProperties(subtype, objectId, maxDepth, depth) + prop.value.properties = await getObjectProperties(subtype, objectId, opts, depth) } else if (type === 'function') { - prop.value.properties = await getFunctionProperties(objectId, maxDepth, depth + 1) + prop.value.properties = await getFunctionProperties(objectId, opts, depth + 1) } } return props } -async function getObjectProperties (subtype, objectId, maxDepth, depth) { +async function getObjectProperties (subtype, objectId, opts, depth) { if (ITERABLE_SUBTYPES.has(subtype)) { - return getIterable(objectId, maxDepth, depth) + return getIterable(objectId, opts, depth) } else if (subtype === 'promise') { - return getInternalProperties(objectId, maxDepth, depth) + return getInternalProperties(objectId, opts, depth) } else if (subtype === 'proxy') { - return getProxy(objectId, maxDepth, depth) + return getProxy(objectId, opts, depth) } else if (subtype === 'arraybuffer') { - return getArrayBuffer(objectId, maxDepth, depth) + return getArrayBuffer(objectId, opts, depth) } else { - return getObject(objectId, maxDepth, depth + 1) + return getObject(objectId, opts, depth + 1, subtype === 'array' || subtype === 'typedarray') } } // TODO: The following extra information from `internalProperties` might be relevant to include for functions: // - Bound function: `[[TargetFunction]]`, `[[BoundThis]]` and `[[BoundArgs]]` // - Non-bound function: `[[FunctionLocation]]`, and `[[Scopes]]` -async function getFunctionProperties (objectId, maxDepth, depth) { +async function getFunctionProperties (objectId, opts, depth) { let { result } = await session.post('Runtime.getProperties', { objectId, ownProperties: true // exclude inherited properties @@ -72,10 +84,12 @@ async function getFunctionProperties (objectId, maxDepth, depth) { // For legacy reasons (I assume) functions has a `prototype` property besides the internal `[[Prototype]]` result = result.filter(({ name }) => name !== 'prototype') - return traverseGetPropertiesResult(result, maxDepth, depth) + return traverseGetPropertiesResult(result, opts, depth) } -async function getIterable (objectId, maxDepth, depth) { +async function getIterable (objectId, opts, depth) { + // TODO: If the iterable has any properties defined on the object directly, instead of in its collection, they will + // exist in the return value below in the `result` property. We currently do not collect these. const { internalProperties } = await session.post('Runtime.getProperties', { objectId, ownProperties: true // exclude inherited properties @@ -93,10 +107,17 @@ async function getIterable (objectId, maxDepth, depth) { ownProperties: true // exclude inherited properties }) - return traverseGetPropertiesResult(result, maxDepth, depth) + removeNonEnumerableProperties(result) // remove the `length` property + const size = result.length + if (size > opts.maxCollectionSize) { + result.splice(opts.maxCollectionSize) + result[collectionSizeSym] = size + } + + return traverseGetPropertiesResult(result, opts, depth) } -async function getInternalProperties (objectId, maxDepth, depth) { +async function getInternalProperties (objectId, opts, depth) { const { internalProperties } = await session.post('Runtime.getProperties', { objectId, ownProperties: true // exclude inherited properties @@ -105,10 +126,10 @@ async function getInternalProperties (objectId, maxDepth, depth) { // We want all internal properties except the prototype const props = internalProperties.filter(({ name }) => name !== '[[Prototype]]') - return traverseGetPropertiesResult(props, maxDepth, depth) + return traverseGetPropertiesResult(props, opts, depth) } -async function getProxy (objectId, maxDepth, depth) { +async function getProxy (objectId, opts, depth) { const { internalProperties } = await session.post('Runtime.getProperties', { objectId, ownProperties: true // exclude inherited properties @@ -127,14 +148,14 @@ async function getProxy (objectId, maxDepth, depth) { ownProperties: true // exclude inherited properties }) - return traverseGetPropertiesResult(result, maxDepth, depth) + return traverseGetPropertiesResult(result, opts, depth) } // Support for ArrayBuffer is a bit trickly because the internal structure stored in `internalProperties` is not // documented and is not straight forward. E.g. ArrayBuffer(3) will internally contain both Int8Array(3) and // UInt8Array(3), whereas ArrayBuffer(8) internally contains both Int8Array(8), Uint8Array(8), Int16Array(4), and // Int32Array(2) - all representing the same data in different ways. -async function getArrayBuffer (objectId, maxDepth, depth) { +async function getArrayBuffer (objectId, opts, depth) { const { internalProperties } = await session.post('Runtime.getProperties', { objectId, ownProperties: true // exclude inherited properties @@ -149,5 +170,13 @@ async function getArrayBuffer (objectId, maxDepth, depth) { ownProperties: true // exclude inherited properties }) - return traverseGetPropertiesResult(result, maxDepth, depth) + return traverseGetPropertiesResult(result, opts, depth) +} + +function removeNonEnumerableProperties (props) { + for (let i = 0; i < props.length; i++) { + if (props[i].enumerable === false) { + props.splice(i--, 1) + } + } } diff --git a/packages/dd-trace/src/debugger/devtools_client/snapshot/index.js b/packages/dd-trace/src/debugger/devtools_client/snapshot/index.js index add097ac755..cca7aa43bae 100644 --- a/packages/dd-trace/src/debugger/devtools_client/snapshot/index.js +++ b/packages/dd-trace/src/debugger/devtools_client/snapshot/index.js @@ -4,6 +4,7 @@ const { getRuntimeObject } = require('./collector') const { processRawState } = require('./processor') const DEFAULT_MAX_REFERENCE_DEPTH = 3 +const DEFAULT_MAX_COLLECTION_SIZE = 100 const DEFAULT_MAX_LENGTH = 255 module.exports = { @@ -12,14 +13,18 @@ module.exports = { async function getLocalStateForCallFrame ( callFrame, - { maxReferenceDepth = DEFAULT_MAX_REFERENCE_DEPTH, maxLength = DEFAULT_MAX_LENGTH } = {} + { + maxReferenceDepth = DEFAULT_MAX_REFERENCE_DEPTH, + maxCollectionSize = DEFAULT_MAX_COLLECTION_SIZE, + maxLength = DEFAULT_MAX_LENGTH + } = {} ) { const rawState = [] let processedState = null for (const scope of callFrame.scopeChain) { if (scope.type === 'global') continue // The global scope is too noisy - rawState.push(...await getRuntimeObject(scope.object.objectId, maxReferenceDepth)) + rawState.push(...await getRuntimeObject(scope.object.objectId, { maxReferenceDepth, maxCollectionSize })) } // Deplay calling `processRawState` so the caller gets a chance to resume the main thread before processing `rawState` diff --git a/packages/dd-trace/src/debugger/devtools_client/snapshot/processor.js b/packages/dd-trace/src/debugger/devtools_client/snapshot/processor.js index 2cac9ef0b1c..9ded1477441 100644 --- a/packages/dd-trace/src/debugger/devtools_client/snapshot/processor.js +++ b/packages/dd-trace/src/debugger/devtools_client/snapshot/processor.js @@ -1,5 +1,7 @@ 'use strict' +const { collectionSizeSym } = require('./symbols') + module.exports = { processRawState: processProperties } @@ -144,31 +146,28 @@ function toArray (type, elements, maxLength) { if (elements === undefined) return notCapturedDepth(type) // Perf: Create array of expected size in advance (expect that it contains only one non-enumrable element) - const expectedLength = elements.length - 1 - const result = { type, elements: new Array(expectedLength) } + const result = { type, elements: new Array(elements.length) } + + setNotCaptureReasonOnCollection(result, elements) let i = 0 for (const elm of elements) { - if (elm.enumerable === false) continue // the value of the `length` property should not be part of the array result.elements[i++] = getPropertyValue(elm, maxLength) } - // Safe-guard in case there were more than one non-enumerable element - if (i < expectedLength) result.elements.length = i - return result } function toMap (type, pairs, maxLength) { if (pairs === undefined) return notCapturedDepth(type) - // Perf: Create array of expected size in advance (expect that it contains only one non-enumrable element) - const expectedLength = pairs.length - 1 - const result = { type, entries: new Array(expectedLength) } + // Perf: Create array of expected size in advance + const result = { type, entries: new Array(pairs.length) } + + setNotCaptureReasonOnCollection(result, pairs) let i = 0 for (const pair of pairs) { - if (pair.enumerable === false) continue // the value of the `length` property should not be part of the map // The following code is based on assumptions made when researching the output of the Chrome DevTools Protocol. // There doesn't seem to be any documentation to back it up: // @@ -180,9 +179,6 @@ function toMap (type, pairs, maxLength) { result.entries[i++] = [key, val] } - // Safe-guard in case there were more than one non-enumerable element - if (i < expectedLength) result.entries.length = i - return result } @@ -190,12 +186,12 @@ function toSet (type, values, maxLength) { if (values === undefined) return notCapturedDepth(type) // Perf: Create array of expected size in advance (expect that it contains only one non-enumrable element) - const expectedLength = values.length - 1 - const result = { type, elements: new Array(expectedLength) } + const result = { type, elements: new Array(values.length) } + + setNotCaptureReasonOnCollection(result, values) let i = 0 for (const value of values) { - if (value.enumerable === false) continue // the value of the `length` property should not be part of the set // The following code is based on assumptions made when researching the output of the Chrome DevTools Protocol. // There doesn't seem to be any documentation to back it up: // @@ -205,9 +201,6 @@ function toSet (type, values, maxLength) { result.elements[i++] = getPropertyValue(value.value.properties[0], maxLength) } - // Safe-guard in case there were more than one non-enumerable element - if (i < expectedLength) result.elements.length = i - return result } @@ -236,6 +229,13 @@ function arrayBufferToString (bytes, size) { return buf.toString() } +function setNotCaptureReasonOnCollection (result, collection) { + if (collectionSizeSym in collection) { + result.notCapturedReason = 'collectionSize' + result.size = collection[collectionSizeSym] + } +} + function notCapturedDepth (type) { return { type, notCapturedReason: 'depth' } } diff --git a/packages/dd-trace/src/debugger/devtools_client/snapshot/symbols.js b/packages/dd-trace/src/debugger/devtools_client/snapshot/symbols.js new file mode 100644 index 00000000000..99efc36e5f6 --- /dev/null +++ b/packages/dd-trace/src/debugger/devtools_client/snapshot/symbols.js @@ -0,0 +1,5 @@ +'use stict' + +module.exports = { + collectionSizeSym: Symbol('datadog.collectionSize') +} diff --git a/packages/dd-trace/test/debugger/devtools_client/snapshot/max-collection-size.spec.js b/packages/dd-trace/test/debugger/devtools_client/snapshot/max-collection-size.spec.js new file mode 100644 index 00000000000..6b63eec715e --- /dev/null +++ b/packages/dd-trace/test/debugger/devtools_client/snapshot/max-collection-size.spec.js @@ -0,0 +1,129 @@ +'use strict' + +require('../../../setup/mocha') + +const { getTargetCodePath, enable, teardown, assertOnBreakpoint, setAndTriggerBreakpoint } = require('./utils') + +const DEFAULT_MAX_COLLECTION_SIZE = 100 +const target = getTargetCodePath(__filename) + +describe('debugger -> devtools client -> snapshot.getLocalStateForCallFrame', function () { + describe('maxCollectionSize', function () { + const configs = [ + undefined, + { maxCollectionSize: 3 } + ] + + beforeEach(enable(__filename)) + + afterEach(teardown) + + for (const config of configs) { + const maxCollectionSize = config?.maxCollectionSize ?? DEFAULT_MAX_COLLECTION_SIZE + const postfix = config === undefined ? 'not set' : `set to ${config.maxCollectionSize}` + + describe(`shold respect the default maxCollectionSize if ${postfix}`, function () { + let state + + const expectedElements = [] + const expectedEntries = [] + for (let i = 1; i <= maxCollectionSize; i++) { + expectedElements.push({ type: 'number', value: i.toString() }) + expectedEntries.push([ + { type: 'number', value: i.toString() }, + { + type: 'Object', + fields: { i: { type: 'number', value: i.toString() } } + } + ]) + } + + beforeEach(function (done) { + assertOnBreakpoint(done, config, (_state) => { + state = _state + }) + setAndTriggerBreakpoint(target, 24) + }) + + it('should have expected number of elements in state', function () { + expect(state).to.have.keys(['arr', 'map', 'set', 'wmap', 'wset', 'typedArray']) + }) + + it('Array', function () { + expect(state).to.have.deep.property('arr', { + type: 'Array', + elements: expectedElements, + notCapturedReason: 'collectionSize', + size: 1000 + }) + }) + + it('Map', function () { + expect(state).to.have.deep.property('map', { + type: 'Map', + entries: expectedEntries, + notCapturedReason: 'collectionSize', + size: 1000 + }) + }) + + it('Set', function () { + expect(state).to.have.deep.property('set', { + type: 'Set', + elements: expectedElements, + notCapturedReason: 'collectionSize', + size: 1000 + }) + }) + + it('WeakMap', function () { + expect(state.wmap).to.include({ + type: 'WeakMap', + notCapturedReason: 'collectionSize', + size: 1000 + }) + + expect(state.wmap.entries).to.have.lengthOf(maxCollectionSize) + + // The order of the entries is not guaranteed, so we don't know which were removed + for (const entry of state.wmap.entries) { + expect(entry).to.have.lengthOf(2) + expect(entry[0]).to.have.property('type', 'Object') + expect(entry[0].fields).to.have.property('i') + expect(entry[0].fields.i).to.have.property('type', 'number') + expect(entry[0].fields.i).to.have.property('value').to.match(/^\d+$/) + expect(entry[1]).to.have.property('type', 'number') + expect(entry[1]).to.have.property('value', entry[0].fields.i.value) + } + }) + + it('WeakSet', function () { + expect(state.wset).to.include({ + type: 'WeakSet', + notCapturedReason: 'collectionSize', + size: 1000 + }) + + expect(state.wset.elements).to.have.lengthOf(maxCollectionSize) + + // The order of the elements is not guaranteed, so we don't know which were removed + for (const element of state.wset.elements) { + expect(element).to.have.property('type', 'Object') + expect(element.fields).to.have.property('i') + expect(element.fields.i).to.have.property('type', 'number') + expect(element.fields.i).to.have.property('value').to.match(/^\d+$/) + } + }) + + it('TypedArray', function () { + expect(state).to.have.deep.property('typedArray', { + type: 'Uint16Array', + elements: expectedElements, + notCapturedReason: 'collectionSize', + size: 1000 + }) + }) + }) + } + }) +}) diff --git a/packages/dd-trace/test/debugger/devtools_client/snapshot/target-code/max-collection-size.js b/packages/dd-trace/test/debugger/devtools_client/snapshot/target-code/max-collection-size.js new file mode 100644 index 00000000000..09c8ca81100 --- /dev/null +++ b/packages/dd-trace/test/debugger/devtools_client/snapshot/target-code/max-collection-size.js @@ -0,0 +1,27 @@ +'use stict' + +function run () { + const arr = [] + const map = new Map() + const set = new Set() + const wmap = new WeakMap() + const wset = new WeakSet() + const typedArray = new Uint16Array(new ArrayBuffer(2000)) + + // 1000 is larger the default maxCollectionSize of 100 + for (let i = 1; i <= 1000; i++) { + // A reference that can be used in WeakMap/WeakSet to avoid GC + const obj = { i } + + arr.push(i) + map.set(i, obj) + set.add(i) + wmap.set(obj, i) + wset.add(obj) + typedArray[i - 1] = i + } + + return 'my return value' // breakpoint at this line +} + +module.exports = { run }