refactor: Updated shim classes to no longer construct specs.

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.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.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.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]

lib/shim/message-shim/index.js

@@ -233,7 +233,7 @@ function recordProduce(nodule, properties, recordNamer) {
       return null
     }
 
-    const name = _nameMessageSegment(shim, msgDesc, shim._metrics.PRODUCE)
+    msgDesc.name = _nameMessageSegment(shim, msgDesc, shim._metrics.PRODUCE)
     if (!shim.agent.config.message_tracer.segment_parameters.enabled) {
       delete msgDesc.parameters
     } else if (msgDesc.routingKey) {
@@ -242,19 +242,13 @@ function recordProduce(nodule, properties, recordNamer) {
       })
     }
 
-    return {
-      name: name,
-      promise: msgDesc.promise || false,
-      callback: msgDesc.callback || null,
-      recorder: genericRecorder,
-      inContext: function generateCATHeaders() {
-        if (msgDesc.headers) {
-          shim.insertCATRequestHeaders(msgDesc.headers, true)
-        }
-      },
-      parameters: msgDesc.parameters || null,
-      opaque: msgDesc.opaque || false
+    msgDesc.inContext = function generateCATHeaders() {
+      if (msgDesc.headers) {
+        shim.insertCATRequestHeaders(msgDesc.headers, true)
+      }
     }
+    msgDesc.recorder = genericRecorder
+    return msgDesc
   })
 }
 /**
@@ -292,9 +286,6 @@ function recordConsume(nodule, properties, spec) {
     spec = properties
     properties = null
   }
-  if (!this.isFunction(spec)) {
-    spec = new specs.MessageSpec(spec)
-  }
 
   // This is using wrap instead of record because the spec allows for a messageHandler
   // which is being used to handle the result of the callback or promise of the
@@ -345,11 +336,7 @@ function recordPurgeQueue(nodule, properties, spec) {
     properties = null
   }
 
-  // Fill the spec with defaults.
   const specIsFunction = this.isFunction(spec)
-  if (!specIsFunction) {
-    spec = new specs.MessageSpec(spec || {})
-  }
 
   return this.record(nodule, properties, function purgeRecorder(shim, fn, name, args) {
     let descriptor = spec
@@ -363,20 +350,16 @@ function recordPurgeQueue(nodule, properties, spec) {
       queue = args[queueIdx]
     }
 
-    return {
-      name: _nameMessageSegment(
-        shim,
-        {
-          destinationType: shim.QUEUE,
-          destinationName: queue
-        },
-        shim._metrics.PURGE
-      ),
-      recorder: genericRecorder,
-      callback: descriptor.callback,
-      promise: descriptor.promise,
-      internal: descriptor.internal
-    }
+    descriptor.name = _nameMessageSegment(
+      shim,
+      {
+        destinationType: shim.QUEUE,
+        destinationName: queue
+      },
+      shim._metrics.PURGE
+    )
+    descriptor.recorder = genericRecorder
+    return descriptor
   })
 }
 
@@ -421,8 +404,6 @@ function recordSubscribedConsume(nodule, properties, spec) {
     properties = null
   }
 
-  spec = new specs.MessageSubscribeSpec(spec)
-
   // Make sure our spec has what we need.
   if (!this.isFunction(spec.messageHandler)) {
     this.logger.debug('spec.messageHandler should be a function')
@@ -456,13 +437,12 @@ function recordSubscribedConsume(nodule, properties, spec) {
       cbIdx = null
     }
 
-    return {
+    return new specs.RecorderSpec({
       name: spec.name || name,
       callback: cbIdx,
       promise: spec.promise,
-
       stream: false,
       internal: false
-    }
+    })
   })
 }