Skip to content

Commit

Permalink
[DI] Use column number from source maps
Browse files Browse the repository at this point in the history
  • Loading branch information
watson committed Feb 17, 2025
1 parent efb8e44 commit 9f91f32
Show file tree
Hide file tree
Showing 14 changed files with 83 additions and 32 deletions.
47 changes: 35 additions & 12 deletions integration-tests/debugger/source-map-support.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
})
})
})

This file was deleted.

11 changes: 11 additions & 0 deletions integration-tests/debugger/target-app/source-map-support/minify.js
Original file line number Diff line number Diff line change
@@ -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 })
})

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
#!/usr/bin/env sh

npx --package=typescript -- tsc --sourceMap integration-tests/debugger/target-app/source-map-support/typescript.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
({ lineNumber, columnNumber } = await getGeneratedPosition(url, source, line, sourceMapURL).line)
} 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
}
})

Expand Down
14 changes: 8 additions & 6 deletions packages/dd-trace/src/debugger/devtools_client/breakpoints.js
Original file line number Diff line number Diff line change
@@ -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')
Expand All @@ -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
Expand All @@ -38,18 +39,19 @@ async function addBreakpoint (probe) {
const { url, scriptId, sourceMapURL, source } = script

if (sourceMapURL) {
line = await getSourceMappedLine(url, source, line, sourceMapURL)
({ lineNumber, 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
url, lineNumber, 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
}
})

Expand Down
4 changes: 2 additions & 2 deletions packages/dd-trace/src/debugger/devtools_client/source-maps.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 })
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand All @@ -30,7 +30,7 @@ describe('source map utils', function () {

loadSourceMap = sourceMaps.loadSourceMap
loadSourceMapSync = sourceMaps.loadSourceMapSync
getSourceMappedLine = sourceMaps.getSourceMappedLine
getGeneratedPosition = sourceMaps.getGeneratedPosition
})

describe('loadSourceMap', function () {
Expand Down Expand Up @@ -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 })
})
})
})
Expand Down

0 comments on commit 9f91f32

Please sign in to comment.