feat: Provided ability to disable instrumentation for a 3rd party package (newrelic#2551)

Author: bizob2828

lib/config/build-instrumentation-config.js

@@ -0,0 +1,19 @@
+/*
+ * Copyright 2024 New Relic Corporation. All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+'use strict'
+const { boolean } = require('./formatters')
+const instrumentedLibraries = require('../instrumentations')()
+const pkgNames = Object.keys(instrumentedLibraries)
+
+/**
+ * Builds the stanza for config.instrumentation.*
+ * It defaults every library to true and assigns a boolean
+ * formatter for the environment variable conversion of the values
+ */
+module.exports = pkgNames.reduce((config, pkg) => {
+  config[pkg] = { enabled: { formatter: boolean, default: true } }
+  return config
+}, {})

lib/config/default.js

@@ -7,6 +7,7 @@
 
 const defaultConfig = module.exports
 const { array, int, float, boolean, object, objectList, allowList, regex } = require('./formatters')
+const pkgInstrumentation = require('./build-instrumentation-config')
 
 /**
  * A function that returns the definition of the agent configuration
@@ -1382,7 +1383,13 @@ defaultConfig.definition = () => ({
         default: true
       }
     }
-  }
+  },
+  /**
+   * Stanza that contains all keys to disable 3rd party package instrumentation(i.e. mongodb, pg, redis, etc)
+   * Note: Disabling a given 3rd party library may affect the instrumentation of 3rd party libraries used after
+   * the disabled library.
+   */
+  instrumentation: pkgInstrumentation
 })
 
 /**

lib/shimmer.js

@@ -286,7 +286,11 @@ const shimmer = (module.exports = {
    */
   registerThirdPartyInstrumentation(agent) {
     for (const [moduleName, instrInfo] of Object.entries(INSTRUMENTATIONS)) {
-      if (instrInfo.module) {
+      if (agent.config.instrumentation?.[moduleName]?.enabled === false) {
+        logger.warn(
+          `Instrumentation for ${moduleName} has been disabled via 'config.instrumentation.${moduleName}.enabled. Not instrumenting package`
+        )
+      } else if (instrInfo.module) {
         // Because external instrumentations can change independent of
         // the agent core, we don't want breakages in them to entirely
         // disable the agent.

test/lib/custom-assertions.js

@@ -80,6 +80,94 @@ function compareSegments(parent, segments) {
   })
 }
 
+/**
+ * @param {TraceSegment} parent     Parent segment
+ * @param {Array} expected          Array of strings that represent segment names.
+ *                                  If an item in the array is another array, it
+ *                                  represents children of the previous item.
+ * @param {boolean} options.exact   If true, then the expected segments must match
+ *                                  exactly, including their position and children on all
+ *                                  levels.  When false, then only check that each child
+ *                                  exists.
+ * @param {array} options.exclude   Array of segment names that should be excluded from
+ *                                  validation.  This is useful, for example, when a
+ *                                  segment may or may not be created by code that is not
+ *                                  directly under test.  Only used when `exact` is true.
+ */
+function assertSegments(parent, expected, options) {
+  let child
+  let childCount = 0
+
+  // rather default to what is more likely to fail than have a false test
+  let exact = true
+  if (options && options.exact === false) {
+    exact = options.exact
+  } else if (options === false) {
+    exact = false
+  }
+
+  function getChildren(_parent) {
+    return _parent.children.filter(function (item) {
+      if (exact && options && options.exclude) {
+        return options.exclude.indexOf(item.name) === -1
+      }
+      return true
+    })
+  }
+
+  const children = getChildren(parent)
+  if (exact) {
+    for (let i = 0; i < expected.length; ++i) {
+      const sequenceItem = expected[i]
+
+      if (typeof sequenceItem === 'string') {
+        child = children[childCount++]
+        assert.equal(
+          child ? child.name : undefined,
+          sequenceItem,
+          'segment "' +
+            parent.name +
+            '" should have child "' +
+            sequenceItem +
+            '" in position ' +
+            childCount
+        )
+
+        // If the next expected item is not array, then check that the current
+        // child has no children
+        if (!Array.isArray(expected[i + 1])) {
+          assert.ok(
+            getChildren(child).length === 0,
+            'segment "' + child.name + '" should not have any children'
+          )
+        }
+      } else if (typeof sequenceItem === 'object') {
+        assertSegments(child, sequenceItem, options)
+      }
+    }
+
+    // check if correct number of children was found
+    assert.equal(children.length, childCount)
+  } else {
+    for (let i = 0; i < expected.length; i++) {
+      const sequenceItem = expected[i]
+
+      if (typeof sequenceItem === 'string') {
+        // find corresponding child in parent
+        for (let j = 0; j < parent.children.length; j++) {
+          if (parent.children[j].name === sequenceItem) {
+            child = parent.children[j]
+          }
+        }
+        assert.ok(child, 'segment "' + parent.name + '" should have child "' + sequenceItem + '"')
+        if (typeof expected[i + 1] === 'object') {
+          assertSegments(child, expected[i + 1], exact)
+        }
+      }
+    }
+  }
+}
+
 /**
  * Like `tap.prototype.match`. Verifies that `actual` satisfies the shape
  * provided by `expected`.
@@ -147,6 +235,7 @@ function match(actual, expected) {
 module.exports = {
   assertCLMAttrs,
   assertExactClmAttrs,
+  assertSegments,
   compareSegments,
   isNonWritable,
   match

test/lib/metrics_helper.js

@@ -161,7 +161,6 @@ function assertSegments(parent, expected, options) {
         // If the next expected item is not array, then check that the current
         // child has no children
         if (!Array.isArray(expected[i + 1])) {
-          // var children = child.children
           this.ok(
             getChildren(child).length === 0,
             'segment "' + child.name + '" should not have any children'

test/unit/config/build-instrumentation-config.test.js

@@ -0,0 +1,20 @@
+/*
+ * Copyright 2022 New Relic Corporation. All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+'use strict'
+
+const test = require('node:test')
+const assert = require('node:assert')
+
+test('should default the instrumentation stanza', () => {
+  const { boolean } = require('../../../lib/config/formatters')
+  const pkgs = require('../../../lib/config/build-instrumentation-config')
+  const instrumentation = require('../../../lib/instrumentations')()
+  const pkgNames = Object.keys(instrumentation)
+
+  pkgNames.forEach((pkg) => {
+    assert.deepEqual(pkgs[pkg], { enabled: { formatter: boolean, default: true } })
+  })
+})

test/unit/config/config-defaults.test.js

@@ -262,4 +262,10 @@ test('with default properties', async (t) => {
     assert.equal(configuration.ai_monitoring.enabled, false)
     assert.equal(configuration.ai_monitoring.streaming.enabled, true)
   })
+
+  await t.test('instrumentation defaults', () => {
+    assert.equal(configuration.instrumentation.express.enabled, true)
+    assert.equal(configuration.instrumentation['@prisma/client'].enabled, true)
+    assert.equal(configuration.instrumentation.npmlog.enabled, true)
+  })
 })

test/unit/config/config-env.test.js

@@ -768,5 +768,19 @@ test('when overriding configuration values via environment variables', async (t)
         end()
       })
     })
+
+    await t.test('should convert NEW_RELIC_INSTRUMENTATION*  accordingly', (t, end) => {
+      const env = {
+        NEW_RELIC_INSTRUMENTATION_IOREDIS_ENABLED: 'false',
+        ['NEW_RELIC_INSTRUMENTATION_@GRPC/GRPC-JS_ENABLED']: 'false',
+        NEW_RELIC_INSTRUMENTATION_KNEX_ENABLED: 'false'
+      }
+      idempotentEnv(env, (config) => {
+        assert.equal(config.instrumentation.ioredis.enabled, false)
+        assert.equal(config.instrumentation['@grpc/grpc-js'].enabled, false)
+        assert.equal(config.instrumentation.knex.enabled, false)
+        end()
+      })
+    })
   }
 })

test/versioned/disabled-instrumentation/disabled-express.test.js

@@ -0,0 +1,56 @@
+/*
+ * Copyright 2024 New Relic Corporation. All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+'use strict'
+const assert = require('node:assert')
+const test = require('node:test')
+const helper = require('../../lib/agent_helper')
+const http = require('http')
+const params = require('../../lib/params')
+const { assertSegments } = require('../../lib/custom-assertions')
+
+test('should still record child segments if express instrumentation is disabled', async (t) => {
+  const agent = helper.instrumentMockedAgent({
+    instrumentation: {
+      express: {
+        enabled: false
+      }
+    }
+  })
+  const express = require('express')
+  const app = express()
+  const Redis = require('ioredis')
+  const client = new Redis(params.redis_port, params.redis_host)
+
+  app.get('/test-me', (_req, res) => {
+    client.get('foo', (err) => {
+      assert.equal(err, undefined)
+      res.end()
+    })
+  })
+
+  const promise = new Promise((resolve) => {
+    agent.on('transactionFinished', (tx) => {
+      assert.equal(tx.name, 'WebTransaction/NormalizedUri/*', 'should not name transactions')
+      const rootSegment = tx.trace.root
+      const expectedSegments = ['WebTransaction/NormalizedUri/*', ['Datastore/operation/Redis/get']]
+      assertSegments(rootSegment, expectedSegments)
+      resolve()
+    })
+  })
+
+  const server = app.listen(() => {
+    const { port } = server.address()
+    http.request({ port, path: '/test-me' }).end()
+  })
+
+  t.after(() => {
+    server.close()
+    client.disconnect()
+    helper.unloadAgent(agent)
+  })
+
+  await promise
+})

test/versioned/disabled-instrumentation/disabled-ioredis.test.js

@@ -0,0 +1,78 @@
+/*
+ * Copyright 2024 New Relic Corporation. All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+'use strict'
+const assert = require('node:assert')
+const test = require('node:test')
+const helper = require('../../lib/agent_helper')
+const params = require('../../lib/params')
+const { assertSegments } = require('../../lib/custom-assertions')
+const mongoCommon = require('../mongodb/common')
+
+test('Disabled PG scenarios', async (t) => {
+  t.beforeEach(async (ctx) => {
+    ctx.nr = {}
+    const agent = helper.instrumentMockedAgent({
+      instrumentation: {
+        ioredis: {
+          enabled: false
+        }
+      }
+    })
+    const Redis = require('ioredis')
+    const mongodb = require('mongodb')
+    const mongo = await mongoCommon.connect({ mongodb })
+    const collection = mongo.db.collection('disabled-inst-test')
+    const redisClient = new Redis(params.redis_port, params.redis_host)
+    await redisClient.select(1)
+    ctx.nr.redisClient = redisClient
+    ctx.nr.agent = agent
+    ctx.nr.collection = collection
+    ctx.nr.db = mongo.db
+    ctx.nr.mongoClient = mongo.client
+  })
+
+  t.afterEach(async (ctx) => {
+    const { agent, redisClient, mongoClient, db } = ctx.nr
+    await mongoCommon.close(mongoClient, db)
+    redisClient.disconnect()
+    helper.unloadAgent(agent)
+  })
+
+  await t.test('should record child segments if pg is disabled and using promises', async (t) => {
+    const { agent, redisClient, collection } = t.nr
+    await helper.runInTransaction(agent, async (tx) => {
+      await redisClient.get('foo')
+      await collection.countDocuments()
+      await redisClient.get('bar')
+      tx.end()
+      assertSegments(tx.trace.root, [
+        'Datastore/statement/MongoDB/disabled-inst-test/aggregate',
+        'Datastore/statement/MongoDB/disabled-inst-test/next'
+      ])
+    })
+  })
+
+  await t.test('should record child segments if pg is disabled and using callbacks', async (t) => {
+    const { agent, redisClient, collection } = t.nr
+    await helper.runInTransaction(agent, async (tx) => {
+      await new Promise((resolve) => {
+        redisClient.get('foo', async (err) => {
+          assert.equal(err, null)
+          await collection.countDocuments()
+          redisClient.get('bar', (innerErr) => {
+            tx.end()
+            assert.equal(innerErr, null)
+            assertSegments(tx.trace.root, [
+              'Datastore/statement/MongoDB/disabled-inst-test/aggregate',
+              'Datastore/statement/MongoDB/disabled-inst-test/next'
+            ])
+            resolve()
+          })
+        })
+      })
+    })
+  })
+})