diff --git a/lib/core-instrumentation.js b/lib/core-instrumentation.js index 7c04bdba2b..efc323905a 100644 --- a/lib/core-instrumentation.js +++ b/lib/core-instrumentation.js @@ -8,10 +8,6 @@ const InstrumentationDescriptor = require('./instrumentation-descriptor') module.exports = { - child_process: { - type: InstrumentationDescriptor.TYPE_GENERIC, - file: 'child_process.js' - }, crypto: { type: InstrumentationDescriptor.TYPE_GENERIC, file: 'crypto.js' diff --git a/lib/instrumentation/core/child_process.js b/lib/instrumentation/core/child_process.js deleted file mode 100644 index f49bb89c00..0000000000 --- a/lib/instrumentation/core/child_process.js +++ /dev/null @@ -1,37 +0,0 @@ -/* - * Copyright 2020 New Relic Corporation. All rights reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -'use strict' - -const { RecorderSpec } = require('../../../lib/shim/specs') - -module.exports = initialize - -function initialize(agent, childProcess, moduleName, shim) { - if (!childProcess) { - shim.logger.debug('Could not find child_process, not instrumenting') - return false - } - - const methods = ['exec', 'execFile'] - - shim.record(childProcess, methods, function recordExec(shim, fn, name) { - return new RecorderSpec({ name: 'child_process.' + name, callback: shim.LAST }) - }) - - makePromisifyCompatible(shim, childProcess) -} - -function makePromisifyCompatible(shim, childProcess) { - const originalExec = shim.getOriginal(childProcess.exec) - for (const symbol of Object.getOwnPropertySymbols(originalExec)) { - childProcess.exec[symbol] = originalExec[symbol] - } - - const originalExecFile = shim.getOriginal(childProcess.execFile) - for (const symbol of Object.getOwnPropertySymbols(originalExecFile)) { - childProcess.execFile[symbol] = originalExecFile[symbol] - } -} diff --git a/lib/subscriber-configs.js b/lib/subscriber-configs.js index b52599868f..6ec6304062 100644 --- a/lib/subscriber-configs.js +++ b/lib/subscriber-configs.js @@ -16,6 +16,7 @@ const subscribers = { ...require('./subscribers/bluebird/config'), ...require('./subscribers/bunyan/config'), ...require('./subscribers/cassandra-driver/config'), + ...require('./subscribers/child_process/config'), ...require('./subscribers/connect/config'), ...require('./subscribers/elasticsearch/config'), ...require('./subscribers/express/config'), diff --git a/lib/subscribers/child_process/config.js b/lib/subscribers/child_process/config.js new file mode 100644 index 0000000000..4f3281d183 --- /dev/null +++ b/lib/subscribers/child_process/config.js @@ -0,0 +1,14 @@ +/* + * Copyright 2026 New Relic Corporation. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +'use strict' + +// instrumentations are blank because the subscriber is not Orchestrion-based +module.exports = { + child_process: [ + { path: './child_process/exec', instrumentations: [] }, + { path: './child_process/exec-file', instrumentations: [] } + ] +} diff --git a/lib/subscribers/child_process/exec-file.js b/lib/subscribers/child_process/exec-file.js new file mode 100644 index 0000000000..dc0c367438 --- /dev/null +++ b/lib/subscribers/child_process/exec-file.js @@ -0,0 +1,14 @@ +/* + * Copyright 2026 New Relic Corporation. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +'use strict' + +const ChildProcessExec = require('./exec') + +module.exports = class ChildProcessExecFile extends ChildProcessExec { + constructor({ agent, logger }) { + super({ agent, logger, methodName: 'execFile' }) + } +} diff --git a/lib/subscribers/child_process/exec.js b/lib/subscribers/child_process/exec.js new file mode 100644 index 0000000000..c787d7978a --- /dev/null +++ b/lib/subscribers/child_process/exec.js @@ -0,0 +1,69 @@ +/* + * Copyright 2026 New Relic Corporation. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +'use strict' + +const childProcess = require('child_process') +const TcBaseSubscriber = require('../tc-base') + +module.exports = class ChildProcessExec extends TcBaseSubscriber { + constructor({ agent, logger, methodName = 'exec' }) { + super({ agent, logger, packageName: 'child_process', channelName: methodName }) + this.methodName = methodName + } + + /** + * Patches `childProcess[methodName]` before binding the store -- guarded so + * `setupSubscribers()` constructing/enabling this on every agent cycle + * only ever wraps the method once, ever, per process. + */ + enable() { + this.patch() + super.enable() + } + + patch() { + const methodName = this.methodName + const original = childProcess[methodName] + if (original.__nr_wrapped === true) { + return + } + const channel = this.channel + + childProcess[methodName] = function wrappedMethod(...args) { + const lastArg = args[args.length - 1] + const hasCallback = typeof lastArg === 'function' + const data = { methodName, callbackName: hasCallback ? lastArg.name || '' : null } + + return hasCallback + ? channel.traceCallback(original, -1, data, this, ...args) + : channel.traceSync(original, data, this, ...args) + } + + for (const symbol of Object.getOwnPropertySymbols(original)) { + childProcess[methodName][symbol] = original[symbol] + } + childProcess[methodName].__nr_wrapped = true + } + + handleStart(data, ctx) { + const segment = this.createSegment(ctx, 'child_process.' + data.methodName) + return segment ? ctx.enterSegment({ segment }) : ctx + } + + handleAsyncStart(data, ctx) { + if (!ctx?.segment) { + return ctx + } + ctx.segment.touch() + + const segment = this.createSegment(ctx, 'Callback: ' + data.callbackName) + if (!segment) { + return ctx + } + data.callbackSegment = segment + return ctx.enterSegment({ segment }) + } +} diff --git a/lib/subscribers/tc-base.js b/lib/subscribers/tc-base.js new file mode 100644 index 0000000000..6763728393 --- /dev/null +++ b/lib/subscribers/tc-base.js @@ -0,0 +1,133 @@ +/* + * Copyright 2026 New Relic Corporation. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +'use strict' + +// eslint-disable-next-line n/no-unsupported-features/node-builtins +const { tracingChannel } = require('node:diagnostics_channel') + +/** + * Base class for subscribers whose `tracingChannel` is self-created (via + * `tracingChannel(id)`) but published by our own monkeypatch rather than by + * Orchestrion's `ModulePatch` e.g. core Node modules where the target + * method can be patched directly. + * + * Subclasses are responsible for actually publishing to `this.channel` + * (typically from an overridden `enable()`, guarded so the patch only ever + * happens once) and for the `handleStart`/`handleAsyncStart` overrides below. + */ +class TcBaseSubscriber { + constructor({ agent, logger, packageName, channelName }) { + this.agent = agent + this.logger = logger.child({ component: `${packageName}-subscriber` }) + this.config = agent.config + this.packageName = packageName + this.channelName = channelName + this.id = `${packageName}.${channelName}` + this.channel = tracingChannel(this.id) + this.tracer = agent.tracer + this.store = agent.tracer._contextManager._asyncLocalStorage + this._onEnd = this.onEnd.bind(this) + this._onAsyncEnd = this.onAsyncEnd.bind(this) + } + + /** + * Checks if the subscriber is enabled based on the agent's configuration. + * @returns {boolean} if subscriber is enabled + */ + get enabled() { + return this.config.instrumentation[this.packageName].enabled === true + } + + /** + * Creates and starts a segment as a child of the given context's segment. + * @param {Context} ctx active context + * @param {string} name segment name + * @returns {TraceSegment|null} the created segment, or null if none was created + */ + createSegment(ctx, name) { + const segment = this.tracer.createSegment({ name, parent: ctx?.segment, transaction: ctx?.transaction }) + if (segment) { + segment.start() + } + return segment + } + + /** + * Override point: called on `channel.start`. Return the context that + * should become active for the duration of the traced call. + * @param {object} data event data published on `channel.start` + * @param {Context} ctx ambient context at the time `start` fired + * @returns {Context} context to enter + */ + handleStart(data, ctx) { + return ctx + } + + /** + * Override point: called on `channel.asyncStart` (i.e. right before a + * traced callback runs). Return the context that should become active for + * the duration of the callback. + * @param {object} data event data published on `channel.start`/`asyncStart` + * @param {Context} ctx context that was entered on `start` (`data.ctx`) + * @returns {Context} context to enter + */ + handleAsyncStart(data, ctx) { + return ctx + } + + /** + * Default `end` handler -- touches whatever segment `handleStart` entered. + * @param {object} data event data + */ + onEnd(data) { + data.ctx?.segment?.touch() + } + + /** + * Default `asyncEnd` handler -- touches whatever segment + * `handleAsyncStart` created, if any. + * @param {object} data event data + */ + onAsyncEnd(data) { + data.callbackSegment?.touch() + } + + /** + * Binds this subscriber's store to `channel.start`/`asyncStart` so + * `handleStart`/`handleAsyncStart` run scoped to the traced call/callback. + */ + enable() { + this.channel.start.bindStore(this.store, (data) => { + data.ctx = this.handleStart(data, this.tracer.getContext()) + return data.ctx + }) + this.channel.asyncStart.bindStore(this.store, (data) => this.handleAsyncStart(data, data.ctx)) + } + + /** + * Unbinds this subscriber's store from `channel.start`/`asyncStart`. + */ + disable() { + this.channel.start.unbindStore(this.store) + this.channel.asyncStart.unbindStore(this.store) + } + + /** + * Subscribes the default `end`/`asyncEnd` touch handlers. + */ + subscribe() { + this.channel.subscribe({ end: this._onEnd, asyncEnd: this._onAsyncEnd }) + } + + /** + * Unsubscribes the `end`/`asyncEnd` touch handlers. + */ + unsubscribe() { + this.channel.unsubscribe({ end: this._onEnd, asyncEnd: this._onAsyncEnd }) + } +} + +module.exports = TcBaseSubscriber diff --git a/test/integration/core/child-process.test.js b/test/integration/core/child-process.test.js index 1a2d8b3994..8f9767a277 100644 --- a/test/integration/core/child-process.test.js +++ b/test/integration/core/child-process.test.js @@ -9,6 +9,8 @@ const test = require('node:test') const assert = require('node:assert') const cp = require('child_process') const fs = require('fs') +const path = require('path') +const util = require('util') const helper = require('../../lib/agent_helper') const verifySegments = require('./verify.js') @@ -51,6 +53,50 @@ test('execFile', function (t, end) { }) }) +test('exec via util.promisify', async function (t) { + const { agent } = t.nr + const asyncExec = util.promisify(cp.exec) + + await helper.runInTransaction(agent, async function (tx) { + const { stdout } = await asyncExec('ls', { cwd: __dirname }) + const files = stdout.trim().split('\n').sort() + assert.deepEqual(files, fs.readdirSync(__dirname).sort()) + + const children = tx.trace.getChildren(tx.trace.root.id) + // TODO: should return a 'exec' segment with an internal 'execFile' child + assert.equal(children.length, 1, 'should have exactly one segment, from the internal execFile call') + assert.equal(children[0].name, 'child_process.execFile') + }) +}) + +test('execFile via util.promisify', async function (t) { + const { agent } = t.nr + const asyncExecFile = util.promisify(cp.execFile) + + await helper.runInTransaction(agent, async function (tx) { + const { stdout } = await asyncExecFile(path.join(__dirname, 'exec-me.js')) + assert.equal(stdout, 'I am stdout\n') + + const children = tx.trace.getChildren(tx.trace.root.id) + // TODO: should return a 'execFile' segment + assert.equal(children.length, 0, 'promisified execFile bypasses instrumentation entirely') + }) +}) + +test('exec segment duration should span the full process lifetime', function (t, end) { + const { agent } = t.nr + helper.runInTransaction(agent, function (tx) { + cp.exec('sleep 0.2', function (err) { + assert.ok(!err, 'should not error') + setTimeout(() => { + const [segment] = tx.trace.getChildren(tx.trace.root.id) + assert.ok(segment.getDurationInMillis() >= 150, `segment duration (${segment.getDurationInMillis()}ms) should reflect the sleep, not just the sync call`) + end() + }, 0) + }) + }) +}) + test('transaction context is preserved in subscribed events', function (t, end) { const { agent } = t.nr helper.runInTransaction(agent, function (transaction) { diff --git a/test/unit/subscribers/child_process/exec.test.js b/test/unit/subscribers/child_process/exec.test.js new file mode 100644 index 0000000000..8c8f39234e --- /dev/null +++ b/test/unit/subscribers/child_process/exec.test.js @@ -0,0 +1,34 @@ +/* + * Copyright 2026 New Relic Corporation. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +'use strict' + +const test = require('node:test') +const assert = require('node:assert') +const cp = require('child_process') +const helper = require('#testlib/agent_helper.js') +const logger = require('#agentlib/logger.js') +const ChildProcessExec = require('#agentlib/subscribers/child_process/exec.js') + +test('ChildProcessExec subscriber', async (t) => { + const agent = helper.loadMockedAgent() + t.after(() => { + helper.unloadAgent(agent) + }) + + await t.test('should only wrap cp.exec once, even across multiple enable() calls', () => { + const original = cp.exec + + const first = new ChildProcessExec({ agent, logger }) + first.enable() + const wrapped = cp.exec + assert.notEqual(wrapped, original, 'exec should be wrapped after enable()') + + const second = new ChildProcessExec({ agent, logger }) + second.enable() + + assert.equal(cp.exec, wrapped, 'exec should not be wrapped a second time') + }) +})