Skip to content

Commit

Permalink
fix: drop no-longer-necessary half-disabling of instrumentations (#2444)
Browse files Browse the repository at this point in the history
When disableInstrumentations support was added in #353 it only
half-disabled these instrumentations, briefly mentioning "continuation
patches". I believe this was about user-land callback queue handling
that should no longer be necessary after run-context and #2430 work.

In #2498 it was discussed and decided that if listed in
disableInstrumentations the agent should not touch the module at all.

Refs: #2498
  • Loading branch information
trentm authored Jan 31, 2022
1 parent 00fe15d commit dbaddc5
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 5 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,13 @@ Notes:
[float]
===== Breaking changes
* Change the `redis` and `mysql` instrumentations to not patch at all if
they are listed in <<disable-instrumentations, `disableInstrumentations`>>*.
This means that an application that uses one of these packages *and* lists
that package in `disableInstrumentations` could see changes in the async
run-context of callbacks. See {issues}2498[#2498] and the
<<release-notes-3.26.0,v3.26.0 release notes>> which has a similar change.
[float]
===== Features
Expand Down
7 changes: 4 additions & 3 deletions lib/instrumentation/modules/mysql.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ var symbols = require('../../symbols')
var { getDBDestination } = require('../context')

module.exports = function (mysql, agent, { version, enabled }) {
if (!enabled) {
return mysql
}
if (!semver.satisfies(version, '^2.0.0')) {
agent.logger.debug('mysql version %s not supported - aborting...', version)
return mysql
Expand All @@ -21,8 +24,6 @@ module.exports = function (mysql, agent, { version, enabled }) {
agent.logger.debug('shimming mysql.createPoolCluster')
shimmer.wrap(mysql, 'createPoolCluster', wrapCreatePoolCluster)

if (!enabled) return mysql

agent.logger.debug('shimming mysql.createConnection')
shimmer.wrap(mysql, 'createConnection', wrapCreateConnection)

Expand Down Expand Up @@ -75,7 +76,7 @@ module.exports = function (mysql, agent, { version, enabled }) {

if (typeof cb === 'function') {
arguments[0] = agent._instrumentation.bindFunction(function wrapedCallback (err, connection) { // eslint-disable-line handle-callback-err
if (connection && enabled) wrapQueryable(connection, 'getConnection() > connection', agent)
if (connection) wrapQueryable(connection, 'getConnection() > connection', agent)
return cb.apply(this, arguments)
})
}
Expand Down
7 changes: 5 additions & 2 deletions lib/instrumentation/modules/redis.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ const TYPE = 'cache'
const SUBTYPE = 'redis'

module.exports = function (redis, agent, { version, enabled }) {
if (!enabled) {
return redis
}
if (!semver.satisfies(version, '>=2.0.0 <4.0.0')) {
agent.logger.debug('redis version %s not supported - aborting...', version)
return redis
Expand Down Expand Up @@ -70,7 +73,7 @@ module.exports = function (redis, agent, { version, enabled }) {

const command = commandObj.command
agent.logger.debug({ command: command }, 'intercepted call to RedisClient.prototype.internal_send_command')
const span = enabled && ins.createSpan(command.toUpperCase(), TYPE, SUBTYPE)
const span = ins.createSpan(command.toUpperCase(), TYPE, SUBTYPE)
if (!span) {
return original.apply(this, arguments)
}
Expand Down Expand Up @@ -102,7 +105,7 @@ module.exports = function (redis, agent, { version, enabled }) {
}

agent.logger.debug({ command: command }, 'intercepted call to RedisClient.prototype.send_command')
var span = enabled && ins.createSpan(command.toUpperCase(), TYPE, SUBTYPE)
var span = ins.createSpan(command.toUpperCase(), TYPE, SUBTYPE)
if (!span) {
return original.apply(this, arguments)
}
Expand Down

0 comments on commit dbaddc5

Please sign in to comment.