From 855450097bab4d341712f8dc1e7288aef0c63bcd Mon Sep 17 00:00:00 2001
From: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com>
Date: Wed, 16 Oct 2024 16:02:13 -0500
Subject: [PATCH 1/2] move recorders to lib/metrics/recorders

---
 lib/db/parsed-statement.js                    | 50 +--------------
 .../recorders/middleware-metric-recorder.js   | 29 +++++++++
 lib/metrics/recorders/record-metrics.js       | 58 +++++++++++++++++
 .../recorders/record-operation-metrics.js     | 62 +++++++++++++++++++
 lib/shim/datastore-shim.js                    | 55 +---------------
 lib/shim/webframework-shim/middleware.js      | 24 +------
 6 files changed, 156 insertions(+), 122 deletions(-)
 create mode 100644 lib/metrics/recorders/middleware-metric-recorder.js
 create mode 100644 lib/metrics/recorders/record-metrics.js
 create mode 100644 lib/metrics/recorders/record-operation-metrics.js

diff --git a/lib/db/parsed-statement.js b/lib/db/parsed-statement.js
index c509e22ab2..332b61e007 100644
--- a/lib/db/parsed-statement.js
+++ b/lib/db/parsed-statement.js
@@ -5,8 +5,7 @@
 
 'use strict'
 
-const { DB, ALL } = require('../metrics/names')
-const { DESTINATIONS } = require('../config/attribute-filter')
+const _recordMetrics = require('../../lib/metrics/recorders/record-metrics')
 
 function ParsedStatement(type, operation, collection, raw) {
   this.type = type
@@ -22,53 +21,10 @@ function ParsedStatement(type, operation, collection, raw) {
 }
 
 ParsedStatement.prototype.recordMetrics = function recordMetrics(segment, scope) {
-  const duration = segment.getDurationInMillis()
-  const exclusive = segment.getExclusiveDurationInMillis()
-  const transaction = segment.transaction
-  const type = transaction.isWeb() ? DB.WEB : DB.OTHER
-  const thisTypeSlash = this.type + '/'
-  const operation = DB.OPERATION + '/' + thisTypeSlash + this.operation
-
-  // Note, an operation metric should _always_ be created even if the action was
-  // a statement. This is part of the spec.
-
-  // Rollups
-  transaction.measure(operation, null, duration, exclusive)
-  transaction.measure(DB.PREFIX + type, null, duration, exclusive)
-  transaction.measure(DB.PREFIX + thisTypeSlash + type, null, duration, exclusive)
-  transaction.measure(DB.PREFIX + thisTypeSlash + ALL, null, duration, exclusive)
-  transaction.measure(DB.ALL, null, duration, exclusive)
-
-  // If we can parse the SQL statement, create a 'statement' metric, and use it
-  // as the scoped metric for transaction breakdowns. Otherwise, skip the
-  // 'statement' metric and use the 'operation' metric as the scoped metric for
-  // transaction breakdowns.
-  let collection
-  if (this.collection) {
-    collection = DB.STATEMENT + '/' + thisTypeSlash + this.collection + '/' + this.operation
-    transaction.measure(collection, null, duration, exclusive)
-    if (scope) {
-      transaction.measure(collection, scope, duration, exclusive)
-    }
-  } else if (scope) {
-    transaction.measure(operation, scope, duration, exclusive)
-  }
-
-  // This recorder is side-effectful Because we are depending on the recorder
-  // setting the transaction name, recorders must always be run before generating
-  // the final transaction trace
-  segment.name = collection || operation
-
-  // Datastore instance metrics.
-  const attributes = segment.attributes.get(DESTINATIONS.TRANS_SEGMENT)
-  if (attributes.host && attributes.port_path_or_id) {
-    const instanceName =
-      DB.INSTANCE + '/' + thisTypeSlash + attributes.host + '/' + attributes.port_path_or_id
-    transaction.measure(instanceName, null, duration, exclusive)
-  }
+  _recordMetrics.bind(this)(segment, scope)
 
   if (this.raw) {
-    transaction.agent.queries.add(segment, this.type.toLowerCase(), this.raw, this.trace)
+    segment.transaction.agent.queries.add(segment, this.type.toLowerCase(), this.raw, this.trace)
   }
 }
 
diff --git a/lib/metrics/recorders/middleware-metric-recorder.js b/lib/metrics/recorders/middleware-metric-recorder.js
new file mode 100644
index 0000000000..61693cfb88
--- /dev/null
+++ b/lib/metrics/recorders/middleware-metric-recorder.js
@@ -0,0 +1,29 @@
+/*
+ * Copyright 2024 New Relic Corporation. All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+'use strict'
+
+/**
+ * Creates a recorder for middleware metrics.
+ *
+ * @private
+ * @param {object} _shim instance of shim
+ * @param {string} metricName name of metric
+ * @returns {Function} recorder for middleware
+ */
+function makeMiddlewareRecorder(_shim, metricName) {
+  return function middlewareMetricRecorder(segment, scope) {
+    const duration = segment.getDurationInMillis()
+    const exclusive = segment.getExclusiveDurationInMillis()
+    const transaction = segment.transaction
+
+    if (scope) {
+      transaction.measure(metricName, scope, duration, exclusive)
+    }
+    transaction.measure(metricName, null, duration, exclusive)
+  }
+}
+
+module.exports = makeMiddlewareRecorder
diff --git a/lib/metrics/recorders/record-metrics.js b/lib/metrics/recorders/record-metrics.js
new file mode 100644
index 0000000000..76dba097c8
--- /dev/null
+++ b/lib/metrics/recorders/record-metrics.js
@@ -0,0 +1,58 @@
+/*
+ * Copyright 2024 New Relic Corporation. All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+'use strict'
+
+const { DB, ALL } = require('../../metrics/names')
+const { DESTINATIONS } = require('../../config/attribute-filter')
+
+function _recordMetrics(segment, scope) {
+  const duration = segment.getDurationInMillis()
+  const exclusive = segment.getExclusiveDurationInMillis()
+  const transaction = segment.transaction
+  const type = transaction.isWeb() ? DB.WEB : DB.OTHER
+  const thisTypeSlash = this.type + '/'
+  const operation = DB.OPERATION + '/' + thisTypeSlash + this.operation
+
+  // Note, an operation metric should _always_ be created even if the action was
+  // a statement. This is part of the spec.
+
+  // Rollups
+  transaction.measure(operation, null, duration, exclusive)
+  transaction.measure(DB.PREFIX + type, null, duration, exclusive)
+  transaction.measure(DB.PREFIX + thisTypeSlash + type, null, duration, exclusive)
+  transaction.measure(DB.PREFIX + thisTypeSlash + ALL, null, duration, exclusive)
+  transaction.measure(DB.ALL, null, duration, exclusive)
+
+  // If we can parse the SQL statement, create a 'statement' metric, and use it
+  // as the scoped metric for transaction breakdowns. Otherwise, skip the
+  // 'statement' metric and use the 'operation' metric as the scoped metric for
+  // transaction breakdowns.
+  let collection
+  if (this.collection) {
+    collection = DB.STATEMENT + '/' + thisTypeSlash + this.collection + '/' + this.operation
+    transaction.measure(collection, null, duration, exclusive)
+    if (scope) {
+      transaction.measure(collection, scope, duration, exclusive)
+    }
+  } else if (scope) {
+    transaction.measure(operation, scope, duration, exclusive)
+  }
+
+  // This recorder is side-effectful Because we are depending on the recorder
+  // setting the transaction name, recorders must always be run before generating
+  // the final transaction trace
+  segment.name = collection || operation
+
+  // Datastore instance metrics.
+  const attributes = segment.attributes.get(DESTINATIONS.TRANS_SEGMENT)
+  if (attributes.host && attributes.port_path_or_id) {
+    const instanceName =
+      DB.INSTANCE + '/' + thisTypeSlash + attributes.host + '/' + attributes.port_path_or_id
+    transaction.measure(instanceName, null, duration, exclusive)
+  }
+}
+
+module.exports = _recordMetrics
diff --git a/lib/metrics/recorders/record-operation-metrics.js b/lib/metrics/recorders/record-operation-metrics.js
new file mode 100644
index 0000000000..4a2fdd69cb
--- /dev/null
+++ b/lib/metrics/recorders/record-operation-metrics.js
@@ -0,0 +1,62 @@
+/*
+ * Copyright 2024 New Relic Corporation. All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+'use strict'
+
+const metrics = require('../../metrics/names')
+
+/**
+ * Records all the metrics required for database operations.
+ *
+ * - `_recordOperationMetrics(segment [, scope])`
+ *
+ * @private
+ * @this DatastoreShim
+ * @implements {MetricFunction}
+ * @param {TraceSegment}  segment - The segment being recorded.
+ * @param {string}        [scope] - The scope of the segment.
+ * @see DatastoreShim#recordOperation
+ * @see MetricFunction
+ */
+function recordOperationMetrics(segment, scope) {
+  if (!segment) {
+    return
+  }
+
+  const duration = segment.getDurationInMillis()
+  const exclusive = segment.getExclusiveDurationInMillis()
+  const transaction = segment.transaction
+  const type = transaction.isWeb() ? 'allWeb' : 'allOther'
+  const operation = segment.name
+
+  if (scope) {
+    transaction.measure(operation, scope, duration, exclusive)
+  }
+
+  transaction.measure(operation, null, duration, exclusive)
+  transaction.measure(metrics.DB.PREFIX + type, null, duration, exclusive)
+  transaction.measure(metrics.DB.ALL, null, duration, exclusive)
+  transaction.measure(this._metrics.ALL, null, duration, exclusive)
+  transaction.measure(
+    metrics.DB.PREFIX + this._metrics.PREFIX + '/' + type,
+    null,
+    duration,
+    exclusive
+  )
+
+  const attributes = segment.getAttributes()
+  if (attributes.host && attributes.port_path_or_id) {
+    const instanceName = [
+      metrics.DB.INSTANCE,
+      this._metrics.PREFIX,
+      attributes.host,
+      attributes.port_path_or_id
+    ].join('/')
+
+    transaction.measure(instanceName, null, duration, exclusive)
+  }
+}
+
+module.exports = recordOperationMetrics
diff --git a/lib/shim/datastore-shim.js b/lib/shim/datastore-shim.js
index 9c1b85bfcc..81bdb3e5ce 100644
--- a/lib/shim/datastore-shim.js
+++ b/lib/shim/datastore-shim.js
@@ -15,6 +15,7 @@ const Shim = require('./shim')
 const urltils = require('../util/urltils')
 const util = require('util')
 const specs = require('./specs')
+const recordOperationMetrics = require('../../lib/metrics/recorders/record-operation-metrics')
 const { DatastoreParameters } = specs.params
 
 /**
@@ -284,7 +285,7 @@ function recordOperation(nodule, properties, opSpec) {
       if (!segDesc?.name?.startsWith(shim._metrics.OPERATION)) {
         segDesc.name = shim._metrics.OPERATION + segDesc.name
       }
-      segDesc.recorder = _recordOperationMetrics.bind(shim)
+      segDesc.recorder = recordOperationMetrics.bind(shim)
     }
 
     return segDesc
@@ -612,58 +613,6 @@ function _recordQueryMetrics(parsed, segment, scope) {
   }
 }
 
-/**
- * Records all the metrics required for database operations.
- *
- * - `_recordOperationMetrics(segment [, scope])`
- *
- * @private
- * @this DatastoreShim
- * @implements {MetricFunction}
- * @param {TraceSegment}  segment - The segment being recorded.
- * @param {string}        [scope] - The scope of the segment.
- * @see DatastoreShim#recordOperation
- * @see MetricFunction
- */
-function _recordOperationMetrics(segment, scope) {
-  if (!segment) {
-    return
-  }
-
-  const duration = segment.getDurationInMillis()
-  const exclusive = segment.getExclusiveDurationInMillis()
-  const transaction = segment.transaction
-  const type = transaction.isWeb() ? 'allWeb' : 'allOther'
-  const operation = segment.name
-
-  if (scope) {
-    transaction.measure(operation, scope, duration, exclusive)
-  }
-
-  transaction.measure(operation, null, duration, exclusive)
-  transaction.measure(metrics.DB.PREFIX + type, null, duration, exclusive)
-  transaction.measure(metrics.DB.ALL, null, duration, exclusive)
-  transaction.measure(this._metrics.ALL, null, duration, exclusive)
-  transaction.measure(
-    metrics.DB.PREFIX + this._metrics.PREFIX + '/' + type,
-    null,
-    duration,
-    exclusive
-  )
-
-  const attributes = segment.getAttributes()
-  if (attributes.host && attributes.port_path_or_id) {
-    const instanceName = [
-      metrics.DB.INSTANCE,
-      this._metrics.PREFIX,
-      attributes.host,
-      attributes.port_path_or_id
-    ].join('/')
-
-    transaction.measure(instanceName, null, duration, exclusive)
-  }
-}
-
 /**
  * Extracts the query string from the arguments according to the given spec.
  *
diff --git a/lib/shim/webframework-shim/middleware.js b/lib/shim/webframework-shim/middleware.js
index d76f4cab86..5950b8ba07 100644
--- a/lib/shim/webframework-shim/middleware.js
+++ b/lib/shim/webframework-shim/middleware.js
@@ -13,6 +13,7 @@ const {
 } = require('./common')
 const { assignCLMSymbol } = require('../../util/code-level-metrics')
 const { RecorderSpec } = require('../specs')
+const makeMiddlewareRecorder = require('../../metrics/recorders/middleware-metric-recorder')
 
 const MIDDLEWARE_TYPE_DETAILS = {
   APPLICATION: { name: 'Mounted App: ', path: true, record: false },
@@ -88,7 +89,7 @@ function constructRecorder({ txInfo, typeDetails, shim, metricName }) {
   let recorder = null
   if (typeDetails.record) {
     const stackPath = txInfo.transaction.nameState.getPath() || ''
-    recorder = _makeMiddlewareRecorder(shim, metricName + '/' + stackPath)
+    recorder = makeMiddlewareRecorder(shim, metricName + '/' + stackPath)
   }
   return recorder
 }
@@ -325,27 +326,6 @@ module.exports._recordMiddleware = function _recordMiddleware(shim, middleware,
   )
 }
 
-/**
- * Creates a recorder for middleware metrics.
- *
- * @private
- * @param {object} _shim instance of shim
- * @param {string} metricName name of metric
- * @returns {Function} recorder for middleware
- */
-function _makeMiddlewareRecorder(_shim, metricName) {
-  return function middlewareMetricRecorder(segment, scope) {
-    const duration = segment.getDurationInMillis()
-    const exclusive = segment.getExclusiveDurationInMillis()
-    const transaction = segment.transaction
-
-    if (scope) {
-      transaction.measure(metricName, scope, duration, exclusive)
-    }
-    transaction.measure(metricName, null, duration, exclusive)
-  }
-}
-
 /**
  * Wrap the `next` middleware function and push on our name state if we find it. We only want to
  * push the name state if there is a next so that we can safely remove it

From efdd04cf553ef70979e3755bb167ffa2cfc0ae55 Mon Sep 17 00:00:00 2001
From: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com>
Date: Wed, 16 Oct 2024 16:22:09 -0500
Subject: [PATCH 2/2] Add jsdocs

---
 lib/metrics/recorders/record-metrics.js           | 6 ++++++
 lib/metrics/recorders/record-operation-metrics.js | 2 +-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/lib/metrics/recorders/record-metrics.js b/lib/metrics/recorders/record-metrics.js
index 76dba097c8..5887126546 100644
--- a/lib/metrics/recorders/record-metrics.js
+++ b/lib/metrics/recorders/record-metrics.js
@@ -8,6 +8,12 @@
 const { DB, ALL } = require('../../metrics/names')
 const { DESTINATIONS } = require('../../config/attribute-filter')
 
+/**
+ * @this ParsedStatement
+ * @param {TraceSegment}  segment - The segment being recorded.
+ * @param {string}        [scope] - The scope of the segment.
+ */
+
 function _recordMetrics(segment, scope) {
   const duration = segment.getDurationInMillis()
   const exclusive = segment.getExclusiveDurationInMillis()
diff --git a/lib/metrics/recorders/record-operation-metrics.js b/lib/metrics/recorders/record-operation-metrics.js
index 4a2fdd69cb..35d31200f2 100644
--- a/lib/metrics/recorders/record-operation-metrics.js
+++ b/lib/metrics/recorders/record-operation-metrics.js
@@ -10,7 +10,7 @@ const metrics = require('../../metrics/names')
 /**
  * Records all the metrics required for database operations.
  *
- * - `_recordOperationMetrics(segment [, scope])`
+ * - `recordOperationMetrics(segment [, scope])`
  *
  * @private
  * @this DatastoreShim