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

[DI] Improve test setup by allowing breakpoint URL to be dynamic #4996

Merged
merged 2 commits into from
Dec 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions integration-tests/debugger/basic.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ describe('Dynamic Instrumentation', function () {
const t = setup()

it('base case: target app should work as expected if no test probe has been added', async function () {
const response = await t.axios.get('/foo')
const response = await t.axios.get(t.breakpoint.url)
assert.strictEqual(response.status, 200)
assert.deepStrictEqual(response.data, { hello: 'foo' })
})
Expand Down Expand Up @@ -51,7 +51,7 @@ describe('Dynamic Instrumentation', function () {
assertUUID(payload.debugger.diagnostics.runtimeId)

if (payload.debugger.diagnostics.status === 'INSTALLED') {
t.axios.get('/foo')
t.axios.get(t.breakpoint.url)
.then((response) => {
assert.strictEqual(response.status, 200)
assert.deepStrictEqual(response.data, { hello: 'foo' })
Expand Down Expand Up @@ -293,13 +293,13 @@ describe('Dynamic Instrumentation', function () {
const expectedMessages = ['Hello World!', 'Hello Updated World!']
const triggers = [
async () => {
await t.axios.get('/foo')
await t.axios.get(t.breakpoint.url)
t.rcConfig.config.version++
t.rcConfig.config.template = 'Hello Updated World!'
t.agent.updateRemoteConfig(t.rcConfig.id, t.rcConfig.config)
},
async () => {
await t.axios.get('/foo')
await t.axios.get(t.breakpoint.url)
}
]

Expand All @@ -319,7 +319,7 @@ describe('Dynamic Instrumentation', function () {
t.agent.on('debugger-diagnostics', ({ payload }) => {
if (payload.debugger.diagnostics.status === 'INSTALLED') {
t.agent.once('remote-confg-responded', async () => {
await t.axios.get('/foo')
await t.axios.get(t.breakpoint.url)
// We want to wait enough time to see if the client triggers on the breakpoint so that the test can fail
// if it does, but not so long that the test times out.
// TODO: Is there some signal we can use instead of a timer?
Expand Down Expand Up @@ -368,7 +368,7 @@ describe('Dynamic Instrumentation', function () {
})

// Perform HTTP request to try and trigger the probe
t.axios.get('/foo').catch((err) => {
t.axios.get(t.breakpoint.url).catch((err) => {
// If the request hasn't fully completed by the time the tests ends and the target app is destroyed, Axios
// will complain with a "socket hang up" error. Hence this sanity check before calling `done(err)`. If we
// later add more tests below this one, this shouuldn't be an issue.
Expand Down
6 changes: 2 additions & 4 deletions integration-tests/debugger/snapshot-pruning.spec.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
'use strict'

const { assert } = require('chai')
const { setup, getBreakpointInfo } = require('./utils')

const { line } = getBreakpointInfo()
const { setup } = require('./utils')

describe('Dynamic Instrumentation', function () {
const t = setup()
Expand All @@ -17,7 +15,7 @@ describe('Dynamic Instrumentation', function () {
assert.isBelow(Buffer.byteLength(JSON.stringify(payload)), 1024 * 1024) // 1MB
assert.deepEqual(payload['debugger.snapshot'].captures, {
lines: {
[line]: {
[t.breakpoint.line]: {
locals: {
notCapturedReason: 'Snapshot was too large',
size: 6
Expand Down
2 changes: 1 addition & 1 deletion integration-tests/debugger/target-app/basic.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ const Fastify = require('fastify')
const fastify = Fastify()

fastify.get('/:name', function handler (request) {
return { hello: request.params.name } // BREAKPOINT
return { hello: request.params.name } // BREAKPOINT: /foo
})

fastify.listen({ port: process.env.APP_PORT }, (err) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ fastify.get('/:name', function handler (request) {
// eslint-disable-next-line no-unused-vars
const obj = generateObjectWithJSONSizeLargerThan1MB()

return { hello: request.params.name } // BREAKPOINT
return { hello: request.params.name } // BREAKPOINT: /foo
})

fastify.listen({ port: process.env.APP_PORT }, (err) => {
Expand Down
2 changes: 1 addition & 1 deletion integration-tests/debugger/target-app/snapshot.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const fastify = Fastify()
fastify.get('/:name', function handler (request) {
// eslint-disable-next-line no-unused-vars
const { nil, undef, bool, num, bigint, str, lstr, sym, regex, arr, obj, emptyObj, fn, p } = getSomeData()
return { hello: request.params.name } // BREAKPOINT
return { hello: request.params.name } // BREAKPOINT: /foo
})

fastify.listen({ port: process.env.APP_PORT }, (err) => {
Expand Down
2 changes: 1 addition & 1 deletion integration-tests/debugger/target-app/unreffed.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ require('dd-trace/init')
const http = require('http')

const server = http.createServer((req, res) => {
res.end('hello world') // BREAKPOINT
res.end('hello world') // BREAKPOINT: /
setImmediate(() => {
server.close()
})
Expand Down
2 changes: 1 addition & 1 deletion integration-tests/debugger/unreffed.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@ describe('Dynamic Instrumentation', function () {
assert.strictEqual(code, 0)
done()
})
t.axios.get('/')
t.axios.get(t.breakpoint.url)
})
})
45 changes: 16 additions & 29 deletions integration-tests/debugger/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,14 @@ const getPort = require('get-port')
const Axios = require('axios')

const { createSandbox, FakeAgent, spawnProc } = require('../helpers')
const { generateProbeConfig } = require('../../packages/dd-trace/test/debugger/devtools_client/utils')

const BREAKPOINT_TOKEN = '// BREAKPOINT'
const pollInterval = 1

module.exports = {
pollInterval,
setup,
getBreakpointInfo
setup
}

function setup () {
Expand All @@ -35,7 +36,7 @@ function setup () {
// Trigger the breakpoint once probe is successfully installed
t.agent.on('debugger-diagnostics', ({ payload }) => {
if (payload.debugger.diagnostics.status === 'INSTALLED') {
t.axios.get('/foo')
t.axios.get(breakpoint.url)
}
})
}
Expand All @@ -45,32 +46,15 @@ function setup () {
return {
product: 'LIVE_DEBUGGING',
id: `logProbe_${overrides.id}`,
config: generateProbeConfig(overrides)
}
}

function generateProbeConfig (overrides = {}) {
overrides.capture = { maxReferenceDepth: 3, ...overrides.capture }
overrides.sampling = { snapshotsPerSecond: 5000, ...overrides.sampling }
return {
id: randomUUID(),
version: 0,
type: 'LOG_PROBE',
language: 'javascript',
where: { sourceFile: breakpoint.file, lines: [String(breakpoint.line)] },
tags: [],
template: 'Hello World!',
segments: [{ str: 'Hello World!' }],
captureSnapshot: false,
evaluateAt: 'EXIT',
...overrides
config: generateProbeConfig(breakpoint, overrides)
}
}

before(async function () {
sandbox = await createSandbox(['fastify']) // TODO: Make this dynamic
cwd = sandbox.folder
t.appFile = join(cwd, ...breakpoint.file.split('/'))
// The sandbox uses the `integration-tests` folder as its root
t.appFile = join(cwd, 'debugger', breakpoint.file)
})

after(async function () {
Expand Down Expand Up @@ -113,12 +97,15 @@ function getBreakpointInfo (stackIndex = 0) {
.split(':')[0]

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

// Finally, find the line number of the breakpoint
const line = readFileSync(join(__dirname, 'target-app', filename), 'utf8')
.split('\n')
.findIndex(line => line.includes('// BREAKPOINT')) + 1

return { file: `debugger/target-app/${filename}`, line }
const lines = readFileSync(join(__dirname, file), 'utf8').split('\n')
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()
return { file, line: i + 1, url }
}
}
}
25 changes: 25 additions & 0 deletions packages/dd-trace/test/debugger/devtools_client/utils.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
'use strict'
uurien marked this conversation as resolved.
Show resolved Hide resolved

const { randomUUID } = require('node:crypto')

module.exports = {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually, we write module.exports at the end of the file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find that it's easier to get a quick overview over what the file "exports" when it's at the top of the file. Is putting them at the end an enforced pattern for the tracer?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

enforced pattern I don't know tbh. It is what I see in other files, there is not a rule in the linter, so I think that it is not enforced pattern. It just catches my attention.

generateProbeConfig
}

function generateProbeConfig (breakpoint, overrides = {}) {
overrides.capture = { maxReferenceDepth: 3, ...overrides.capture }
overrides.sampling = { snapshotsPerSecond: 5000, ...overrides.sampling }
return {
id: randomUUID(),
version: 0,
type: 'LOG_PROBE',
language: 'javascript',
where: { sourceFile: breakpoint.file, lines: [String(breakpoint.line)] },
tags: [],
template: 'Hello World!',
segments: [{ str: 'Hello World!' }],
captureSnapshot: false,
evaluateAt: 'EXIT',
...overrides
}
}
Loading