[DI] Add source map support (#5205)

eslint.config.mjs

@@ -41,6 +41,7 @@ export default [
       '**/versions', // This is effectively a node_modules tree.
       '**/acmeair-nodejs', // We don't own this.
       '**/vendor', // Generally, we didn't author this code.
+      'integration-tests/debugger/target-app/source-map-support/index.js', // Generated
       'integration-tests/esbuild/out.js', // Generated
       'integration-tests/esbuild/aws-sdk-out.js', // Generated
       'packages/dd-trace/src/appsec/blocked_templates.js', // TODO Why is this ignored?

integration-tests/debugger/basic.spec.js

@@ -535,7 +535,7 @@ function assertBasicInputPayload (t, payload) {
     service: 'node',
     message: 'Hello World!',
     logger: {
-      name: t.breakpoint.file,
+      name: t.breakpoint.deployedFile,
       method: 'fooHandler',
       version,
       thread_name: 'MainThread'
@@ -544,7 +544,7 @@ function assertBasicInputPayload (t, payload) {
       probe: {
         id: t.rcConfig.config.id,
         version: 0,
-        location: { file: t.breakpoint.file, lines: [String(t.breakpoint.line)] }
+        location: { file: t.breakpoint.deployedFile, lines: [String(t.breakpoint.line)] }
       },
       language: 'javascript'
     }

integration-tests/debugger/source-map-support.spec.js

@@ -0,0 +1,27 @@
+'use strict'
+
+const { assert } = require('chai')
+const { setup } = require('./utils')
+
+describe('Dynamic Instrumentation', function () {
+  describe('source map support', function () {
+    const t = setup({
+      testApp: 'target-app/source-map-support/index.js',
+      testAppSource: 'target-app/source-map-support/index.ts'
+    })
+
+    beforeEach(t.triggerBreakpoint)
+
+    it('should support source maps', function (done) {
+      t.agent.on('debugger-input', ({ payload: [{ 'debugger.snapshot': { probe: { location } } }] }) => {
+        assert.deepEqual(location, {
+          file: 'target-app/source-map-support/index.ts',
+          lines: ['9']
+        })
+        done()
+      })
+
+      t.agent.addRemoteConfig(t.rcConfig)
+    })
+  })
+})

integration-tests/debugger/target-app/source-map-support/index.ts

@@ -0,0 +1,14 @@
+require('dd-trace/init')
+
+import { createServer } from 'node:http'
+
+const server = createServer((req, res) => {
+  // Blank lines below to ensure line numbers in transpiled file differ from original file
+
+
+  res.end('hello world') // BREAKPOINT: /
+})
+
+server.listen(process.env.APP_PORT, () => {
+  process.send?.({ port: process.env.APP_PORT })
+})

integration-tests/debugger/utils.js

@@ -18,9 +18,13 @@ module.exports = {
   setup
 }
 
-function setup ({ env, testApp } = {}) {
+function setup ({ env, testApp, testAppSource } = {}) {
   let sandbox, cwd, appPort
-  const breakpoints = getBreakpointInfo({ file: testApp, stackIndex: 1 }) // `1` to disregard the `setup` function
+  const breakpoints = getBreakpointInfo({
+    deployedFile: testApp,
+    sourceFile: testAppSource,
+    stackIndex: 1 // `1` to disregard the `setup` function
+  })
   const t = {
     breakpoint: breakpoints[0],
     breakpoints,
@@ -71,7 +75,7 @@ function setup ({ env, testApp } = {}) {
     sandbox = await createSandbox(['fastify']) // TODO: Make this dynamic
     cwd = sandbox.folder
     // The sandbox uses the `integration-tests` folder as its root
-    t.appFile = join(cwd, 'debugger', breakpoints[0].file)
+    t.appFile = join(cwd, 'debugger', breakpoints[0].deployedFile)
   })
 
   after(async function () {
@@ -110,8 +114,8 @@ function setup ({ env, testApp } = {}) {
   return t
 }
 
-function getBreakpointInfo ({ file, stackIndex = 0 }) {
-  if (!file) {
+function getBreakpointInfo ({ deployedFile, sourceFile = deployedFile, stackIndex = 0 } = {}) {
+  if (!deployedFile) {
     // First, get the filename of file that called this function
     const testFile = new Error().stack
       .split('\n')[stackIndex + 2] // +2 to skip this function + the first line, which is the error message
@@ -120,17 +124,17 @@ function getBreakpointInfo ({ file, stackIndex = 0 }) {
       .split(':')[0]
 
     // Then, find the corresponding file in which the breakpoint(s) exists
-    file = join('target-app', basename(testFile).replace('.spec', ''))
+    deployedFile = sourceFile = join('target-app', basename(testFile).replace('.spec', ''))
   }
 
   // Finally, find the line number(s) of the breakpoint(s)
-  const lines = readFileSync(join(__dirname, file), 'utf8').split('\n')
+  const lines = readFileSync(join(__dirname, sourceFile), 'utf8').split('\n')
   const result = []
   for (let i = 0; i < lines.length; i++) {
     const index = lines[i].indexOf(BREAKPOINT_TOKEN)
     if (index !== -1) {
       const url = lines[i].slice(index + BREAKPOINT_TOKEN.length + 1).trim()
-      result.push({ file, line: i + 1, url })
+      result.push({ sourceFile, deployedFile, line: i + 1, url })
     }
   }
 

packages/datadog-plugin-cucumber/src/index.js

@@ -47,6 +47,7 @@ const {
 const id = require('../../dd-trace/src/id')
 
 const BREAKPOINT_HIT_GRACE_PERIOD_MS = 200
+const BREAKPOINT_SET_GRACE_PERIOD_MS = 200
 const isCucumberWorker = !!process.env.CUCUMBER_WORKER_ID
 
 function getTestSuiteTags (testSuiteSpan) {
@@ -251,7 +252,12 @@ class CucumberPlugin extends CiPlugin {
           const { file, line, stackIndex } = probeInformation
           this.runningTestProbe = { file, line }
           this.testErrorStackIndex = stackIndex
-          // TODO: we're not waiting for setProbePromise to be resolved, so there might be race conditions
+          const waitUntil = Date.now() + BREAKPOINT_SET_GRACE_PERIOD_MS
+          while (Date.now() < waitUntil) {
+            // TODO: To avoid a race condition, we should wait until `probeInformation.setProbePromise` has resolved.
+            // However, Cucumber doesn't have a mechanism for waiting asyncrounously here, so for now, we'll have to
+            // fall back to a fixed syncronous delay.
+          }
         }
       }
       span.setTag(TEST_STATUS, 'fail')

packages/datadog-plugin-mocha/src/index.js

@@ -48,6 +48,8 @@ const {
 const id = require('../../dd-trace/src/id')
 const log = require('../../dd-trace/src/log')
 
+const BREAKPOINT_SET_GRACE_PERIOD_MS = 200
+
 function getTestSuiteLevelVisibilityTags (testSuiteSpan) {
   const testSuiteSpanContext = testSuiteSpan.context()
   const suiteTags = {
@@ -279,7 +281,12 @@ class MochaPlugin extends CiPlugin {
             this.runningTestProbe = { file, line }
             this.testErrorStackIndex = stackIndex
             test._ddShouldWaitForHitProbe = true
-            // TODO: we're not waiting for setProbePromise to be resolved, so there might be race conditions
+            const waitUntil = Date.now() + BREAKPOINT_SET_GRACE_PERIOD_MS
+            while (Date.now() < waitUntil) {
+              // TODO: To avoid a race condition, we should wait until `probeInformation.setProbePromise` has resolved.
+              // However, Mocha doesn't have a mechanism for waiting asyncrounously here, so for now, we'll have to
+              // fall back to a fixed syncronous delay.
+            }
           }
         }
 

packages/dd-trace/src/ci-visibility/dynamic-instrumentation/worker/index.js

@@ -1,5 +1,5 @@
 'use strict'
-const path = require('path')
+
 const {
   workerData: {
     breakpointSetChannel,
@@ -8,10 +8,11 @@ const {
   }
 } = require('worker_threads')
 const { randomUUID } = require('crypto')
-const sourceMap = require('source-map')
 
 // TODO: move debugger/devtools_client/session to common place
 const session = require('../../../debugger/devtools_client/session')
+// TODO: move debugger/devtools_client/source-maps to common place
+const { getSourceMappedLine } = require('../../../debugger/devtools_client/source-maps')
 // TODO: move debugger/devtools_client/snapshot to common place
 const { getLocalStateForCallFrame } = require('../../../debugger/devtools_client/snapshot')
 // TODO: move debugger/devtools_client/state to common place
@@ -98,17 +99,21 @@ async function addBreakpoint (probe) {
     throw new Error(`No loaded script found for ${file}`)
   }
 
-  const [path, scriptId, sourceMapURL] = script
+  const { url, scriptId, sourceMapURL, source } = script
 
-  log.warn(`Adding breakpoint at ${path}:${line}`)
+  log.warn(`Adding breakpoint at ${url}:${line}`)
 
   let lineNumber = line
 
-  if (sourceMapURL && sourceMapURL.startsWith('data:')) {
+  if (sourceMapURL) {
     try {
-      lineNumber = await processScriptWithInlineSourceMap({ file, line, sourceMapURL })
+      lineNumber = await getSourceMappedLine(url, source, line, sourceMapURL)
     } catch (err) {
-      log.error('Error processing script with inline source map', err)
+      log.error('Error processing script with source map', err)
+    }
+    if (lineNumber === null) {
+      log.error('Could not find generated position for %s:%s', url, line)
+      lineNumber = line
     }
   }
 
@@ -123,51 +128,11 @@ async function addBreakpoint (probe) {
     breakpointIdToProbe.set(breakpointId, probe)
     probeIdToBreakpointId.set(probe.id, breakpointId)
   } catch (e) {
-    log.error(`Error setting breakpoint at ${path}:${line}:`, e)
+    log.error('Error setting breakpoint at %s:%s', url, line, e)
   }
 }
 
 function start () {
   sessionStarted = true
   return session.post('Debugger.enable') // return instead of await to reduce number of promises created
 }
-
-async function processScriptWithInlineSourceMap (params) {
-  const { file, line, sourceMapURL } = params
-
-  // Extract the base64-encoded source map
-  const base64SourceMap = sourceMapURL.split('base64,')[1]
-
-  // Decode the base64 source map
-  const decodedSourceMap = Buffer.from(base64SourceMap, 'base64').toString('utf8')
-
-  // Parse the source map
-  const consumer = await new sourceMap.SourceMapConsumer(decodedSourceMap)
-
-  let generatedPosition
-
-  // Map to the generated position. We'll attempt with the full file path first, then with the basename.
-  // TODO: figure out why sometimes the full path doesn't work
-  generatedPosition = consumer.generatedPositionFor({
-    source: file,
-    line,
-    column: 0
-  })
-  if (generatedPosition.line === null) {
-    generatedPosition = consumer.generatedPositionFor({
-      source: path.basename(file),
-      line,
-      column: 0
-    })
-  }
-
-  consumer.destroy()
-
-  // If we can't find the line, just return the original line
-  if (generatedPosition.line === null) {
-    log.error(`Could not find generated position for ${file}:${line}`)
-    return line
-  }
-
-  return generatedPosition.line
-}

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

@@ -1,5 +1,6 @@
 'use strict'
 
+const { getSourceMappedLine } = require('./source-maps')
 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')
@@ -16,7 +17,7 @@ async function addBreakpoint (probe) {
   if (!sessionStarted) await start()
 
   const file = probe.where.sourceFile
-  const line = Number(probe.where.lines[0]) // Tracer doesn't support multiple-line breakpoints
+  let line = Number(probe.where.lines[0]) // Tracer doesn't support multiple-line breakpoints
 
   // Optimize for sending data to /debugger/v1/input endpoint
   probe.location = { file, lines: [String(line)] }
@@ -34,11 +35,15 @@ async function addBreakpoint (probe) {
   // not continue untill all scripts have been parsed?
   const script = findScriptFromPartialPath(file)
   if (!script) throw new Error(`No loaded script found for ${file} (probe: ${probe.id}, version: ${probe.version})`)
-  const [path, scriptId] = script
+  const { url, scriptId, sourceMapURL, source } = script
+
+  if (sourceMapURL) {
+    line = await getSourceMappedLine(url, source, line, sourceMapURL)
+  }
 
   log.debug(
     '[debugger:devtools_client] Adding breakpoint at %s:%d (probe: %s, version: %d)',
-    path, line, probe.id, probe.version
+    url, line, probe.id, probe.version
   )
 
   const { breakpointId } = await session.post('Debugger.setBreakpoint', {
@@ -66,7 +71,7 @@ async function removeBreakpoint ({ id }) {
   probes.delete(id)
   breakpoints.delete(breakpointId)
 
-  if (breakpoints.size === 0) await stop()
+  if (breakpoints.size === 0) return stop() // return instead of await to reduce number of promises created
 }
 
 async function start () {

packages/dd-trace/src/debugger/devtools_client/source-maps.js

@@ -0,0 +1,50 @@
+'use strict'
+
+const { join, dirname } = require('path')
+const { readFileSync } = require('fs')
+const { readFile } = require('fs/promises')
+const { SourceMapConsumer } = require('source-map')
+
+const cache = new Map()
+let cacheTimer = null
+
+const self = module.exports = {
+  async loadSourceMap (dir, url) {
+    if (url.startsWith('data:')) return loadInlineSourceMap(url)
+    const path = join(dir, url)
+    if (cache.has(path)) return cache.get(path)
+    return cacheIt(path, JSON.parse(await readFile(path, 'utf8')))
+  },
+
+  loadSourceMapSync (dir, url) {
+    if (url.startsWith('data:')) return loadInlineSourceMap(url)
+    const path = join(dir, url)
+    if (cache.has(path)) return cache.get(path)
+    return cacheIt(path, JSON.parse(readFileSync(path, 'utf8')))
+  },
+
+  async getSourceMappedLine (url, source, line, sourceMapURL) {
+    const dir = dirname(new URL(url).pathname)
+    return await SourceMapConsumer.with(
+      await self.loadSourceMap(dir, sourceMapURL),
+      null,
+      (consumer) => consumer.generatedPositionFor({ source, line, column: 0 }).line
+    )
+  }
+}
+
+function cacheIt (key, value) {
+  clearTimeout(cacheTimer)
+  cacheTimer = setTimeout(function () {
+    // Optimize for app boot, where a lot of reads might happen
+    // Clear cache a few seconds after it was last used
+    cache.clear()
+  }, 10_000).unref()
+  cache.set(key, value)
+  return value
+}
+
+function loadInlineSourceMap (data) {
+  data = data.slice(data.indexOf('base64,') + 7)
+  return JSON.parse(Buffer.from(data, 'base64').toString('utf8'))
+}