Skip to content

Commit bedcd11

Browse files
committed
ref(node): Remove remaining OTel tracing APIs from redis instrumentation
Drop the OTel context API from the v2-v3 path: translate the command callback binding to withActiveSpan(getActiveSpan()) and remove the createClient/create_stream patches that existed only to context.bind the client/stream event emitters (span creation already happens at command call time, and callback context is preserved by the command wrapper). Replace the OTel DiagLogger (_diag) with Sentry's debug logger, and switch the leaked OTel Span type in index.ts and types.ts to @sentry/core. SpanKind and the setTracerProvider base-class override are kept as part of the instrumentation/patching base.
1 parent 26bdeea commit bedcd11

3 files changed

Lines changed: 22 additions & 70 deletions

File tree

packages/node/src/integrations/tracing/redis/index.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
import type { Span } from '@opentelemetry/api';
2-
import type { IntegrationFn } from '@sentry/core';
1+
import type { IntegrationFn, Span } from '@sentry/core';
32
import {
43
defineIntegration,
54
SEMANTIC_ATTRIBUTE_CACHE_HIT,

packages/node/src/integrations/tracing/redis/vendored/redis-instrumentation.ts

Lines changed: 20 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -20,17 +20,20 @@
2020
* - Refactored to use Sentry's span APIs instead of OpenTelemetry tracing APIs
2121
*/
2222

23-
import { context, SpanKind } from '@opentelemetry/api';
24-
import type { DiagLogger, TracerProvider } from '@opentelemetry/api';
23+
import { SpanKind } from '@opentelemetry/api';
24+
import type { TracerProvider } from '@opentelemetry/api';
2525
import { InstrumentationBase, InstrumentationNodeModuleDefinition, isWrapped } from '@opentelemetry/instrumentation';
2626
import type { Span, SpanAttributes } from '@sentry/core';
2727
import {
28+
debug,
29+
getActiveSpan,
2830
SDK_VERSION,
2931
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
3032
SPAN_STATUS_ERROR,
3133
startInactiveSpan,
3234
withActiveSpan,
3335
} from '@sentry/core';
36+
import { DEBUG_BUILD } from '../../../../debug-build';
3437
import { InstrumentationNodeModuleFile } from '../../InstrumentationNodeModuleFile';
3538
import { defaultDbStatementSerializer } from './redis-common';
3639
import {
@@ -111,10 +114,7 @@ function runResponseHook(
111114

112115
// ---- v4-v5 utils ----
113116

114-
function removeCredentialsFromDBConnectionStringAttribute(
115-
diagLogger: DiagLogger,
116-
url: string | undefined,
117-
): string | undefined {
117+
function removeCredentialsFromDBConnectionStringAttribute(url: string | undefined): string | undefined {
118118
if (typeof url !== 'string' || !url) {
119119
return undefined;
120120
}
@@ -125,47 +125,21 @@ function removeCredentialsFromDBConnectionStringAttribute(
125125
u.password = '';
126126
return u.href;
127127
} catch (err) {
128-
diagLogger.error('failed to sanitize redis connection url', err);
128+
DEBUG_BUILD && debug.error('failed to sanitize redis connection url', err);
129129
}
130130
return undefined;
131131
}
132132

133-
function getClientAttributes(diagLogger: DiagLogger, options: any): SpanAttributes {
133+
function getClientAttributes(options: any): SpanAttributes {
134134
return {
135135
[ATTR_DB_SYSTEM]: DB_SYSTEM_VALUE_REDIS,
136136
[ATTR_NET_PEER_NAME]: options?.socket?.host,
137137
[ATTR_NET_PEER_PORT]: options?.socket?.port,
138-
[ATTR_DB_CONNECTION_STRING]: removeCredentialsFromDBConnectionStringAttribute(diagLogger, options?.url),
138+
[ATTR_DB_CONNECTION_STRING]: removeCredentialsFromDBConnectionStringAttribute(options?.url),
139139
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: ORIGIN,
140140
};
141141
}
142142

143-
// ---- v2-v3 utils ----
144-
145-
function getTracedCreateClient(original: Function): Function {
146-
return function createClientTrace(this: any) {
147-
const client = original.apply(this, arguments);
148-
return context.bind(context.active(), client);
149-
};
150-
}
151-
152-
function getTracedCreateStreamTrace(original: Function): Function {
153-
return function create_stream_trace(this: any) {
154-
if (!Object.prototype.hasOwnProperty.call(this, 'stream')) {
155-
Object.defineProperty(this, 'stream', {
156-
get() {
157-
return this._patched_redis_stream;
158-
},
159-
set(val: any) {
160-
context.bind(context.active(), val);
161-
this._patched_redis_stream = val;
162-
},
163-
});
164-
}
165-
return original.apply(this, arguments);
166-
};
167-
}
168-
169143
// ---- RedisInstrumentationV2_V3 ----
170144

171145
class RedisInstrumentationV2_V3 extends InstrumentationBase<RedisInstrumentationConfig> {
@@ -185,21 +159,11 @@ class RedisInstrumentationV2_V3 extends InstrumentationBase<RedisInstrumentation
185159
this._unwrap(moduleExports.RedisClient.prototype, 'internal_send_command');
186160
}
187161
this._wrap(moduleExports.RedisClient.prototype, 'internal_send_command', this._getPatchInternalSendCommand());
188-
if (isWrapped(moduleExports.RedisClient.prototype['create_stream'])) {
189-
this._unwrap(moduleExports.RedisClient.prototype, 'create_stream');
190-
}
191-
this._wrap(moduleExports.RedisClient.prototype, 'create_stream', this._getPatchCreateStream());
192-
if (isWrapped(moduleExports.createClient)) {
193-
this._unwrap(moduleExports, 'createClient');
194-
}
195-
this._wrap(moduleExports, 'createClient', this._getPatchCreateClient());
196162
return moduleExports;
197163
},
198164
(moduleExports: any) => {
199165
if (moduleExports === undefined) return;
200166
this._unwrap(moduleExports.RedisClient.prototype, 'internal_send_command');
201-
this._unwrap(moduleExports.RedisClient.prototype, 'create_stream');
202-
this._unwrap(moduleExports, 'createClient');
203167
},
204168
),
205169
];
@@ -231,11 +195,11 @@ class RedisInstrumentationV2_V3 extends InstrumentationBase<RedisInstrumentation
231195
});
232196
const originalCallback = arguments[0].callback;
233197
if (originalCallback) {
234-
const originalContext = context.active();
198+
const parentSpan = getActiveSpan();
235199
arguments[0].callback = function callback(this: any, err: Error | null, reply: unknown) {
236200
runResponseHook(instrumentation.getConfig().responseHook, span, cmd.command, cmd.args, reply);
237201
endSpan(span, err);
238-
return context.with(originalContext, originalCallback, this, ...arguments);
202+
return withActiveSpan(parentSpan ?? null, () => originalCallback.apply(this, arguments));
239203
};
240204
}
241205
try {
@@ -247,18 +211,6 @@ class RedisInstrumentationV2_V3 extends InstrumentationBase<RedisInstrumentation
247211
};
248212
};
249213
}
250-
251-
private _getPatchCreateClient() {
252-
return function createClient(original: Function) {
253-
return getTracedCreateClient(original);
254-
};
255-
}
256-
257-
private _getPatchCreateStream() {
258-
return function createReadStream(original: Function) {
259-
return getTracedCreateStreamTrace(original);
260-
};
261-
}
262214
}
263215

264216
// ---- RedisInstrumentationV4_V5 ----
@@ -284,7 +236,7 @@ class RedisInstrumentationV4_V5 extends InstrumentationBase<RedisInstrumentation
284236
(moduleExports: any, moduleVersion?: string) => {
285237
const transformCommandArguments = moduleExports.transformCommandArguments;
286238
if (!transformCommandArguments) {
287-
this._diag.error('internal instrumentation error, missing transformCommandArguments function');
239+
DEBUG_BUILD && debug.error('internal instrumentation error, missing transformCommandArguments function');
288240
return moduleExports;
289241
}
290242
const functionToPatch = moduleVersion?.startsWith('1.0.') ? 'extendWithCommands' : 'attachCommands';
@@ -413,7 +365,7 @@ class RedisInstrumentationV4_V5 extends InstrumentationBase<RedisInstrumentation
413365
return function execPatch(this: any) {
414366
const execRes = original.apply(this, arguments);
415367
if (typeof execRes?.then !== 'function') {
416-
plugin._diag.error('non-promise result when patching exec/execAsPipeline');
368+
DEBUG_BUILD && debug.error('non-promise result when patching exec/execAsPipeline');
417369
return execRes;
418370
}
419371
return execRes
@@ -425,7 +377,7 @@ class RedisInstrumentationV4_V5 extends InstrumentationBase<RedisInstrumentation
425377
.catch((err: any) => {
426378
const openSpans: OpenSpanInfo[] = this[OTEL_OPEN_SPANS];
427379
if (!openSpans) {
428-
plugin._diag.error('cannot find open spans to end for multi/pipeline');
380+
DEBUG_BUILD && debug.error('cannot find open spans to end for multi/pipeline');
429381
} else {
430382
const replies =
431383
err.constructor.name === 'MultiErrorReply'
@@ -468,10 +420,9 @@ class RedisInstrumentationV4_V5 extends InstrumentationBase<RedisInstrumentation
468420
}
469421

470422
private _getPatchedClientConnect() {
471-
const plugin = this;
472423
return function connectWrapper(original: Function) {
473424
return function patchedConnect(this: any) {
474-
const attributes = getClientAttributes(plugin._diag, this.options);
425+
const attributes = getClientAttributes(this.options);
475426
const span = startInactiveSpan({
476427
name: `${RedisInstrumentationV4_V5.COMPONENT}-connect`,
477428
kind: SpanKind.CLIENT,
@@ -501,7 +452,7 @@ class RedisInstrumentationV4_V5 extends InstrumentationBase<RedisInstrumentation
501452
const clientOptions = origThis.options || origThis[MULTI_COMMAND_OPTIONS];
502453
const commandName = redisCommandArguments[0] as string;
503454
const commandArgs = redisCommandArguments.slice(1);
504-
const attributes = getClientAttributes(this._diag, clientOptions);
455+
const attributes = getClientAttributes(clientOptions);
505456
const dbStatement = defaultDbStatementSerializer(commandName, commandArgs);
506457
if (dbStatement != null) {
507458
attributes[ATTR_DB_STATEMENT] = dbStatement;
@@ -535,10 +486,12 @@ class RedisInstrumentationV4_V5 extends InstrumentationBase<RedisInstrumentation
535486

536487
_endSpansWithRedisReplies(openSpans: OpenSpanInfo[] | undefined, replies: unknown[]): void {
537488
if (!openSpans) {
538-
return this._diag.error('cannot find open spans to end for redis multi/pipeline');
489+
DEBUG_BUILD && debug.error('cannot find open spans to end for redis multi/pipeline');
490+
return;
539491
}
540492
if (replies.length !== openSpans.length) {
541-
return this._diag.error('number of multi command spans does not match response from redis');
493+
DEBUG_BUILD && debug.error('number of multi command spans does not match response from redis');
494+
return;
542495
}
543496
for (let i = 0; i < openSpans.length; i++) {
544497
const { span, commandName, commandArgs } = openSpans[i]!;

packages/node/src/integrations/tracing/redis/vendored/types.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
*/
2121
/* eslint-disable -- vendored @opentelemetry/instrumentation-redis */
2222

23-
import type { Span } from '@opentelemetry/api';
23+
import type { Span } from '@sentry/core';
2424
import type { InstrumentationConfig } from '@opentelemetry/instrumentation';
2525

2626
// ---- redis types ----

0 commit comments

Comments
 (0)