Merge pull request newrelic#122 from michaelgoin/fix-version-checking

Fixes nr-hooks version checking circular reference issue.

merged/aws-sdk/lib/v3/check-agent-compatible.js

@@ -0,0 +1,30 @@
+/*
+ * Copyright 2020 New Relic Corporation. All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+'use strict'
+
+const semver = require('semver')
+
+const ON_REQUIRE_COMPATIBLE_VERSIONS = '>=8.7.0'
+
+// TODO: Remove this semver check and semver module when we ship Node 18 support
+// A bug existed in 8.6.0 when we introduced the `onResolved` hook.
+// See: https://github.com/newrelic/node-newrelic/pull/986
+// To avoid unnecessary support issues we will require agent version >= 8.7.0 to
+// register AWS SDK v3 instrumentation
+function checkAgentCompatible(agentVersion, logger, moduleName) {
+  const isCompatible = semver.satisfies(agentVersion, ON_REQUIRE_COMPATIBLE_VERSIONS)
+
+  if (!isCompatible) {
+    logger.warn(
+      `Agent version must be ${ON_REQUIRE_COMPATIBLE_VERSIONS} to instrument ${moduleName}. ` +
+        `Current version: ${agentVersion}`
+    )
+  }
+
+  return isCompatible
+}
+
+module.exports = checkAgentCompatible

merged/aws-sdk/nr-hooks.js

@@ -4,56 +4,77 @@
  */
 
 'use strict'
-const newrelic = require('newrelic')
-const semver = require('semver')
-const agentVersion = newrelic && newrelic.agent && newrelic.agent.version
 
 const instrumentations = [
   {
     type: 'conglomerate',
     moduleName: 'aws-sdk',
     onRequire: require('./lib/v2/instrumentation')
+  },
+  {
+    type: 'generic',
+    moduleName: '@aws-sdk/smithy-client',
+    onResolved: require('./lib/v3/smithy-client')
+  },
+  {
+    type: 'message',
+    moduleName: '@aws-sdk/client-sns',
+    onResolved: require('./lib/v3/sns')
+  },
+  {
+    type: 'message',
+    moduleName: '@aws-sdk/client-sqs',
+    onResolved: require('./lib/v3/sqs')
+  },
+  {
+    type: 'datastore',
+    moduleName: '@aws-sdk/client-dynamodb',
+    onResolved: require('./lib/v3/client-dynamodb')
+  },
+  {
+    type: 'datastore',
+    moduleName: '@aws-sdk/lib-dynamodb',
+    onResolved: require('./lib/v3/lib-dynamodb')
   }
 ]
 
-// TODO: Remove this semver check and semver module when we ship Node 18 support
-// A bug existed in 8.6.0 when we introduced the `onResolved` hook.
-// See: https://github.com/newrelic/node-newrelic/pull/986
-// To avoid unnecessary support issues we will require agent version >= 8.7.0 to
-// register AWS SDK v3 instrumentation
-if (semver.satisfies(agentVersion, '>=8.7.0')) {
-  instrumentations.push(
-    {
-      type: 'generic',
-      moduleName: '@aws-sdk/smithy-client',
-      onResolved: require('./lib/v3/smithy-client')
-    },
-    {
-      type: 'message',
-      moduleName: '@aws-sdk/client-sns',
-      onResolved: require('./lib/v3/sns')
-    },
-    {
-      type: 'message',
-      moduleName: '@aws-sdk/client-sqs',
-      onResolved: require('./lib/v3/sqs')
-    },
-    {
-      type: 'datastore',
-      moduleName: '@aws-sdk/client-dynamodb',
-      onResolved: require('./lib/v3/client-dynamodb')
-    },
-    {
-      type: 'datastore',
-      moduleName: '@aws-sdk/lib-dynamodb',
-      onResolved: require('./lib/v3/lib-dynamodb')
+// TODO: Remove code block next major release of module by moving to a peer-dependency check.
+// See further comments in check-agent-compatible.js.
+const checkAgentCompatible = require('./lib/v3/check-agent-compatible')
+const NOOP_ON_REQUIRE = () => false
+
+instrumentations
+  .filter((definition) => {
+    return definition.onResolved
+  })
+  .forEach(addCompatibleAgentCheck)
+
+/**
+ * Adds a check on resolve to ensure on a version of the agent that does
+ * not have the multiple invocation bug. If compatible, replaces onResolved
+ * with original and returns value of invoked original. If not compatible,
+ * sets onResolved to null so the related instrumentation gets skipped on each
+ * resolve.
+ * @param {object} definition Object definition instrumentation parameters
+ */
+function addCompatibleAgentCheck(definition) {
+  // Silence old agent versions from warning about missing require.
+  definition.onRequire = NOOP_ON_REQUIRE
+
+  const originalOnResolved = definition.onResolved
+
+  definition.onResolved = function checkCompatibleOnResolved(shim, name) {
+    if (!checkAgentCompatible(shim.agent.config.version, shim.logger, name)) {
+      // Prevent future attempted execution which avoids allocating a shim each time.
+      definition.onResolved = null
+      return false
     }
-  )
-} else {
-  newrelic.shim.logger.warn(
-    'The New Relic Node.js agent must be >= 8.7.0 to instrument AWS SDK v3, current version: %s',
-    agentVersion
-  )
+
+    definition.onResolved = originalOnResolved
+    return originalOnResolved.apply(this, arguments)
+  }
 }
 
+// ------------------------------------------------------------------
+
 module.exports = instrumentations