Skip to content

Commit

Permalink
[DI] Add stack trace to log probe results
Browse files Browse the repository at this point in the history
This stack trace represents the point in the execution when the
breakpoint associated with the probe was hit.

The strack frames also include a `columnNumber` property, even though
the API doesn't yet use this property. However, support for column
information in the backend is being added in parallel.
  • Loading branch information
watson committed Sep 26, 2024
1 parent 1d2543c commit 37af606
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 6 deletions.
16 changes: 16 additions & 0 deletions integration-tests/debugger/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,22 @@ describe('Dynamic Instrumentation', function () {
assert.isTrue(payload['debugger.snapshot'].timestamp > Date.now() - 1000 * 60)
assert.isTrue(payload['debugger.snapshot'].timestamp <= Date.now())

assert.isArray(payload['debugger.snapshot'].stack)
assert.isAbove(payload['debugger.snapshot'].stack.length, 0)
for (const frame of payload['debugger.snapshot'].stack) {
assert.isObject(frame)
assert.hasAllKeys(frame, ['fileName', 'function', 'lineNumber', 'columnNumber'])
assert.isString(frame.fileName)
assert.isString(frame.function)
assert.isAbove(frame.lineNumber, 0)
assert.isAbove(frame.columnNumber, 0)
}
const topFrame = payload['debugger.snapshot'].stack[0]
assert.match(topFrame.fileName, new RegExp(`${appFile}$`)) // path seems to be prefeixed with `/private` on Mac
assert.strictEqual(topFrame.function, 'handler')
assert.strictEqual(topFrame.lineNumber, probeLineNo)
assert.strictEqual(topFrame.columnNumber, 3)

done()
})

Expand Down
13 changes: 13 additions & 0 deletions packages/dd-trace/src/debugger/devtools_client/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const { randomUUID } = require('crypto')
const { breakpoints } = require('./state')
const session = require('./session')
const send = require('./send')
const { getScriptUrlFromId } = require('./state')
const { ackEmitting } = require('./status')
const { parentThreadId } = require('./config')
const log = require('../../log')
Expand Down Expand Up @@ -35,6 +36,17 @@ session.on('Debugger.paused', async ({ params }) => {
thread_name: threadName
}

const stack = params.callFrames.map((frame) => {
let fileName = getScriptUrlFromId(frame.location.scriptId)
if (fileName.startsWith('file://')) fileName = fileName.substr(7) // TODO: This might not be required
return {
fileName,
function: frame.functionName,
lineNumber: frame.location.lineNumber + 1, // Beware! lineNumber is zero-indexed
columnNumber: frame.location.columnNumber + 1 // Beware! columnNumber is zero-indexed
}
})

// TODO: Send multiple probes in one HTTP request as an array
for (const probe of probes) {
const snapshot = {
Expand All @@ -45,6 +57,7 @@ session.on('Debugger.paused', async ({ params }) => {
version: probe.version,
location: probe.location
},
stack,
language: 'javascript'
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
'use strict'

const { workerData: { rcPort } } = require('node:worker_threads')
const { getScript, probes, breakpoints } = require('./state')
const { findScriptFromPartialPath, probes, breakpoints } = require('./state')
const session = require('./session')
const { ackReceived, ackInstalled, ackError } = require('./status')
const log = require('../../log')
Expand Down Expand Up @@ -120,7 +120,7 @@ async function addBreakpoint (probe) {
// 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?
const script = getScript(file)
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

Expand Down
14 changes: 10 additions & 4 deletions packages/dd-trace/src/debugger/devtools_client/state.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@

const session = require('./session')

const scripts = []
const scriptIds = []
const scriptUrls = new Map()

module.exports = {
probes: new Map(),
Expand All @@ -25,10 +26,14 @@ module.exports = {
* @param {string} path
* @returns {[string, string] | undefined}
*/
getScript (path) {
return scripts
findScriptFromPartialPath (path) {
return scriptIds
.filter(([url]) => url.endsWith(path))
.sort(([a], [b]) => a.length - b.length)[0]
},

getScriptUrlFromId (id) {
return scriptUrls.get(id)
}
}

Expand All @@ -41,7 +46,8 @@ module.exports = {
// - `` - Not sure what this is, but should just be ignored
// TODO: Event fired for all files, every time debugger is enabled. So when we disable it, we need to reset the state
session.on('Debugger.scriptParsed', ({ params }) => {
scriptUrls.set(params.scriptId, params.url)
if (params.url.startsWith('file:')) {
scripts.push([params.url, params.scriptId])
scriptIds.push([params.url, params.scriptId])
}
})

0 comments on commit 37af606

Please sign in to comment.