chore: Refactored module instrumentation tracking to use a tracker class

.github/workflows/ci-workflow.yml

@@ -35,7 +35,7 @@ jobs:
       with:
         node-version: ${{ matrix.node-version }}
     - name: Install Dependencies
-      run: npm ci
+      run: npm ci --verbose
     - name: Run Linting
       run: npm run lint
     - name: Inspect Lockfile
@@ -58,7 +58,7 @@ jobs:
       with:
         node-version: ${{ matrix.node-version }}
     - name: Install Dependencies
-      run: npm ci
+      run: npm ci --verbose
     - name: Run CI Script Unit Tests
       run: npm run unit:scripts
 
@@ -79,7 +79,7 @@ jobs:
       with:
         node-version: ${{ matrix.node-version }}
     - name: Install Dependencies
-      run: npm ci
+      run: npm ci --verbose
     - name: Run Unit Tests
       run: npm run unit
     - name: Archive Unit Test Coverage
@@ -104,7 +104,7 @@ jobs:
       with:
         node-version: ${{ matrix.node-version }}
     - name: Install Dependencies
-      run: npm ci
+      run: npm ci --verbose
     - name: Run Docker Services
       run: npm run services
     - name: Run Integration Tests
@@ -132,7 +132,7 @@ jobs:
       with:
         node-version: ${{ matrix.node-version }}
     - name: Install Dependencies
-      run: npm ci
+      run: npm ci --verbose
     - name: Run Docker Services
       run: npm run services
     - name: Run Versioned Tests
@@ -161,7 +161,7 @@ jobs:
       with:
         name: logs-${{ matrix.node-version }}.tgz
         path: ./logs-${{ matrix.node-version }}.tgz
-  
+
   # There is no coverage for external as that's tracked in their respective repos
   versioned-external:
     needs: skip_if_release
@@ -180,7 +180,7 @@ jobs:
       with:
         node-version: ${{ matrix.node-version }}
     - name: Install Dependencies
-      run: npm ci
+      run: npm ci --verbose
     - name: Run Versioned Tests
       run: TEST_CHILD_TIMEOUT=600000 npm run versioned:external
       env:

jsdoc-conf.jsonc

@@ -0,0 +1,38 @@
+{
+  "opts": {
+    "destination": "out/",
+    "readme": "./README.md",
+    "template": "./node_modules/clean-jsdoc-theme",
+    "theme_opts": {
+      "search": false,
+      "shouldRemoveScrollbarStyle": true
+    },
+    "tutorials": "examples/shim",
+    "recurse": true
+  },
+  "source": {
+    "exclude": ["node_modules", "test"],
+    "includePattern": ".+\\.js(doc)?$",
+    // Note: we cannot add patterns to the "include" strings. They must be
+    // explicit strings that point to either specific files or directories.
+    // Recursion is enabled for the `jsdoc` command, thus any directories
+    // specified here will be processed recursively. So make sure any files in
+    // these directories should be included in our public docs.
+    "include": [
+      "lib/shim/",
+      "lib/transaction/handle.js",
+      "lib/instrumentation-descriptor.js"
+    ]
+  },
+  "plugins": [
+    "plugins/markdown"
+  ],
+  "templates": {
+    "cleverLinks": true,
+    "showInheritedInNav": false
+  },
+  "markdown": {
+    "hardwrap": false,
+    "idInHeadings": true
+  }
+}

lib/instrumentation-descriptor.js

@@ -30,6 +30,7 @@ const idGen = new IdGen()
  * @property {string} absolutePath
  * @property {string} module
  * @property {string} moduleName
+ * @property {string} shimName
  * @property {InstrumentationOnError} onError
  * @property {InstrumentationOnRequire} onRequire
  * @property {string} resolvedName
@@ -117,6 +118,14 @@ class InstrumentationDescriptor {
    */
   moduleName
 
+  /**
+   * Used when instrumenting a module to determine if a module has already
+   * been wrapped by a specific shim instance. It is used in conjunction with
+   * the `shim.id` value.
+   * @type {string}
+   */
+  shimName
+
   /**
    * The absolute path to the module to instrument. This should only be set
    * when the module being instrumented does not reside in a `node_modules`
@@ -163,6 +172,7 @@ class InstrumentationDescriptor {
     this.absolutePath = params.absolutePath
     this.module = params.module
     this.moduleName = params.moduleName
+    this.shimName = params.shimName
     this.onError = params.onError
     this.onRequire = params.onRequire
     this.resolvedName = params.resolvedName

lib/shimmer.js

@@ -14,6 +14,8 @@ const shims = require('./shim')
 const { Hook } = require('@newrelic/ritm')
 const IitmHook = require('import-in-the-middle')
 const { nrEsmProxy } = require('./symbols')
+const InstrumentationDescriptor = require('./instrumentation-descriptor')
+const InstrumentationTracker = require('./instrumentation-tracker')
 let pkgsToHook = []
 
 const NAMES = require('./metrics/names')
@@ -361,6 +363,7 @@ const shimmer = (module.exports = {
       _firstPartyInstrumentation(agent, filePath, shim, uninstrumented, mojule)
     }
   },
+
   registerHooks(agent) {
     this._ritm = new Hook(pkgsToHook, function onHook(exports, name, basedir) {
       return _postLoad(agent, exports, name, basedir)
@@ -369,6 +372,7 @@ const shimmer = (module.exports = {
       return _postLoad(agent, exports, name, basedir, true)
     })
   },
+
   removeHooks() {
     if (this._ritm) {
       this._ritm.unhook()
@@ -394,10 +398,11 @@ const shimmer = (module.exports = {
       return
     }
 
-    const registeredInstrumentation = shimmer.registeredInstrumentations[opts.moduleName]
+    const registeredInstrumentation = shimmer.registeredInstrumentations.getAllByName(
+      opts.moduleName
+    )
 
     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
@@ -412,12 +417,13 @@ const shimmer = (module.exports = {
       }
     }
 
-    opts[symbols.instrumented] = new Set()
-    opts[symbols.instrumentedErrored] = new Set()
-    shimmer.registeredInstrumentations[opts.moduleName].push({ ...opts })
+    shimmer.registeredInstrumentations.track(
+      opts.moduleName,
+      new InstrumentationDescriptor({ ...opts })
+    )
   },
 
-  registeredInstrumentations: Object.create(null),
+  registeredInstrumentations: new InstrumentationTracker(),
 
   /**
    * NOT FOR USE IN PRODUCTION CODE
@@ -467,23 +473,26 @@ const shimmer = (module.exports = {
    * only if every hook succeeded.
    *
    * @param {string} moduleName name of registered instrumentation
-   * @returns {boolean} if all instrumentation hooks ran for a given version
+   * @param {object} resolvedName the fully resolve path to the module
+   * @returns {boolean} if all instrumentation hooks successfully ran for a
+   * module
    */
-  isInstrumented(moduleName) {
-    const pkgVersion = shimmer.getPackageVersion(moduleName)
-    const instrumentations = shimmer.registeredInstrumentations[moduleName]
-    const didInstrument = instrumentations.filter((instrumentation) =>
-      instrumentation[symbols.instrumented].has(pkgVersion)
-    )
-    return didInstrument.length === instrumentations.length
+  isInstrumented(moduleName, resolvedName) {
+    const items = shimmer.registeredInstrumentations
+      .getAllByName(moduleName)
+      .filter(
+        (item) =>
+          item.instrumentation.resolvedName === resolvedName && item.meta.instrumented === true
+      )
+    return items.length > 0
   },
 
   instrumentPostLoad(agent, module, moduleName, resolvedName, returnModule = false) {
     const result = _postLoad(agent, module, moduleName, resolvedName)
     // This is to not break the public API
     // previously it would just call instrumentation
     // and not check the result
-    return returnModule ? result : shimmer.isInstrumented(moduleName)
+    return returnModule ? result : shimmer.isInstrumented(moduleName, resolvedName)
   },
 
   /**
@@ -494,8 +503,15 @@ const shimmer = (module.exports = {
    */
   getPackageVersion(moduleName) {
     try {
-      const { basedir } = shimmer.registeredInstrumentations[moduleName]
-      const pkg = require(path.resolve(basedir, 'package.json'))
+      const trackedItems = shimmer.registeredInstrumentations.getAllByName(moduleName)
+      if (trackedItems === undefined) {
+        throw Error(`no tracked items for module '${moduleName}'`)
+      }
+      const item = trackedItems.find((item) => item.instrumentation.resolvedName !== undefined)
+      if (item === undefined) {
+        return process.version
+      }
+      const pkg = require(path.resolve(item.instrumentation.resolvedName, 'package.json'))
       return pkg.version
     } catch (err) {
       logger.debug('Failed to get version for `%s`, reason: %s', moduleName, err.message)
@@ -534,16 +550,17 @@ function applyDebugState(shim, nodule, inEsm) {
 function instrumentPostLoad(agent, nodule, moduleName, resolvedName, esmResolver) {
   // default to Node.js version, this occurs for core libraries
   const pkgVersion = resolvedName ? shimmer.getPackageVersion(moduleName) : process.version
-  const instrumentations = shimmer.registeredInstrumentations[moduleName]
-  instrumentations.forEach((instrumentation) => {
-    const isInstrumented = instrumentation[symbols.instrumented].has(pkgVersion)
-    const failedInstrumentation = instrumentation[symbols.instrumentedErrored].has(pkgVersion)
-    if (isInstrumented || failedInstrumentation) {
+  const trackedInstrumentations = shimmer.registeredInstrumentations.getAllByName(moduleName)
+  trackedInstrumentations.forEach((trackedInstrumentation) => {
+    const isInstrumented = trackedInstrumentation.meta.instrumented === true
+    const failedInstrumentation = trackedInstrumentation.meta.didError === true
+    if (isInstrumented === true || failedInstrumentation === true) {
       const msg = isInstrumented ? 'Already instrumented' : 'Failed to instrument'
       logger.trace(`${msg} ${moduleName}@${pkgVersion}, skipping registering instrumentation`)
       return
     }
 
+    const { instrumentation } = trackedInstrumentation
     const resolvedNodule = resolveNodule({ nodule, instrumentation, esmResolver })
     const shim = shims.createShimFromType({
       type: instrumentation.type,
@@ -555,18 +572,13 @@ function instrumentPostLoad(agent, nodule, moduleName, resolvedName, esmResolver
     })
 
     applyDebugState(shim, resolvedNodule, esmResolver)
-    trackInstrumentationUsage(
-      agent,
-      shim,
-      instrumentation.specifier || moduleName,
-      NAMES.FEATURES.INSTRUMENTATION.ON_REQUIRE
-    )
+    trackInstrumentationUsage(agent, shim, moduleName, NAMES.FEATURES.INSTRUMENTATION.ON_REQUIRE)
 
     // Tracking instrumentation is only used to add the supportability metrics
     // that occur directly above this.  No reason to attempt to load instrumentation
     // as it does not exist.
     if (instrumentation.type === MODULE_TYPE.TRACKING) {
-      instrumentation[symbols.instrumented].add(pkgVersion)
+      shimmer.registeredInstrumentations.setHookSuccess(trackedInstrumentation)
       return
     }
 
@@ -644,27 +656,20 @@ function resolveNodule({ nodule, instrumentation, esmResolver }) {
  * @param {object} params wrapping object to function
  * @param {*} params.shim The instance of the shim used to instrument the module.
  * @param {object} params.nodule Class or module containing the function to wrap.
- * @param {object} params.resolvedNodule returns xport of the default property
- * @param {string} params.pkgVersion version of module
+ * @param {object} params.resolvedNodule returns export of the default property
  * @param {string} params.moduleName module name
- * @param {object} params.instrumentation hooks for a give module
- * @returns {object} updated xport module
+ * @param {InstrumentationDescriptor} params.instrumentation hooks for a give module
+ * @returns {object} updated export module
  */
-function loadInstrumentation({
-  shim,
-  resolvedNodule,
-  pkgVersion,
-  moduleName,
-  nodule,
-  instrumentation
-}) {
+function loadInstrumentation({ shim, resolvedNodule, moduleName, nodule, instrumentation }) {
+  const trackedItem = shimmer.registeredInstrumentations.getTrackedItem(moduleName, instrumentation)
   try {
     if (instrumentation.onRequire(shim, resolvedNodule, moduleName) !== false) {
+      shimmer.registeredInstrumentations.setHookSuccess(trackedItem)
       nodule = shim.getExport(nodule)
-      instrumentation[symbols.instrumented].add(pkgVersion)
     }
   } catch (instrumentationError) {
-    instrumentation[symbols.instrumentedErrored].add(pkgVersion)
+    shimmer.registeredInstrumentations.setHookFailure(trackedItem)
     if (instrumentation.onError) {
       try {
         instrumentation.onError(instrumentationError)
@@ -706,20 +711,34 @@ function _firstPartyInstrumentation(agent, fileName, shim, nodule, moduleName) {
   }
 }
 
+/**
+ * Invoked directly after a module is loaded via `require` (or `import`). This
+ * is the first opportunity for us to work with the newly loaded module. Prior
+ * to this, the only information we have is the simple name (the string used
+ * to load the module), as well as the `onRequire` and `onError` hooks to
+ * attach to the module.
+ *
+ * @param {object} agent
+ * @param {object} nodule The newly loaded module.
+ * @param {string} name The simple name used to load the module.
+ * @param {string} resolvedName The full file system path to the module.
+ * @param {object} [esmResolver]
+ * @returns {*|Object|undefined}
+ * @private
+ */
 function _postLoad(agent, nodule, name, resolvedName, esmResolver) {
-  const instrumentation = shimmer.getInstrumentationNameFromModuleName(name)
-  const registeredInstrumentation = shimmer.registeredInstrumentations[instrumentation]
+  const simpleName = shimmer.getInstrumentationNameFromModuleName(name)
+  const registeredInstrumentations = shimmer.registeredInstrumentations.getAllByName(simpleName)
   const hasPostLoadInstrumentation =
-    registeredInstrumentation &&
-    registeredInstrumentation.length &&
-    registeredInstrumentation.filter((hook) => hook.onRequire).length
+    registeredInstrumentations &&
+    registeredInstrumentations.length &&
+    registeredInstrumentations.filter((ri) => ri.instrumentation.onRequire).length
 
   // Check if this is a known instrumentation and then run it.
   if (hasPostLoadInstrumentation) {
-    // Add the basedir to the instrumentation to be used later to parse version from package.json
-    registeredInstrumentation.basedir = resolvedName
+    shimmer.registeredInstrumentations.setResolvedName(simpleName, resolvedName)
     logger.trace('Instrumenting %s with onRequire (module loaded) hook.', name)
-    return instrumentPostLoad(agent, nodule, instrumentation, resolvedName, esmResolver)
+    return instrumentPostLoad(agent, nodule, simpleName, resolvedName, esmResolver)
   }
 
   return nodule