Skip to content

Commit

Permalink
fix: Updated instrumentation registration to allow for instrumenting …
Browse files Browse the repository at this point in the history
…of a local file that does not exist within node_modules. You must pass in `absolutePath` with the absolute path to the file that is being instrumented along with the moduleName which in this case is just the file name without the extension (#1974)
  • Loading branch information
bizob2828 committed Jan 25, 2024
1 parent f800281 commit f545b4e
Show file tree
Hide file tree
Showing 5 changed files with 117 additions and 7 deletions.
10 changes: 5 additions & 5 deletions api.js
Expand Up @@ -1312,7 +1312,7 @@ API.prototype.recordCustomEvent = function recordCustomEvent(eventType, attribut
*
* @param {string|object} moduleName The module name given to require to load the module, or the instrumentation specification
* @param {string} moduleName.moduleName The module name given to require to load the module
* @param {Function} moduleName.onResolved The function to call prior to module load after the filepath has been resolved
* @param {string} [moduleName.absolutePath] Must provide absolute path to module if it does not exist within node_modules. This is used to instrument a file within the same application.
* @param {Function} moduleName.onRequire The function to call when the module has been loaded
* @param {Function} [moduleName.onError] If provided, should `onRequire` throw an error, the error will be passed to
* @param {Function} onRequire The function to call when the module has been loaded
Expand Down Expand Up @@ -1343,7 +1343,7 @@ API.prototype.instrument = function instrument(moduleName, onRequire, onError) {
*
* @param {string|object} moduleName The module name given to require to load the module, or the instrumentation specification
* @param {string} moduleName.moduleName The module name given to require to load the module
* @param {Function} moduleName.onResolved The function to call prior to module load after the filepath has been resolved
* @param {string} [moduleName.absolutePath] Must provide absolute path to module if it does not exist within node_modules. This is used to instrument a file within the same application.
* @param {Function} moduleName.onRequire The function to call when the module has been loaded
* @param {Function} [moduleName.onError] If provided, should `onRequire` throw an error, the error will be passed to
* @param {Function} onRequire The function to call when the module has been loaded
Expand Down Expand Up @@ -1375,7 +1375,7 @@ API.prototype.instrumentConglomerate = function instrumentConglomerate(
*
* @param {string|object} moduleName The module name given to require to load the module, or the instrumentation specification
* @param {string} moduleName.moduleName The module name given to require to load the module
* @param {Function} moduleName.onResolved The function to call prior to module load after the filepath has been resolved
* @param {string} [moduleName.absolutePath] Must provide absolute path to module if it does not exist within node_modules. This is used to instrument a file within the same application.
* @param {Function} moduleName.onRequire The function to call when the module has been loaded
* @param {Function} [moduleName.onError] If provided, should `onRequire` throw an error, the error will be passed to
* @param {Function} onRequire The function to call when the module has been loaded
Expand Down Expand Up @@ -1408,7 +1408,7 @@ API.prototype.instrumentDatastore = function instrumentDatastore(moduleName, onR
*
* @param {string|object} moduleName The module name given to require to load the module, or the instrumentation specification
* @param {string} moduleName.moduleName The module name given to require to load the module
* @param {Function} moduleName.onResolved The function to call prior to module load after the filepath has been resolved
* @param {string} [moduleName.absolutePath] Must provide absolute path to module if it does not exist within node_modules. This is used to instrument a file within the same application.
* @param {Function} moduleName.onRequire The function to call when the module has been loaded
* @param {Function} [moduleName.onError] If provided, should `onRequire` throw an error, the error will be passed to
* @param {Function} onRequire The function to call when the module has been loaded
Expand Down Expand Up @@ -1445,7 +1445,7 @@ API.prototype.instrumentWebframework = function instrumentWebframework(
*
* @param {string|object} moduleName The module name given to require to load the module, or the instrumentation specification
* @param {string} moduleName.moduleName The module name given to require to load the module
* @param {Function} moduleName.onResolved The function to call prior to module load after the filepath has been resolved
* @param {string} [moduleName.absolutePath] Must provide absolute path to module if it does not exist within node_modules. This is used to instrument a file within the same application.
* @param {Function} moduleName.onRequire The function to call when the module has been loaded
* @param {Function} [moduleName.onError] If provided, should `onRequire` throw an error, the error will be passed to
* @param {Function} onRequire The function to call when the module has been loaded
Expand Down
10 changes: 8 additions & 2 deletions lib/shimmer.js
Expand Up @@ -398,11 +398,17 @@ const shimmer = (module.exports = {

if (!registeredInstrumentation) {
shimmer.registeredInstrumentations[opts.moduleName] = []
// In cases where a customer is trying to instrument a file
// that is not within node_modules, they must provide the absolutePath
// so require-in-the-middle can call our callback. the moduleName
// still needs to be the resolved name so we can look up our instrumentation correctly
const pkgHook = opts.absolutePath || opts.moduleName

// not using a set because this is shared by reference
// to allow custom instrumentation to be loaded after the
// agent is bootstrapped
if (!pkgsToHook.includes(opts.moduleName)) {
pkgsToHook.push(opts.moduleName)
if (!pkgsToHook.includes(pkgHook)) {
pkgsToHook.push(pkgHook)
}
}

Expand Down
8 changes: 8 additions & 0 deletions test/integration/module-loading/local-package.js
@@ -0,0 +1,8 @@
/*
* Copyright 2020 New Relic Corporation. All rights reserved.
* SPDX-License-Identifier: Apache-2.0
*/

'use strict'

module.exports = () => ({ hello: 'world' })
32 changes: 32 additions & 0 deletions test/integration/module-loading/module-loading.tap.js
Expand Up @@ -13,6 +13,8 @@ const shimmer = require('../../../lib/shimmer')
const symbols = require('../../../lib/symbols')
const { FEATURES } = require('../../../lib/metrics/names')

const LOCAL_MODULE = 'local-package'
const LOCAL_MODULE_PATH = require.resolve('./local-package')
const CUSTOM_MODULE = 'customTestPackage'
const CUSTOM_MODULE_PATH = `./node_modules/${CUSTOM_MODULE}`
const CUSTOM_MODULE_PATH_SUB = `./node_modules/subPkg/node_modules/${CUSTOM_MODULE}`
Expand Down Expand Up @@ -177,6 +179,36 @@ tap.test('Should create usage version metric onRequire', (t) => {
}
})

tap.test('should instrument a local package', (t) => {
let agent = helper.instrumentMockedAgent()

t.teardown(() => {
helper.unloadAgent(agent)
agent = null
})

shimmer.registerInstrumentation({
moduleName: LOCAL_MODULE,
absolutePath: LOCAL_MODULE_PATH,
onRequire: onRequireHandler
})

require('./local-package')

function onRequireHandler(shim, localPkg, name) {
t.equal(
shim.pkgVersion,
process.version,
'defaults to node version for pkgVersion as this is not a package'
)
t.ok(shim.id)
t.equal(name, LOCAL_MODULE)
const result = localPkg()
t.same(result, { hello: 'world' })
t.end()
}
})

function simulateTestLoadAndUnload() {
const agent = helper.instrumentMockedAgent()

Expand Down
64 changes: 64 additions & 0 deletions test/unit/api/api-instrument-messages.test.js
@@ -0,0 +1,64 @@
/*
* Copyright 2020 New Relic Corporation. All rights reserved.
* SPDX-License-Identifier: Apache-2.0
*/

'use strict'

const tap = require('tap')
const API = require('../../../api')
const helper = require('../../lib/agent_helper')
const sinon = require('sinon')
const shimmer = require('../../../lib/shimmer')

tap.test('Agent API - instrumentMessages', (t) => {
t.autoend()

let agent = null
let api = null

t.beforeEach(() => {
agent = helper.loadMockedAgent()
api = new API(agent)

sinon.spy(shimmer, 'registerInstrumentation')
})

t.afterEach(() => {
helper.unloadAgent(agent)
agent = null

shimmer.registerInstrumentation.restore()
})

t.test('should register the instrumentation with shimmer', (t) => {
const opts = {
moduleName: 'foobar',
absolutePath: `${__dirname}/foobar`,
onRequire: function () {}
}
api.instrumentMessages(opts)

t.ok(shimmer.registerInstrumentation.calledOnce)
const args = shimmer.registerInstrumentation.getCall(0).args
const [actualOpts] = args

t.same(actualOpts, opts)
t.equal(actualOpts.type, 'message')

t.end()
})

t.test('should convert separate args into an options object', (t) => {
function onRequire() {}
function onError() {}
api.instrumentMessages('foobar', onRequire, onError)

const opts = shimmer.registerInstrumentation.getCall(0).args[0]
t.equal(opts.moduleName, 'foobar')
t.equal(opts.onRequire, onRequire)
t.equal(opts.onError, onError)

t.end()
})
})

0 comments on commit f545b4e

Please sign in to comment.