Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DI] Add support for sampling #4998

Merged
merged 1 commit into from
Dec 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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
Loading