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

Fix child process not maintaining previous parent span after execution #4752

Merged
merged 2 commits into from
Oct 4, 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
25 changes: 17 additions & 8 deletions packages/datadog-instrumentations/src/child_process.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,17 @@ function wrapChildProcessSyncMethod (shell = false) {

const childProcessInfo = normalizeArgs(arguments, shell)

return childProcessChannel.traceSync(
childProcessMethod,
{
command: childProcessInfo.command,
shell: childProcessInfo.shell
},
this,
...arguments)
const innerResource = new AsyncResource('bound-anonymous-fn')
return innerResource.runInAsyncScope(() => {
return childProcessChannel.traceSync(
childProcessMethod,
{
command: childProcessInfo.command,
shell: childProcessInfo.shell
},
this,
...arguments)
})
}
}
}
Expand Down Expand Up @@ -101,6 +104,12 @@ function wrapChildProcessAsyncMethod (shell = false) {

const childProcessInfo = normalizeArgs(arguments, shell)

const cb = arguments[arguments.length - 1]
if (typeof cb === 'function') {
const callbackResource = new AsyncResource('bound-anonymous-fn')
arguments[arguments.length - 1] = callbackResource.bind(cb)
}

const innerResource = new AsyncResource('bound-anonymous-fn')
return innerResource.runInAsyncScope(() => {
childProcessChannel.start.publish({ command: childProcessInfo.command, shell: childProcessInfo.shell })
Expand Down
120 changes: 113 additions & 7 deletions packages/datadog-plugin-child_process/test/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,82 @@ describe('Child process plugin', () => {
})
})

describe('context maintenance', () => {
let parent
let childProcess
let tracer

before(() => {
return agent.load(['child_process'])
.then(() => {
childProcess = require('child_process')
tracer = require('../../dd-trace')
tracer.init()
parent = tracer.startSpan('parent')
parent.finish()
}).then(_port => {
return new Promise(resolve => setImmediate(resolve))
})
})

after(() => {
return agent.close()
})

it('should preserve context around execSync calls', () => {
tracer.scope().activate(parent, () => {
expect(tracer.scope().active()).to.equal(parent)
childProcess.execSync('ls')
expect(tracer.scope().active()).to.equal(parent)
})
})

it('should preserve context around exec calls', (done) => {
tracer.scope().activate(parent, () => {
expect(tracer.scope().active()).to.equal(parent)
childProcess.exec('ls', () => {
expect(tracer.scope().active()).to.equal(parent)
done()
})
})
})

it('should preserve context around execFileSync calls', () => {
tracer.scope().activate(parent, () => {
expect(tracer.scope().active()).to.equal(parent)
childProcess.execFileSync('ls')
expect(tracer.scope().active()).to.equal(parent)
})
})

it('should preserve context around execFile calls', (done) => {
tracer.scope().activate(parent, () => {
expect(tracer.scope().active()).to.equal(parent)
childProcess.execFile('ls', () => {
expect(tracer.scope().active()).to.equal(parent)
done()
})
})
})

it('should preserve context around spawnSync calls', () => {
tracer.scope().activate(parent, () => {
expect(tracer.scope().active()).to.equal(parent)
childProcess.spawnSync('ls')
expect(tracer.scope().active()).to.equal(parent)
})
})

it('should preserve context around spawn calls', (done) => {
tracer.scope().activate(parent, () => {
expect(tracer.scope().active()).to.equal(parent)
childProcess.spawn('ls')
expect(tracer.scope().active()).to.equal(parent)
done()
})
})
})

describe('Integration', () => {
describe('Methods which spawn a shell by default', () => {
const execAsyncMethods = ['exec']
Expand All @@ -299,19 +375,25 @@ describe('Child process plugin', () => {

afterEach(() => agent.close({ ritmReset: false }))
const parentSpanList = [true, false]
parentSpanList.forEach(parentSpan => {
describe(`${parentSpan ? 'with' : 'without'} parent span`, () => {
parentSpanList.forEach(hasParentSpan => {
let parentSpan

describe(`${hasParentSpan ? 'with' : 'without'} parent span`, () => {
const methods = [
...execAsyncMethods.map(methodName => ({ methodName, async: true })),
...execSyncMethods.map(methodName => ({ methodName, async: false }))
]
if (parentSpan) {
beforeEach((done) => {
const parentSpan = tracer.startSpan('parent')

beforeEach((done) => {
if (hasParentSpan) {
parentSpan = tracer.startSpan('parent')
parentSpan.finish()
tracer.scope().activate(parentSpan, done)
})
}
} else {
storage.enterWith({})
done()
}
})

methods.forEach(({ methodName, async }) => {
describe(methodName, () => {
Expand All @@ -335,6 +417,30 @@ describe('Child process plugin', () => {
}
})

it('should maintain previous span after the execution', (done) => {
const res = childProcess[methodName]('ls')
const span = storage.getStore()?.span
expect(span).to.be.equals(parentSpan)
if (async) {
res.on('close', () => {
expect(span).to.be.equals(parentSpan)
done()
})
} else {
done()
}
})

if (async) {
it('should maintain previous span in the callback', (done) => {
childProcess[methodName]('ls', () => {
const span = storage.getStore()?.span
expect(span).to.be.equals(parentSpan)
done()
})
})
}

it('command should be scrubbed', (done) => {
const expected = {
type: 'system',
Expand Down
Loading