diff --git a/packages/collector/test/tracing/databases/redis/app.js b/packages/collector/test/tracing/databases/redis/app.js index 446ea12222..31edf26acc 100644 --- a/packages/collector/test/tracing/databases/redis/app.js +++ b/packages/collector/test/tracing/databases/redis/app.js @@ -291,6 +291,18 @@ app.post('/two-different-target-hosts', async (req, res) => { } }); +app.get('/legacy-mode', async (req, res) => { + // legacy mode is redis v4 with callback style + connection.get('xxxx', (err, result) => { + if (err) { + log('Get with key xxxx failed', err); + res.sendStatus(500); + } else { + res.send(result); + } + }); +}); + app.listen(port, () => { log(`Listening on port: ${port}`); }); diff --git a/packages/collector/test/tracing/databases/redis/connect-via/default.js b/packages/collector/test/tracing/databases/redis/connect-via/default.js index 79f2e49bfd..5bb612b985 100644 --- a/packages/collector/test/tracing/databases/redis/connect-via/default.js +++ b/packages/collector/test/tracing/databases/redis/connect-via/default.js @@ -5,17 +5,18 @@ 'use strict'; module.exports = async function (redis, log) { - const client = redis.createClient({ url: `redis://${process.env.REDIS}` }); - const client2 = redis.createClient({ url: `redis://${process.env.REDIS_ALTERNATIVE}` }); + const legacyMode = process.env.REDIS_LEGACY_MODE === 'true'; + const client = redis.createClient({ url: `redis://${process.env.REDIS}`, legacyMode }); + const client2 = redis.createClient({ url: `redis://${process.env.REDIS_ALTERNATIVE}`, legacyMode }); [client, client2].forEach(c => { c.on('error', err => log('Redis Client Error', err)); }); - log('Connecting to client 1'); + log(`Connecting to client 1 (legacyMode: ${legacyMode})`); await client.connect(); log(`Connected to client 1 (${process.env.REDIS}).`); - log('Connecting to client 2'); + log(`Connecting to client 2 (legacyMode: ${legacyMode})`); await client2.connect(); log(`Connected to client 2 (${process.env.REDIS_ALTERNATIVE}).`); diff --git a/packages/collector/test/tracing/databases/redis/default_test.js b/packages/collector/test/tracing/databases/redis/default_test.js index 3dffd44eb6..5ba47a8c2f 100644 --- a/packages/collector/test/tracing/databases/redis/default_test.js +++ b/packages/collector/test/tracing/databases/redis/default_test.js @@ -8,4 +8,7 @@ const setupType = 'default'; describe(`tracing/redis/${setupType}`, function () { require('./test_definition').call(this, setupType); + + // legacy mode + require('./test_definition').call(this, setupType, true); }); diff --git a/packages/collector/test/tracing/databases/redis/test_definition.js b/packages/collector/test/tracing/databases/redis/test_definition.js index 535703b60c..71caf591fe 100644 --- a/packages/collector/test/tracing/databases/redis/test_definition.js +++ b/packages/collector/test/tracing/databases/redis/test_definition.js @@ -49,7 +49,7 @@ const versionsSinceV5 = ['latest', 'v5.8.3']; * node bin/start-test-containers.js --redis --redis-slave --redis-sentinel */ -module.exports = function (setupType) { +module.exports = function (setupType, legacyMode = false) { const allVersions = ['latest', 'v5.8.3', 'v4', 'v3']; let versionsToRun; @@ -78,7 +78,7 @@ module.exports = function (setupType) { } // The allowRootExitSpanApp is compatable with Redis v4 and v5 (latest). - if (redisVersion !== legacyVersion) { + if (redisVersion !== legacyVersion && legacyMode === false) { mochaSuiteFn('When allowRootExitSpan: true is set', function () { globalAgent.setUpCleanUpHooks(); let controls; @@ -126,7 +126,7 @@ module.exports = function (setupType) { // In v5, Redis moved "Isolation Pool" into RedisClientPool. // see: https://github.com/redis/node-redis/blob/master/docs/pool.md // Only for this test the connection is established via the pool. - if (versionsSinceV5.includes(redisVersion) && setupType === 'default') { + if (versionsSinceV5.includes(redisVersion) && setupType === 'default' && legacyMode === false) { mochaSuiteFn('When connected via clientpool', function () { globalAgent.setUpCleanUpHooks(); let controls; @@ -174,7 +174,8 @@ module.exports = function (setupType) { env: { REDIS_VERSION: redisVersion, REDIS_PKG: redisPkg, - REDIS_SETUP_TYPE: setupType + REDIS_SETUP_TYPE: setupType, + REDIS_LEGACY_MODE: legacyMode } }); @@ -200,6 +201,34 @@ module.exports = function (setupType) { await controls.clearIpcMessages(); }); + if (redisVersion === 'v4' && legacyMode === true) { + it('must trace legacy mode', async () => { + await controls.sendRequest({ + method: 'GET', + path: '/legacy-mode' + }); + + return retry(() => + agentControls.getSpans().then(spans => { + expect(spans.length).to.be.eql(2); + + expectAtLeastOneMatching(spans, [ + span => expect(span.n).to.equal('redis'), + span => expect(span.k).to.equal(constants.EXIT), + span => expect(span.f.e).to.equal(String(controls.getPid())), + span => expect(span.f.h).to.equal('agent-stub-uuid'), + span => expect(span.async).to.not.exist, + span => expect(span.error).to.not.exist, + span => expect(span.ec).to.equal(0), + span => expect(span.d).to.be.greaterThan(1) + ]); + }) + ); + }); + + return; + } + it('must trace set/get calls', () => controls .sendRequest({ diff --git a/packages/core/src/tracing/instrumentation/databases/redis.js b/packages/core/src/tracing/instrumentation/databases/redis.js index 809c667e6b..4f88ec8c85 100644 --- a/packages/core/src/tracing/instrumentation/databases/redis.js +++ b/packages/core/src/tracing/instrumentation/databases/redis.js @@ -182,7 +182,10 @@ function instrument(redis) { addressUrl = 'localhost:6379'; } - shimAllCommands(redisClient, addressUrl, false, redisCommandList); + // legacyMode is a Redis v4 option used to maintain backward-compatible command behavior + const isLegacyMode = redisClient.options?.legacyMode === true; + const cbStyle = !!isLegacyMode; + shimAllCommands(redisClient, addressUrl, cbStyle, redisCommandList); if (redisClient.multi) { shimmer.wrap(redisClient, 'multi', wrapMulti(addressUrl, false)); @@ -357,7 +360,7 @@ function instrumentCommand(original, command, address, cbStyle) { return error; }); } else { - // No promise returned, call onResult immediately to close the span + // UNKNOWN CASE onResult(); } return promise;