From 66044df99bb1b6f359d844d65d190c599548ed6e Mon Sep 17 00:00:00 2001 From: Tom Z Date: Mon, 19 Jul 2021 17:53:53 +0300 Subject: [PATCH 1/6] fix: redis instrumentation loses context when using callbacks --- .../opentelemetry-instrumentation-redis/src/utils.ts | 6 +++++- .../test/redis.test.ts | 12 ++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/plugins/node/opentelemetry-instrumentation-redis/src/utils.ts b/plugins/node/opentelemetry-instrumentation-redis/src/utils.ts index 9ad22b2b43..a701e18bfa 100644 --- a/plugins/node/opentelemetry-instrumentation-redis/src/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-redis/src/utils.ts @@ -125,6 +125,7 @@ export const getTracedInternalSendCommand = ( ); } + const originalContext = context.active(); const originalCallback = arguments[0].callback; if (originalCallback) { (arguments[0] as RedisCommand).callback = function callback( @@ -146,7 +147,10 @@ export const getTracedInternalSendCommand = ( } endSpan(span, err); - return originalCallback.apply(this, arguments); + const callbackThis = this; + return context.with(originalContext, () => { + return originalCallback.apply(callbackThis, arguments); + }); }; } try { diff --git a/plugins/node/opentelemetry-instrumentation-redis/test/redis.test.ts b/plugins/node/opentelemetry-instrumentation-redis/test/redis.test.ts index 18b6be3107..038fc1211c 100644 --- a/plugins/node/opentelemetry-instrumentation-redis/test/redis.test.ts +++ b/plugins/node/opentelemetry-instrumentation-redis/test/redis.test.ts @@ -214,6 +214,18 @@ describe('redis@2.x', () => { }); }); + describe('Instrumenting redis when using callbacks', () => { + it('should contain the root span when inside callback', () => { + const rootSpan = tracer.startSpan('test span'); + context.with(trace.setSpan(context.active(), rootSpan), () => { + client.set('callbacksTestKey', 'value', () => { + const activeSpan = trace.getSpan(context.active()); + assert.strictEqual(activeSpan, rootSpan); + }); + }); + }); + }); + describe('Removing instrumentation', () => { before(() => { instrumentation.disable(); From c479e46b366594113a5e47698607fd299faf0c9b Mon Sep 17 00:00:00 2001 From: Tom Z Date: Mon, 19 Jul 2021 18:00:49 +0300 Subject: [PATCH 2/6] fix: rename test --- .../node/opentelemetry-instrumentation-redis/test/redis.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/node/opentelemetry-instrumentation-redis/test/redis.test.ts b/plugins/node/opentelemetry-instrumentation-redis/test/redis.test.ts index 038fc1211c..39dd6e5bd8 100644 --- a/plugins/node/opentelemetry-instrumentation-redis/test/redis.test.ts +++ b/plugins/node/opentelemetry-instrumentation-redis/test/redis.test.ts @@ -215,7 +215,7 @@ describe('redis@2.x', () => { }); describe('Instrumenting redis when using callbacks', () => { - it('should contain the root span when inside callback', () => { + it('should invoke callback with original command context', () => { const rootSpan = tracer.startSpan('test span'); context.with(trace.setSpan(context.active(), rootSpan), () => { client.set('callbacksTestKey', 'value', () => { From 5fe0bc253e913b00b40812a08f4caf7fba1bd8ea Mon Sep 17 00:00:00 2001 From: Tom Z Date: Mon, 19 Jul 2021 18:02:59 +0300 Subject: [PATCH 3/6] fix: merge new test into existing describe --- .../node/opentelemetry-instrumentation-redis/test/redis.test.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-redis/test/redis.test.ts b/plugins/node/opentelemetry-instrumentation-redis/test/redis.test.ts index 39dd6e5bd8..8805317191 100644 --- a/plugins/node/opentelemetry-instrumentation-redis/test/redis.test.ts +++ b/plugins/node/opentelemetry-instrumentation-redis/test/redis.test.ts @@ -212,9 +212,7 @@ describe('redis@2.x', () => { }); }); }); - }); - describe('Instrumenting redis when using callbacks', () => { it('should invoke callback with original command context', () => { const rootSpan = tracer.startSpan('test span'); context.with(trace.setSpan(context.active(), rootSpan), () => { From 89d8384a221f8edc1a92b06f8f1ec43a801d0df0 Mon Sep 17 00:00:00 2001 From: Tom Z Date: Mon, 19 Jul 2021 18:12:28 +0300 Subject: [PATCH 4/6] fix: refactor context.with as requested --- .../opentelemetry-instrumentation-redis/src/utils.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-redis/src/utils.ts b/plugins/node/opentelemetry-instrumentation-redis/src/utils.ts index a701e18bfa..b87b4a1cf4 100644 --- a/plugins/node/opentelemetry-instrumentation-redis/src/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-redis/src/utils.ts @@ -148,9 +148,12 @@ export const getTracedInternalSendCommand = ( endSpan(span, err); const callbackThis = this; - return context.with(originalContext, () => { - return originalCallback.apply(callbackThis, arguments); - }); + return context.with( + originalContext, + originalCallback, + callbackThis, + ...arguments + ); }; } try { From 860d515f924ba0eeaa2ea4d671090f167635cae2 Mon Sep 17 00:00:00 2001 From: Tom Z Date: Mon, 19 Jul 2021 18:15:16 +0300 Subject: [PATCH 5/6] fix: remove redundant callbackThis const --- plugins/node/opentelemetry-instrumentation-redis/src/utils.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-redis/src/utils.ts b/plugins/node/opentelemetry-instrumentation-redis/src/utils.ts index b87b4a1cf4..952bb1fdcc 100644 --- a/plugins/node/opentelemetry-instrumentation-redis/src/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-redis/src/utils.ts @@ -147,11 +147,10 @@ export const getTracedInternalSendCommand = ( } endSpan(span, err); - const callbackThis = this; return context.with( originalContext, originalCallback, - callbackThis, + this, ...arguments ); }; From d4c24fe8a3218376177dd5564908fd52e572bb67 Mon Sep 17 00:00:00 2001 From: Tom Z Date: Tue, 20 Jul 2021 12:15:16 +0300 Subject: [PATCH 6/6] fix: move originalContext to inside if --- plugins/node/opentelemetry-instrumentation-redis/src/utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/node/opentelemetry-instrumentation-redis/src/utils.ts b/plugins/node/opentelemetry-instrumentation-redis/src/utils.ts index 952bb1fdcc..52729c3df5 100644 --- a/plugins/node/opentelemetry-instrumentation-redis/src/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-redis/src/utils.ts @@ -125,9 +125,9 @@ export const getTracedInternalSendCommand = ( ); } - const originalContext = context.active(); const originalCallback = arguments[0].callback; if (originalCallback) { + const originalContext = context.active(); (arguments[0] as RedisCommand).callback = function callback( this: unknown, err: Error | null,