[DI] Use column number from source maps (#5279)

eslint.config.mjs

@@ -41,7 +41,8 @@ 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/debugger/target-app/source-map-support/minify.min.js', // Generated
+      'integration-tests/debugger/target-app/source-map-support/typescript.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/source-map-support.spec.js

@@ -5,23 +5,46 @@ 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'
-    })
+    describe('Different file extention (TypeScript)', function () {
+      const t = setup({
+        testApp: 'target-app/source-map-support/typescript.js',
+        testAppSource: 'target-app/source-map-support/typescript.ts'
+      })
 
-    beforeEach(t.triggerBreakpoint)
+      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']
+      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/typescript.ts',
+            lines: ['9']
+          })
+          done()
         })
-        done()
+
+        t.agent.addRemoteConfig(t.rcConfig)
+      })
+    })
+
+    describe('Column information required (Minified)', function () {
+      const t = setup({
+        testApp: 'target-app/source-map-support/minify.min.js',
+        testAppSource: 'target-app/source-map-support/minify.js'
       })
 
-      t.agent.addRemoteConfig(t.rcConfig)
+      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/minify.js',
+            lines: ['6']
+          })
+          done()
+        })
+
+        t.agent.addRemoteConfig(t.rcConfig)
+      })
     })
   })
 })

integration-tests/debugger/target-app/source-map-support/minify.js

@@ -0,0 +1,11 @@
+require('dd-trace/init')
+
+const { createServer } = require('node:http')
+
+const server = createServer((req, res) => {
+  res.end('hello world') // BREAKPOINT: /
+})
+
+server.listen(process.env.APP_PORT, () => {
+  process.send?.({ port: process.env.APP_PORT })
+})

integration-tests/debugger/target-app/source-map-support/scripts/build-minifiy.sh

@@ -0,0 +1,6 @@
+#!/usr/bin/env sh
+
+npx uglify-js integration-tests/debugger/target-app/source-map-support/minify.js \
+  -o integration-tests/debugger/target-app/source-map-support/minify.min.js \
+  --v8 \
+  --source-map url=minify.min.js.map

integration-tests/debugger/target-app/source-map-support/scripts/build-typescript.sh

@@ -0,0 +1,3 @@
+#!/usr/bin/env sh
+
+npx --package=typescript -- tsc --sourceMap integration-tests/debugger/target-app/source-map-support/typescript.ts

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

@@ -12,7 +12,7 @@ const { randomUUID } = require('crypto')
 // 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')
+const { getGeneratedPosition } = 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
@@ -104,24 +104,27 @@ async function addBreakpoint (probe) {
   log.warn(`Adding breakpoint at ${url}:${line}`)
 
   let lineNumber = line
+  let columnNumber = 0
 
   if (sourceMapURL) {
     try {
-      lineNumber = await getSourceMappedLine(url, source, line, sourceMapURL)
+      ({ line: lineNumber, column: columnNumber } = await getGeneratedPosition(url, source, line, sourceMapURL))
     } catch (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
+      columnNumber = 0
     }
   }
 
   try {
     const { breakpointId } = await session.post('Debugger.setBreakpoint', {
       location: {
         scriptId,
-        lineNumber: lineNumber - 1
+        lineNumber: lineNumber - 1,
+        columnNumber
       }
     })
 

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

@@ -1,6 +1,6 @@
 'use strict'
 
-const { getSourceMappedLine } = require('./source-maps')
+const { getGeneratedPosition } = 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')
@@ -17,10 +17,11 @@ async function addBreakpoint (probe) {
   if (!sessionStarted) await start()
 
   const file = probe.where.sourceFile
-  let line = Number(probe.where.lines[0]) // Tracer doesn't support multiple-line breakpoints
+  let lineNumber = Number(probe.where.lines[0]) // Tracer doesn't support multiple-line breakpoints
+  let columnNumber = 0 // Probes do not contain/support column information
 
   // Optimize for sending data to /debugger/v1/input endpoint
-  probe.location = { file, lines: [String(line)] }
+  probe.location = { file, lines: [String(lineNumber)] }
   delete probe.where
 
   // Optimize for fast calculations when probe is hit
@@ -38,18 +39,19 @@ async function addBreakpoint (probe) {
   const { url, scriptId, sourceMapURL, source } = script
 
   if (sourceMapURL) {
-    line = await getSourceMappedLine(url, source, line, sourceMapURL)
+    ({ line: lineNumber, column: columnNumber } = await getGeneratedPosition(url, source, lineNumber, sourceMapURL))
   }
 
   log.debug(
-    '[debugger:devtools_client] Adding breakpoint at %s:%d (probe: %s, version: %d)',
-    url, line, probe.id, probe.version
+    '[debugger:devtools_client] Adding breakpoint at %s:%d:%d (probe: %s, version: %d)',
+    url, lineNumber, columnNumber, probe.id, probe.version
   )
 
   const { breakpointId } = await session.post('Debugger.setBreakpoint', {
     location: {
       scriptId,
-      lineNumber: line - 1 // Beware! lineNumber is zero-indexed
+      lineNumber: lineNumber - 1, // Beware! lineNumber is zero-indexed
+      columnNumber
     }
   })
 

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

@@ -23,12 +23,12 @@ const self = module.exports = {
     return cacheIt(path, JSON.parse(readFileSync(path, 'utf8')))
   },
 
-  async getSourceMappedLine (url, source, line, sourceMapURL) {
+  async getGeneratedPosition (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
+      (consumer) => consumer.generatedPositionFor({ source, line, column: 0 })
     )
   }
 }

packages/dd-trace/test/debugger/devtools_client/source-maps.spec.js

@@ -16,7 +16,7 @@ const rawSourceMap = JSON.stringify(parsedSourceMap)
 const inlineSourceMap = `data:application/json;base64,${Buffer.from(rawSourceMap).toString('base64')}`
 
 describe('source map utils', function () {
-  let loadSourceMap, loadSourceMapSync, getSourceMappedLine, readFileSync, readFile
+  let loadSourceMap, loadSourceMapSync, getGeneratedPosition, readFileSync, readFile
 
   describe('basic', function () {
     beforeEach(function () {
@@ -30,7 +30,7 @@ describe('source map utils', function () {
 
       loadSourceMap = sourceMaps.loadSourceMap
       loadSourceMapSync = sourceMaps.loadSourceMapSync
-      getSourceMappedLine = sourceMaps.getSourceMappedLine
+      getGeneratedPosition = sourceMaps.getGeneratedPosition
     })
 
     describe('loadSourceMap', function () {
@@ -77,19 +77,19 @@ describe('source map utils', function () {
       })
     })
 
-    describe('getSourceMappedLine', function () {
+    describe('getGeneratedPosition', function () {
       const url = `file://${dir}/${parsedSourceMap.file}`
       const source = parsedSourceMap.sources[0]
       const line = 1
 
       it('should return expected line for inline source map', async function () {
-        const result = await getSourceMappedLine(url, source, line, sourceMapURL)
-        expect(result).to.equal(2)
+        const pos = await getGeneratedPosition(url, source, line, sourceMapURL)
+        expect(pos).to.deep.equal({ line: 2, column: 0, lastColumn: 5 })
       })
 
       it('should return expected line for non-inline source map', async function () {
-        const result = await getSourceMappedLine(url, source, line, inlineSourceMap)
-        expect(result).to.equal(2)
+        const pos = await getGeneratedPosition(url, source, line, inlineSourceMap)
+        expect(pos).to.deep.equal({ line: 2, column: 0, lastColumn: 5 })
       })
     })
   })