Skip to content

Commit

Permalink
[DI] Adhere to maxFieldCount limit in snapshots (#4829)
Browse files Browse the repository at this point in the history
The limit controls the maximum number of properties collected on an
object. The default is 20.

It's also applied on each scope when collecting properties. If there's
for example more than maxFieldCount properties in the current scope,
they are not all collected.
  • Loading branch information
watson authored and rochdev committed Nov 6, 2024
1 parent 7aedf2f commit 1c03da2
Show file tree
Hide file tree
Showing 11 changed files with 190 additions and 9 deletions.
50 changes: 49 additions & 1 deletion integration-tests/debugger/snapshot.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,8 @@ describe('Dynamic Instrumentation', function () {

// from closure scope
// There's no reason to test the `fastify` object 100%, instead just check its fingerprint
assert.deepEqual(Object.keys(fastify), ['type', 'fields'])
assert.equal(fastify.type, 'Object')
assert.typeOf(fastify.fields, 'Object')

assert.deepEqual(getSomeData, {
type: 'Function',
Expand Down Expand Up @@ -186,6 +186,54 @@ describe('Dynamic Instrumentation', function () {

t.agent.addRemoteConfig(t.generateRemoteConfig({ captureSnapshot: true, capture: { maxCollectionSize: 3 } }))
})

it('should respect maxFieldCount', (done) => {
const maxFieldCount = 3

function assertMaxFieldCount (prop) {
if ('fields' in prop) {
if (prop.notCapturedReason === 'fieldCount') {
assert.strictEqual(Object.keys(prop.fields).length, maxFieldCount)
assert.isAbove(prop.size, maxFieldCount)
} else {
assert.isBelow(Object.keys(prop.fields).length, maxFieldCount)
}
}

for (const value of Object.values(prop.fields || prop.elements || prop.entries || {})) {
assertMaxFieldCount(value)
}
}

t.agent.on('debugger-input', ({ payload: { 'debugger.snapshot': { captures } } }) => {
const { locals } = captures.lines[t.breakpoint.line]

assert.deepEqual(Object.keys(locals), [
// Up to 3 properties from the local scope
'request', 'nil', 'undef',
// Up to 3 properties from the closure scope
'fastify', 'getSomeData'
])

assert.strictEqual(locals.request.type, 'Request')
assert.strictEqual(Object.keys(locals.request.fields).length, maxFieldCount)
assert.strictEqual(locals.request.notCapturedReason, 'fieldCount')
assert.isAbove(locals.request.size, maxFieldCount)

assert.strictEqual(locals.fastify.type, 'Object')
assert.strictEqual(Object.keys(locals.fastify.fields).length, maxFieldCount)
assert.strictEqual(locals.fastify.notCapturedReason, 'fieldCount')
assert.isAbove(locals.fastify.size, maxFieldCount)

for (const value of Object.values(locals)) {
assertMaxFieldCount(value)
}

done()
})

t.agent.addRemoteConfig(t.generateRemoteConfig({ captureSnapshot: true, capture: { maxFieldCount } }))
})
})
})
})
5 changes: 3 additions & 2 deletions packages/dd-trace/src/debugger/devtools_client/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,14 @@ session.on('Debugger.paused', async ({ params }) => {
const timestamp = Date.now()

let captureSnapshotForProbe = null
let maxReferenceDepth, maxCollectionSize, maxLength
let maxReferenceDepth, maxCollectionSize, maxFieldCount, 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)
maxFieldCount = highestOrUndefined(probe.capture.maxFieldCount, maxFieldCount)
maxLength = highestOrUndefined(probe.capture.maxLength, maxLength)
}
return probe
Expand All @@ -41,7 +42,7 @@ session.on('Debugger.paused', async ({ params }) => {
// TODO: Create unique states for each affected probe based on that probes unique `capture` settings (DEBUG-2863)
processLocalState = await getLocalStateForCallFrame(
params.callFrames[0],
{ maxReferenceDepth, maxCollectionSize, maxLength }
{ maxReferenceDepth, maxCollectionSize, maxFieldCount, maxLength }
)
} catch (err) {
// TODO: This error is not tied to a specific probe, but to all probes with `captureSnapshot: true`.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict'

const { collectionSizeSym } = require('./symbols')
const { collectionSizeSym, fieldCountSym } = require('./symbols')
const session = require('../session')

const LEAF_SUBTYPES = new Set(['date', 'regexp'])
Expand Down Expand Up @@ -30,6 +30,11 @@ async function getObject (objectId, opts, depth = 0, collection = false) {
result.splice(opts.maxCollectionSize)
result[collectionSizeSym] = size
}
} else if (result.length > opts.maxFieldCount) {
// Trim the number of properties on the object if there's too many.
const size = result.length
result.splice(opts.maxFieldCount)
result[fieldCountSym] = size
} else if (privateProperties) {
result.push(...privateProperties)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const { processRawState } = require('./processor')

const DEFAULT_MAX_REFERENCE_DEPTH = 3
const DEFAULT_MAX_COLLECTION_SIZE = 100
const DEFAULT_MAX_FIELD_COUNT = 20
const DEFAULT_MAX_LENGTH = 255

module.exports = {
Expand All @@ -16,6 +17,7 @@ async function getLocalStateForCallFrame (
{
maxReferenceDepth = DEFAULT_MAX_REFERENCE_DEPTH,
maxCollectionSize = DEFAULT_MAX_COLLECTION_SIZE,
maxFieldCount = DEFAULT_MAX_FIELD_COUNT,
maxLength = DEFAULT_MAX_LENGTH
} = {}
) {
Expand All @@ -24,7 +26,10 @@ async function getLocalStateForCallFrame (

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, maxCollectionSize }))
rawState.push(...await getRuntimeObject(
scope.object.objectId,
{ maxReferenceDepth, maxCollectionSize, maxFieldCount }
))
}

// Deplay calling `processRawState` so the caller gets a chance to resume the main thread before processing `rawState`
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict'

const { collectionSizeSym } = require('./symbols')
const { collectionSizeSym, fieldCountSym } = require('./symbols')

module.exports = {
processRawState: processProperties
Expand Down Expand Up @@ -139,7 +139,18 @@ function toString (str, maxLength) {

function toObject (type, props, maxLength) {
if (props === undefined) return notCapturedDepth(type)
return { type, fields: processProperties(props, maxLength) }

const result = {
type,
fields: processProperties(props, maxLength)
}

if (fieldCountSym in props) {
result.notCapturedReason = 'fieldCount'
result.size = props[fieldCountSym]
}

return result
}

function toArray (type, elements, maxLength) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
'use stict'

module.exports = {
collectionSizeSym: Symbol('datadog.collectionSize')
collectionSizeSym: Symbol('datadog.collectionSize'),
fieldCountSym: Symbol('datadog.fieldCount')
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ describe('debugger -> devtools client -> snapshot.getLocalStateForCallFrame', fu
session.once('Debugger.paused', async ({ params }) => {
expect(params.hitBreakpoints.length).to.eq(1)

resolve((await getLocalStateForCallFrame(params.callFrames[0]))())
resolve((await getLocalStateForCallFrame(params.callFrames[0], { maxFieldCount: Infinity }))())
})

await setAndTriggerBreakpoint(target, 10)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
'use strict'

require('../../../setup/mocha')

const { getTargetCodePath, enable, teardown, assertOnBreakpoint, setAndTriggerBreakpoint } = require('./utils')

const target = getTargetCodePath(__filename)

describe('debugger -> devtools client -> snapshot.getLocalStateForCallFrame', function () {
describe('maxFieldCount', function () {
beforeEach(enable(__filename))

afterEach(teardown)

describe('shold respect maxFieldCount on each collected scope', function () {
const maxFieldCount = 3
let state

beforeEach(function (done) {
assertOnBreakpoint(done, { maxFieldCount }, (_state) => {
state = _state
})
setAndTriggerBreakpoint(target, 11)
})

it('should capture expected snapshot', function () {
// Expect the snapshot to have captured the first 3 fields from each scope
expect(state).to.have.keys(['a1', 'b1', 'c1', 'a2', 'b2', 'c2'])
})
})
})
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
'use strict'

require('../../../setup/mocha')

const { getTargetCodePath, enable, teardown, assertOnBreakpoint, setAndTriggerBreakpoint } = require('./utils')

const DEFAULT_MAX_FIELD_COUNT = 20
const target = getTargetCodePath(__filename)

describe('debugger -> devtools client -> snapshot.getLocalStateForCallFrame', function () {
describe('maxFieldCount', function () {
beforeEach(enable(__filename))

afterEach(teardown)

describe('shold respect the default maxFieldCount if not set', generateTestCases())

describe('shold respect maxFieldCount if set to 10', generateTestCases({ maxFieldCount: 10 }))
})
})

function generateTestCases (config) {
const maxFieldCount = config?.maxFieldCount ?? DEFAULT_MAX_FIELD_COUNT
let state

const expectedFields = {}
for (let i = 1; i <= maxFieldCount; i++) {
expectedFields[`field${i}`] = { type: 'number', value: i.toString() }
}

return function () {
beforeEach(function (done) {
assertOnBreakpoint(done, config, (_state) => {
state = _state
})
setAndTriggerBreakpoint(target, 11)
})

it('should capture expected snapshot', function () {
expect(state).to.have.keys(['obj'])
expect(state).to.have.deep.property('obj', {
type: 'Object',
fields: expectedFields,
notCapturedReason: 'fieldCount',
size: 40
})
})
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
'use stict'

function run () {
// local scope
const { a1, b1, c1, d1 } = {}

{
// block scope
const { a2, b2, c2, d2 } = {}

return { a1, b1, c1, d1, a2, b2, c2, d2 } // breakpoint at this line
}
}

module.exports = { run }
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
'use stict'

function run () {
const obj = {}

// 40 is larger the default maxFieldCount of 20
for (let i = 1; i <= 40; i++) {
obj[`field${i}`] = i
}

return 'my return value' // breakpoint at this line
}

module.exports = { run }

0 comments on commit 1c03da2

Please sign in to comment.