diff --git a/.github/workflows/ci-workflow.yml b/.github/workflows/ci-workflow.yml index 12eed48c50..879633918c 100644 --- a/.github/workflows/ci-workflow.yml +++ b/.github/workflows/ci-workflow.yml @@ -86,6 +86,7 @@ jobs: name: unit-tests-${{ matrix.node-version }} path: ./coverage/unit/lcov.info - name: Run ESM Unit Tests + if: matrix.node-version != '20.x' run: npm run unit:esm - name: Archive ESM Unit Test Coverage uses: actions/upload-artifact@v3 diff --git a/esm-loader.mjs b/esm-loader.mjs index dd84ef9fa7..796e3fa955 100644 --- a/esm-loader.mjs +++ b/esm-loader.mjs @@ -3,12 +3,11 @@ * SPDX-License-Identifier: Apache-2.0 */ -import newrelic from './index.js' -import shimmer from './lib/shimmer.js' -import loggingModule from './lib/logger.js' +//import newrelic from './index.js' +//import shimmer from './lib/shimmer.js' +//import loggingModule from './lib/logger.js' import NAMES from './lib/metrics/names.js' import semver from 'semver' -import path from 'node:path' import { fileURLToPath } from 'node:url' const isSupportedVersion = () => semver.gte(process.version, 'v16.12.0') @@ -17,22 +16,41 @@ const isSupportedVersion = () => semver.gte(process.version, 'v16.12.0') const isFromEsmLoader = (context) => context && context.parentURL && context.parentURL.includes('newrelic/esm-loader.mjs') -const logger = loggingModule.child({ component: 'esm-loader' }) +//const logger = loggingModule.child({ component: 'esm-loader' }) const esmShimPath = new URL('./lib/esm-shim.mjs', import.meta.url) -const customEntryPoint = newrelic?.agent?.config?.api.esm.custom_instrumentation_entrypoint - -// Hook point within agent for customers to register their custom instrumentation. -if (customEntryPoint) { - const resolvedEntryPoint = path.resolve(customEntryPoint) - logger.debug('Registering custom ESM instrumentation at %s', resolvedEntryPoint) - await import(resolvedEntryPoint) -} +// TODO: do this in globalPreload +/* addESMSupportabilityMetrics(newrelic.agent) +*/ // exporting for testing purposes export const registeredSpecifiers = new Map() +export function globalPreload() { + return ` + const { createRequire } = getBuiltin('module') + const path = getBuiltin('path') + const { cwd } = getBuiltin('process') + const require = createRequire(cwd()) + // load agent in main thread + const newrelic = require(cwd() + '/index.js') + debugger + const logger = require(cwd() + '/lib/logger.js') + // Have to do this in function as top level await does not work + /* + async function loadCustomInstrumentation() { + const customEntryPoint = newrelic?.agent?.config?.api.esm.custom_instrumentation_entrypoint + // Hook point within agent for customers to register their custom instrumentation. + if (customEntryPoint) { + const resolvedEntryPoint = path.resolve(customEntryPoint) + logger.debug('Registering custom ESM instrumentation at %s', resolvedEntryPoint) + await import(resolvedEntryPoint) + } + }*/ + ` +} + /** * Hook chain responsible for resolving a file URL for a given module specifier * @@ -48,7 +66,7 @@ export const registeredSpecifiers = new Map() * @returns {Promise} Promise object representing the resolution of a given specifier */ export async function resolve(specifier, context, nextResolve) { - if (!newrelic.agent || !isSupportedVersion() || isFromEsmLoader(context)) { + if (!isSupportedVersion() || isFromEsmLoader(context)) { return nextResolve(specifier, context, nextResolve) } @@ -59,27 +77,37 @@ export async function resolve(specifier, context, nextResolve) { * duplicating the logic of the Node.js hook */ const resolvedModule = await nextResolve(specifier, context, nextResolve) - const instrumentationName = shimmer.getInstrumentationNameFromModuleName(specifier) - const instrumentationDefinition = shimmer.registeredInstrumentations[instrumentationName] - + const { pkgs, registerInstrumentation } = await import('./lib/loaded-instrumentation.js') + debugger + const instrumentationDefinition = pkgs[specifier] if (instrumentationDefinition) { const { url, format } = resolvedModule - logger.debug(`Instrumentation exists for ${specifier} ${format} package.`) - - if (format === 'commonjs') { + //logger.debug(`Instrumentation exists for ${specifier} ${format} package.`) + + if (registeredSpecifiers.get(url)) { + /*logger.debug( + `Instrumentation already registered for ${specifier} under ${fileURLToPath( + url + )}, skipping resolve hook...` + )*/ + } else if (format === 'commonjs') { // ES Modules translate import statements into fully qualified filepaths, so we create a copy of our instrumentation under this filepath const instrumentationDefinitionCopy = [...instrumentationDefinition] + instrumentationDefinitionCopy.forEach((copy) => { // Stripping the prefix is necessary because the code downstream gets this url without it copy.moduleName = fileURLToPath(url) // Added to keep our Supportability metrics from exploding/including customer info via full filepath copy.specifier = specifier - shimmer.registerInstrumentation(copy) - logger.debug( + registerInstrumentation(copy) + /*logger.debug( `Registered CommonJS instrumentation for ${specifier} under ${copy.moduleName}` - ) + )*/ }) + + // Keep track of what we've registered so we don't double register (see: https://github.com/newrelic/node-newrelic/issues/1646) + registeredSpecifiers.set(url, specifier) } else if (format === 'module') { registeredSpecifiers.set(url, specifier) const modifiedUrl = new URL(url) @@ -109,7 +137,7 @@ export async function resolve(specifier, context, nextResolve) { * @returns {Promise} Promise object representing the load of a given url */ export async function load(url, context, nextLoad) { - if (!newrelic.agent || !isSupportedVersion()) { + if (!isSupportedVersion()) { return nextLoad(url, context, nextLoad) } diff --git a/esm-preload.js b/esm-preload.js new file mode 100644 index 0000000000..9ed77fddb1 --- /dev/null +++ b/esm-preload.js @@ -0,0 +1,4 @@ +function preload() { + require('./index') +} +module.exports = preload() diff --git a/lib/loaded-instrumentation.js b/lib/loaded-instrumentation.js new file mode 100644 index 0000000000..751d1e1579 --- /dev/null +++ b/lib/loaded-instrumentation.js @@ -0,0 +1,44 @@ +const pkgs = Object.create(null) +function registerInstrumentation(opts) { + if (!hasValidRegisterOptions(opts)) { + return + } + + const registeredInstrumentation = pkgs[opts.moduleName] + + if (!registeredInstrumentation) { + pkgs[opts.moduleName] = [] + } + + pkgs[opts.moduleName].push({ ...opts }) +} + +function hasValidRegisterOptions(opts) { + if (!opts) { + //logger.warn('Instrumentation registration failed, no options provided') + return false + } + + if (!opts.moduleName) { + //logger.warn(`Instrumentation registration failed, 'moduleName' not provided`) + return false + } + + if (!opts.onRequire && !opts.onResolved) { + /*logger.warn( + 'Instrumentation registration for %s failed, no require hooks provided.', + opts.moduleName + ) + */ + + return false + } + + return true +} + + +module.exports = { + pkgs, + registerInstrumentation +} diff --git a/lib/shimmer.js b/lib/shimmer.js index af876594bb..94830a2a9b 100644 --- a/lib/shimmer.js +++ b/lib/shimmer.js @@ -17,6 +17,7 @@ const NAMES = require('./metrics/names') const symbols = require('./symbols') const MODULE_TYPE = shims.constants.MODULE_TYPE +const loadedInstrumentations = require('./loaded-instrumentation') const CORE_INSTRUMENTATION = { child_process: { @@ -430,21 +431,8 @@ const shimmer = (module.exports = { shimmer.registerThirdPartyInstrumentation(agent) }, - registerInstrumentation: function registerInstrumentation(opts) { - if (!hasValidRegisterOptions(opts)) { - return - } - - const registeredInstrumentation = shimmer.registeredInstrumentations[opts.moduleName] - - if (!registeredInstrumentation) { - shimmer.registeredInstrumentations[opts.moduleName] = [] - } - - shimmer.registeredInstrumentations[opts.moduleName].push({ ...opts }) - }, - - registeredInstrumentations: Object.create(null), + registerInstrumentation: loadedInstrumentations.registerInstrumentation, + registeredInstrumentations: loadedInstrumentations.pkgs, /** * NOT FOR USE IN PRODUCTION CODE @@ -702,29 +690,6 @@ function _instrumentOnResolved(agent, moduleName, resolvedFilepath) { }) } -function hasValidRegisterOptions(opts) { - if (!opts) { - logger.warn('Instrumentation registration failed, no options provided') - return false - } - - if (!opts.moduleName) { - logger.warn(`Instrumentation registration failed, 'moduleName' not provided`) - return false - } - - if (!opts.onRequire && !opts.onResolved) { - logger.warn( - 'Instrumentation registration for %s failed, no require hooks provided.', - opts.moduleName - ) - - return false - } - - return true -} - /** * Adds metrics to indicate instrumentation was used for a particular module and * what major version the module was at, if possible. diff --git a/test/integration/logger-test-case/hello.js b/test/integration/logger-test-case/hello.js index 02f7bf0ef9..7b285c1507 100644 --- a/test/integration/logger-test-case/hello.js +++ b/test/integration/logger-test-case/hello.js @@ -5,7 +5,9 @@ 'use strict' +debugger const newrelic = require('../../../index.js') +const express = require('express') function greeter(name) { return `Hello ${name}` diff --git a/test/integration/logger-test-case/package.json b/test/integration/logger-test-case/package.json new file mode 100644 index 0000000000..f4524b250d --- /dev/null +++ b/test/integration/logger-test-case/package.json @@ -0,0 +1,14 @@ +{ + "name": "logger-test-case", + "version": "1.0.0", + "description": "", + "main": "hello.js", + "scripts": { + "test": "echo \"Error: no test specified\" && exit 1" + }, + "author": "", + "license": "ISC", + "dependencies": { + "express": "^4.18.2" + } +}