Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Debug failing execution #5220

Draft
wants to merge 14 commits into
base: master
Choose a base branch
from
1 change: 1 addition & 0 deletions eslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ 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/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?
Expand Down
4 changes: 2 additions & 2 deletions integration-tests/debugger/basic.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,7 @@ function assertBasicInputPayload (t, payload) {
service: 'node',
message: 'Hello World!',
logger: {
name: t.breakpoint.file,
name: t.breakpoint.deployedFile,
method: 'fooHandler',
version,
thread_name: 'MainThread'
Expand All @@ -544,7 +544,7 @@ function assertBasicInputPayload (t, payload) {
probe: {
id: t.rcConfig.config.id,
version: 0,
location: { file: t.breakpoint.file, lines: [String(t.breakpoint.line)] }
location: { file: t.breakpoint.deployedFile, lines: [String(t.breakpoint.line)] }
},
language: 'javascript'
}
Expand Down
27 changes: 27 additions & 0 deletions integration-tests/debugger/source-map-support.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
'use strict'

const { assert } = require('chai')
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'
})

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']
})
done()
})

t.agent.addRemoteConfig(t.rcConfig)
})
})
})

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,14 @@
require('dd-trace/init')

import { createServer } from 'node:http'

const server = createServer((req, res) => {
// Blank lines below to ensure line numbers in transpiled file differ from original file


res.end('hello world') // BREAKPOINT: /
})

server.listen(process.env.APP_PORT, () => {
process.send?.({ port: process.env.APP_PORT })
})
20 changes: 12 additions & 8 deletions integration-tests/debugger/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,13 @@ module.exports = {
setup
}

function setup ({ env, testApp } = {}) {
function setup ({ env, testApp, testAppSource } = {}) {
let sandbox, cwd, appPort
const breakpoints = getBreakpointInfo({ file: testApp, stackIndex: 1 }) // `1` to disregard the `setup` function
const breakpoints = getBreakpointInfo({
deployedFile: testApp,
sourceFile: testAppSource,
stackIndex: 1 // `1` to disregard the `setup` function
})
const t = {
breakpoint: breakpoints[0],
breakpoints,
Expand Down Expand Up @@ -71,7 +75,7 @@ function setup ({ env, testApp } = {}) {
sandbox = await createSandbox(['fastify']) // TODO: Make this dynamic
cwd = sandbox.folder
// The sandbox uses the `integration-tests` folder as its root
t.appFile = join(cwd, 'debugger', breakpoints[0].file)
t.appFile = join(cwd, 'debugger', breakpoints[0].deployedFile)
})

after(async function () {
Expand Down Expand Up @@ -110,8 +114,8 @@ function setup ({ env, testApp } = {}) {
return t
}

function getBreakpointInfo ({ file, stackIndex = 0 }) {
if (!file) {
function getBreakpointInfo ({ deployedFile, sourceFile = deployedFile, stackIndex = 0 } = {}) {
if (!deployedFile) {
// First, get the filename of file that called this function
const testFile = new Error().stack
.split('\n')[stackIndex + 2] // +2 to skip this function + the first line, which is the error message
Expand All @@ -120,17 +124,17 @@ function getBreakpointInfo ({ file, stackIndex = 0 }) {
.split(':')[0]

// Then, find the corresponding file in which the breakpoint(s) exists
file = join('target-app', basename(testFile).replace('.spec', ''))
deployedFile = sourceFile = join('target-app', basename(testFile).replace('.spec', ''))
}

// Finally, find the line number(s) of the breakpoint(s)
const lines = readFileSync(join(__dirname, file), 'utf8').split('\n')
const lines = readFileSync(join(__dirname, sourceFile), 'utf8').split('\n')
const result = []
for (let i = 0; i < lines.length; i++) {
const index = lines[i].indexOf(BREAKPOINT_TOKEN)
if (index !== -1) {
const url = lines[i].slice(index + BREAKPOINT_TOKEN.length + 1).trim()
result.push({ file, line: i + 1, url })
result.push({ sourceFile, deployedFile, line: i + 1, url })
}
}

Expand Down
11 changes: 8 additions & 3 deletions integration-tests/mocha/mocha.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2206,7 +2206,7 @@
})
})

context('dynamic instrumentation', () => {
context.only('dynamic instrumentation', () => {

Check warning on line 2209 in integration-tests/mocha/mocha.spec.js

View workflow job for this annotation

GitHub Actions / lint

Unexpected exclusive mocha test
it('does not activate dynamic instrumentation if DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED is not set', (done) => {
receiver.setSettings({
flaky_test_retries_enabled: true,
Expand Down Expand Up @@ -2376,7 +2376,7 @@
spanIdByLog = diLog.dd.span_id
traceIdByLog = diLog.dd.trace_id
snapshotIdByLog = diLog.debugger.snapshot.id
}, 5000)
}, 25000)

childProcess = exec(
'node ./ci-visibility/run-mocha.js',
Expand All @@ -2388,12 +2388,17 @@
'./dynamic-instrumentation/test-hit-breakpoint'
]),
DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true',
DD_CIVISIBILITY_FLAKY_RETRY_COUNT: '1'
DD_CIVISIBILITY_FLAKY_RETRY_COUNT: '1',
DD_TRACE_DEBUG: '1',
DD_TRACE_LOG_LEVEL: 'warn'
},
stdio: 'inherit'
}
)

childProcess.stdout.pipe(process.stdout)
childProcess.stderr.pipe(process.stderr)

childProcess.on('exit', () => {
Promise.all([eventsPromise, logsPromise]).then(() => {
assert.equal(snapshotIdByTest, snapshotIdByLog)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ class TestVisDynamicInstrumentation {
this.breakpointHitChannel.port2.on('message', ({ snapshot }) => {
const { probe: { id: probeId } } = snapshot
const onHit = this.onHitBreakpointByProbeId.get(probeId)
log.warn(`received snapshot ${JSON.stringify(snapshot, null, 2)}`)
if (onHit) {
onHit({ snapshot })
} else {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
'use strict'
const path = require('path')

const {
workerData: {
breakpointSetChannel,
Expand All @@ -8,10 +8,11 @@
}
} = require('worker_threads')
const { randomUUID } = require('crypto')
const sourceMap = require('source-map')

// 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')

Check failure on line 15 in packages/dd-trace/src/ci-visibility/dynamic-instrumentation/worker/index.js

View workflow job for this annotation

GitHub Actions / lint

'getSourceMappedLine' is assigned a value but never used
// 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 All @@ -27,7 +28,9 @@
const probeIdToBreakpointId = new Map()

session.on('Debugger.paused', async ({ params: { hitBreakpoints: [hitBreakpoint], callFrames } }) => {
log.warn(`Debugger.paused: ${JSON.stringify(hitBreakpoint, null, 2)}`)
const probe = breakpointIdToProbe.get(hitBreakpoint)
log.warn(`probe: ${JSON.stringify(probe, null, 2)}`)
if (!probe) {
log.warn(`No probe found for breakpoint ${hitBreakpoint}`)
return session.post('Debugger.resume')
Expand All @@ -51,6 +54,8 @@
language: 'javascript'
}

log.warn(`snapshot: ${JSON.stringify(snapshot, null, 2)}`)

const state = getLocalState()
if (state) {
snapshot.captures = {
Expand All @@ -62,13 +67,17 @@
})

breakpointRemoveChannel.on('message', async (probeId) => {
log.warn(`remove breakpoint ${probeId}`)
await removeBreakpoint(probeId)
log.warn(`removed breakpoint ${probeId}`)
breakpointRemoveChannel.postMessage(probeId)
})

breakpointSetChannel.on('message', async (probe) => {
log.warn(`set breakpoint ${JSON.stringify(probe, null, 2)}`)
await addBreakpoint(probe)
breakpointSetChannel.postMessage(probe.id)
log.warn(`added breakpoint ${probe.id}`)
})

async function removeBreakpoint (probeId) {
Expand Down Expand Up @@ -98,19 +107,27 @@
throw new Error(`No loaded script found for ${file}`)
}

const [path, scriptId, sourceMapURL] = script
const { url, scriptId, sourceMapURL } = script

log.warn(`Adding breakpoint at ${path}:${line}`)
log.warn(`Adding breakpoint at ${url}:${line}`)
log.warn(`scriptId: ${scriptId}`)

let lineNumber = line

Check failure on line 115 in packages/dd-trace/src/ci-visibility/dynamic-instrumentation/worker/index.js

View workflow job for this annotation

GitHub Actions / lint

'lineNumber' is never reassigned. Use 'const' instead

if (sourceMapURL && sourceMapURL.startsWith('data:')) {
try {
lineNumber = await processScriptWithInlineSourceMap({ file, line, sourceMapURL })
} catch (err) {
log.error('Error processing script with inline source map', err)
}
}
// if (sourceMapURL) {
// try {
// lineNumber = await getSourceMappedLine(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
// }
// }

log.warn(`Source map: ${sourceMapURL}`)
log.warn(`Actual line number ${lineNumber}`)

try {
const { breakpointId } = await session.post('Debugger.setBreakpoint', {
Expand All @@ -119,55 +136,16 @@
lineNumber: lineNumber - 1
}
})
log.warn(`breakpointId: ${breakpointId}`)

breakpointIdToProbe.set(breakpointId, probe)
probeIdToBreakpointId.set(probe.id, breakpointId)
} catch (e) {
log.error(`Error setting breakpoint at ${path}:${line}:`, e)
log.error('Error setting breakpoint at %s:%s', url, line, e)
}
}

function start () {
sessionStarted = true
return session.post('Debugger.enable') // return instead of await to reduce number of promises created
}

async function processScriptWithInlineSourceMap (params) {
const { file, line, sourceMapURL } = params

// Extract the base64-encoded source map
const base64SourceMap = sourceMapURL.split('base64,')[1]

// Decode the base64 source map
const decodedSourceMap = Buffer.from(base64SourceMap, 'base64').toString('utf8')

// Parse the source map
const consumer = await new sourceMap.SourceMapConsumer(decodedSourceMap)

let generatedPosition

// Map to the generated position. We'll attempt with the full file path first, then with the basename.
// TODO: figure out why sometimes the full path doesn't work
generatedPosition = consumer.generatedPositionFor({
source: file,
line,
column: 0
})
if (generatedPosition.line === null) {
generatedPosition = consumer.generatedPositionFor({
source: path.basename(file),
line,
column: 0
})
}

consumer.destroy()

// If we can't find the line, just return the original line
if (generatedPosition.line === null) {
log.error(`Could not find generated position for ${file}:${line}`)
return line
}

return generatedPosition.line
}
13 changes: 9 additions & 4 deletions packages/dd-trace/src/debugger/devtools_client/breakpoints.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
'use strict'

const { getSourceMappedLine } = 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 @@ -16,7 +17,7 @@ async function addBreakpoint (probe) {
if (!sessionStarted) await start()

const file = probe.where.sourceFile
const line = Number(probe.where.lines[0]) // Tracer doesn't support multiple-line breakpoints
let line = Number(probe.where.lines[0]) // Tracer doesn't support multiple-line breakpoints

// Optimize for sending data to /debugger/v1/input endpoint
probe.location = { file, lines: [String(line)] }
Expand All @@ -34,11 +35,15 @@ async function addBreakpoint (probe) {
// not continue untill all scripts have been parsed?
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
const { url, scriptId, sourceMapURL, source } = script

if (sourceMapURL) {
line = await getSourceMappedLine(url, source, line, sourceMapURL)
}

log.debug(
'[debugger:devtools_client] Adding breakpoint at %s:%d (probe: %s, version: %d)',
path, line, probe.id, probe.version
url, line, probe.id, probe.version
)

const { breakpointId } = await session.post('Debugger.setBreakpoint', {
Expand Down Expand Up @@ -66,7 +71,7 @@ async function removeBreakpoint ({ id }) {
probes.delete(id)
breakpoints.delete(breakpointId)

if (breakpoints.size === 0) await stop()
if (breakpoints.size === 0) return stop() // return instead of await to reduce number of promises created
}

async function start () {
Expand Down
Loading
Loading