Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Sep 10, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions lib/config/build-instrumentation-config.js
Original file line number Diff line number Diff line change
@@ -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
}, {})
9 changes: 8 additions & 1 deletion lib/config/default.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
bizob2828 marked this conversation as resolved.
Show resolved Hide resolved
})

/**
Expand Down
6 changes: 5 additions & 1 deletion lib/shimmer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
90 changes: 90 additions & 0 deletions test/lib/custom-assertions.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,95 @@ 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])) {
// var children = child.children
bizob2828 marked this conversation as resolved.
Show resolved Hide resolved
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`.
Expand Down Expand Up @@ -147,6 +236,7 @@ function match(actual, expected) {
module.exports = {
assertCLMAttrs,
assertExactClmAttrs,
assertSegments,
compareSegments,
isNonWritable,
match
Expand Down
20 changes: 20 additions & 0 deletions test/unit/config/build-instrumentation-config.test.js
Original file line number Diff line number Diff line change
@@ -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 } })
})
})
6 changes: 6 additions & 0 deletions test/unit/config/config-defaults.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
})
14 changes: 14 additions & 0 deletions test/unit/config/config-env.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
})
})
}
})
56 changes: 56 additions & 0 deletions test/versioned/disabled-instrumentation/disabled-express.test.js
Original file line number Diff line number Diff line change
@@ -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
})
78 changes: 78 additions & 0 deletions test/versioned/disabled-instrumentation/disabled-ioredis.test.js
Original file line number Diff line number Diff line change
@@ -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()
})
})
})
})
})
})
24 changes: 24 additions & 0 deletions test/versioned/disabled-instrumentation/newrelic.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/*
* Copyright 2020 New Relic Corporation. All rights reserved.
* SPDX-License-Identifier: Apache-2.0
*/

'use strict'

exports.config = {
app_name: ['My Application'],
license_key: 'license key here',
logging: {
level: 'trace'
},
utilization: {
detect_aws: false,
detect_pcf: false,
detect_azure: false,
detect_gcp: false,
detect_docker: false
},
transaction_tracer: {
enabled: true
}
}
22 changes: 22 additions & 0 deletions test/versioned/disabled-instrumentation/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
{
"name": "disabled-instrumentation-tests",
"targets": [],
"version": "0.0.0",
"private": true,
"tests": [
{
"engines": {
"node": ">=18"
},
"dependencies": {
"express": "latest",
bizob2828 marked this conversation as resolved.
Show resolved Hide resolved
"ioredis": "latest",
"mongodb": "latest"
},
"files": [
"disabled-express.test.js",
"disabled-ioredis.test.js"
]
}
]
}
Loading