[DI] Add support for sampling

Author: watson

integration-tests/debugger/basic.spec.js

@@ -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()

packages/dd-trace/src/debugger/devtools_client/breakpoints.js

@@ -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')
 
@@ -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?

packages/dd-trace/src/debugger/devtools_client/defaults.js

@@ -0,0 +1,6 @@
+'use strict'
+
+module.exports = {
+  MAX_SNAPSHOTS_PER_SECOND_PER_PROBE: 1,
+  MAX_NON_SNAPSHOTS_PER_SECOND_PER_PROBE: 5_000
+}

packages/dd-trace/src/debugger/devtools_client/index.js

@@ -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) {
@@ -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.