From 92515a65e37d079ff3b3ea79546bc9d02dba82b0 Mon Sep 17 00:00:00 2001 From: Thomas Watson Date: Wed, 2 Oct 2024 12:13:37 +0200 Subject: [PATCH] [DI] Add stack trace to log probe results (#4727) --- integration-tests/debugger/index.spec.js | 16 ++++++++++++++++ .../src/debugger/devtools_client/index.js | 13 +++++++++++++ .../debugger/devtools_client/remote_config.js | 4 ++-- .../src/debugger/devtools_client/state.js | 14 ++++++++++---- 4 files changed, 41 insertions(+), 6 deletions(-) diff --git a/integration-tests/debugger/index.spec.js b/integration-tests/debugger/index.spec.js index 241f57f722..4f975e179f 100644 --- a/integration-tests/debugger/index.spec.js +++ b/integration-tests/debugger/index.spec.js @@ -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() }) diff --git a/packages/dd-trace/src/debugger/devtools_client/index.js b/packages/dd-trace/src/debugger/devtools_client/index.js index f4789ea65a..e1100f99ab 100644 --- a/packages/dd-trace/src/debugger/devtools_client/index.js +++ b/packages/dd-trace/src/debugger/devtools_client/index.js @@ -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') @@ -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 = { @@ -45,6 +57,7 @@ session.on('Debugger.paused', async ({ params }) => { version: probe.version, location: probe.location }, + stack, language: 'javascript' } diff --git a/packages/dd-trace/src/debugger/devtools_client/remote_config.js b/packages/dd-trace/src/debugger/devtools_client/remote_config.js index 25ac070cc9..b030b09372 100644 --- a/packages/dd-trace/src/debugger/devtools_client/remote_config.js +++ b/packages/dd-trace/src/debugger/devtools_client/remote_config.js @@ -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') @@ -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 diff --git a/packages/dd-trace/src/debugger/devtools_client/state.js b/packages/dd-trace/src/debugger/devtools_client/state.js index 316841667f..8be9c80836 100644 --- a/packages/dd-trace/src/debugger/devtools_client/state.js +++ b/packages/dd-trace/src/debugger/devtools_client/state.js @@ -2,7 +2,8 @@ const session = require('./session') -const scripts = [] +const scriptIds = [] +const scriptUrls = new Map() module.exports = { probes: new Map(), @@ -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) } } @@ -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]) } })