fix!: Replaced esm loader with import-in-the-middle to fix instrumentation firing for both commonjs and esm

.github/workflows/ci-workflow.yml

@@ -85,15 +85,6 @@ jobs:
       with:
         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
-      with:
-        name: esm-unit-tests-${{ matrix.node-version }}
-        path: ./coverage/esm-unit/lcov.info
-
   integration:
     needs: skip_if_release
     if: needs.skip_if_release.outputs.should_skip != 'true'
@@ -170,12 +161,6 @@ jobs:
           token: ${{ secrets.CODECOV_TOKEN }}
           directory: unit-tests-${{ matrix.node-version }}
           flags: unit-tests-${{ matrix.node-version }}
-      - name: Post ESM Unit Test Coverage
-        uses: codecov/codecov-action@v3
-        with:
-          token: ${{ secrets.CODECOV_TOKEN }}
-          directory: esm-unit-tests-${{ matrix.node-version }}
-          flags: esm-unit-tests-${{ matrix.node-version }}
       - name: Post Integration Test Coverage
         uses: codecov/codecov-action@v3
         with:

README.md

@@ -78,40 +78,28 @@ $ mv newrelic.js newrelic.cjs
  2. To use the newrelic ESM loader, start your program with node and use the `--experimental-loader` flag and a path to the loader file, like this:
 
 ```sh
-$ node --experimental-loader newrelic/esm-loader.mjs your-program.js
+$ node --experimental-loader newrelic/esm-loader.mjs -r newrelic your-program.js
 ```
 
 **Note**: Unlike the CommonJS methods listed above, there are no alternatives to running the agent without the `--experimental-loader` flag.
 
 ### Custom Instrumentation
 
-The agent supports adding your own custom instrumentation to ES module applications. In order to load custom instrumentation in an ES module app, you'll need to update your `newrelic.cjs` file to include the following:
+The agent supports adding your own custom instrumentation to ES module applications. You can use the instrumentation API methods. The only other difference between CommonJS custom instrumentation and ESM is you must provide a property of `isEsm: true`. 
 
 ```js
-    /* File: newrelic.cjs */
-    'use strict'
-    /**
-     * New Relic agent configuration.
-     *
-     * See lib/config/default.js in the agent distribution for a more complete
-     * description of configuration variables and their potential values.
-     */
-    exports.config = {
-      app_name: ['Your application or service name'],
-      license_key: 'your new relic license key',
-      api: {
-        esm: {
-            custom_instrumentation_entrypoint: '/path/to/my/instrumentation.js'
-        }
+import newrelic from 'newrelic'
+newrelic.instrument({ moduleName: 'parse-json', isEsm: true }, function wrap(shim, parseJson, moduleName) {
+  shim.wrap(parseJson.default, function wrapParseJson(shim, orig) {
+      return function wrappedParseJson() {
+          const result = orig.apply(this, arguments)
+          result.instrumented = true
+          return true
       }
-      /* ... rest of configuration .. */
-    }
+  })
+})
 ```
 
-If you do not use a configuration file, then use the environment variable `NEW_RELIC_API_ESM_CUSTOM_INSTRUMENTATION_ENTRYPOINT` instead.
-
-By updating the configuration, the agent's ES module loader will ensure that your custom instrumentation is added at module load. This is required in ES module applications due to the immutability of module export bindings: we are unable to apply our instrumentation after loading is complete.
-
 We support the following custom instrumentation API methods in ES module apps:
 
 * `newrelic.instrument`

THIRD_PARTY_NOTICES.md

@@ -24,6 +24,7 @@ code, the source code can be found at [https://github.com/newrelic/node-newrelic
 * [@tyriar/fibonacci-heap](#tyriarfibonacci-heap)
 * [concat-stream](#concat-stream)
 * [https-proxy-agent](#https-proxy-agent)
+* [import-in-the-middle](#import-in-the-middle)
 * [json-bigint](#json-bigint)
 * [json-stringify-safe](#json-stringify-safe)
 * [readable-stream](#readable-stream)
@@ -74,7 +75,6 @@ code, the source code can be found at [https://github.com/newrelic/node-newrelic
 * [sinon](#sinon)
 * [tap](#tap)
 * [temp](#temp)
-* [testdouble](#testdouble)
 * [when](#when)
 
 **[optionalDependencies](#optionalDependencies)**
@@ -1288,6 +1288,27 @@ The above copyright notice and this permission notice shall be included in all c
 THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 ```
 
+### import-in-the-middle
+
+This product includes source derived from [import-in-the-middle](https://github.com/DataDog/import-in-the-middle) ([v1.4.2](https://github.com/DataDog/import-in-the-middle/tree/v1.4.2)), distributed under the [Apache-2.0 License](https://github.com/DataDog/import-in-the-middle/blob/v1.4.2/LICENSE):
+
+```
+Copyright 2021 Datadog, Inc.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+
+```
+
 ### json-bigint
 
 This product includes source derived from [json-bigint](https://github.com/sidorares/json-bigint) ([v1.0.0](https://github.com/sidorares/json-bigint/tree/v1.0.0)), distributed under the [MIT License](https://github.com/sidorares/json-bigint/blob/v1.0.0/LICENSE):
@@ -9496,35 +9517,6 @@ CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 ```
 
-### testdouble
-
-This product includes source derived from [testdouble](https://github.com/testdouble/testdouble.js) ([v3.17.2](https://github.com/testdouble/testdouble.js/tree/v3.17.2)), distributed under the [MIT License](https://github.com/testdouble/testdouble.js/blob/v3.17.2/LICENSE.txt):
-
-```
-The MIT License (MIT)
-
-Copyright (c) 2015 Test Double, LLC.
-
-Permission is hereby granted, free of charge, to any person obtaining a copy
-of this software and associated documentation files (the "Software"), to deal
-in the Software without restriction, including without limitation the rights
-to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
-copies of the Software, and to permit persons to whom the Software is
-furnished to do so, subject to the following conditions:
-
-The above copyright notice and this permission notice shall be included in all
-copies or substantial portions of the Software.
-
-THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
-IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
-FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
-AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
-LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
-OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
-SOFTWARE.
-
-```
-
 ### when
 
 This product includes source derived from [when](https://github.com/cujojs/when) ([v3.7.8](https://github.com/cujojs/when/tree/v3.7.8)), distributed under the [MIT License](https://github.com/cujojs/when/blob/v3.7.8/LICENSE.txt):

bin/run-versioned-tests.sh

@@ -61,6 +61,7 @@ if [ ! -z "$JOBS" ];
 then
   JOBS_ARGS="--jobs $JOBS"
 fi
+export NR_LOADER=./esm-loader.mjs
 
 if [[ "${NPM7}" = 1 ]];
 then

esm-loader.mjs

@@ -3,228 +3,4 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-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')
-// This check will prevent resolve hooks executing from within this file
-// If I do `import('foo')` in here it'll hit the resolve hook multiple times
-const isFromEsmLoader = (context) =>
-  context && context.parentURL && context.parentURL.includes('newrelic/esm-loader.mjs')
-
-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)
-}
-
-addESMSupportabilityMetrics(newrelic.agent)
-
-// exporting for testing purposes
-export const registeredSpecifiers = new Map()
-
-/**
- * Hook chain responsible for resolving a file URL for a given module specifier
- *
- * Our loader has to be the last user-supplied loader if chaining is happening,
- * as we rely on `nextResolve` being the default Node.js resolve hook to get our URL
- *
- * Docs: https://nodejs.org/api/esm.html#resolvespecifier-context-nextresolve
- *
- * @param {string} specifier string identifier in an import statement or import() expression
- * @param {object} context metadata about the specifier, including url of the parent module and any import assertions
- *        Optional argument that only needs to be passed when changed
- * @param {Function} nextResolve The subsequent resolve hook in the chain, or the Node.js default resolve hook after the last user-supplied resolve hook
- * @returns {Promise} Promise object representing the resolution of a given specifier
- */
-export async function resolve(specifier, context, nextResolve) {
-  if (!newrelic.agent || !isSupportedVersion() || isFromEsmLoader(context)) {
-    return nextResolve(specifier, context, nextResolve)
-  }
-
-  /**
-   * We manually call the default Node.js resolve hook so
-   * that we can get the fully qualified URL path and the
-   * package type (commonjs/module/builtin) without
-   * 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]
-
-  if (instrumentationDefinition) {
-    const { url, format } = resolvedModule
-    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(
-          `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') {
-      addNrInstrumentation(resolvedModule)
-      registeredSpecifiers.set(url, { specifier, hasNrInstrumentation: true })
-    } else {
-      logger.debug(`${specifier} is not a CommonJS nor ESM package, skipping for now.`)
-    }
-  }
-
-  return resolvedModule
-}
-
-/**
- * This is purely done so that we can import the incoming specifier
- * in the load hook.  It used to be used to determine if instrumentation existed
- * for specifier but we found a RCE with that solution.
- *
- * @param {object} resolvedModule the result of call resolve on a specifier
- */
-function addNrInstrumentation(resolvedModule) {
-  const modifiedUrl = new URL(resolvedModule.url)
-  modifiedUrl.searchParams.set('hasNrInstrumentation', 'true')
-  resolvedModule.url = modifiedUrl.href
-}
-
-/**
- * Extracts the href without query params
- *
- * @param {string} url url of specifier
- * @returns {string} new url without hasNrInstrumentation query param
- */
-function removeNrInstrumentation(url) {
-  let parsedUrl
-
-  try {
-    parsedUrl = new URL(url)
-    url = parsedUrl.href.split('?')[0]
-  } catch (err) {
-    logger.error('Unable to parse url: %s, msg: %s', url, err.message)
-  }
-
-  return url
-}
-
-/**
- * Hook chain responsible for determining how a URL should be interpreted, retrieved, and parsed.
- *
- * Our loader has to be the last user-supplied loader if chaining is happening,
- * as we rely on `nextLoad` being the default Node.js resolve hook to load the ESM.
- *
- * Docs: https://nodejs.org/dist/latest-v18.x/docs/api/esm.html#loadurl-context-nextload
- *
- * @param {string} url the URL returned by the resolve chain
- * @param {object} context metadata about the url, including conditions, format and import assertions
- * @param {Function} nextLoad the subsequent load hook in the chain, or the Node.js default load hook after the last user-supplied load hook
- * @returns {Promise} Promise object representing the load of a given url
- */
-export async function load(url, context, nextLoad) {
-  if (!newrelic.agent || !isSupportedVersion()) {
-    return nextLoad(url, context, nextLoad)
-  }
-
-  url = removeNrInstrumentation(url)
-
-  const pkg = registeredSpecifiers.get(url)
-
-  if (!pkg?.hasNrInstrumentation) {
-    return nextLoad(url, context, nextLoad)
-  }
-
-  const { specifier } = pkg
-
-  const rewrittenSource = await wrapEsmSource(url, specifier)
-  logger.debug(`Registered module instrumentation for ${specifier}.`)
-
-  return {
-    format: 'module',
-    source: rewrittenSource,
-    shortCircuit: true
-  }
-}
-
-/**
- * Helper function for determining which of our Supportability metrics to use for the current loader invocation
- *
- * @param {object} agent
- *        instantiation of the New Relic agent
- * @returns {void}
- */
-function addESMSupportabilityMetrics(agent) {
-  if (!agent) {
-    return
-  }
-
-  if (isSupportedVersion()) {
-    agent.metrics.getOrCreateMetric(NAMES.FEATURES.ESM.LOADER).incrementCallCount()
-  } else {
-    logger.warn(
-      'New Relic for Node.js ESM loader requires a version of Node >= v16.12.0; your version is %s.  Instrumentation will not be registered.',
-      process.version
-    )
-    agent.metrics.getOrCreateMetric(NAMES.FEATURES.ESM.UNSUPPORTED_LOADER).incrementCallCount()
-  }
-
-  if (customEntryPoint) {
-    agent.metrics.getOrCreateMetric(NAMES.FEATURES.ESM.CUSTOM_INSTRUMENTATION).incrementCallCount()
-  }
-}
-
-/**
- * Rewrites the source code of a ES module we want to instrument.
- * This is done by injecting the ESM shim which proxies every property on the exported
- * module and registers the module with shimmer so instrumentation can be registered properly.
- *
- * Note: this autogenerated code _requires_ that the import have the file:// prefix!
- * Without it, Node.js throws an ERR_INVALID_URL error: you've been warned.
- *
- * @param {string} url the URL returned by the resolve chain
- * @param {string} specifier string identifier in an import statement or import() expression
- * @returns {string} source code rewritten to wrap with our esm-shim
- */
-async function wrapEsmSource(url, specifier) {
-  const pkg = await import(url)
-  const props = Object.keys(pkg)
-  const trimmedUrl = fileURLToPath(url)
-
-  return `
-    import wrapModule from '${esmShimPath.href}'
-    import * as _originalModule from '${url}'
-    const _wrappedModule = wrapModule(_originalModule, '${specifier}', '${trimmedUrl}')
-    ${props
-      .map((propName) => {
-        return `
-    let _${propName} = _wrappedModule.${propName}
-    export { _${propName} as ${propName} }`
-      })
-      .join('\n')}
-  `
-}
+export * from 'import-in-the-middle/hook.mjs'