wip: tried to load agent in globalPreload and split out shimmer regis…

…trations

.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

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)
  }
 

esm-preload.js

@@ -0,0 +1,4 @@
+function preload() {
+ require('./index')
+}
+module.exports = preload()

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
+}

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.

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}`

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"
+ }
+}