Skip to content

Commit 5861dfa

Browse files
blumamirblumamir
authored
fix(redis): use new tracer after setTracerProvider (#2865)
1 parent 2317e2f commit 5861dfa

File tree

3 files changed

+147
-127
lines changed

3 files changed

+147
-127
lines changed

‎plugins/node/opentelemetry-instrumentation-redis/src/instrumentation.ts‎

Lines changed: 109 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,27 @@ import {
1818
isWrapped,
1919
InstrumentationBase,
2020
InstrumentationNodeModuleDefinition,
21+
safeExecuteInTheMiddle,
2122
} from '@opentelemetry/instrumentation';
2223
import {
24+
endSpan,
2325
getTracedCreateClient,
2426
getTracedCreateStreamTrace,
25-
getTracedInternalSendCommand,
2627
} from './utils';
27-
import { RedisInstrumentationConfig } from './types';
28+
import { RedisCommand, RedisInstrumentationConfig } from './types';
2829
/** @knipignore */
2930
import { PACKAGE_NAME, PACKAGE_VERSION } from './version';
31+
import { RedisPluginClientTypes } from './internal-types';
32+
import { SpanKind, context, trace } from '@opentelemetry/api';
33+
import {
34+
DBSYSTEMVALUES_REDIS,
35+
SEMATTRS_DB_CONNECTION_STRING,
36+
SEMATTRS_DB_STATEMENT,
37+
SEMATTRS_DB_SYSTEM,
38+
SEMATTRS_NET_PEER_NAME,
39+
SEMATTRS_NET_PEER_PORT,
40+
} from '@opentelemetry/semantic-conventions';
41+
import { defaultDbStatementSerializer } from '@opentelemetry/redis-common';
3042

3143
const DEFAULT_CONFIG: RedisInstrumentationConfig = {
3244
requireParentSpan: false,
@@ -96,28 +108,116 @@ export class RedisInstrumentation extends InstrumentationBase<RedisInstrumentati
96108
),
97109
];
98110
}
111+
99112
/**
100113
* Patch internal_send_command(...) to trace requests
101114
*/
102115
private _getPatchInternalSendCommand() {
103-
const tracer = this.tracer;
104-
const config = this.getConfig();
116+
const instrumentation = this;
105117
return function internal_send_command(original: Function) {
106-
return getTracedInternalSendCommand(tracer, original, config);
118+
return function internal_send_command_trace(
119+
this: RedisPluginClientTypes,
120+
cmd?: RedisCommand
121+
) {
122+
// Versions of redis (2.4+) use a single options object
123+
// instead of named arguments
124+
if (arguments.length !== 1 || typeof cmd !== 'object') {
125+
// We don't know how to trace this call, so don't start/stop a span
126+
return original.apply(this, arguments);
127+
}
128+
129+
const config = instrumentation.getConfig();
130+
131+
const hasNoParentSpan = trace.getSpan(context.active()) === undefined;
132+
if (config.requireParentSpan === true && hasNoParentSpan) {
133+
return original.apply(this, arguments);
134+
}
135+
136+
const dbStatementSerializer =
137+
config?.dbStatementSerializer || defaultDbStatementSerializer;
138+
const span = instrumentation.tracer.startSpan(
139+
`${RedisInstrumentation.COMPONENT}-${cmd.command}`,
140+
{
141+
kind: SpanKind.CLIENT,
142+
attributes: {
143+
[SEMATTRS_DB_SYSTEM]: DBSYSTEMVALUES_REDIS,
144+
[SEMATTRS_DB_STATEMENT]: dbStatementSerializer(
145+
cmd.command,
146+
cmd.args
147+
),
148+
},
149+
}
150+
);
151+
152+
// Set attributes for not explicitly typed RedisPluginClientTypes
153+
if (this.connection_options) {
154+
span.setAttributes({
155+
[SEMATTRS_NET_PEER_NAME]: this.connection_options.host,
156+
[SEMATTRS_NET_PEER_PORT]: this.connection_options.port,
157+
});
158+
}
159+
if (this.address) {
160+
span.setAttribute(
161+
SEMATTRS_DB_CONNECTION_STRING,
162+
`redis://${this.address}`
163+
);
164+
}
165+
166+
const originalCallback = arguments[0].callback;
167+
if (originalCallback) {
168+
const originalContext = context.active();
169+
(arguments[0] as RedisCommand).callback = function callback<T>(
170+
this: unknown,
171+
err: Error | null,
172+
reply: T
173+
) {
174+
if (config?.responseHook) {
175+
const responseHook = config.responseHook;
176+
safeExecuteInTheMiddle(
177+
() => {
178+
responseHook(span, cmd.command, cmd.args, reply);
179+
},
180+
err => {
181+
if (err) {
182+
instrumentation._diag.error(
183+
'Error executing responseHook',
184+
err
185+
);
186+
}
187+
},
188+
true
189+
);
190+
}
191+
192+
endSpan(span, err);
193+
return context.with(
194+
originalContext,
195+
originalCallback,
196+
this,
197+
...arguments
198+
);
199+
};
200+
}
201+
try {
202+
// Span will be ended in callback
203+
return original.apply(this, arguments);
204+
} catch (rethrow: any) {
205+
endSpan(span, rethrow);
206+
throw rethrow; // rethrow after ending span
207+
}
208+
};
107209
};
108210
}
109211

110212
private _getPatchCreateClient() {
111-
const tracer = this.tracer;
112213
return function createClient(original: Function) {
113-
return getTracedCreateClient(tracer, original);
214+
return getTracedCreateClient(original);
114215
};
115216
}
116217

117218
private _getPatchCreateStream() {
118-
const tracer = this.tracer;
119219
return function createReadStream(original: Function) {
120-
return getTracedCreateStreamTrace(tracer, original);
220+
return getTracedCreateStreamTrace(original);
121221
};
122222
}
123223
}

‎plugins/node/opentelemetry-instrumentation-redis/src/utils.ts‎

Lines changed: 4 additions & 118 deletions
Original file line numberDiff line numberDiff line change
@@ -15,31 +15,10 @@
1515
*/
1616

1717
import type * as redisTypes from 'redis';
18-
import {
19-
context,
20-
Tracer,
21-
SpanKind,
22-
Span,
23-
SpanStatusCode,
24-
trace,
25-
diag,
26-
} from '@opentelemetry/api';
27-
import { RedisCommand, RedisInstrumentationConfig } from './types';
18+
import { context, Span, SpanStatusCode } from '@opentelemetry/api';
2819
import { EventEmitter } from 'events';
29-
import { RedisInstrumentation } from './';
30-
import {
31-
DBSYSTEMVALUES_REDIS,
32-
SEMATTRS_DB_CONNECTION_STRING,
33-
SEMATTRS_DB_STATEMENT,
34-
SEMATTRS_DB_SYSTEM,
35-
SEMATTRS_NET_PEER_NAME,
36-
SEMATTRS_NET_PEER_PORT,
37-
} from '@opentelemetry/semantic-conventions';
38-
import { safeExecuteInTheMiddle } from '@opentelemetry/instrumentation';
39-
import { RedisPluginClientTypes } from './internal-types';
40-
import { defaultDbStatementSerializer } from '@opentelemetry/redis-common';
4120

42-
const endSpan = (span: Span, err?: Error | null) => {
21+
export const endSpan = (span: Span, err?: Error | null) => {
4322
if (err) {
4423
span.setStatus({
4524
code: SpanStatusCode.ERROR,
@@ -49,17 +28,14 @@ const endSpan = (span: Span, err?: Error | null) => {
4928
span.end();
5029
};
5130

52-
export const getTracedCreateClient = (tracer: Tracer, original: Function) => {
31+
export const getTracedCreateClient = (original: Function) => {
5332
return function createClientTrace(this: redisTypes.RedisClient) {
5433
const client: redisTypes.RedisClient = original.apply(this, arguments);
5534
return context.bind(context.active(), client);
5635
};
5736
};
5837

59-
export const getTracedCreateStreamTrace = (
60-
tracer: Tracer,
61-
original: Function
62-
) => {
38+
export const getTracedCreateStreamTrace = (original: Function) => {
6339
return function create_stream_trace(this: redisTypes.RedisClient) {
6440
if (!Object.prototype.hasOwnProperty.call(this, 'stream')) {
6541
Object.defineProperty(this, 'stream', {
@@ -75,93 +51,3 @@ export const getTracedCreateStreamTrace = (
7551
return original.apply(this, arguments);
7652
};
7753
};
78-
79-
export const getTracedInternalSendCommand = (
80-
tracer: Tracer,
81-
original: Function,
82-
config?: RedisInstrumentationConfig
83-
) => {
84-
return function internal_send_command_trace(
85-
this: RedisPluginClientTypes,
86-
cmd?: RedisCommand
87-
) {
88-
// New versions of redis (2.4+) use a single options object
89-
// instead of named arguments
90-
if (arguments.length !== 1 || typeof cmd !== 'object') {
91-
// We don't know how to trace this call, so don't start/stop a span
92-
return original.apply(this, arguments);
93-
}
94-
95-
const hasNoParentSpan = trace.getSpan(context.active()) === undefined;
96-
if (config?.requireParentSpan === true && hasNoParentSpan) {
97-
return original.apply(this, arguments);
98-
}
99-
100-
const dbStatementSerializer =
101-
config?.dbStatementSerializer || defaultDbStatementSerializer;
102-
const span = tracer.startSpan(
103-
`${RedisInstrumentation.COMPONENT}-${cmd.command}`,
104-
{
105-
kind: SpanKind.CLIENT,
106-
attributes: {
107-
[SEMATTRS_DB_SYSTEM]: DBSYSTEMVALUES_REDIS,
108-
[SEMATTRS_DB_STATEMENT]: dbStatementSerializer(cmd.command, cmd.args),
109-
},
110-
}
111-
);
112-
113-
// Set attributes for not explicitly typed RedisPluginClientTypes
114-
if (this.connection_options) {
115-
span.setAttributes({
116-
[SEMATTRS_NET_PEER_NAME]: this.connection_options.host,
117-
[SEMATTRS_NET_PEER_PORT]: this.connection_options.port,
118-
});
119-
}
120-
if (this.address) {
121-
span.setAttribute(
122-
SEMATTRS_DB_CONNECTION_STRING,
123-
`redis://${this.address}`
124-
);
125-
}
126-
127-
const originalCallback = arguments[0].callback;
128-
if (originalCallback) {
129-
const originalContext = context.active();
130-
(arguments[0] as RedisCommand).callback = function callback<T>(
131-
this: unknown,
132-
err: Error | null,
133-
reply: T
134-
) {
135-
if (config?.responseHook) {
136-
const responseHook = config.responseHook;
137-
safeExecuteInTheMiddle(
138-
() => {
139-
responseHook(span, cmd.command, cmd.args, reply);
140-
},
141-
err => {
142-
if (err) {
143-
diag.error('Error executing responseHook', err);
144-
}
145-
},
146-
true
147-
);
148-
}
149-
150-
endSpan(span, err);
151-
return context.with(
152-
originalContext,
153-
originalCallback,
154-
this,
155-
...arguments
156-
);
157-
};
158-
}
159-
try {
160-
// Span will be ended in callback
161-
return original.apply(this, arguments);
162-
} catch (rethrow: any) {
163-
endSpan(span, rethrow);
164-
throw rethrow; // rethrow after ending span
165-
}
166-
};
167-
};

‎plugins/node/opentelemetry-instrumentation-redis/test/redis.test.ts‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,9 @@ describe('[email protected]', () => {
8080
beforeEach(() => {
8181
contextManager = new AsyncLocalStorageContextManager().enable();
8282
context.setGlobalContextManager(contextManager);
83+
// set the default tracer provider before each test
84+
// specific ones can override it to assert certain things
85+
instrumentation.setTracerProvider(provider);
8386
});
8487

8588
afterEach(() => {
@@ -389,5 +392,36 @@ describe('[email protected]', () => {
389392
});
390393
});
391394
});
395+
396+
describe('setTracerProvider', () => {
397+
before(() => {
398+
instrumentation.disable();
399+
instrumentation.setConfig({});
400+
instrumentation.enable();
401+
});
402+
403+
it('should use new tracer provider after setTracerProvider is called', done => {
404+
const testSpecificMemoryExporter = new InMemorySpanExporter();
405+
const spanProcessor = new SimpleSpanProcessor(
406+
testSpecificMemoryExporter
407+
);
408+
const tracerProvider = new NodeTracerProvider({
409+
spanProcessors: [spanProcessor],
410+
});
411+
412+
// key point of this test, setting new tracer provider and making sure
413+
// new spans use it.
414+
instrumentation.setTracerProvider(tracerProvider);
415+
416+
client.set('test', 'value-with-new-tracer-provider', err => {
417+
assert.ifError(err);
418+
// assert that the span was exported by the new tracer provider
419+
// which is using the test specific span processor
420+
const spans = testSpecificMemoryExporter.getFinishedSpans();
421+
assert.strictEqual(spans.length, 1);
422+
done();
423+
});
424+
});
425+
});
392426
});
393427
});

0 commit comments

Comments
 (0)