refactor: Updated shim classes to no longer construct specs. (#2096)

THIRD_PARTY_NOTICES.md

@@ -712,7 +712,7 @@ This product includes source derived from [@newrelic/aws-sdk](https://github.com
 
 ### @newrelic/koa
 
-This product includes source derived from [@newrelic/koa](https://github.com/newrelic/node-newrelic-koa) ([v9.0.0](https://github.com/newrelic/node-newrelic-koa/tree/v9.0.0)), distributed under the [Apache-2.0 License](https://github.com/newrelic/node-newrelic-koa/blob/v9.0.0/LICENSE):
+This product includes source derived from [@newrelic/koa](https://github.com/newrelic/node-newrelic-koa) ([v9.1.0](https://github.com/newrelic/node-newrelic-koa/tree/v9.1.0)), distributed under the [Apache-2.0 License](https://github.com/newrelic/node-newrelic-koa/blob/v9.1.0/LICENSE):
 
 ```
 Apache License

lib/instrumentation/cassandra-driver.js

@@ -28,7 +28,9 @@ module.exports = function initialize(_agent, cassandra, _moduleName, shim) {
   shim.recordOperation(
     ClientProto,
     ['connect', 'shutdown'],
-    new OperationSpec({ callback: shim.LAST })
+    function operationSpec(shim, _fn, name) {
+      return new OperationSpec({ callback: shim.LAST, name })
+    }
   )
 
   if (semver.satisfies(cassandraVersion, '>=4.4.0')) {

lib/instrumentation/mongodb/common.js

@@ -23,12 +23,9 @@ common.NR_ATTRS = Symbol('NR_ATTRS')
 common.instrumentCursor = function instrumentCursor(shim, Cursor) {
   if (Cursor && Cursor.prototype) {
     const proto = Cursor.prototype
-    for (let i = 0; i < CURSOR_OPS.length; i++) {
-      shim.recordQuery(proto, CURSOR_OPS[i], common.makeQueryDescFunc(shim, CURSOR_OPS[i]))
-    }
-
-    shim.recordQuery(proto, 'each', common.makeQueryDescFunc(shim, 'each'))
-    shim.recordOperation(proto, 'pipe', new OperationSpec({ opaque: true }))
+    shim.recordQuery(proto, CURSOR_OPS, common.makeQueryDescFunc)
+    shim.recordQuery(proto, 'each', common.makeQueryDescFunc)
+    shim.recordOperation(proto, 'pipe', new OperationSpec({ opaque: true, name: 'pipe' }))
   }
 }
 
@@ -42,9 +39,7 @@ common.instrumentCursor = function instrumentCursor(shim, Cursor) {
 common.instrumentCollection = function instrumentCollection(shim, Collection) {
   if (Collection && Collection.prototype) {
     const proto = Collection.prototype
-    for (let i = 0; i < COLLECTION_OPS.length; i++) {
-      shim.recordQuery(proto, COLLECTION_OPS[i], common.makeQueryDescFunc(shim, COLLECTION_OPS[i]))
-    }
+    shim.recordQuery(proto, COLLECTION_OPS, common.makeQueryDescFunc)
   }
 }
 
@@ -72,43 +67,48 @@ common.instrumentBulkOperation = function instrumentBulkOperation(shim, BulkOper
 common.instrumentDb = function instrumentDb(shim, Db) {
   if (Db && Db.prototype) {
     const proto = Db.prototype
+    shim.recordOperation(proto, DB_OPS, function makeOperationDescFunc(shim, _fn, methodName) {
+      return new OperationSpec({
+        callback: shim.LAST,
+        opaque: true,
+        promise: true,
+        name: methodName
+      })
+    })
+    // link to client.connect(removed in v4.0)
     shim.recordOperation(
-      proto,
-      DB_OPS,
-      new OperationSpec({ callback: shim.LAST, opaque: true, promise: true })
+      Db,
+      'connect',
+      new OperationSpec({ callback: shim.LAST, promise: true, name: 'connect' })
     )
-    // link to client.connect(removed in v4.0)
-    shim.recordOperation(Db, 'connect', new OperationSpec({ callback: shim.LAST, promise: true }))
   }
 }
 
 /**
  * Sets up the desc for all instrumented query methods
  *
  * @param {Shim} shim instance of shim
- * @param {string} methodName name of method getting executed
- * @returns {object} query spec
+ * @param {Function} fn function getting instrumented
+ * @param _fn
+ * @param {string} methodName name of function
+ * @returns {QuerySpec} query spec
  */
-common.makeQueryDescFunc = function makeQueryDescFunc(shim, methodName) {
+common.makeQueryDescFunc = function makeQueryDescFunc(shim, _fn, methodName) {
   if (methodName === 'each') {
-    return function eachDescFunc() {
-      const parameters = getInstanceAttributeParameters(shim, this)
-      return new QuerySpec({ query: methodName, parameters, rowCallback: shim.LAST, opaque: true })
-    }
-  }
-
-  return function queryDescFunc() {
-    // segment name does not actually use query string
-    // method name is set as query so the query parser has access to the op name
     const parameters = getInstanceAttributeParameters(shim, this)
-    return new QuerySpec({
-      query: methodName,
-      parameters,
-      promise: true,
-      callback: shim.LAST,
-      opaque: true
-    })
+    return new QuerySpec({ query: methodName, parameters, rowCallback: shim.LAST, opaque: true })
   }
+
+  // segment name does not actually use query string
+  // method name is set as query so the query parser has access to the op name
+  const parameters = getInstanceAttributeParameters(shim, this)
+  return new QuerySpec({
+    query: methodName,
+    parameters,
+    promise: true,
+    callback: shim.LAST,
+    opaque: true
+  })
 }
 
 /**

lib/instrumentation/mongodb/v2-mongo.js

@@ -38,7 +38,7 @@ module.exports = function instrument(shim, mongodb) {
   const recordDesc = {
     Gridstore: {
       isQuery: false,
-      makeDescFunc: function makeGridDesc(opName) {
+      makeDescFunc: function makeGridDesc(shim, fn, opName) {
         return new OperationSpec({ name: 'GridFS-' + opName, callback: shim.LAST })
       }
     },
@@ -50,8 +50,8 @@ module.exports = function instrument(shim, mongodb) {
     Collection: { isQuery: true, makeDescFunc: makeQueryDescFunc },
     Db: {
       isQuery: false,
-      makeDescFunc: function makeDbDesc() {
-        return new OperationSpec({ callback: shim.LAST })
+      makeDescFunc: function makeDbDesc(shim, fn, method) {
+        return new OperationSpec({ callback: shim.LAST, name: method })
       }
     }
   }
@@ -95,10 +95,10 @@ module.exports = function instrument(shim, mongodb) {
         const { isQuery, makeDescFunc } = recordDesc[objectName]
         const proto = object.prototype
         if (isQuery) {
-          shim.recordQuery(proto, method, makeDescFunc(shim, method))
+          shim.recordQuery(proto, method, makeDescFunc)
         } else if (isQuery === false) {
           // could be unset
-          shim.recordOperation(proto, method, makeDescFunc(shim, method))
+          shim.recordOperation(proto, method, makeDescFunc)
         } else {
           shim.logger.trace('No wrapping method found for %s', objectName)
         }
@@ -108,7 +108,11 @@ module.exports = function instrument(shim, mongodb) {
     // the cursor object implements Readable stream and internally calls nextObject on
     // each read, in which case we do not want to record each nextObject() call
     if (/Cursor$/.test(objectName)) {
-      shim.recordOperation(object.prototype, 'pipe', new OperationSpec({ opaque: true }))
+      shim.recordOperation(
+        object.prototype,
+        'pipe',
+        new OperationSpec({ opaque: true, name: 'pipe' })
+      )
     }
   }
 }

lib/instrumentation/mongodb/v3-mongo.js

@@ -48,7 +48,7 @@ function instrumentClient(shim, mongodb) {
     // Add the connection url to the MongoClient to retrieve later in the `lib/instrumentation/mongo/common`
     // captureAttributesOnStarted listener
     this[NR_ATTRS] = args[0]
-    return new RecorderSpec({ callback: shim.LAST })
+    return new RecorderSpec({ callback: shim.LAST, name: 'connect' })
   })
 }
 

lib/instrumentation/mongodb/v4-mongo.js

@@ -73,7 +73,7 @@ function cmdStartedHandler(shim, evnt) {
 function wrapConnect(shim) {
   this.monitorCommands = true
   this.on('commandStarted', cmdStartedHandler.bind(this, shim))
-  return new OperationSpec({ callback: shim.LAST })
+  return new OperationSpec({ callback: shim.LAST, name: 'connect' })
 }
 
 /**

lib/instrumentation/pg.js

@@ -6,7 +6,7 @@
 'use strict'
 
 const { nrEsmProxy } = require('../symbols')
-const { RecorderSpec, QuerySpec } = require('../shim/specs')
+const { RecorderSpec, QuerySpec, ClassWrapSpec } = require('../shim/specs')
 const DatastoreParameters = require('../shim/specs/params/datastore')
 
 function getQuery(shim, original, name, args) {
@@ -58,12 +58,12 @@ module.exports = function initialize(agent, pgsql, moduleName, shim) {
   // wrapping for native
   function instrumentPGNative(pg) {
     shim.wrapReturn(pg, 'Client', clientFactoryWrapper)
-    shim.wrapClass(pg, 'Pool', { post: poolPostConstructor, es6: true })
+    shim.wrapClass(pg, 'Pool', new ClassWrapSpec({ post: poolPostConstructor, es6: true }))
   }
 
   function poolPostConstructor(shim) {
     if (!shim.isWrapped(this.Client)) {
-      shim.wrapClass(this, 'Client', clientPostConstructor)
+      shim.wrapClass(this, 'Client', new ClassWrapSpec({ post: clientPostConstructor }))
     }
   }
 

lib/shim/datastore-shim.js

@@ -14,9 +14,8 @@ const ParsedStatement = require('../db/parsed-statement')
 const Shim = require('./shim')
 const urltils = require('../util/urltils')
 const util = require('util')
-const {
-  params: { DatastoreParameters }
-} = require('./specs/')
+const specs = require('./specs')
+const { DatastoreParameters } = specs.params
 
 /**
  * An enumeration of well-known datastores so that new instrumentations can use
@@ -263,18 +262,28 @@ function recordOperation(nodule, properties, opSpec) {
     opSpec = properties
     properties = null
   }
+
+  // TODO: not a fan of this. people should always pass in some sort of spec
   if (!opSpec) {
     opSpec = Object.create(null)
   }
 
   return this.record(nodule, properties, function operationRecorder(shim, fn, fnName, args) {
     shim.logger.trace('Recording datastore operation "%s"', fnName)
 
-    const segDesc = _getSpec.call(this, { spec: opSpec, shim, fn, fnName, args })
+    const segDesc = _getSpec.call(this, {
+      spec: opSpec,
+      shim,
+      fn,
+      fnName,
+      args
+    })
 
     // Adjust the segment name with the metric prefix and add a recorder.
     if (!hasOwnProperty(segDesc, 'record') || segDesc.record !== false) {
-      segDesc.name = shim._metrics.OPERATION + segDesc.name
+      if (!segDesc?.name?.startsWith(shim._metrics.OPERATION)) {
+        segDesc.name = shim._metrics.OPERATION + segDesc.name
+      }
       segDesc.recorder = _recordOperationMetrics.bind(shim)
     }
 
@@ -534,6 +543,7 @@ function _recordQuery(suffix, nodule, properties, querySpec) {
     querySpec = properties
     properties = null
   }
+
   if (!querySpec) {
     this.logger.debug('Missing query spec for recordQuery, not wrapping.')
     return nodule
@@ -579,20 +589,13 @@ function _recordQuery(suffix, nodule, properties, querySpec) {
  *
  */
 function _getSpec({ spec, shim, fn, fnName, args }) {
-  let dsSpec = null
+  let dsSpec = spec
   if (shim.isFunction(spec)) {
     dsSpec = spec.call(this, shim, fn, fnName, args)
-  } else {
-    dsSpec = { ...spec }
   }
 
-  dsSpec.name = dsSpec.name || fnName || 'other'
-
-  const parameters = _normalizeParameters.call(shim, dsSpec.parameters)
-  return {
-    ...dsSpec,
-    parameters
-  }
+  dsSpec.parameters = _normalizeParameters.call(shim, dsSpec.parameters)
+  return dsSpec
 }
 
 /**

lib/shim/message-shim/consume.js

@@ -26,9 +26,8 @@ function updateSpecFromArgs({ shim, fn, fnName, args, spec }) {
   let msgDesc = null
   if (shim.isFunction(spec)) {
     msgDesc = spec.call(this, shim, fn, fnName, args)
-    msgDesc = new specs.MessageSpec(msgDesc)
   } else {
-    msgDesc = new specs.MessageSpec(spec)
+    msgDesc = spec
     const destIdx = shim.normalizeIndex(args.length, spec.destinationName)
     if (destIdx !== null) {
       msgDesc.destinationName = args[destIdx]