refactor!: Updated agent to use require-in-the-middle to register Com…

…monJS instrumentation (#1758) * Removed onResolved hook. If you're using custom instrumentation with an onResolved hook, you must update to use onRequire * You can no longer instrument files that are not within a node_module unless you provide an absolute path to the file when registering the instrumentation. * You cannot instrument both the base module and a sub module.
newrelic · Aug 17, 2023 · d4b4f11 · d4b4f11
1 parent 6788f9e
commit d4b4f11
Show file tree

Hide file tree

Showing 36 changed files with 429 additions and 677 deletions.
diff --git a/THIRD_PARTY_NOTICES.md b/THIRD_PARTY_NOTICES.md
@@ -27,6 +27,7 @@ code, the source code can be found at [https://github.com/newrelic/node-newrelic
 * [json-bigint](#json-bigint)
 * [json-stringify-safe](#json-stringify-safe)
 * [readable-stream](#readable-stream)
+* [require-in-the-middle](#require-in-the-middle)
 * [semver](#semver)
 * [winston-transport](#winston-transport)
 
@@ -1393,6 +1394,36 @@ IN THE SOFTWARE.
 
 ```
 
+### require-in-the-middle
+
+This product includes source derived from [require-in-the-middle](https://github.com/elastic/require-in-the-middle) ([v7.2.0](https://github.com/elastic/require-in-the-middle/tree/v7.2.0)), distributed under the [MIT License](https://github.com/elastic/require-in-the-middle/blob/v7.2.0/LICENSE):
+
+```
+The MIT License (MIT)
+
+Copyright (c) 2016-2019, Thomas Watson Steen
+Copyright (c) 2019-2023, Elasticsearch B.V.
+
+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.
+
+```
+
 ### semver
 
 This product includes source derived from [semver](https://github.com/npm/node-semver) ([v7.5.4](https://github.com/npm/node-semver/tree/v7.5.4)), distributed under the [ISC License](https://github.com/npm/node-semver/blob/v7.5.4/LICENSE):

diff --git a/index.js b/index.js
@@ -150,12 +150,12 @@ function createAgent(config) {
   }
 
   const shimmer = require('./lib/shimmer')
-  shimmer.patchModule(agent)
   shimmer.bootstrapInstrumentation(agent)
 
   // Check for already loaded modules and warn about them.
   const uninstrumented = require('./lib/uninstrumented')
   uninstrumented.check(shimmer.registeredInstrumentations)
+  shimmer.registerHooks(agent)
 
   agent.start(function afterStart(error) {
     if (error) {

diff --git a/lib/instrumentation/amqplib.js → lib/instrumentation/amqplib/amqplib.js b/lib/instrumentation/amqplib.js → lib/instrumentation/amqplib/amqplib.js
@@ -7,34 +7,6 @@
 
 const url = require('url')
 
-module.exports.selfRegister = function selfRegister(shimmer) {
-  shimmer.registerInstrumentation({
-    moduleName: 'amqplib',
-    type: 'message',
-    onRequire: instrumentChannelAPI
-  })
-  shimmer.registerInstrumentation({
-    moduleName: 'amqplib/channel_api',
-    type: 'message',
-    onRequire: instrumentChannelAPI
-  })
-  shimmer.registerInstrumentation({
-    moduleName: 'amqplib/channel_api.js',
-    type: 'message',
-    onRequire: instrumentChannelAPI
-  })
-  shimmer.registerInstrumentation({
-    moduleName: 'amqplib/callback_api',
-    type: 'message',
-    onRequire: instrumentCallbackAPI
-  })
-  shimmer.registerInstrumentation({
-    moduleName: 'amqplib/callback_api.js',
-    type: 'message',
-    onRequire: instrumentCallbackAPI
-  })
-}
-
 module.exports.instrumentPromiseAPI = instrumentChannelAPI
 module.exports.instrumentCallbackAPI = instrumentCallbackAPI
 

diff --git a/lib/instrumentation/amqplib/nr-hooks.js b/lib/instrumentation/amqplib/nr-hooks.js
@@ -0,0 +1,20 @@
+/*
+ * Copyright 2023 New Relic Corporation. All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+'use strict'
+const amqplib = require('./amqplib')
+
+module.exports = [
+  {
+    moduleName: 'amqplib/callback_api',
+    type: 'message',
+    onRequire: amqplib.instrumentCallbackAPI
+  },
+  {
+    moduleName: 'amqplib/channel_api',
+    type: 'message',
+    onRequire: amqplib.instrumentPromiseAPI
+  }
+]
diff --git a/lib/instrumentation/grpc-js/grpc.js b/lib/instrumentation/grpc-js/grpc.js
@@ -11,27 +11,23 @@ const { DESTINATIONS } = require('../../config/attribute-filter')
 const DESTINATION = DESTINATIONS.TRANS_EVENT | DESTINATIONS.ERROR_EVENT
 const semver = require('semver')
 
-module.exports = function instrument(shim) {
-  const grpcVersion = shim.require('./package.json').version
-  const genericShim = shim.makeSpecializedShim(shim.GENERIC, 'grpc-client-interceptor')
-
-  if (semver.gte(grpcVersion, '1.8.0')) {
-    const resolvingCall = genericShim.require('./build/src/resolving-call')
-    genericShim.wrap(resolvingCall.ResolvingCall.prototype, 'start', wrapStart)
-  } else {
-    const callStream = genericShim.require('./build/src/call-stream')
-    genericShim.wrap(callStream.Http2CallStream.prototype, 'start', wrapStart)
-  }
+module.exports.wrapStartResolve = function wrappedClient(shim, resolvingCall) {
+  shim.wrap(resolvingCall.ResolvingCall.prototype, 'start', wrapStart)
+}
 
-  const webFrameworkShim = shim.makeSpecializedShim(shim.WEB_FRAMEWORK, 'server')
+module.exports.wrapStartCall = function wrappedClient(shim, callStream) {
+  shim.wrap(callStream.Http2CallStream.prototype, 'start', wrapStart)
+}
+
+module.exports.wrapServer = function wrapServer(shim, server) {
+  const grpcVersion = shim.require('./package.json').version
   if (semver.lt(grpcVersion, '1.4.0')) {
     shim.logger.debug('gRPC server-side instrumentation only supported on grpc-js >=1.4.0')
     return
   }
 
-  const server = webFrameworkShim.require('./build/src/server')
-  webFrameworkShim.setFramework('gRPC')
-  webFrameworkShim.wrap(server.Server.prototype, 'register', wrapRegister)
+  shim.setFramework('gRPC')
+  shim.wrap(server.Server.prototype, 'register', wrapRegister)
 }
 
 /**

diff --git a/lib/instrumentation/grpc-js/nr-hooks.js b/lib/instrumentation/grpc-js/nr-hooks.js
@@ -7,13 +7,22 @@
 const grpc = require('./grpc')
 
 /**
- * Need to use nr-hooks style for grpc because we're using the onResolved hook
- * to register instrumentation.
+ * Need to use nr-hooks style for grpc because we're instrumentation a submodule.
  */
 module.exports = [
   {
-    type: 'conglomerate', // generic shim for client, web framework shim for server
-    moduleName: '@grpc/grpc-js',
-    onResolved: grpc
+    type: 'generic',
+    moduleName: '@grpc/grpc-js/build/src/resolving-call',
+    onRequire: grpc.wrapStartResolve
+  },
+  {
+    type: 'generic',
+    moduleName: '@grpc/grpc-js/build/src/call-stream',
+    onRequire: grpc.wrapStartCall
+  },
+  {
+    type: 'web-framework',
+    moduleName: '@grpc/grpc-js/build/src/server',
+    onRequire: grpc.wrapServer
   }
 ]
diff --git a/lib/instrumentation/pino/nr-hooks.js b/lib/instrumentation/pino/nr-hooks.js
@@ -7,13 +7,12 @@
 const pino = require('./pino')
 
 /**
- * Need to use nr-hooks style for pino because we're using the onResolved hook
- * to register instrumentation.
+ * Need to use nr-hooks style because we are instrumenting a submodule.
  */
 module.exports = [
   {
     type: 'generic',
-    moduleName: 'pino',
-    onResolved: pino
+    moduleName: 'pino/lib/tools',
+    onRequire: pino
   }
 ]
diff --git a/lib/instrumentation/pino/pino.js b/lib/instrumentation/pino/pino.js
@@ -15,15 +15,14 @@ const {
 } = require('../../util/application-logging')
 const semver = require('semver')
 
-module.exports = function instrument(shim) {
+module.exports = function instrument(shim, tools) {
   const pinoVersion = shim.require('./package.json').version
 
   if (semver.lt(pinoVersion, '7.0.0')) {
     shim.logger.debug('Instrumentation only supported on pino >=7.0.0.')
     return
   }
 
-  const tools = shim.require('./lib/tools')
   const agent = shim.agent
   const config = agent.config
 
@@ -35,18 +34,6 @@ module.exports = function instrument(shim) {
   const metrics = agent.metrics
   createModuleUsageMetric('pino', metrics)
 
-  const levelUtils = shim.require('./lib/levels')
-
-  /**
-   * Creates an object where the keys are the level labels and values are the level number.
-   * Pino passes in level as number but our spec needs it to be the label
-   */
-  const levelMap = Object.entries(levelUtils.levels).reduce((levels, level) => {
-    const [label, number] = level
-    levels[number] = label
-    return levels
-  }, {})
-
   const symbols = shim.require('./lib/symbols')
 
   shim.wrap(tools, 'asJson', function wrapJson(shim, asJson) {
@@ -62,10 +49,10 @@ module.exports = function instrument(shim) {
      */
     return function wrappedAsJson() {
       const args = shim.argsToArray.apply(shim, arguments)
+      const level = this?.levels?.labels?.[args[2]]
 
       if (isMetricsEnabled(config)) {
-        const level = args[2]
-        incrementLoggingLinesMetrics(levelMap[level], metrics)
+        incrementLoggingLinesMetrics(level, metrics)
       }
 
       if (isLocalDecoratingEnabled(config)) {
@@ -86,7 +73,7 @@ module.exports = function instrument(shim) {
           logLine,
           agent,
           chindings,
-          levelMap,
+          level,
           logger: shim.logger
         })
 
@@ -110,10 +97,10 @@ module.exports = function instrument(shim) {
  * @param logLine.agent
  * @param logLine.chindings
  * @param logLine.msg
- * @param logLine.levelMap
+ * @param logLine.level
  * @param logLine.logger
  */
-function reformatLogLine({ logLine, msg, agent, chindings = '', levelMap, logger }) {
+function reformatLogLine({ logLine, msg, agent, chindings = '', level, logger }) {
   const metadata = agent.getLinkingMetadata()
 
   /**
@@ -145,7 +132,7 @@ function reformatLogLine({ logLine, msg, agent, chindings = '', levelMap, logger
       reformatError(formattedLog)
     }
     Object.assign(formattedLog, agentMeta)
-    formattedLog.level = levelMap[formattedLog.level]
+    formattedLog.level = level
     delete formattedLog.time
     delete formattedLog.msg
     return formattedLog

diff --git a/lib/instrumentations.js b/lib/instrumentations.js
@@ -11,7 +11,7 @@ const MODULE_TYPE = require('./shim/constants').MODULE_TYPE
 module.exports = function instrumentations() {
   return {
     'aws-sdk': { module: '@newrelic/aws-sdk' },
-    'amqplib': { type: MODULE_TYPE.MESSAGE },
+    'amqplib': { module: './instrumentation/amqplib' },
     'cassandra-driver': { type: MODULE_TYPE.DATASTORE },
     'connect': { type: MODULE_TYPE.WEB_FRAMEWORK },
     'bluebird': { type: MODULE_TYPE.PROMISE },