From 58a83dee40157c418588891bb6ed74c1cdad2b92 Mon Sep 17 00:00:00 2001 From: kirrg001 Date: Tue, 20 Jan 2026 11:30:56 +0100 Subject: [PATCH 1/4] fix(redis): improved duration for legacy mode refs https://jsw.ibm.com/browse/INSTA-70365 https://github.com/redis/node-redis/blob/master/docs/v3-to-v4.md#legacy-mode --- .../test/tracing/databases/redis/app.js | 12 ++++++ .../databases/redis/connect-via/default.js | 9 +++-- .../tracing/databases/redis/default_test.js | 5 ++- .../databases/redis/test_definition.js | 38 +++++++++++++++++-- .../instrumentation/databases/redis.js | 6 ++- 5 files changed, 59 insertions(+), 11 deletions(-) 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..ad574ef1f0 100644 --- a/packages/collector/test/tracing/databases/redis/default_test.js +++ b/packages/collector/test/tracing/databases/redis/default_test.js @@ -6,6 +6,9 @@ const setupType = 'default'; -describe(`tracing/redis/${setupType}`, function () { +describe.only(`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..c5576acaf2 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,35 @@ 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), + // check duration + 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..5be781c1b5 100644 --- a/packages/core/src/tracing/instrumentation/databases/redis.js +++ b/packages/core/src/tracing/instrumentation/databases/redis.js @@ -182,7 +182,9 @@ function instrument(redis) { addressUrl = 'localhost:6379'; } - shimAllCommands(redisClient, addressUrl, false, redisCommandList); + 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 +359,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; From 761e7bcbdbbb3e9fcf4776ecb53fdcf8236a9854 Mon Sep 17 00:00:00 2001 From: kirrg001 Date: Tue, 20 Jan 2026 11:31:34 +0100 Subject: [PATCH 2/4] chore: lint --- packages/collector/test/tracing/databases/redis/default_test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/collector/test/tracing/databases/redis/default_test.js b/packages/collector/test/tracing/databases/redis/default_test.js index ad574ef1f0..5ba47a8c2f 100644 --- a/packages/collector/test/tracing/databases/redis/default_test.js +++ b/packages/collector/test/tracing/databases/redis/default_test.js @@ -6,7 +6,7 @@ const setupType = 'default'; -describe.only(`tracing/redis/${setupType}`, function () { +describe(`tracing/redis/${setupType}`, function () { require('./test_definition').call(this, setupType); // legacy mode From a8de24f05a95104169653218959f315e4228d428 Mon Sep 17 00:00:00 2001 From: kirrg001 Date: Tue, 20 Jan 2026 12:55:26 +0100 Subject: [PATCH 3/4] chore: comment cleanup --- .../collector/test/tracing/databases/redis/test_definition.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/collector/test/tracing/databases/redis/test_definition.js b/packages/collector/test/tracing/databases/redis/test_definition.js index c5576acaf2..71caf591fe 100644 --- a/packages/collector/test/tracing/databases/redis/test_definition.js +++ b/packages/collector/test/tracing/databases/redis/test_definition.js @@ -220,7 +220,6 @@ module.exports = function (setupType, legacyMode = false) { span => expect(span.async).to.not.exist, span => expect(span.error).to.not.exist, span => expect(span.ec).to.equal(0), - // check duration span => expect(span.d).to.be.greaterThan(1) ]); }) From f0210f2d14222b66ad8f7484bf1cb588755b28f7 Mon Sep 17 00:00:00 2001 From: kirrg001 Date: Tue, 20 Jan 2026 13:08:30 +0100 Subject: [PATCH 4/4] chore: suggestion Co-authored-by: Arya --- packages/core/src/tracing/instrumentation/databases/redis.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/core/src/tracing/instrumentation/databases/redis.js b/packages/core/src/tracing/instrumentation/databases/redis.js index 5be781c1b5..4f88ec8c85 100644 --- a/packages/core/src/tracing/instrumentation/databases/redis.js +++ b/packages/core/src/tracing/instrumentation/databases/redis.js @@ -182,6 +182,7 @@ function instrument(redis) { addressUrl = 'localhost:6379'; } + // 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);