diff --git a/plugins/node/opentelemetry-instrumentation-ioredis/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-ioredis/src/instrumentation.ts index 7c74454c55..d3a26a9ec3 100644 --- a/plugins/node/opentelemetry-instrumentation-ioredis/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-ioredis/src/instrumentation.ts @@ -14,15 +14,17 @@ * limitations under the License. */ -import { diag } from '@opentelemetry/api'; +import { diag, trace, context, SpanKind } from '@opentelemetry/api'; import type * as ioredisTypes from 'ioredis'; import { InstrumentationBase, InstrumentationNodeModuleDefinition, isWrapped, } from '@opentelemetry/instrumentation'; -import { IORedisInstrumentationConfig } from './types'; -import { traceConnection, traceSendCommand } from './utils'; +import { IORedisInstrumentationConfig, IORedisCommand } from './types'; +import { SemanticAttributes } from '@opentelemetry/semantic-conventions'; +import { safeExecuteInTheMiddle } from '@opentelemetry/instrumentation'; +import { endSpan, defaultDbStatementSerializer } from './utils'; import { VERSION } from './version'; const DEFAULT_CONFIG: IORedisInstrumentationConfig = { @@ -82,18 +84,127 @@ export class IORedisInstrumentation extends InstrumentationBase< */ private _patchSendCommand(moduleVersion?: string) { return (original: Function) => { - return traceSendCommand( - this.tracer, - original, - this._config, - moduleVersion - ); + return this.traceSendCommand(original, moduleVersion); }; } private _patchConnection() { return (original: Function) => { - return traceConnection(this.tracer, original); + return this.traceConnection(original); }; } + + private traceSendCommand = (original: Function, moduleVersion?: string) => { + const instrumentation = this; + return function (this: ioredisTypes.Redis, cmd?: IORedisCommand) { + if (arguments.length < 1 || typeof cmd !== 'object') { + return original.apply(this, arguments); + } + const config = + instrumentation.getConfig() as IORedisInstrumentationConfig; + const dbStatementSerializer = + config?.dbStatementSerializer || defaultDbStatementSerializer; + + const hasNoParentSpan = trace.getSpan(context.active()) === undefined; + if (config?.requireParentSpan === true && hasNoParentSpan) { + return original.apply(this, arguments); + } + + const span = instrumentation.tracer.startSpan(cmd.name, { + kind: SpanKind.CLIENT, + attributes: { + [SemanticAttributes.DB_SYSTEM]: IORedisInstrumentation.DB_SYSTEM, + [SemanticAttributes.DB_STATEMENT]: dbStatementSerializer( + cmd.name, + cmd.args + ), + }, + }); + + if (config?.requestHook) { + safeExecuteInTheMiddle( + () => + config?.requestHook!(span, { + moduleVersion, + cmdName: cmd.name, + cmdArgs: cmd.args, + }), + e => { + if (e) { + diag.error('ioredis instrumentation: request hook failed', e); + } + }, + true + ); + } + + const { host, port } = this.options; + + span.setAttributes({ + [SemanticAttributes.NET_PEER_NAME]: host, + [SemanticAttributes.NET_PEER_PORT]: port, + [SemanticAttributes.NET_PEER_IP]: `redis://${host}:${port}`, + }); + + try { + const result = original.apply(this, arguments); + + const origResolve = cmd.resolve; + /* eslint-disable @typescript-eslint/no-explicit-any */ + cmd.resolve = function (result: any) { + safeExecuteInTheMiddle( + () => config?.responseHook?.(span, cmd.name, cmd.args, result), + e => { + if (e) { + diag.error('ioredis instrumentation: response hook failed', e); + } + }, + true + ); + + endSpan(span, null); + origResolve(result); + }; + + const origReject = cmd.reject; + cmd.reject = function (err: Error) { + endSpan(span, err); + origReject(err); + }; + + return result; + } catch (error) { + endSpan(span, error); + throw error; + } + }; + }; + + private traceConnection = (original: Function) => { + const instrumentation = this; + return function (this: ioredisTypes.Redis) { + const span = instrumentation.tracer.startSpan('connect', { + kind: SpanKind.CLIENT, + attributes: { + [SemanticAttributes.DB_SYSTEM]: IORedisInstrumentation.DB_SYSTEM, + [SemanticAttributes.DB_STATEMENT]: 'connect', + }, + }); + const { host, port } = this.options; + + span.setAttributes({ + [SemanticAttributes.NET_PEER_NAME]: host, + [SemanticAttributes.NET_PEER_PORT]: port, + [SemanticAttributes.NET_PEER_IP]: `redis://${host}:${port}`, + }); + try { + const client = original.apply(this, arguments); + endSpan(span, null); + return client; + } catch (error) { + endSpan(span, error); + throw error; + } + }; + }; } diff --git a/plugins/node/opentelemetry-instrumentation-ioredis/src/utils.ts b/plugins/node/opentelemetry-instrumentation-ioredis/src/utils.ts index 036b95cfae..dc52e6e4a8 100644 --- a/plugins/node/opentelemetry-instrumentation-ioredis/src/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-ioredis/src/utils.ts @@ -14,26 +14,13 @@ * limitations under the License. */ -import type * as ioredisTypes from 'ioredis'; -import { - Tracer, - SpanKind, - Span, - SpanStatusCode, - trace, - context, - diag, -} from '@opentelemetry/api'; -import { - IORedisCommand, - IORedisInstrumentationConfig, - DbStatementSerializer, -} from './types'; -import { IORedisInstrumentation } from './'; -import { SemanticAttributes } from '@opentelemetry/semantic-conventions'; -import { safeExecuteInTheMiddle } from '@opentelemetry/instrumentation'; +import { Span, SpanStatusCode } from '@opentelemetry/api'; +import { DbStatementSerializer } from './types'; -const endSpan = (span: Span, err: NodeJS.ErrnoException | null | undefined) => { +export const endSpan = ( + span: Span, + err: NodeJS.ErrnoException | null | undefined +) => { if (err) { span.recordException(err); span.setStatus({ @@ -44,125 +31,10 @@ const endSpan = (span: Span, err: NodeJS.ErrnoException | null | undefined) => { span.end(); }; -export const traceConnection = (tracer: Tracer, original: Function) => { - return function (this: ioredisTypes.Redis) { - const span = tracer.startSpan('connect', { - kind: SpanKind.CLIENT, - attributes: { - [SemanticAttributes.DB_SYSTEM]: IORedisInstrumentation.DB_SYSTEM, - [SemanticAttributes.DB_STATEMENT]: 'connect', - }, - }); - const { host, port } = this.options; - - span.setAttributes({ - [SemanticAttributes.NET_PEER_NAME]: host, - [SemanticAttributes.NET_PEER_PORT]: port, - [SemanticAttributes.NET_PEER_IP]: `redis://${host}:${port}`, - }); - try { - const client = original.apply(this, arguments); - endSpan(span, null); - return client; - } catch (error) { - endSpan(span, error); - throw error; - } - }; -}; - -const defaultDbStatementSerializer: DbStatementSerializer = ( +export const defaultDbStatementSerializer: DbStatementSerializer = ( cmdName, cmdArgs ) => Array.isArray(cmdArgs) && cmdArgs.length ? `${cmdName} ${cmdArgs.join(' ')}` : cmdName; - -export const traceSendCommand = ( - tracer: Tracer, - original: Function, - config?: IORedisInstrumentationConfig, - moduleVersion?: string -) => { - const dbStatementSerializer = - config?.dbStatementSerializer || defaultDbStatementSerializer; - return function (this: ioredisTypes.Redis, cmd?: IORedisCommand) { - if (arguments.length < 1 || typeof cmd !== 'object') { - return original.apply(this, arguments); - } - - const hasNoParentSpan = trace.getSpan(context.active()) === undefined; - if (config?.requireParentSpan === true && hasNoParentSpan) { - return original.apply(this, arguments); - } - - const span = tracer.startSpan(cmd.name, { - kind: SpanKind.CLIENT, - attributes: { - [SemanticAttributes.DB_SYSTEM]: IORedisInstrumentation.DB_SYSTEM, - [SemanticAttributes.DB_STATEMENT]: dbStatementSerializer( - cmd.name, - cmd.args - ), - }, - }); - - if (config?.requestHook) { - safeExecuteInTheMiddle( - () => - config?.requestHook!(span, { - moduleVersion, - cmdName: cmd.name, - cmdArgs: cmd.args, - }), - e => { - if (e) { - diag.error('ioredis instrumentation: request hook failed', e); - } - }, - true - ); - } - - const { host, port } = this.options; - - span.setAttributes({ - [SemanticAttributes.NET_PEER_NAME]: host, - [SemanticAttributes.NET_PEER_PORT]: port, - [SemanticAttributes.NET_PEER_IP]: `redis://${host}:${port}`, - }); - - try { - const result = original.apply(this, arguments); - - const origResolve = cmd.resolve; - /* eslint-disable @typescript-eslint/no-explicit-any */ - cmd.resolve = function (result: any) { - safeExecuteInTheMiddle( - () => config?.responseHook?.(span, cmd.name, cmd.args, result), - e => { - if (e) { - diag.error('ioredis instrumentation: response hook failed', e); - } - }, - true - ); - - endSpan(span, null); - origResolve(result); - }; - - const origReject = cmd.reject; - cmd.reject = function (err: Error) { - endSpan(span, err); - origReject(err); - }; - - return result; - } catch (error) { - endSpan(span, error); - throw error; - } - }; -}; diff --git a/plugins/node/opentelemetry-instrumentation-ioredis/test/ioredis.test.ts b/plugins/node/opentelemetry-instrumentation-ioredis/test/ioredis.test.ts index fab08a76c6..d3dceb029e 100644 --- a/plugins/node/opentelemetry-instrumentation-ioredis/test/ioredis.test.ts +++ b/plugins/node/opentelemetry-instrumentation-ioredis/test/ioredis.test.ts @@ -548,11 +548,10 @@ describe('ioredis', () => { }); it('should create a child span for lua', done => { - instrumentation = new IORedisInstrumentation({ + const config: IORedisInstrumentationConfig = { requireParentSpan: false, - }); - instrumentation.setTracerProvider(provider); - require('ioredis'); + }; + instrumentation.setConfig(config); const attributes = { ...DEFAULT_ATTRIBUTES, @@ -624,13 +623,10 @@ describe('ioredis', () => { describe('Instrumenting without parent span', () => { before(() => { - instrumentation.disable(); - instrumentation = new IORedisInstrumentation({ + const config: IORedisInstrumentationConfig = { requireParentSpan: true, - }); - instrumentation.setTracerProvider(provider); - require('ioredis'); - instrumentation.enable(); + }; + instrumentation.setConfig(config); }); it('should not create child span', async () => { await client.set(testKeyName, 'data'); @@ -642,13 +638,10 @@ describe('ioredis', () => { describe('Instrumentation with requireParentSpan', () => { it('should instrument with requireParentSpan equal false', async () => { - instrumentation.disable(); const config: IORedisInstrumentationConfig = { requireParentSpan: false, }; - instrumentation = new IORedisInstrumentation(config); - instrumentation.setTracerProvider(provider); - require('ioredis'); + instrumentation.setConfig(config); await client.set(testKeyName, 'data'); const result = await client.del(testKeyName); @@ -670,13 +663,10 @@ describe('ioredis', () => { }); it('should not instrument with requireParentSpan equal true', async () => { - instrumentation.disable(); const config: IORedisInstrumentationConfig = { requireParentSpan: true, }; - instrumentation = new IORedisInstrumentation(config); - instrumentation.setTracerProvider(provider); - require('ioredis'); + instrumentation.setConfig(config); await client.set(testKeyName, 'data'); const result = await client.del(testKeyName); @@ -690,13 +680,10 @@ describe('ioredis', () => { const dbStatementSerializer: DbStatementSerializer = (cmdName, cmdArgs) => `FOOBAR_${cmdName}: ${cmdArgs[0]}`; before(() => { - instrumentation.disable(); const config: IORedisInstrumentationConfig = { dbStatementSerializer, }; - instrumentation = new IORedisInstrumentation(config); - instrumentation.setTracerProvider(provider); - require('ioredis'); + instrumentation.setConfig(config); }); IOREDIS_CALLBACK_OPERATIONS.forEach(command => { @@ -776,8 +763,14 @@ describe('ioredis', () => { }); describe('Instrumenting with a custom hooks', () => { - it('should call requestHook when set in config', async () => { + before(() => { instrumentation.disable(); + instrumentation = new IORedisInstrumentation(); + instrumentation.setTracerProvider(provider); + require('ioredis'); + }); + + it('should call requestHook when set in config', async () => { const config: IORedisInstrumentationConfig = { requestHook: ( span: Span, @@ -797,9 +790,7 @@ describe('ioredis', () => { ); }, }; - instrumentation = new IORedisInstrumentation(config); - instrumentation.setTracerProvider(provider); - require('ioredis'); + instrumentation.setConfig(config); const span = provider.getTracer('ioredis-test').startSpan('test span'); await context.with(trace.setSpan(context.active(), span), async () => { @@ -814,7 +805,6 @@ describe('ioredis', () => { }); it('should ignore requestHook which throws exception', async () => { - instrumentation.disable(); const config: IORedisInstrumentationConfig = { requestHook: ( span: Span, @@ -827,9 +817,7 @@ describe('ioredis', () => { throw Error('error thrown in requestHook'); }, }; - instrumentation = new IORedisInstrumentation(config); - instrumentation.setTracerProvider(provider); - require('ioredis'); + instrumentation.setConfig(config); const span = provider.getTracer('ioredis-test').startSpan('test span'); await context.with(trace.setSpan(context.active(), span), async () => { @@ -844,7 +832,6 @@ describe('ioredis', () => { }); it('should call responseHook when set in config', async () => { - instrumentation.disable(); const config: IORedisInstrumentationConfig = { responseHook: ( span: Span, @@ -861,9 +848,7 @@ describe('ioredis', () => { ); }, }; - instrumentation = new IORedisInstrumentation(config); - instrumentation.setTracerProvider(provider); - require('ioredis'); + instrumentation.setConfig(config); const span = provider.getTracer('ioredis-test').startSpan('test span'); await context.with(trace.setSpan(context.active(), span), async () => { @@ -878,7 +863,6 @@ describe('ioredis', () => { }); it('should ignore responseHook which throws exception', async () => { - instrumentation.disable(); const config: IORedisInstrumentationConfig = { responseHook: ( _span: Span, @@ -889,9 +873,7 @@ describe('ioredis', () => { throw Error('error thrown in responseHook'); }, }; - instrumentation = new IORedisInstrumentation(config); - instrumentation.setTracerProvider(provider); - require('ioredis'); + instrumentation.setConfig(config); const span = provider.getTracer('ioredis-test').startSpan('test span'); await context.with(trace.setSpan(context.active(), span), async () => { @@ -903,5 +885,47 @@ describe('ioredis', () => { }); }); }); + + describe('setConfig - custom dbStatementSerializer config', () => { + const dbStatementSerializer = ( + cmdName: string, + cmdArgs: Array + ) => { + return Array.isArray(cmdArgs) && cmdArgs.length + ? `FooBar_${cmdName} ${cmdArgs.join(',')}` + : cmdName; + }; + const config: IORedisInstrumentationConfig = { + dbStatementSerializer: dbStatementSerializer, + }; + before(() => { + instrumentation.setConfig(config); + }); + + IOREDIS_CALLBACK_OPERATIONS.forEach(operation => { + it(`should properly execute the db statement serializer for operation ${operation.description}`, done => { + const span = provider + .getTracer('ioredis-test') + .startSpan('test span'); + context.with(trace.setSpan(context.active(), span), () => { + operation.method((err, _) => { + assert.ifError(err); + span.end(); + const endedSpans = memoryExporter.getFinishedSpans(); + assert.strictEqual(endedSpans.length, 2); + const expectedStatement = dbStatementSerializer( + operation.name, + operation.args + ); + assert.strictEqual( + endedSpans[0].attributes[SemanticAttributes.DB_STATEMENT], + expectedStatement + ); + done(); + }); + }); + }); + }); + }); }); });