Skip to content

Commit

Permalink
[DI] Add support for sampling
Browse files Browse the repository at this point in the history
  • Loading branch information
watson committed Dec 12, 2024
1 parent 5e6b008 commit acbba64
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 5 deletions.
39 changes: 39 additions & 0 deletions integration-tests/debugger/basic.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,45 @@ describe('Dynamic Instrumentation', function () {
})
})

describe('sampling', function () {
it('should respect sampling rate for single probe', function (done) {
let start, timer
let payloadsReceived = 0
const rcConfig = t.generateRemoteConfig({ sampling: { snapshotsPerSecond: 1 } })

function triggerBreakpointContinuously () {
t.axios.get(t.breakpoint.url).catch(done)
timer = setTimeout(triggerBreakpointContinuously, 10)
}

t.agent.on('debugger-diagnostics', ({ payload }) => {
if (payload.debugger.diagnostics.status === 'INSTALLED') triggerBreakpointContinuously()
})

t.agent.on('debugger-input', () => {
payloadsReceived++
if (payloadsReceived === 1) {
start = Date.now()
} else if (payloadsReceived === 2) {
const duration = Date.now() - start
clearTimeout(timer)

// Allow for a variance of -5/+50ms (time will tell if this is enough)
assert.isAbove(duration, 995)
assert.isBelow(duration, 1050)

// Wait at least a full sampling period, to see if we get any more payloads
timer = setTimeout(done, 1250)
} else {
clearTimeout(timer)
done(new Error('Too many payloads received!'))
}
})

t.agent.addRemoteConfig(rcConfig)
})
})

describe('race conditions', function () {
it('should remove the last breakpoint completely before trying to add a new one', function (done) {
const rcConfig2 = t.generateRemoteConfig()
Expand Down
8 changes: 8 additions & 0 deletions packages/dd-trace/src/debugger/devtools_client/breakpoints.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
'use strict'

const session = require('./session')
const { MAX_SNAPSHOTS_PER_SECOND_PER_PROBE, MAX_NON_SNAPSHOTS_PER_SECOND_PER_PROBE } = require('./defaults')
const { findScriptFromPartialPath, probes, breakpoints } = require('./state')
const log = require('../../log')

Expand All @@ -21,6 +22,13 @@ async function addBreakpoint (probe) {
probe.location = { file, lines: [String(line)] }
delete probe.where

// Optimize for fast calculations when probe is hit
const snapshotsPerSecond = probe.sampling.snapshotsPerSecond ?? (probe.captureSnapshot
? MAX_SNAPSHOTS_PER_SECOND_PER_PROBE
: MAX_NON_SNAPSHOTS_PER_SECOND_PER_PROBE)
probe.sampling.nsBetweenSampling = BigInt(1 / snapshotsPerSecond * 1e9)
probe.lastCaptureNs = 0n

// TODO: Inbetween `await session.post('Debugger.enable')` and here, the scripts are parsed and cached.
// Maybe there's a race condition here or maybe we're guraenteed that `await session.post('Debugger.enable')` will
// not continue untill all scripts have been parsed?
Expand Down
6 changes: 6 additions & 0 deletions packages/dd-trace/src/debugger/devtools_client/defaults.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
'use strict'

module.exports = {
MAX_SNAPSHOTS_PER_SECOND_PER_PROBE: 1,
MAX_NON_SNAPSHOTS_PER_SECOND_PER_PROBE: 5_000
}
37 changes: 32 additions & 5 deletions packages/dd-trace/src/debugger/devtools_client/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,23 +18,47 @@ require('./remote_config')
const threadId = parentThreadId === 0 ? `pid:${process.pid}` : `pid:${process.pid};tid:${parentThreadId}`
const threadName = parentThreadId === 0 ? 'MainThread' : `WorkerThread:${parentThreadId}`

// WARNING: The code above the line `await session.post('Debugger.resume')` is highly optimized. Please edit with care!
session.on('Debugger.paused', async ({ params }) => {
const start = process.hrtime.bigint()
const timestamp = Date.now()

let captureSnapshotForProbe = null
let maxReferenceDepth, maxCollectionSize, maxFieldCount, maxLength
const probes = params.hitBreakpoints.map((id) => {

// V8 doesn't allow seting more than one breakpoint at a specific location, however, it's possible to set two
// breakpoints just next to eachother that will "snap" to the same logical location, which in turn will be hit at the
// same time. E.g. index.js:1:1 and index.js:1:2.
// TODO: Investigate if it will improve performance to create a fast-path for when there's only a single breakpoint
let sampled = false
const length = params.hitBreakpoints.length
let probes = new Array(length)
for (let i = 0; i < length; i++) {
const id = params.hitBreakpoints[i]
const probe = breakpoints.get(id)
if (probe.captureSnapshot) {

if (start - probe.lastCaptureNs < probe.sampling.nsBetweenSampling) {
continue
}

sampled = true
probe.lastCaptureNs = start

if (probe.captureSnapshot === true) {
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
})

probes[i] = probe
}

if (sampled === false) {
return session.post('Debugger.resume')
}

const timestamp = Date.now()

let processLocalState
if (captureSnapshotForProbe !== null) {
Expand All @@ -56,6 +80,9 @@ session.on('Debugger.paused', async ({ params }) => {

log.debug(`Finished processing breakpoints - main thread paused for: ${Number(diff) / 1000000} ms`)

// Due to the highly optimized algorithm above, the `probes` array might have gaps
probes = probes.filter((probe) => !!probe)

const logger = {
// We can safely use `location.file` from the first probe in the array, since all probes hit by `hitBreakpoints`
// must exist in the same file since the debugger can only pause the main thread in one location.
Expand Down

0 comments on commit acbba64

Please sign in to comment.