Skip to content

Commit f8c4334

Browse files
watsonwatson
authored
fix(debugger): apply source maps to probe stack traces (#7336)
When a probe is hit, stack traces now correctly show original source file locations (e.g., TypeScript files) instead of transpiled code, matching the behavior of breakpoint locations. Added async source map transformation to getStackFromCallFrames using the existing SourceMapConsumer.originalPositionFor method. Stack frames gracefully fall back to generated positions if transformation fails.
1 parent 62d6580 commit f8c4334

File tree

6 files changed

+326
-25
lines changed

6 files changed

+326
-25
lines changed

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

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,19 @@ describe('Dynamic Instrumentation', function () {
1414
beforeEach(() => { t.triggerBreakpoint() })
1515

1616
it('should support source maps', function (done) {
17-
t.agent.on('debugger-input', ({ payload: [{ debugger: { snapshot: { probe: { location } } } }] }) => {
17+
t.agent.on('debugger-input', ({ payload: [{ debugger: { snapshot: { probe: { location }, stack } } }] }) => {
1818
assert.deepStrictEqual(location, {
1919
file: 'target-app/source-map-support/typescript.ts',
2020
lines: ['11']
2121
})
22+
23+
// Verify stack trace also uses original source locations
24+
assert.ok(Array.isArray(stack), 'stack should be an array')
25+
assert.ok(stack.length > 0, 'stack should have at least one frame')
26+
const topFrame = stack[0]
27+
assert.match(topFrame.fileName, /typescript\.ts$/, 'Top frame should reference original TypeScript file')
28+
assert.strictEqual(topFrame.lineNumber, 11, 'Top frame should have original line number')
29+
2230
done()
2331
})
2432

@@ -35,11 +43,19 @@ describe('Dynamic Instrumentation', function () {
3543
beforeEach(() => { t.triggerBreakpoint() })
3644

3745
it('should support source maps', function (done) {
38-
t.agent.on('debugger-input', ({ payload: [{ debugger: { snapshot: { probe: { location } } } }] }) => {
46+
t.agent.on('debugger-input', ({ payload: [{ debugger: { snapshot: { probe: { location }, stack } } }] }) => {
3947
assert.deepStrictEqual(location, {
4048
file: 'target-app/source-map-support/minify.js',
4149
lines: ['9']
4250
})
51+
52+
// Verify stack trace also uses original source locations
53+
assert.ok(Array.isArray(stack), 'stack should be an array')
54+
assert.ok(stack.length > 0, 'stack should have at least one frame')
55+
const topFrame = stack[0]
56+
assert.match(topFrame.fileName, /minify\.js$/, 'Top frame should reference original minified source file')
57+
assert.strictEqual(topFrame.lineNumber, 9, 'Top frame should have original line number')
58+
4359
done()
4460
})
4561

@@ -59,11 +75,19 @@ describe('Dynamic Instrumentation', function () {
5975
beforeEach(() => { t.triggerBreakpoint() })
6076

6177
it('should support relative source paths in source maps', function (done) {
62-
t.agent.on('debugger-input', ({ payload: [{ debugger: { snapshot: { probe: { location } } } }] }) => {
78+
t.agent.on('debugger-input', ({ payload: [{ debugger: { snapshot: { probe: { location }, stack } } }] }) => {
6379
assert.deepStrictEqual(location, {
6480
file: 'target-app/source-map-support/hello/world.ts',
6581
lines: ['2']
6682
})
83+
84+
// Verify stack trace also uses original source locations with relative paths
85+
assert.ok(Array.isArray(stack), 'stack should be an array')
86+
assert.ok(stack.length > 0, 'stack should have at least one frame')
87+
const topFrame = stack[0]
88+
assert.match(topFrame.fileName, /hello\/world\.ts$/, 'Top frame should reference original TypeScript file')
89+
assert.strictEqual(topFrame.lineNumber, 2, 'Top frame should have original line number')
90+
6791
done()
6892
})
6993

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ session.on('Debugger.paused', async ({ params: { hitBreakpoints: [hitBreakpoint]
3636
return session.post('Debugger.resume')
3737
}
3838

39-
const stack = getStackFromCallFrames(callFrames)
39+
const stack = await getStackFromCallFrames(callFrames)
4040

4141
const { processLocalState } = await getLocalStateForCallFrame(callFrames[0])
4242

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

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ const log = require('./log')
1616

1717
require('./remote_config')
1818

19+
/** @typedef {import('node:inspector').Debugger.EvaluateOnCallFrameReturnType} EvaluateOnCallFrameResult */
20+
1921
// Expression to run on a call frame of the paused thread to get its active trace and span id.
2022
const templateExpressionSetupCode = `
2123
const $dd_inspect = global.require('node:util').inspect;
@@ -124,12 +126,14 @@ session.on('Debugger.paused', async ({ params }) => {
124126
if (probe.condition !== undefined) {
125127
// TODO: Bundle all conditions and evaluate them in a single call
126128
// TODO: Handle errors
127-
// eslint-disable-next-line no-await-in-loop
128-
const { result } = await session.post('Debugger.evaluateOnCallFrame', {
129-
callFrameId: params.callFrames[0].callFrameId,
130-
expression: probe.condition,
131-
returnByValue: true
132-
})
129+
const { result } = /** @type {EvaluateOnCallFrameResult} */ (
130+
// eslint-disable-next-line no-await-in-loop
131+
await session.post('Debugger.evaluateOnCallFrame', {
132+
callFrameId: params.callFrames[0].callFrameId,
133+
expression: probe.condition,
134+
returnByValue: true
135+
})
136+
)
133137
if (result.value !== true) continue
134138
}
135139

@@ -151,14 +155,16 @@ session.on('Debugger.paused', async ({ params }) => {
151155
const timestamp = Date.now()
152156

153157
let evalResults
154-
const { result } = await session.post('Debugger.evaluateOnCallFrame', {
155-
callFrameId: params.callFrames[0].callFrameId,
156-
expression: templateExpressions.length === 0
157-
? `[${getDDTagsExpression}]`
158-
: `${templateExpressionSetupCode}[${getDDTagsExpression}${templateExpressions}]`,
159-
returnByValue: true,
160-
includeCommandLineAPI: true
161-
})
158+
const { result } = /** @type {EvaluateOnCallFrameResult} */ (
159+
await session.post('Debugger.evaluateOnCallFrame', {
160+
callFrameId: params.callFrames[0].callFrameId,
161+
expression: templateExpressions.length === 0
162+
? `[${getDDTagsExpression}]`
163+
: `${templateExpressionSetupCode}[${getDDTagsExpression}${templateExpressions}]`,
164+
returnByValue: true,
165+
includeCommandLineAPI: true
166+
})
167+
)
162168
if (result?.subtype === 'error') {
163169
log.error('[debugger:devtools_client] Error evaluating code on call frame: %s', result?.description)
164170
evalResults = []
@@ -198,7 +204,7 @@ session.on('Debugger.paused', async ({ params }) => {
198204
thread_name: threadName
199205
}
200206

201-
const stack = getStackFromCallFrames(params.callFrames)
207+
const stack = await getStackFromCallFrames(params.callFrames)
202208
const dd = processDD(evalResults[0]) // the first result is the dd tags, the rest are the probe template results
203209
let messageIndex = 1
204210

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,15 @@ const self = module.exports = {
3131
null,
3232
(consumer) => consumer.generatedPositionFor({ source, line, column: 0 })
3333
)
34+
},
35+
36+
async getOriginalPosition (url, line, column, sourceMapURL) {
37+
const dir = dirname(new URL(url).pathname)
38+
return SourceMapConsumer.with(
39+
await self.loadSourceMap(dir, sourceMapURL),
40+
null,
41+
(consumer) => consumer.originalPositionFor({ line, column })
42+
)
3443
}
3544
}
3645

‎packages/dd-trace/src/debugger/devtools_client/state.js‎

Lines changed: 63 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,31 @@
22

33
const { join, dirname } = require('path')
44
const { normalize } = require('../../../../../vendor/dist/source-map/lib/util')
5-
const { loadSourceMapSync } = require('./source-maps')
5+
const { loadSourceMapSync, getOriginalPosition } = require('./source-maps')
66
const session = require('./session')
77
const log = require('./log')
88

9+
/**
10+
* @typedef {object} StackFrame
11+
* @property {string} fileName - The file name
12+
* @property {string} function - The function name
13+
* @property {number} lineNumber - The line number (1-indexed)
14+
* @property {number} columnNumber - The column number (1-indexed)
15+
*/
16+
17+
/**
18+
* @typedef {object} ScriptInfo
19+
* @property {string | null} url - The URL of the script
20+
* @property {string | null} scriptId - The script identifier
21+
* @property {string | null} sourceMapURL - The source map URL if available
22+
* @property {string | null} source - The source content if available
23+
*/
24+
925
const WINDOWS_DRIVE_LETTER_REGEX = /[a-zA-Z]/
1026

1127
const loadedScripts = []
1228
const scriptUrls = new Map()
29+
const scriptSourceMaps = new Map()
1330
let reEvaluateProbesTimer = null
1431

1532
module.exports = {
@@ -24,7 +41,8 @@ module.exports = {
2441
* Find the script to inspect based on a partial or absolute path. Handles both Windows and POSIX paths.
2542
*
2643
* @param {string} path - Partial or absolute path to match against loaded scripts
27-
* @returns {object | null} - Object containing `url`, `scriptId`, `sourceMapURL`, and `source` - or null if no match
44+
* @returns {ScriptInfo | null} - Object containing `url`, `scriptId`, `sourceMapURL`, and `source` - or null
45+
* if no match
2846
*/
2947
findScriptFromPartialPath (path) {
3048
if (!path) return null // This shouldn't happen, but better safe than sorry
@@ -101,21 +119,55 @@ module.exports = {
101119
return maxMatchLength === -1 ? null : bestMatch
102120
},
103121

122+
/**
123+
* Get the stack from call frames.
124+
*
125+
* @param {Array<import('inspector').Debugger.CallFrame>} callFrames - The call frames to get the stack from.
126+
* @returns {Promise<Array<StackFrame>>} - The stack from call frames.
127+
*/
104128
getStackFromCallFrames (callFrames) {
105-
return callFrames.map((frame) => {
129+
return Promise.all(callFrames.map(async (frame) => {
106130
// TODO: Possible race condition: If the breakpoint is in the process of being removed, and this is the last
107131
// breakpoint, it will also stop the debugging session, which in turn will clear the state, which means clearing
108132
// the `scriptUrls` map. That might result in this the `scriptUrls.get` call above returning `undefined`, which
109133
// will throw when `startsWith` is called on it.
110134
let fileName = scriptUrls.get(frame.location.scriptId)
111135
if (fileName.startsWith('file://')) fileName = fileName.slice(7) // TODO: This might not be required
136+
137+
let lineNumber = frame.location.lineNumber + 1 // Beware! lineNumber is zero-indexed
138+
let columnNumber = (frame.location.columnNumber ?? 0) + 1 // Beware! columnNumber is zero-indexed
139+
140+
// Check if this script has a source map
141+
const sourceMapInfo = scriptSourceMaps.get(frame.location.scriptId)
142+
if (sourceMapInfo) {
143+
try {
144+
const original = await getOriginalPosition(
145+
sourceMapInfo.url,
146+
frame.location.lineNumber + 1, // CDP uses 0-indexed
147+
(frame.location.columnNumber ?? 0) + 1,
148+
sourceMapInfo.sourceMapURL
149+
)
150+
151+
if (original.source && original.line !== null) {
152+
// Convert source map source path to absolute file path
153+
const dir = dirname(new URL(sourceMapInfo.url).pathname)
154+
fileName = new URL(join(dir, original.source), 'file:').href.slice(7)
155+
lineNumber = original.line
156+
columnNumber = original.column ?? columnNumber
157+
}
158+
} catch (err) {
159+
// If source map transformation fails, use generated positions
160+
log.warn('[debugger:devtools_client] Failed to apply source map to stack frame', err)
161+
}
162+
}
163+
112164
return {
113165
fileName,
114166
function: frame.functionName,
115-
lineNumber: frame.location.lineNumber + 1, // Beware! lineNumber is zero-indexed
116-
columnNumber: frame.location.columnNumber + 1 // Beware! columnNumber is zero-indexed
167+
lineNumber,
168+
columnNumber
117169
}
118-
})
170+
}))
119171
},
120172

121173
// The maps locationToBreakpoint, breakpointToProbes, and probeToLocation are always updated when breakpoints are
@@ -124,6 +176,7 @@ module.exports = {
124176
clearState () {
125177
loadedScripts.length = 0
126178
scriptUrls.clear()
179+
scriptSourceMaps.clear()
127180
}
128181
}
129182

@@ -138,6 +191,10 @@ session.on('Debugger.scriptParsed', ({ params }) => {
138191
scriptUrls.set(params.scriptId, params.url)
139192
if (params.url.startsWith('file:')) {
140193
if (params.sourceMapURL) {
194+
scriptSourceMaps.set(params.scriptId, {
195+
url: params.url,
196+
sourceMapURL: params.sourceMapURL
197+
})
141198
const dir = dirname(new URL(params.url).pathname)
142199
let sources
143200
try {

0 commit comments

Comments
 (0)