From a63098d61bc56ab704cdcebaecfc8cd5098c169d Mon Sep 17 00:00:00 2001 From: Amir Blum Date: Sun, 17 Apr 2022 14:55:47 +0300 Subject: [PATCH 01/22] fix: add patch for queue addCommand function --- .../src/instrumentation.ts | 53 ++++++++++++++++++- 1 file changed, 51 insertions(+), 2 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-redis/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-redis/src/instrumentation.ts index 676a9a374b..62857c45b2 100644 --- a/plugins/node/opentelemetry-instrumentation-redis/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-redis/src/instrumentation.ts @@ -19,6 +19,7 @@ import { isWrapped, InstrumentationBase, InstrumentationNodeModuleDefinition, + InstrumentationNodeModuleFile, } from '@opentelemetry/instrumentation'; import type * as redisTypes from 'redis'; import { @@ -48,6 +49,43 @@ export class RedisInstrumentation extends InstrumentationBase< protected init() { return [ + + // @node-redis/client is a new package introduced and consumed by 'redis ^4.0.0' + new InstrumentationNodeModuleDefinition( + '@node-redis/client', + ['^1.0.0'], + () => {}, + () => {}, + [ new InstrumentationNodeModuleFile( + '@node-redis/client/dist/lib/client/commands-queue.js', + ['^1.0.0'], + (moduleExports: any, moduleVersion?: string) => { + const commandsQueuePrototype = moduleExports.default.prototype; + if ( + isWrapped( + commandsQueuePrototype?.addCommand + ) + ) { + this._unwrap( + commandsQueuePrototype, + 'addCommand' + ); + } + this._wrap( + commandsQueuePrototype, + 'addCommand', + this._getPatchAddCommandV4() + ); + + + console.log(moduleExports.default.prototype['addCommand']); + }, + (moduleExports: unknown, moduleVersion?: string) => { + + }, + )] + ), + new InstrumentationNodeModuleDefinition( 'redis', ['^2.6.0', '^3.0.0'], @@ -67,7 +105,7 @@ export class RedisInstrumentation extends InstrumentationBase< this._wrap( moduleExports.RedisClient.prototype, 'internal_send_command', - this._getPatchInternalSendCommand() + this._getPatchInternalSendCommandV2V3() ); diag.debug('patching redis.RedisClient.create_stream'); @@ -103,10 +141,21 @@ export class RedisInstrumentation extends InstrumentationBase< ), ]; } + + private _getPatchAddCommandV4() { + return function addCommandPatchWrapper(original: Function) { + return function addCommandPatch() { + console.log('here )))))))))))))))))))))))))'); + return original.apply(this, arguments); + } + } + } + /** * Patch internal_send_command(...) to trace requests + * This is for v2 and v3 only */ - private _getPatchInternalSendCommand() { + private _getPatchInternalSendCommandV2V3() { const tracer = this.tracer; const config = this._config; return function internal_send_command(original: Function) { From 3e3aa44c1d812b7c037ef52e36cf1881593c5146 Mon Sep 17 00:00:00 2001 From: Amir Blum Date: Tue, 26 Apr 2022 15:57:30 +0300 Subject: [PATCH 02/22] feat(instrumentation-redis): support for redis@^4.0.0 --- .github/workflows/test-all-versions.yml | 2 + .github/workflows/unit-test.yml | 2 + .../.tav.yml | 10 +- .../README.md | 2 +- .../package.json | 12 +- .../src/instrumentation.ts | 331 ++++++++++++-- .../src/types.ts | 8 +- .../src/utils.ts | 3 +- .../{redis.test.ts => redis_v2_v3.test.ts} | 21 +- .../test/redis_v4.test.ts | 409 ++++++++++++++++++ .../test/utils.ts | 24 + 11 files changed, 765 insertions(+), 59 deletions(-) rename plugins/node/opentelemetry-instrumentation-redis/test/{redis.test.ts => redis_v2_v3.test.ts} (96%) create mode 100644 plugins/node/opentelemetry-instrumentation-redis/test/redis_v4.test.ts create mode 100644 plugins/node/opentelemetry-instrumentation-redis/test/utils.ts diff --git a/.github/workflows/test-all-versions.yml b/.github/workflows/test-all-versions.yml index e48b0a66e4..0239a26653 100644 --- a/.github/workflows/test-all-versions.yml +++ b/.github/workflows/test-all-versions.yml @@ -24,11 +24,13 @@ jobs: --ignore @opentelemetry/instrumentation-tedious --ignore @opentelemetry/instrumentation-amqplib --ignore @opentelemetry/instrumentation-mongodb + --ignore @opentelemetry/instrumentation-redis - node: "10" lerna-extra-args: >- --ignore @opentelemetry/instrumentation-aws-sdk --ignore @opentelemetry/instrumentation-pino --ignore @opentelemetry/instrumentation-mongodb + --ignore @opentelemetry/instrumentation-redis runs-on: ubuntu-latest services: memcached: diff --git a/.github/workflows/unit-test.yml b/.github/workflows/unit-test.yml index d3e4fbfd2a..1e1f63d861 100644 --- a/.github/workflows/unit-test.yml +++ b/.github/workflows/unit-test.yml @@ -17,10 +17,12 @@ jobs: --ignore @opentelemetry/instrumentation-pino --ignore @opentelemetry/instrumentation-tedious --ignore @opentelemetry/instrumentation-amqplib + --ignore @opentelemetry/instrumentation-redis - node: "10" lerna-extra-args: >- --ignore @opentelemetry/instrumentation-aws-sdk --ignore @opentelemetry/instrumentation-pino + --ignore @opentelemetry/instrumentation-redis runs-on: ubuntu-latest services: memcached: diff --git a/plugins/node/opentelemetry-instrumentation-redis/.tav.yml b/plugins/node/opentelemetry-instrumentation-redis/.tav.yml index d22ee83fdf..3150bc1c8f 100644 --- a/plugins/node/opentelemetry-instrumentation-redis/.tav.yml +++ b/plugins/node/opentelemetry-instrumentation-redis/.tav.yml @@ -1,6 +1,12 @@ redis: - versions: ^2.6.0 || ^3.0.0 - commands: npm run test + + jobs: + - versions: "^2.6.0" + commands: npm run test:v2 + - versions: "^3.0.0" + commands: npm run test:v3 + - versions: "^4.0.0" + commands: npm run test:v4 # Fix missing `contrib-test-utils` package pretest: npm run --prefix ../../../ lerna:link diff --git a/plugins/node/opentelemetry-instrumentation-redis/README.md b/plugins/node/opentelemetry-instrumentation-redis/README.md index f60d39ba48..80d856fb9c 100644 --- a/plugins/node/opentelemetry-instrumentation-redis/README.md +++ b/plugins/node/opentelemetry-instrumentation-redis/README.md @@ -18,7 +18,7 @@ npm install --save @opentelemetry/instrumentation-redis ### Supported Versions -- `^2.6.0 || ^3.0.0` (version `4` is not yet supported) +- `^2.6.0 || ^3.0.0 || ^4.0.0` ## Usage diff --git a/plugins/node/opentelemetry-instrumentation-redis/package.json b/plugins/node/opentelemetry-instrumentation-redis/package.json index 046b5f34b5..32c5cbb2c6 100644 --- a/plugins/node/opentelemetry-instrumentation-redis/package.json +++ b/plugins/node/opentelemetry-instrumentation-redis/package.json @@ -6,9 +6,14 @@ "types": "build/src/index.d.ts", "repository": "open-telemetry/opentelemetry-js-contrib", "scripts": { - "test": "nyc ts-mocha -p tsconfig.json 'test/**/*.test.ts'", - "test:debug": "cross-env RUN_REDIS_TESTS_LOCAL=true ts-mocha --inspect-brk --no-timeouts -p tsconfig.json 'test/**/*.test.ts'", + "test": "npm run test:v3", + "test:v2": "nyc ts-mocha -p tsconfig.json 'test/redis_v2_v3.test.ts'", + "test:v3": "nyc ts-mocha -p tsconfig.json 'test/redis_v2_v3.test.ts'", + "test:v4": "nyc ts-mocha -p tsconfig.json --require '@opentelemetry/contrib-test-utils' 'test/redis_v4.test.ts'", + "test:debug": "cross-env RUN_REDIS_TESTS_LOCAL=true ts-mocha --inspect-brk --no-timeouts -p tsconfig.json 'test/redis_v2_v3.test.ts'", "test:local": "cross-env RUN_REDIS_TESTS_LOCAL=true npm run test", + "test:docker:run": "docker run --rm -d --name otel-redis -p 63790:6379 redis:alpine", + "test:docker:stop": "docker stop otel-redis", "test-all-versions": "tav", "test-all-versions:local": "cross-env RUN_REDIS_TESTS_LOCAL=true npm run test-all-versions", "tdd": "npm run test -- --watch-extensions ts --watch", @@ -32,7 +37,7 @@ "author": "OpenTelemetry Authors", "license": "Apache-2.0", "engines": { - "node": ">=8.12.0" + "node": ">=12.0.0" }, "files": [ "build/src/**/*.js", @@ -51,6 +56,7 @@ "devDependencies": { "@opentelemetry/api": "1.0.2", "@opentelemetry/context-async-hooks": "1.0.1", + "@opentelemetry/core": "1.0.1", "@opentelemetry/contrib-test-utils": "^0.29.0", "@opentelemetry/sdk-trace-base": "1.0.1", "@opentelemetry/sdk-trace-node": "1.0.1", diff --git a/plugins/node/opentelemetry-instrumentation-redis/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-redis/src/instrumentation.ts index 62857c45b2..4ec1364083 100644 --- a/plugins/node/opentelemetry-instrumentation-redis/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-redis/src/instrumentation.ts @@ -14,7 +14,14 @@ * limitations under the License. */ -import { diag } from '@opentelemetry/api'; +import { + diag, + trace, + context, + SpanKind, + Span, + SpanStatusCode, +} from '@opentelemetry/api'; import { isWrapped, InstrumentationBase, @@ -23,12 +30,30 @@ import { } from '@opentelemetry/instrumentation'; import type * as redisTypes from 'redis'; import { + defaultDbStatementSerializer, getTracedCreateClient, getTracedCreateStreamTrace, getTracedInternalSendCommand, } from './utils'; import { RedisInstrumentationConfig } from './types'; import { VERSION } from './version'; +import { + DbSystemValues, + SemanticAttributes, +} from '@opentelemetry/semantic-conventions'; + +const OTEL_OPEN_SPANS = Symbol( + 'opentelemetry.instruemntation.redis.open_spans' +); +const MULTI_COMMAND_OPTIONS = Symbol( + 'opentelemetry.instruemntation.redis.multi_command_options' +); + +interface MutliCommandInfo { + span: Span; + commandName: string; + commandArgs: Array; +} const DEFAULT_CONFIG: RedisInstrumentationConfig = { requireParentSpan: false, @@ -49,49 +74,116 @@ export class RedisInstrumentation extends InstrumentationBase< protected init() { return [ - // @node-redis/client is a new package introduced and consumed by 'redis ^4.0.0' new InstrumentationNodeModuleDefinition( '@node-redis/client', ['^1.0.0'], - () => {}, - () => {}, - [ new InstrumentationNodeModuleFile( - '@node-redis/client/dist/lib/client/commands-queue.js', - ['^1.0.0'], - (moduleExports: any, moduleVersion?: string) => { - const commandsQueuePrototype = moduleExports.default.prototype; - if ( - isWrapped( - commandsQueuePrototype?.addCommand - ) - ) { - this._unwrap( - commandsQueuePrototype, - 'addCommand' + (moduleExports: any, moduleVersion?: string) => { + diag.debug( + `Patching @node-redis/client@${moduleVersion} (redis@^4.x.x)` + ); + return moduleExports; + }, + (_moduleExports: any, moduleVersion?: string) => { + diag.debug( + `Unpatching @node-redis/client@${moduleVersion} (redis@^4.x.x)` + ); + }, + [ + new InstrumentationNodeModuleFile( + '@node-redis/client/dist/lib/commander.js', + ['^1.0.0'], + (moduleExports: any) => { + const transformCommandArguments = + moduleExports.transformCommandArguments; + if (!transformCommandArguments) { + this._diag.error( + 'internal instrumentation error, missing transformCommandArguments function' + ); + return moduleExports; + } + + this._diag.debug('Patching redis commands executor'); + if (isWrapped(moduleExports?.extendWithCommands)) { + this._unwrap(moduleExports, 'extendWithCommands'); + } + this._wrap( + moduleExports, + 'extendWithCommands', + this._getPatchExtendWithCommandsV4(transformCommandArguments) ); + + return moduleExports; + }, + (moduleExports: any) => { + this._diag.debug('Unpatching redis commands executor'); + if (isWrapped(moduleExports?.extendWithCommands)) { + this._unwrap(moduleExports, 'extendWithCommands'); + } } - this._wrap( - commandsQueuePrototype, - 'addCommand', - this._getPatchAddCommandV4() - ); - - - console.log(moduleExports.default.prototype['addCommand']); - }, - (moduleExports: unknown, moduleVersion?: string) => { - - }, - )] + ), + new InstrumentationNodeModuleFile( + '@node-redis/client/dist/lib/client/multi-command.js', + ['^1.0.0'], + (moduleExports: any) => { + this._diag.debug('Patching redis multi commands executor'); + const redisClientMultiCommandPrototype = + moduleExports?.default?.prototype; + if (isWrapped(redisClientMultiCommandPrototype?.exec)) { + this._unwrap(redisClientMultiCommandPrototype, 'exec'); + } + this._wrap( + redisClientMultiCommandPrototype, + 'exec', + this._getPatchMultiCommandsExecV4() + ); + + return moduleExports; + }, + (moduleExports: any) => { + this._diag.debug('Unpatching redis multi commands executor'); + const redisClientMultiCommandPrototype = + moduleExports?.default?.prototype; + if (isWrapped(redisClientMultiCommandPrototype?.exec)) { + this._unwrap(redisClientMultiCommandPrototype, 'exec'); + } + } + ), + new InstrumentationNodeModuleFile( + '@node-redis/client/dist/lib/client/index.js', + ['^1.0.0'], + (moduleExports: any) => { + this._diag.debug('Patching redis client'); + const redisClientPrototype = moduleExports?.default?.prototype; + + if (isWrapped(redisClientPrototype?.multi)) { + this._unwrap(redisClientPrototype, 'multi'); + } + this._wrap( + redisClientPrototype, + 'multi', + this._getPatchRedisClientMultiV4() + ); + + return moduleExports; + }, + (moduleExports: any) => { + this._diag.debug('Unpatching redis client'); + const redisClientPrototype = moduleExports?.default?.prototype; + if (isWrapped(redisClientPrototype?.multi)) { + this._unwrap(redisClientPrototype, 'multi'); + } + } + ), + ] ), new InstrumentationNodeModuleDefinition( 'redis', ['^2.6.0', '^3.0.0'], (moduleExports, moduleVersion) => { - diag.debug(`Patching redis@${moduleVersion}`); - diag.debug('Patching redis.RedisClient.internal_send_command'); + this._diag.debug(`Patching redis@${moduleVersion}`); + this._diag.debug('Patching redis.RedisClient.internal_send_command'); if ( isWrapped( moduleExports.RedisClient.prototype['internal_send_command'] @@ -142,13 +234,180 @@ export class RedisInstrumentation extends InstrumentationBase< ]; } - private _getPatchAddCommandV4() { - return function addCommandPatchWrapper(original: Function) { - return function addCommandPatch() { - console.log('here )))))))))))))))))))))))))'); + private _getPatchExtendWithCommandsV4(transformCommandArguments: Function) { + const plugin = this; + return function extendWithCommandsPatchWrapper(original: Function) { + return function extendWithCommandsPatch(this: any, config: any) { + const origExecutor = config.executor; + config.executor = function ( + this: any, + command: any, + args: Array + ) { + const hasNoParentSpan = trace.getSpan(context.active()) === undefined; + if (hasNoParentSpan && plugin._config?.requireParentSpan) { + return origExecutor.apply(this, arguments); + } + + const redisCommandArguments = transformCommandArguments( + command, + args + ).args; + const clientOptions = this.options || this[MULTI_COMMAND_OPTIONS]; + + const commandName = redisCommandArguments[0] as string; // types also allows it to be a Buffer, but in practice it only string + const commandArgs = redisCommandArguments.slice(1); + + const dbStatementSerializer = + plugin._config?.dbStatementSerializer || + defaultDbStatementSerializer; + + const attributes = { + [SemanticAttributes.DB_SYSTEM]: DbSystemValues.REDIS, + [SemanticAttributes.NET_PEER_NAME]: clientOptions?.socket?.host, + [SemanticAttributes.NET_PEER_PORT]: clientOptions?.socket?.port, + [SemanticAttributes.DB_CONNECTION_STRING]: clientOptions?.url, + }; + + try { + const dbStatement = dbStatementSerializer(commandName, commandArgs); + if (dbStatement != null) { + attributes[SemanticAttributes.DB_STATEMENT] = dbStatement; + } + } catch (e) { + plugin._diag.error('dbStatementSerializer throw an exception', e); + } + + const span = plugin.tracer.startSpan( + `${RedisInstrumentation.COMPONENT}-${commandName}`, + { + kind: SpanKind.CLIENT, + attributes, + } + ); + + const res = origExecutor.apply(this, arguments); + if (res.then) { + res.then( + (redisRes: unknown) => { + plugin._endSpanWithResponse( + span, + commandName, + commandArgs, + redisRes, + undefined + ); + }, + (err: any) => { + plugin._endSpanWithResponse( + span, + commandName, + commandArgs, + null, + err + ); + } + ); + } else { + const redisClientMultiCommand = res as { + [OTEL_OPEN_SPANS]?: Array; + }; + redisClientMultiCommand[OTEL_OPEN_SPANS] = + redisClientMultiCommand[OTEL_OPEN_SPANS] || []; + redisClientMultiCommand[OTEL_OPEN_SPANS]!.push({ + span, + commandName, + commandArgs, + }); + } + return res; + }; return original.apply(this, arguments); + }; + }; + } + + private _getPatchMultiCommandsExecV4() { + const plugin = this; + return function execPatchWrapper(original: Function) { + return function execPatch(this: any) { + const execRes = original.apply(this, arguments); + if (!execRes.then) { + this._diag( + 'got non promise result when patching RedisClientMultiCommand.exec' + ); + return execRes; + } + + execRes.then((redisRes: unknown[]) => { + const openSpans = this[OTEL_OPEN_SPANS]; + if (!openSpans) { + return this._diag.error( + 'cannot find open spans to end for redis multi command' + ); + } + if (redisRes.length !== openSpans.length) { + return this._diag.error( + 'number of multi command spans does not match response from redis' + ); + } + for (let i = 0; i < openSpans.length; i++) { + const { span, commandName, commandArgs } = openSpans[i]; + const currCommandRes = redisRes[i]; + if (currCommandRes instanceof Error) { + plugin._endSpanWithResponse( + span, + commandName, + commandArgs, + null, + currCommandRes + ); + } else { + plugin._endSpanWithResponse( + span, + commandName, + commandArgs, + currCommandRes, + undefined + ); + } + } + }); + return execRes; + }; + }; + } + + private _getPatchRedisClientMultiV4() { + const plugin = this; + return function multiPatchWrapper(original: Function) { + return function multiPatch(this: any) { + const multiRes = original.apply(this, arguments); + multiRes[MULTI_COMMAND_OPTIONS] = this.options; + return multiRes; + }; + }; + } + + private _endSpanWithResponse( + span: Span, + commandName: string, + commandArgs: string[], + response: unknown, + error: Error | undefined + ) { + if (!error && this._config.responseHook) { + try { + this._config.responseHook(span, commandName, commandArgs, response); + } catch (err) { + this._diag.error('responseHook throw an exception', err); } } + if (error) { + span.recordException(error); + span.setStatus({ code: SpanStatusCode.ERROR, message: error?.message }); + } + span.end(); } /** diff --git a/plugins/node/opentelemetry-instrumentation-redis/src/types.ts b/plugins/node/opentelemetry-instrumentation-redis/src/types.ts index 245e7611e8..cd9866ed62 100644 --- a/plugins/node/opentelemetry-instrumentation-redis/src/types.ts +++ b/plugins/node/opentelemetry-instrumentation-redis/src/types.ts @@ -45,8 +45,8 @@ export interface RedisPluginClientTypes { * @returns serialized string that will be used as the db.statement attribute. */ export type DbStatementSerializer = ( - cmdName: RedisCommand['command'], - cmdArgs: RedisCommand['args'] + cmdName: string, + cmdArgs: Array ) => string; /** @@ -61,8 +61,8 @@ export type DbStatementSerializer = ( export interface RedisResponseCustomAttributeFunction { ( span: Span, - cmdName: RedisCommand['command'], - cmdArgs: RedisCommand['args'], + cmdName: string, + cmdArgs: Array, response: unknown ): void; } diff --git a/plugins/node/opentelemetry-instrumentation-redis/src/utils.ts b/plugins/node/opentelemetry-instrumentation-redis/src/utils.ts index 4f74b17a19..f4419fb9ba 100644 --- a/plugins/node/opentelemetry-instrumentation-redis/src/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-redis/src/utils.ts @@ -75,7 +75,8 @@ export const getTracedCreateStreamTrace = ( }; }; -const defaultDbStatementSerializer: DbStatementSerializer = cmdName => cmdName; +export const defaultDbStatementSerializer: DbStatementSerializer = cmdName => + cmdName; export const getTracedInternalSendCommand = ( tracer: Tracer, diff --git a/plugins/node/opentelemetry-instrumentation-redis/test/redis.test.ts b/plugins/node/opentelemetry-instrumentation-redis/test/redis_v2_v3.test.ts similarity index 96% rename from plugins/node/opentelemetry-instrumentation-redis/test/redis.test.ts rename to plugins/node/opentelemetry-instrumentation-redis/test/redis_v2_v3.test.ts index 09842809fc..9c6212a783 100644 --- a/plugins/node/opentelemetry-instrumentation-redis/test/redis.test.ts +++ b/plugins/node/opentelemetry-instrumentation-redis/test/redis_v2_v3.test.ts @@ -42,21 +42,20 @@ instrumentation.disable(); import * as redisTypes from 'redis'; import { RedisResponseCustomAttributeFunction } from '../src/types'; +import { + redisTestConfig, + redisTestUrl, + shouldTest, + shouldTestLocal, +} from './utils'; const memoryExporter = new InMemorySpanExporter(); -const CONFIG = { - host: process.env.OPENTELEMETRY_REDIS_HOST || 'localhost', - port: process.env.OPENTELEMETRY_REDIS_PORT || '63790', -}; - -const URL = `redis://${CONFIG.host}:${CONFIG.port}`; - const DEFAULT_ATTRIBUTES = { [SemanticAttributes.DB_SYSTEM]: DbSystemValues.REDIS, - [SemanticAttributes.NET_PEER_NAME]: CONFIG.host, - [SemanticAttributes.NET_PEER_PORT]: CONFIG.port, - [SemanticAttributes.DB_CONNECTION_STRING]: URL, + [SemanticAttributes.NET_PEER_NAME]: redisTestConfig.host, + [SemanticAttributes.NET_PEER_PORT]: redisTestConfig.port, + [SemanticAttributes.DB_CONNECTION_STRING]: redisTestUrl, }; const unsetStatus: SpanStatus = { @@ -67,8 +66,6 @@ describe('redis@2.x', () => { const provider = new NodeTracerProvider(); const tracer = provider.getTracer('external'); let redis: typeof redisTypes; - const shouldTestLocal = process.env.RUN_REDIS_TESTS_LOCAL; - const shouldTest = process.env.RUN_REDIS_TESTS || shouldTestLocal; let contextManager: AsyncHooksContextManager; beforeEach(() => { diff --git a/plugins/node/opentelemetry-instrumentation-redis/test/redis_v4.test.ts b/plugins/node/opentelemetry-instrumentation-redis/test/redis_v4.test.ts new file mode 100644 index 0000000000..4afeaee7cf --- /dev/null +++ b/plugins/node/opentelemetry-instrumentation-redis/test/redis_v4.test.ts @@ -0,0 +1,409 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { + getTestSpans, + registerInstrumentationTesting, +} from '@opentelemetry/contrib-test-utils'; +import { RedisInstrumentation } from '../src'; +import * as assert from 'assert'; + +import { + redisTestConfig, + redisTestUrl, + shouldTest, + shouldTestLocal, +} from './utils'; +import * as testUtils from '@opentelemetry/contrib-test-utils'; + +const instrumentation = registerInstrumentationTesting( + new RedisInstrumentation() +); + +import { createClient } from 'redis'; +import { + Span, + SpanKind, + SpanStatusCode, + trace, + context, +} from '@opentelemetry/api'; +import { SemanticAttributes } from '@opentelemetry/semantic-conventions'; +import { RedisResponseCustomAttributeFunction } from '../src/types'; +import { hrTimeToMilliseconds } from '@opentelemetry/core'; + +describe('redis@^4.0.0', () => { + before(function () { + // needs to be "function" to have MochaContext "this" context + if (!shouldTest) { + // this.skip() workaround + // https://github.com/mochajs/mocha/issues/2683#issuecomment-375629901 + this.test!.parent!.pending = true; + this.skip(); + } + + if (shouldTestLocal) { + testUtils.startDocker('redis'); + } + }); + + after(() => { + if (shouldTestLocal) { + testUtils.cleanUpDocker('redis'); + } + }); + + let client: any; + + beforeEach(async () => { + client = createClient({ + url: redisTestUrl, + }); + await client.connect(); + }); + + afterEach(async () => { + await client?.disconnect(); + }); + + describe('redis commands', () => { + it('simple set and get', async () => { + await client.set('key', 'value'); + const value = await client.get('key'); + assert.strictEqual(value, 'value'); // verify we did not screw up the normal functionality + + const spans = getTestSpans(); + assert.strictEqual(spans.length, 2); + + const redisSetSpan = spans.find(s => s.name.includes('SET')); + assert.ok(redisSetSpan); + assert.strictEqual(redisSetSpan?.kind, SpanKind.CLIENT); + assert.strictEqual(redisSetSpan?.name, 'redis-SET'); + assert.strictEqual( + redisSetSpan?.attributes[SemanticAttributes.DB_SYSTEM], + 'redis' + ); + assert.strictEqual( + redisSetSpan?.attributes[SemanticAttributes.DB_STATEMENT], + 'SET' + ); + assert.strictEqual( + redisSetSpan?.attributes[SemanticAttributes.NET_PEER_NAME], + redisTestConfig.host + ); + assert.strictEqual( + redisSetSpan?.attributes[SemanticAttributes.NET_PEER_PORT], + redisTestConfig.port + ); + assert.strictEqual( + redisSetSpan?.attributes[SemanticAttributes.DB_CONNECTION_STRING], + redisTestUrl + ); + + const redisGetSpan = spans.find(s => s.name.includes('GET')); + assert.ok(redisGetSpan); + assert.strictEqual(redisGetSpan?.kind, SpanKind.CLIENT); + assert.strictEqual(redisGetSpan?.name, 'redis-GET'); + assert.strictEqual( + redisGetSpan?.attributes[SemanticAttributes.DB_SYSTEM], + 'redis' + ); + assert.strictEqual( + redisGetSpan?.attributes[SemanticAttributes.DB_STATEMENT], + 'GET' + ); + assert.strictEqual( + redisGetSpan?.attributes[SemanticAttributes.NET_PEER_NAME], + redisTestConfig.host + ); + assert.strictEqual( + redisGetSpan?.attributes[SemanticAttributes.NET_PEER_PORT], + redisTestConfig.port + ); + assert.strictEqual( + redisGetSpan?.attributes[SemanticAttributes.DB_CONNECTION_STRING], + redisTestUrl + ); + }); + + it('span with error', async () => { + await client.set('string-key', 'string-value'); + await assert.rejects(async () => await client.incr('string-key')); + + const [_setSpan, incrSpan] = getTestSpans(); + + assert.ok(incrSpan); + assert.strictEqual(incrSpan?.status.code, SpanStatusCode.ERROR); + assert.strictEqual( + incrSpan?.status.message, + 'ERR value is not an integer or out of range' + ); + + const exceptions = incrSpan.events.filter( + event => event.name === 'exception' + ); + assert.strictEqual(exceptions.length, 1); + assert.strictEqual( + exceptions?.[0].attributes?.[SemanticAttributes.EXCEPTION_TYPE], + 'ReplyError' + ); + assert.strictEqual( + exceptions?.[0].attributes?.[SemanticAttributes.EXCEPTION_MESSAGE], + 'ERR value is not an integer or out of range' + ); + }); + }); + + describe('multi (transactions) commands', () => { + it('multi commands', async () => { + await client.set('another-key', 'another-value'); + const [setKeyReply, otherKeyValue] = await client + .multi() + .set('key', 'value') + .get('another-key') + .exec(); // ['OK', 'another-value'] + + assert.strictEqual(setKeyReply, 'OK'); // verify we did not screw up the normal functionality + assert.strictEqual(otherKeyValue, 'another-value'); // verify we did not screw up the normal functionality + + const [setSpan, multiSetSpan, multiGetSpan] = getTestSpans(); + + assert.ok(setSpan); + + assert.ok(multiSetSpan); + assert.strictEqual(multiSetSpan.name, 'redis-SET'); + assert.strictEqual( + multiSetSpan.attributes[SemanticAttributes.DB_STATEMENT], + 'SET' + ); + assert.strictEqual( + multiSetSpan?.attributes[SemanticAttributes.NET_PEER_NAME], + redisTestConfig.host + ); + assert.strictEqual( + multiSetSpan?.attributes[SemanticAttributes.NET_PEER_PORT], + redisTestConfig.port + ); + assert.strictEqual( + multiSetSpan?.attributes[SemanticAttributes.DB_CONNECTION_STRING], + redisTestUrl + ); + + assert.ok(multiGetSpan); + assert.strictEqual(multiGetSpan.name, 'redis-GET'); + assert.strictEqual( + multiGetSpan.attributes[SemanticAttributes.DB_STATEMENT], + 'GET' + ); + assert.strictEqual( + multiGetSpan?.attributes[SemanticAttributes.NET_PEER_NAME], + redisTestConfig.host + ); + assert.strictEqual( + multiGetSpan?.attributes[SemanticAttributes.NET_PEER_PORT], + redisTestConfig.port + ); + assert.strictEqual( + multiGetSpan?.attributes[SemanticAttributes.DB_CONNECTION_STRING], + redisTestUrl + ); + }); + + it('multi command with error', async () => { + const [setReply, incrReply] = await client + .multi() + .set('key', 'value') + .incr('key') + .exec(); // ['OK', 'ReplyError'] + assert.strictEqual(setReply, 'OK'); // verify we did not screw up the normal functionality + assert.ok(incrReply instanceof Error); // verify we did not screw up the normal functionality + + const [multiSetSpan, multiIncrSpan] = getTestSpans(); + + assert.ok(multiSetSpan); + assert.strictEqual(multiSetSpan.status.code, SpanStatusCode.UNSET); + + assert.ok(multiIncrSpan); + assert.strictEqual(multiIncrSpan.status.code, SpanStatusCode.ERROR); + assert.strictEqual( + multiIncrSpan.status.message, + 'ERR value is not an integer or out of range' + ); + }); + + it('duration covers create until server response', async () => { + await client.set('another-key', 'another-value'); + const multiClient = client.multi(); + let commands = multiClient.set('key', 'value'); + // wait 10 ms before adding next command + // simulate long operation + await new Promise(resolve => setTimeout(resolve, 10)); + commands = commands.get('another-key'); + const [setKeyReply, otherKeyValue] = await commands.exec(); // ['OK', 'another-value'] + + assert.strictEqual(setKeyReply, 'OK'); // verify we did not screw up the normal functionality + assert.strictEqual(otherKeyValue, 'another-value'); // verify we did not screw up the normal functionality + + const [_setSpan, multiSetSpan, multiGetSpan] = getTestSpans(); + // verify that commands span started when it was added to multi and not when "sent". + // they were called with 10 ms gap between them, so it should be reflected in the span start time + // could be nice feature in the future to capture an event for when it is actually sent + const startTimeDiff = + hrTimeToMilliseconds(multiGetSpan.startTime) - + hrTimeToMilliseconds(multiSetSpan.startTime); + assert.ok( + startTimeDiff >= 9, + `diff of start time should be >= 10 and it's ${startTimeDiff}` + ); + + const endTimeDiff = + hrTimeToMilliseconds(multiGetSpan.endTime) - + hrTimeToMilliseconds(multiSetSpan.endTime); + assert.ok(endTimeDiff < 10); // spans should all end together when multi response arrives from redis server + }); + + it('response hook for multi commands', async () => { + const responseHook: RedisResponseCustomAttributeFunction = ( + span: Span, + cmdName: string, + cmdArgs: Array, + response: unknown + ) => { + span.setAttribute('test.cmd.name', cmdName); + span.setAttribute('test.cmd.args', cmdArgs as string[]); + span.setAttribute('test.db.response', response as string); + }; + instrumentation.setConfig({ responseHook }); + + await client.set('another-key', 'another-value'); + const [setKeyReply, otherKeyValue] = await client + .multi() + .set('key', 'value') + .get('another-key') + .exec(); // ['OK', 'another-value'] + assert.strictEqual(setKeyReply, 'OK'); // verify we did not screw up the normal functionality + assert.strictEqual(otherKeyValue, 'another-value'); // verify we did not screw up the normal functionality + + const [_setSpan, multiSetSpan, multiGetSpan] = getTestSpans(); + + assert.ok(multiSetSpan); + assert.strictEqual(multiSetSpan.attributes['test.cmd.name'], 'SET'); + assert.deepStrictEqual(multiSetSpan.attributes['test.cmd.args'], [ + 'key', + 'value', + ]); + assert.strictEqual(multiSetSpan.attributes['test.db.response'], 'OK'); + + assert.ok(multiGetSpan); + assert.strictEqual(multiGetSpan.attributes['test.cmd.name'], 'GET'); + assert.deepStrictEqual(multiGetSpan.attributes['test.cmd.args'], [ + 'another-key', + ]); + assert.strictEqual( + multiGetSpan.attributes['test.db.response'], + 'another-value' + ); + }); + }); + + describe('config', () => { + describe('dbStatementSerializer', () => { + it('custom dbStatementSerializer', async () => { + const dbStatementSerializer = ( + cmdName: string, + cmdArgs: Array + ) => { + return `${cmdName} ${cmdArgs.join(' ')}`; + }; + + instrumentation.setConfig({ dbStatementSerializer }); + await client.set('key', 'value'); + const [span] = getTestSpans(); + assert.strictEqual( + span.attributes[SemanticAttributes.DB_STATEMENT], + 'SET key value' + ); + }); + + it('dbStatementSerializer throws', async () => { + const dbStatementSerializer = () => { + throw new Error('dbStatementSerializer error'); + }; + + instrumentation.setConfig({ dbStatementSerializer }); + await client.set('key', 'value'); + const [span] = getTestSpans(); + assert.ok(span); + assert.ok(!(SemanticAttributes.DB_STATEMENT in span.attributes)); + }); + }); + + describe('responseHook', () => { + it('valid response hook', async () => { + const responseHook: RedisResponseCustomAttributeFunction = ( + span: Span, + cmdName: string, + cmdArgs: Array, + response: unknown + ) => { + span.setAttribute('test.cmd.name', cmdName); + span.setAttribute('test.cmd.args', cmdArgs as string[]); + span.setAttribute('test.db.response', response as string); + }; + instrumentation.setConfig({ responseHook }); + await client.set('key', 'value'); + const [span] = getTestSpans(); + assert.ok(span); + assert.strictEqual(span.attributes['test.cmd.name'], 'SET'); + assert.deepStrictEqual(span.attributes['test.cmd.args'], [ + 'key', + 'value', + ]); + assert.strictEqual(span.attributes['test.db.response'], 'OK'); + }); + + it('responseHook throws', async () => { + const responseHook = () => { + throw new Error('responseHook error'); + }; + instrumentation.setConfig({ responseHook }); + const res = await client.set('key', 'value'); + assert.strictEqual(res, 'OK'); // package is still functional + const [span] = getTestSpans(); + assert.ok(span); + }); + }); + + describe('requireParentSpan', () => { + it('set to true', async () => { + instrumentation.setConfig({ requireParentSpan: true }); + + // no parent span => no redis span + const res = await client.set('key', 'value'); + assert.strictEqual(res, 'OK'); // verify we did not screw up the normal functionality + assert.ok(getTestSpans().length === 0); + + // has ambient span => redis span + const span = trace.getTracer('test').startSpan('test span'); + await context.with(trace.setSpan(context.active(), span), async () => { + const res = await client.set('key', 'value'); + assert.strictEqual(res, 'OK'); // verify we did not screw up the normal functionality + assert.ok(getTestSpans().length === 1); + }); + span.end(); + }); + }); + }); +}); diff --git a/plugins/node/opentelemetry-instrumentation-redis/test/utils.ts b/plugins/node/opentelemetry-instrumentation-redis/test/utils.ts new file mode 100644 index 0000000000..c60c703e56 --- /dev/null +++ b/plugins/node/opentelemetry-instrumentation-redis/test/utils.ts @@ -0,0 +1,24 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +export const redisTestConfig = { + host: process.env.OPENTELEMETRY_REDIS_HOST || 'localhost', + port: process.env.OPENTELEMETRY_REDIS_PORT || 63790, +}; + +export const redisTestUrl = `redis://${redisTestConfig.host}:${redisTestConfig.port}`; + +export const shouldTestLocal = process.env.RUN_REDIS_TESTS_LOCAL; +export const shouldTest = process.env.RUN_REDIS_TESTS || shouldTestLocal; From 42e958221dcdf30d89b61a65d4cda30c39935706 Mon Sep 17 00:00:00 2001 From: Amir Blum Date: Tue, 26 Apr 2022 16:30:05 +0300 Subject: [PATCH 03/22] fix(redis): compilation issues --- .../src/instrumentation.ts | 3 ++- .../test/redis_v2_v3.test.ts | 13 ++++++++----- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-redis/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-redis/src/instrumentation.ts index 4ec1364083..70e086c90d 100644 --- a/plugins/node/opentelemetry-instrumentation-redis/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-redis/src/instrumentation.ts @@ -103,6 +103,8 @@ export class RedisInstrumentation extends InstrumentationBase< return moduleExports; } + // this is the function that extend a redis client with a list of commands. + // the function patches the commandExecutor to record a span this._diag.debug('Patching redis commands executor'); if (isWrapped(moduleExports?.extendWithCommands)) { this._unwrap(moduleExports, 'extendWithCommands'); @@ -379,7 +381,6 @@ export class RedisInstrumentation extends InstrumentationBase< } private _getPatchRedisClientMultiV4() { - const plugin = this; return function multiPatchWrapper(original: Function) { return function multiPatch(this: any) { const multiRes = original.apply(this, arguments); diff --git a/plugins/node/opentelemetry-instrumentation-redis/test/redis_v2_v3.test.ts b/plugins/node/opentelemetry-instrumentation-redis/test/redis_v2_v3.test.ts index 9c6212a783..c8fc8f21f0 100644 --- a/plugins/node/opentelemetry-instrumentation-redis/test/redis_v2_v3.test.ts +++ b/plugins/node/opentelemetry-instrumentation-redis/test/redis_v2_v3.test.ts @@ -116,7 +116,7 @@ describe('redis@2.x', () => { }; context.with(trace.setSpan(context.active(), span), () => { - client = redis.createClient(URL); + client = redis.createClient(redisTestUrl); client.on('ready', readyHandler); client.on('error', errorHandler); }); @@ -155,7 +155,7 @@ describe('redis@2.x', () => { ]; before(done => { - client = redis.createClient(URL); + client = redis.createClient(redisTestUrl); client.on('error', err => { done(err); }); @@ -248,7 +248,10 @@ describe('redis@2.x', () => { }); describe('dbStatementSerializer config', () => { - const dbStatementSerializer = (cmdName: string, cmdArgs: string[]) => { + const dbStatementSerializer = ( + cmdName: string, + cmdArgs: Array + ) => { return Array.isArray(cmdArgs) && cmdArgs.length ? `${cmdName} ${cmdArgs.join(' ')}` : cmdName; @@ -291,7 +294,7 @@ describe('redis@2.x', () => { const responseHook: RedisResponseCustomAttributeFunction = ( span: Span, _cmdName: string, - _cmdArgs: string[], + _cmdArgs: Array, response: unknown ) => { span.setAttribute(dataFieldName, new String(response).toString()); @@ -322,7 +325,7 @@ describe('redis@2.x', () => { const badResponseHook: RedisResponseCustomAttributeFunction = ( _span: Span, _cmdName: string, - _cmdArgs: string[], + _cmdArgs: Array, _response: unknown ) => { throw 'Some kind of error'; From 8ba5a8e0c575b275f9711ec385ef1c66f64d0e53 Mon Sep 17 00:00:00 2001 From: Amir Blum Date: Wed, 27 Apr 2022 15:03:31 +0300 Subject: [PATCH 04/22] fix(instrumentation-redis): make sure port is integer --- 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 f4419fb9ba..6ff0a09c32 100644 --- a/plugins/node/opentelemetry-instrumentation-redis/src/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-redis/src/utils.ts @@ -119,7 +119,7 @@ export const getTracedInternalSendCommand = ( if (this.options) { span.setAttributes({ [SemanticAttributes.NET_PEER_NAME]: this.options.host, - [SemanticAttributes.NET_PEER_PORT]: this.options.port, + [SemanticAttributes.NET_PEER_PORT]: parseInt(this.options.port), }); } if (this.address) { From e8ae6fb93961a460b83569b64f2f0ac50534727a Mon Sep 17 00:00:00 2001 From: Amir Blum Date: Sat, 30 Apr 2022 14:13:11 +0300 Subject: [PATCH 05/22] chore(redis): separate redis instrumentation into it's own package --- .github/component_owners.yml | 2 + .github/workflows/test-all-versions.yml | 4 +- .github/workflows/unit-test.yml | 4 +- .../.eslintignore | 1 + .../.eslintrc.js | 7 + .../.npmignore | 4 + .../.tav.yml | 8 + .../LICENSE | 201 +++++++++ .../README.md | 59 +++ .../package.json | 76 ++++ .../src/index.ts | 18 + .../src/instrumentation.ts | 357 +++++++++++++++ .../src/types.ts | 59 +++ .../src/utils.ts | 5 + .../test/redis.test.ts | 409 ++++++++++++++++++ .../test/utils.ts | 24 + .../tsconfig.json | 11 + 17 files changed, 1245 insertions(+), 4 deletions(-) create mode 100644 plugins/node/opentelemetry-instrumentation-redis-4/.eslintignore create mode 100644 plugins/node/opentelemetry-instrumentation-redis-4/.eslintrc.js create mode 100644 plugins/node/opentelemetry-instrumentation-redis-4/.npmignore create mode 100644 plugins/node/opentelemetry-instrumentation-redis-4/.tav.yml create mode 100644 plugins/node/opentelemetry-instrumentation-redis-4/LICENSE create mode 100644 plugins/node/opentelemetry-instrumentation-redis-4/README.md create mode 100644 plugins/node/opentelemetry-instrumentation-redis-4/package.json create mode 100644 plugins/node/opentelemetry-instrumentation-redis-4/src/index.ts create mode 100644 plugins/node/opentelemetry-instrumentation-redis-4/src/instrumentation.ts create mode 100644 plugins/node/opentelemetry-instrumentation-redis-4/src/types.ts create mode 100644 plugins/node/opentelemetry-instrumentation-redis-4/src/utils.ts create mode 100644 plugins/node/opentelemetry-instrumentation-redis-4/test/redis.test.ts create mode 100644 plugins/node/opentelemetry-instrumentation-redis-4/test/utils.ts create mode 100644 plugins/node/opentelemetry-instrumentation-redis-4/tsconfig.json diff --git a/.github/component_owners.yml b/.github/component_owners.yml index 8452497362..478d0da901 100644 --- a/.github/component_owners.yml +++ b/.github/component_owners.yml @@ -44,6 +44,8 @@ components: - rauno56 plugins/node/opentelemetry-instrumentation-redis: - blumamir + plugins/node/opentelemetry-instrumentation-redis-4: + - blumamir plugins/node/opentelemetry-instrumentation-restify: - rauno56 plugins/node/opentelemetry-instrumentation-router: diff --git a/.github/workflows/test-all-versions.yml b/.github/workflows/test-all-versions.yml index 0239a26653..b97006a8f1 100644 --- a/.github/workflows/test-all-versions.yml +++ b/.github/workflows/test-all-versions.yml @@ -24,13 +24,13 @@ jobs: --ignore @opentelemetry/instrumentation-tedious --ignore @opentelemetry/instrumentation-amqplib --ignore @opentelemetry/instrumentation-mongodb - --ignore @opentelemetry/instrumentation-redis + --ignore @opentelemetry/instrumentation-redis-4 - node: "10" lerna-extra-args: >- --ignore @opentelemetry/instrumentation-aws-sdk --ignore @opentelemetry/instrumentation-pino --ignore @opentelemetry/instrumentation-mongodb - --ignore @opentelemetry/instrumentation-redis + --ignore @opentelemetry/instrumentation-redis-4 runs-on: ubuntu-latest services: memcached: diff --git a/.github/workflows/unit-test.yml b/.github/workflows/unit-test.yml index 1e1f63d861..8afa198ab6 100644 --- a/.github/workflows/unit-test.yml +++ b/.github/workflows/unit-test.yml @@ -17,12 +17,12 @@ jobs: --ignore @opentelemetry/instrumentation-pino --ignore @opentelemetry/instrumentation-tedious --ignore @opentelemetry/instrumentation-amqplib - --ignore @opentelemetry/instrumentation-redis + --ignore @opentelemetry/instrumentation-redis-4 - node: "10" lerna-extra-args: >- --ignore @opentelemetry/instrumentation-aws-sdk --ignore @opentelemetry/instrumentation-pino - --ignore @opentelemetry/instrumentation-redis + --ignore @opentelemetry/instrumentation-redis-4 runs-on: ubuntu-latest services: memcached: diff --git a/plugins/node/opentelemetry-instrumentation-redis-4/.eslintignore b/plugins/node/opentelemetry-instrumentation-redis-4/.eslintignore new file mode 100644 index 0000000000..378eac25d3 --- /dev/null +++ b/plugins/node/opentelemetry-instrumentation-redis-4/.eslintignore @@ -0,0 +1 @@ +build diff --git a/plugins/node/opentelemetry-instrumentation-redis-4/.eslintrc.js b/plugins/node/opentelemetry-instrumentation-redis-4/.eslintrc.js new file mode 100644 index 0000000000..f756f4488b --- /dev/null +++ b/plugins/node/opentelemetry-instrumentation-redis-4/.eslintrc.js @@ -0,0 +1,7 @@ +module.exports = { + "env": { + "mocha": true, + "node": true + }, + ...require('../../../eslint.config.js') +} diff --git a/plugins/node/opentelemetry-instrumentation-redis-4/.npmignore b/plugins/node/opentelemetry-instrumentation-redis-4/.npmignore new file mode 100644 index 0000000000..9505ba9450 --- /dev/null +++ b/plugins/node/opentelemetry-instrumentation-redis-4/.npmignore @@ -0,0 +1,4 @@ +/bin +/coverage +/doc +/test diff --git a/plugins/node/opentelemetry-instrumentation-redis-4/.tav.yml b/plugins/node/opentelemetry-instrumentation-redis-4/.tav.yml new file mode 100644 index 0000000000..f478be1116 --- /dev/null +++ b/plugins/node/opentelemetry-instrumentation-redis-4/.tav.yml @@ -0,0 +1,8 @@ +redis: + + jobs: + - versions: "^4.0.0" + commands: npm run test + + # Fix missing `contrib-test-utils` package + pretest: npm run --prefix ../../../ lerna:link diff --git a/plugins/node/opentelemetry-instrumentation-redis-4/LICENSE b/plugins/node/opentelemetry-instrumentation-redis-4/LICENSE new file mode 100644 index 0000000000..e50e8c80f9 --- /dev/null +++ b/plugins/node/opentelemetry-instrumentation-redis-4/LICENSE @@ -0,0 +1,201 @@ + Apache License + Version 2.0, January 2004 + http://www.apache.org/licenses/ + + TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION + + 1. Definitions. + + "License" shall mean the terms and conditions for use, reproduction, + and distribution as defined by Sections 1 through 9 of this document. + + "Licensor" shall mean the copyright owner or entity authorized by + the copyright owner that is granting the License. + + "Legal Entity" shall mean the union of the acting entity and all + other entities that control, are controlled by, or are under common + control with that entity. For the purposes of this definition, + "control" means (i) the power, direct or indirect, to cause the + direction or management of such entity, whether by contract or + otherwise, or (ii) ownership of fifty percent (50%) or more of the + outstanding shares, or (iii) beneficial ownership of such entity. + + "You" (or "Your") shall mean an individual or Legal Entity + exercising permissions granted by this License. + + "Source" form shall mean the preferred form for making modifications, + including but not limited to software source code, documentation + source, and configuration files. + + "Object" form shall mean any form resulting from mechanical + transformation or translation of a Source form, including but + not limited to compiled object code, generated documentation, + and conversions to other media types. + + "Work" shall mean the work of authorship, whether in Source or + Object form, made available under the License, as indicated by a + copyright notice that is included in or attached to the work + (an example is provided in the Appendix below). + + "Derivative Works" shall mean any work, whether in Source or Object + form, that is based on (or derived from) the Work and for which the + editorial revisions, annotations, elaborations, or other modifications + represent, as a whole, an original work of authorship. For the purposes + of this License, Derivative Works shall not include works that remain + separable from, or merely link (or bind by name) to the interfaces of, + the Work and Derivative Works thereof. + + "Contribution" shall mean any work of authorship, including + the original version of the Work and any modifications or additions + to that Work or Derivative Works thereof, that is intentionally + submitted to Licensor for inclusion in the Work by the copyright owner + or by an individual or Legal Entity authorized to submit on behalf of + the copyright owner. For the purposes of this definition, "submitted" + means any form of electronic, verbal, or written communication sent + to the Licensor or its representatives, including but not limited to + communication on electronic mailing lists, source code control systems, + and issue tracking systems that are managed by, or on behalf of, the + Licensor for the purpose of discussing and improving the Work, but + excluding communication that is conspicuously marked or otherwise + designated in writing by the copyright owner as "Not a Contribution." + + "Contributor" shall mean Licensor and any individual or Legal Entity + on behalf of whom a Contribution has been received by Licensor and + subsequently incorporated within the Work. + + 2. Grant of Copyright License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + copyright license to reproduce, prepare Derivative Works of, + publicly display, publicly perform, sublicense, and distribute the + Work and such Derivative Works in Source or Object form. + + 3. Grant of Patent License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + (except as stated in this section) patent license to make, have made, + use, offer to sell, sell, import, and otherwise transfer the Work, + where such license applies only to those patent claims licensable + by such Contributor that are necessarily infringed by their + Contribution(s) alone or by combination of their Contribution(s) + with the Work to which such Contribution(s) was submitted. If You + institute patent litigation against any entity (including a + cross-claim or counterclaim in a lawsuit) alleging that the Work + or a Contribution incorporated within the Work constitutes direct + or contributory patent infringement, then any patent licenses + granted to You under this License for that Work shall terminate + as of the date such litigation is filed. + + 4. Redistribution. You may reproduce and distribute copies of the + Work or Derivative Works thereof in any medium, with or without + modifications, and in Source or Object form, provided that You + meet the following conditions: + + (a) You must give any other recipients of the Work or + Derivative Works a copy of this License; and + + (b) You must cause any modified files to carry prominent notices + stating that You changed the files; and + + (c) You must retain, in the Source form of any Derivative Works + that You distribute, all copyright, patent, trademark, and + attribution notices from the Source form of the Work, + excluding those notices that do not pertain to any part of + the Derivative Works; and + + (d) If the Work includes a "NOTICE" text file as part of its + distribution, then any Derivative Works that You distribute must + include a readable copy of the attribution notices contained + within such NOTICE file, excluding those notices that do not + pertain to any part of the Derivative Works, in at least one + of the following places: within a NOTICE text file distributed + as part of the Derivative Works; within the Source form or + documentation, if provided along with the Derivative Works; or, + within a display generated by the Derivative Works, if and + wherever such third-party notices normally appear. The contents + of the NOTICE file are for informational purposes only and + do not modify the License. You may add Your own attribution + notices within Derivative Works that You distribute, alongside + or as an addendum to the NOTICE text from the Work, provided + that such additional attribution notices cannot be construed + as modifying the License. + + You may add Your own copyright statement to Your modifications and + may provide additional or different license terms and conditions + for use, reproduction, or distribution of Your modifications, or + for any such Derivative Works as a whole, provided Your use, + reproduction, and distribution of the Work otherwise complies with + the conditions stated in this License. + + 5. Submission of Contributions. Unless You explicitly state otherwise, + any Contribution intentionally submitted for inclusion in the Work + by You to the Licensor shall be under the terms and conditions of + this License, without any additional terms or conditions. + Notwithstanding the above, nothing herein shall supersede or modify + the terms of any separate license agreement you may have executed + with Licensor regarding such Contributions. + + 6. Trademarks. This License does not grant permission to use the trade + names, trademarks, service marks, or product names of the Licensor, + except as required for reasonable and customary use in describing the + origin of the Work and reproducing the content of the NOTICE file. + + 7. Disclaimer of Warranty. Unless required by applicable law or + agreed to in writing, Licensor provides the Work (and each + Contributor provides its Contributions) on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied, including, without limitation, any warranties or conditions + of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A + PARTICULAR PURPOSE. You are solely responsible for determining the + appropriateness of using or redistributing the Work and assume any + risks associated with Your exercise of permissions under this License. + + 8. Limitation of Liability. In no event and under no legal theory, + whether in tort (including negligence), contract, or otherwise, + unless required by applicable law (such as deliberate and grossly + negligent acts) or agreed to in writing, shall any Contributor be + liable to You for damages, including any direct, indirect, special, + incidental, or consequential damages of any character arising as a + result of this License or out of the use or inability to use the + Work (including but not limited to damages for loss of goodwill, + work stoppage, computer failure or malfunction, or any and all + other commercial damages or losses), even if such Contributor + has been advised of the possibility of such damages. + + 9. Accepting Warranty or Additional Liability. While redistributing + the Work or Derivative Works thereof, You may choose to offer, + and charge a fee for, acceptance of support, warranty, indemnity, + or other liability obligations and/or rights consistent with this + License. However, in accepting such obligations, You may act only + on Your own behalf and on Your sole responsibility, not on behalf + of any other Contributor, and only if You agree to indemnify, + defend, and hold each Contributor harmless for any liability + incurred by, or claims asserted against, such Contributor by reason + of your accepting any such warranty or additional liability. + + END OF TERMS AND CONDITIONS + + APPENDIX: How to apply the Apache License to your work. + + To apply the Apache License to your work, attach the following + boilerplate notice, with the fields enclosed by brackets "[]" + replaced with your own identifying information. (Don't include + the brackets!) The text should be enclosed in the appropriate + comment syntax for the file format. We also recommend that a + file or class name and description of purpose be included on the + same "printed page" as the copyright notice for easier + identification within third-party archives. + + Copyright [2022] OpenTelemetry Authors + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. diff --git a/plugins/node/opentelemetry-instrumentation-redis-4/README.md b/plugins/node/opentelemetry-instrumentation-redis-4/README.md new file mode 100644 index 0000000000..21a5a9c99c --- /dev/null +++ b/plugins/node/opentelemetry-instrumentation-redis-4/README.md @@ -0,0 +1,59 @@ +# OpenTelemetry redis Instrumentation for Node.js + +[![NPM Published Version][npm-img]][npm-url] +[![Apache License][license-image]][license-image] + +This module provides automatic instrumentation for [`redis@^4.0.0`](https://github.com/NodeRedis/node_redis). + +For automatic instrumentation see the +[@opentelemetry/sdk-trace-node](https://github.com/open-telemetry/opentelemetry-js/tree/main/packages/opentelemetry-sdk-trace-node) package. + +Compatible with OpenTelemetry JS API and SDK `1.0+`. + +## Installation + +```bash +npm install --save @opentelemetry/instrumentation-redis-4 +``` + +### Supported Versions + +This package supports `redis@^4.0.0` +For versions `redis@^2.6.0` and `redis@^3.0.0`, please use `@opentelemetry/instrumentation-redis` + +## Usage + +OpenTelemetry Redis Instrumentation allows the user to automatically collect trace data and export them to the backend of choice, to give observability to distributed systems when working with [redis](https://www.npmjs.com/package/redis). + +To load a specific instrumentation (**redis** in this case), specify it in the registerInstrumentations' configuration + +```javascript +const { NodeTracerProvider } = require('@opentelemetry/sdk-trace-node'); +const { RedisInstrumentation } = require('@opentelemetry/instrumentation-redis-4'); +const { registerInstrumentations } = require('@opentelemetry/instrumentation'); + +const provider = new NodeTracerProvider(); +provider.register(); + +registerInstrumentations({ + instrumentations: [ + new RedisInstrumentation(), + ], +}) +``` + +## Useful links + +- For more information on OpenTelemetry, visit: +- For more about OpenTelemetry JavaScript: +- For help or feedback on this project, join us in [GitHub Discussions][discussions-url] + +## License + +Apache 2.0 - See [LICENSE][license-url] for more information. + +[discussions-url]: https://github.com/open-telemetry/opentelemetry-js/discussions +[license-url]: https://github.com/open-telemetry/opentelemetry-js-contrib/blob/main/LICENSE +[license-image]: https://img.shields.io/badge/license-Apache_2.0-green.svg?style=flat +[npm-url]: https://www.npmjs.com/package/@opentelemetry/instrumentation-redis-4 +[npm-img]: https://badge.fury.io/js/%40opentelemetry%2Finstrumentation-redis-4.svg diff --git a/plugins/node/opentelemetry-instrumentation-redis-4/package.json b/plugins/node/opentelemetry-instrumentation-redis-4/package.json new file mode 100644 index 0000000000..3193ba5fe1 --- /dev/null +++ b/plugins/node/opentelemetry-instrumentation-redis-4/package.json @@ -0,0 +1,76 @@ +{ + "name": "@opentelemetry/instrumentation-redis-4", + "version": "0.29.0", + "description": "OpenTelemetry redis automatic instrumentation package.", + "main": "build/src/index.js", + "types": "build/src/index.d.ts", + "repository": "open-telemetry/opentelemetry-js-contrib", + "scripts": { + "test": "nyc ts-mocha -p tsconfig.json --require '@opentelemetry/contrib-test-utils' 'test/redis.test.ts'", + "test:debug": "cross-env RUN_REDIS_TESTS_LOCAL=true ts-mocha --inspect-brk --no-timeouts -p tsconfig.json 'test/redis.test.ts'", + "test:local": "cross-env RUN_REDIS_TESTS_LOCAL=true npm run test", + "test:docker:run": "docker run --rm -d --name otel-redis -p 63790:6379 redis:alpine", + "test:docker:stop": "docker stop otel-redis", + "test-all-versions": "tav", + "test-all-versions:local": "cross-env RUN_REDIS_TESTS_LOCAL=true npm run test-all-versions", + "tdd": "npm run test -- --watch-extensions ts --watch", + "clean": "rimraf build/*", + "lint": "eslint . --ext .ts", + "lint:fix": "eslint . --ext .ts --fix", + "precompile": "tsc --version && lerna run version:update --scope @opentelemetry/instrumentation-redis --include-dependencies", + "prewatch": "npm run precompile", + "version:update": "node ../../../scripts/version-update.js", + "compile": "tsc -p .", + "prepare": "npm run compile" + }, + "keywords": [ + "instrumentation", + "nodejs", + "opentelemetry", + "profiling", + "redis", + "tracing" + ], + "author": "OpenTelemetry Authors", + "license": "Apache-2.0", + "engines": { + "node": ">=12.0.0" + }, + "files": [ + "build/src/**/*.js", + "build/src/**/*.js.map", + "build/src/**/*.d.ts", + "doc", + "LICENSE", + "README.md" + ], + "publishConfig": { + "access": "public" + }, + "peerDependencies": { + "@opentelemetry/api": "^1.0.0" + }, + "devDependencies": { + "@opentelemetry/api": "1.0.0", + "@opentelemetry/context-async-hooks": "1.0.1", + "@opentelemetry/core": "1.0.1", + "@opentelemetry/contrib-test-utils": "^0.29.0", + "@opentelemetry/sdk-trace-base": "1.0.1", + "@opentelemetry/sdk-trace-node": "1.0.1", + "@types/mocha": "7.0.2", + "@types/node": "16.11.21", + "cross-env": "7.0.3", + "gts": "3.1.0", + "mocha": "7.2.0", + "nyc": "15.1.0", + "redis": "4.0.6", + "rimraf": "3.0.2", + "test-all-versions": "5.0.1", + "ts-mocha": "8.0.0", + "typescript": "4.3.5" + }, + "dependencies": { + "@opentelemetry/instrumentation": "^0.27.0", + "@opentelemetry/semantic-conventions": "^1.0.0" + } +} diff --git a/plugins/node/opentelemetry-instrumentation-redis-4/src/index.ts b/plugins/node/opentelemetry-instrumentation-redis-4/src/index.ts new file mode 100644 index 0000000000..cad137182f --- /dev/null +++ b/plugins/node/opentelemetry-instrumentation-redis-4/src/index.ts @@ -0,0 +1,18 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export * from './instrumentation'; +export { RedisInstrumentationConfig } from './types'; diff --git a/plugins/node/opentelemetry-instrumentation-redis-4/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-redis-4/src/instrumentation.ts new file mode 100644 index 0000000000..40a11ae5cf --- /dev/null +++ b/plugins/node/opentelemetry-instrumentation-redis-4/src/instrumentation.ts @@ -0,0 +1,357 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { + diag, + trace, + context, + SpanKind, + Span, + SpanStatusCode, +} from '@opentelemetry/api'; +import { + isWrapped, + InstrumentationBase, + InstrumentationNodeModuleDefinition, + InstrumentationNodeModuleFile, +} from '@opentelemetry/instrumentation'; +import { + defaultDbStatementSerializer, +} from './utils'; +import { RedisInstrumentationConfig } from './types'; +import { VERSION } from './version'; +import { + DbSystemValues, + SemanticAttributes, +} from '@opentelemetry/semantic-conventions'; + +const OTEL_OPEN_SPANS = Symbol( + 'opentelemetry.instruemntation.redis.open_spans' +); +const MULTI_COMMAND_OPTIONS = Symbol( + 'opentelemetry.instruemntation.redis.multi_command_options' +); + +interface MutliCommandInfo { + span: Span; + commandName: string; + commandArgs: Array; +} + +const DEFAULT_CONFIG: RedisInstrumentationConfig = { + requireParentSpan: false, +}; + +export class RedisInstrumentation extends InstrumentationBase< + any +> { + static readonly COMPONENT = 'redis'; + + constructor(protected override _config: RedisInstrumentationConfig = {}) { + super('@opentelemetry/instrumentation-redis', VERSION, _config); + } + + override setConfig(config: RedisInstrumentationConfig = {}) { + this._config = Object.assign({}, DEFAULT_CONFIG, config); + } + + protected init() { + return [ + // @node-redis/client is a new package introduced and consumed by 'redis ^4.0.0' + new InstrumentationNodeModuleDefinition( + '@node-redis/client', + ['^1.0.0'], + (moduleExports: any, moduleVersion?: string) => { + diag.debug( + `Patching @node-redis/client@${moduleVersion} (redis@^4.x.x)` + ); + return moduleExports; + }, + (_moduleExports: any, moduleVersion?: string) => { + diag.debug( + `Unpatching @node-redis/client@${moduleVersion} (redis@^4.x.x)` + ); + }, + [ + new InstrumentationNodeModuleFile( + '@node-redis/client/dist/lib/commander.js', + ['^1.0.0'], + (moduleExports: any) => { + const transformCommandArguments = + moduleExports.transformCommandArguments; + if (!transformCommandArguments) { + this._diag.error( + 'internal instrumentation error, missing transformCommandArguments function' + ); + return moduleExports; + } + + // this is the function that extend a redis client with a list of commands. + // the function patches the commandExecutor to record a span + this._diag.debug('Patching redis commands executor'); + if (isWrapped(moduleExports?.extendWithCommands)) { + this._unwrap(moduleExports, 'extendWithCommands'); + } + this._wrap( + moduleExports, + 'extendWithCommands', + this._getPatchExtendWithCommands(transformCommandArguments) + ); + + return moduleExports; + }, + (moduleExports: any) => { + this._diag.debug('Unpatching redis commands executor'); + if (isWrapped(moduleExports?.extendWithCommands)) { + this._unwrap(moduleExports, 'extendWithCommands'); + } + } + ), + new InstrumentationNodeModuleFile( + '@node-redis/client/dist/lib/client/multi-command.js', + ['^1.0.0'], + (moduleExports: any) => { + this._diag.debug('Patching redis multi commands executor'); + const redisClientMultiCommandPrototype = + moduleExports?.default?.prototype; + if (isWrapped(redisClientMultiCommandPrototype?.exec)) { + this._unwrap(redisClientMultiCommandPrototype, 'exec'); + } + this._wrap( + redisClientMultiCommandPrototype, + 'exec', + this._getPatchMultiCommandsExec() + ); + + return moduleExports; + }, + (moduleExports: any) => { + this._diag.debug('Unpatching redis multi commands executor'); + const redisClientMultiCommandPrototype = + moduleExports?.default?.prototype; + if (isWrapped(redisClientMultiCommandPrototype?.exec)) { + this._unwrap(redisClientMultiCommandPrototype, 'exec'); + } + } + ), + new InstrumentationNodeModuleFile( + '@node-redis/client/dist/lib/client/index.js', + ['^1.0.0'], + (moduleExports: any) => { + this._diag.debug('Patching redis client'); + const redisClientPrototype = moduleExports?.default?.prototype; + + if (isWrapped(redisClientPrototype?.multi)) { + this._unwrap(redisClientPrototype, 'multi'); + } + this._wrap( + redisClientPrototype, + 'multi', + this._getPatchRedisClientMulti() + ); + + return moduleExports; + }, + (moduleExports: any) => { + this._diag.debug('Unpatching redis client'); + const redisClientPrototype = moduleExports?.default?.prototype; + if (isWrapped(redisClientPrototype?.multi)) { + this._unwrap(redisClientPrototype, 'multi'); + } + } + ), + ] + ), + + ]; + } + + private _getPatchExtendWithCommands(transformCommandArguments: Function) { + const plugin = this; + return function extendWithCommandsPatchWrapper(original: Function) { + return function extendWithCommandsPatch(this: any, config: any) { + const origExecutor = config.executor; + config.executor = function ( + this: any, + command: any, + args: Array + ) { + const hasNoParentSpan = trace.getSpan(context.active()) === undefined; + if (hasNoParentSpan && plugin._config?.requireParentSpan) { + return origExecutor.apply(this, arguments); + } + + const redisCommandArguments = transformCommandArguments( + command, + args + ).args; + const clientOptions = this.options || this[MULTI_COMMAND_OPTIONS]; + + const commandName = redisCommandArguments[0] as string; // types also allows it to be a Buffer, but in practice it only string + const commandArgs = redisCommandArguments.slice(1); + + const dbStatementSerializer = + plugin._config?.dbStatementSerializer || + defaultDbStatementSerializer; + + const attributes = { + [SemanticAttributes.DB_SYSTEM]: DbSystemValues.REDIS, + [SemanticAttributes.NET_PEER_NAME]: clientOptions?.socket?.host, + [SemanticAttributes.NET_PEER_PORT]: clientOptions?.socket?.port, + [SemanticAttributes.DB_CONNECTION_STRING]: clientOptions?.url, + }; + + try { + const dbStatement = dbStatementSerializer(commandName, commandArgs); + if (dbStatement != null) { + attributes[SemanticAttributes.DB_STATEMENT] = dbStatement; + } + } catch (e) { + plugin._diag.error('dbStatementSerializer throw an exception', e); + } + + const span = plugin.tracer.startSpan( + `${RedisInstrumentation.COMPONENT}-${commandName}`, + { + kind: SpanKind.CLIENT, + attributes, + } + ); + + const res = origExecutor.apply(this, arguments); + if (res.then) { + res.then( + (redisRes: unknown) => { + plugin._endSpanWithResponse( + span, + commandName, + commandArgs, + redisRes, + undefined + ); + }, + (err: any) => { + plugin._endSpanWithResponse( + span, + commandName, + commandArgs, + null, + err + ); + } + ); + } else { + const redisClientMultiCommand = res as { + [OTEL_OPEN_SPANS]?: Array; + }; + redisClientMultiCommand[OTEL_OPEN_SPANS] = + redisClientMultiCommand[OTEL_OPEN_SPANS] || []; + redisClientMultiCommand[OTEL_OPEN_SPANS]!.push({ + span, + commandName, + commandArgs, + }); + } + return res; + }; + return original.apply(this, arguments); + }; + }; + } + + private _getPatchMultiCommandsExec() { + const plugin = this; + return function execPatchWrapper(original: Function) { + return function execPatch(this: any) { + const execRes = original.apply(this, arguments); + if (typeof(execRes?.then) !== 'function') { + this._diag( + 'got non promise result when patching RedisClientMultiCommand.exec' + ); + return execRes; + } + + execRes.then((redisRes: unknown[]) => { + const openSpans = this[OTEL_OPEN_SPANS]; + if (!openSpans) { + return this._diag.error( + 'cannot find open spans to end for redis multi command' + ); + } + if (redisRes.length !== openSpans.length) { + return this._diag.error( + 'number of multi command spans does not match response from redis' + ); + } + for (let i = 0; i < openSpans.length; i++) { + const { span, commandName, commandArgs } = openSpans[i]; + const currCommandRes = redisRes[i]; + if (currCommandRes instanceof Error) { + plugin._endSpanWithResponse( + span, + commandName, + commandArgs, + null, + currCommandRes + ); + } else { + plugin._endSpanWithResponse( + span, + commandName, + commandArgs, + currCommandRes, + undefined + ); + } + } + }); + return execRes; + }; + }; + } + + private _getPatchRedisClientMulti() { + return function multiPatchWrapper(original: Function) { + return function multiPatch(this: any) { + const multiRes = original.apply(this, arguments); + multiRes[MULTI_COMMAND_OPTIONS] = this.options; + return multiRes; + }; + }; + } + + private _endSpanWithResponse( + span: Span, + commandName: string, + commandArgs: string[], + response: unknown, + error: Error | undefined + ) { + if (!error && this._config.responseHook) { + try { + this._config.responseHook(span, commandName, commandArgs, response); + } catch (err) { + this._diag.error('responseHook throw an exception', err); + } + } + if (error) { + span.recordException(error); + span.setStatus({ code: SpanStatusCode.ERROR, message: error?.message }); + } + span.end(); + } + +} diff --git a/plugins/node/opentelemetry-instrumentation-redis-4/src/types.ts b/plugins/node/opentelemetry-instrumentation-redis-4/src/types.ts new file mode 100644 index 0000000000..e15141a22e --- /dev/null +++ b/plugins/node/opentelemetry-instrumentation-redis-4/src/types.ts @@ -0,0 +1,59 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Span } from '@opentelemetry/api'; +import { InstrumentationConfig } from '@opentelemetry/instrumentation'; + +/** + * Function that can be used to serialize db.statement tag + * @param cmdName - The name of the command (eg. set, get, mset) + * @param cmdArgs - Array of arguments passed to the command + * + * @returns serialized string that will be used as the db.statement attribute. + */ +export type DbStatementSerializer = ( + cmdName: string, + cmdArgs: Array +) => string; + +/** + * Function that can be used to add custom attributes to span on response from redis server + * @param span - The span created for the redis command, on which attributes can be set + * @param cmdName - The name of the command (eg. set, get, mset) + * @param cmdArgs - Array of arguments passed to the command + * @param response - The response object which is returned to the user who called this command. + * Can be used to set custom attributes on the span. + * The type of the response varies depending on the specific command. + */ +export interface RedisResponseCustomAttributeFunction { + ( + span: Span, + cmdName: string, + cmdArgs: Array, + response: unknown + ): void; +} + +export interface RedisInstrumentationConfig extends InstrumentationConfig { + /** Custom serializer function for the db.statement tag */ + dbStatementSerializer?: DbStatementSerializer; + + /** Function for adding custom attributes on db response */ + responseHook?: RedisResponseCustomAttributeFunction; + + /** Require parent to create redis span, default when unset is false */ + requireParentSpan?: boolean; +} diff --git a/plugins/node/opentelemetry-instrumentation-redis-4/src/utils.ts b/plugins/node/opentelemetry-instrumentation-redis-4/src/utils.ts new file mode 100644 index 0000000000..ce7567a08f --- /dev/null +++ b/plugins/node/opentelemetry-instrumentation-redis-4/src/utils.ts @@ -0,0 +1,5 @@ +import { DbStatementSerializer } from "./types"; + +export const defaultDbStatementSerializer: DbStatementSerializer = cmdName => + cmdName; + diff --git a/plugins/node/opentelemetry-instrumentation-redis-4/test/redis.test.ts b/plugins/node/opentelemetry-instrumentation-redis-4/test/redis.test.ts new file mode 100644 index 0000000000..4afeaee7cf --- /dev/null +++ b/plugins/node/opentelemetry-instrumentation-redis-4/test/redis.test.ts @@ -0,0 +1,409 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { + getTestSpans, + registerInstrumentationTesting, +} from '@opentelemetry/contrib-test-utils'; +import { RedisInstrumentation } from '../src'; +import * as assert from 'assert'; + +import { + redisTestConfig, + redisTestUrl, + shouldTest, + shouldTestLocal, +} from './utils'; +import * as testUtils from '@opentelemetry/contrib-test-utils'; + +const instrumentation = registerInstrumentationTesting( + new RedisInstrumentation() +); + +import { createClient } from 'redis'; +import { + Span, + SpanKind, + SpanStatusCode, + trace, + context, +} from '@opentelemetry/api'; +import { SemanticAttributes } from '@opentelemetry/semantic-conventions'; +import { RedisResponseCustomAttributeFunction } from '../src/types'; +import { hrTimeToMilliseconds } from '@opentelemetry/core'; + +describe('redis@^4.0.0', () => { + before(function () { + // needs to be "function" to have MochaContext "this" context + if (!shouldTest) { + // this.skip() workaround + // https://github.com/mochajs/mocha/issues/2683#issuecomment-375629901 + this.test!.parent!.pending = true; + this.skip(); + } + + if (shouldTestLocal) { + testUtils.startDocker('redis'); + } + }); + + after(() => { + if (shouldTestLocal) { + testUtils.cleanUpDocker('redis'); + } + }); + + let client: any; + + beforeEach(async () => { + client = createClient({ + url: redisTestUrl, + }); + await client.connect(); + }); + + afterEach(async () => { + await client?.disconnect(); + }); + + describe('redis commands', () => { + it('simple set and get', async () => { + await client.set('key', 'value'); + const value = await client.get('key'); + assert.strictEqual(value, 'value'); // verify we did not screw up the normal functionality + + const spans = getTestSpans(); + assert.strictEqual(spans.length, 2); + + const redisSetSpan = spans.find(s => s.name.includes('SET')); + assert.ok(redisSetSpan); + assert.strictEqual(redisSetSpan?.kind, SpanKind.CLIENT); + assert.strictEqual(redisSetSpan?.name, 'redis-SET'); + assert.strictEqual( + redisSetSpan?.attributes[SemanticAttributes.DB_SYSTEM], + 'redis' + ); + assert.strictEqual( + redisSetSpan?.attributes[SemanticAttributes.DB_STATEMENT], + 'SET' + ); + assert.strictEqual( + redisSetSpan?.attributes[SemanticAttributes.NET_PEER_NAME], + redisTestConfig.host + ); + assert.strictEqual( + redisSetSpan?.attributes[SemanticAttributes.NET_PEER_PORT], + redisTestConfig.port + ); + assert.strictEqual( + redisSetSpan?.attributes[SemanticAttributes.DB_CONNECTION_STRING], + redisTestUrl + ); + + const redisGetSpan = spans.find(s => s.name.includes('GET')); + assert.ok(redisGetSpan); + assert.strictEqual(redisGetSpan?.kind, SpanKind.CLIENT); + assert.strictEqual(redisGetSpan?.name, 'redis-GET'); + assert.strictEqual( + redisGetSpan?.attributes[SemanticAttributes.DB_SYSTEM], + 'redis' + ); + assert.strictEqual( + redisGetSpan?.attributes[SemanticAttributes.DB_STATEMENT], + 'GET' + ); + assert.strictEqual( + redisGetSpan?.attributes[SemanticAttributes.NET_PEER_NAME], + redisTestConfig.host + ); + assert.strictEqual( + redisGetSpan?.attributes[SemanticAttributes.NET_PEER_PORT], + redisTestConfig.port + ); + assert.strictEqual( + redisGetSpan?.attributes[SemanticAttributes.DB_CONNECTION_STRING], + redisTestUrl + ); + }); + + it('span with error', async () => { + await client.set('string-key', 'string-value'); + await assert.rejects(async () => await client.incr('string-key')); + + const [_setSpan, incrSpan] = getTestSpans(); + + assert.ok(incrSpan); + assert.strictEqual(incrSpan?.status.code, SpanStatusCode.ERROR); + assert.strictEqual( + incrSpan?.status.message, + 'ERR value is not an integer or out of range' + ); + + const exceptions = incrSpan.events.filter( + event => event.name === 'exception' + ); + assert.strictEqual(exceptions.length, 1); + assert.strictEqual( + exceptions?.[0].attributes?.[SemanticAttributes.EXCEPTION_TYPE], + 'ReplyError' + ); + assert.strictEqual( + exceptions?.[0].attributes?.[SemanticAttributes.EXCEPTION_MESSAGE], + 'ERR value is not an integer or out of range' + ); + }); + }); + + describe('multi (transactions) commands', () => { + it('multi commands', async () => { + await client.set('another-key', 'another-value'); + const [setKeyReply, otherKeyValue] = await client + .multi() + .set('key', 'value') + .get('another-key') + .exec(); // ['OK', 'another-value'] + + assert.strictEqual(setKeyReply, 'OK'); // verify we did not screw up the normal functionality + assert.strictEqual(otherKeyValue, 'another-value'); // verify we did not screw up the normal functionality + + const [setSpan, multiSetSpan, multiGetSpan] = getTestSpans(); + + assert.ok(setSpan); + + assert.ok(multiSetSpan); + assert.strictEqual(multiSetSpan.name, 'redis-SET'); + assert.strictEqual( + multiSetSpan.attributes[SemanticAttributes.DB_STATEMENT], + 'SET' + ); + assert.strictEqual( + multiSetSpan?.attributes[SemanticAttributes.NET_PEER_NAME], + redisTestConfig.host + ); + assert.strictEqual( + multiSetSpan?.attributes[SemanticAttributes.NET_PEER_PORT], + redisTestConfig.port + ); + assert.strictEqual( + multiSetSpan?.attributes[SemanticAttributes.DB_CONNECTION_STRING], + redisTestUrl + ); + + assert.ok(multiGetSpan); + assert.strictEqual(multiGetSpan.name, 'redis-GET'); + assert.strictEqual( + multiGetSpan.attributes[SemanticAttributes.DB_STATEMENT], + 'GET' + ); + assert.strictEqual( + multiGetSpan?.attributes[SemanticAttributes.NET_PEER_NAME], + redisTestConfig.host + ); + assert.strictEqual( + multiGetSpan?.attributes[SemanticAttributes.NET_PEER_PORT], + redisTestConfig.port + ); + assert.strictEqual( + multiGetSpan?.attributes[SemanticAttributes.DB_CONNECTION_STRING], + redisTestUrl + ); + }); + + it('multi command with error', async () => { + const [setReply, incrReply] = await client + .multi() + .set('key', 'value') + .incr('key') + .exec(); // ['OK', 'ReplyError'] + assert.strictEqual(setReply, 'OK'); // verify we did not screw up the normal functionality + assert.ok(incrReply instanceof Error); // verify we did not screw up the normal functionality + + const [multiSetSpan, multiIncrSpan] = getTestSpans(); + + assert.ok(multiSetSpan); + assert.strictEqual(multiSetSpan.status.code, SpanStatusCode.UNSET); + + assert.ok(multiIncrSpan); + assert.strictEqual(multiIncrSpan.status.code, SpanStatusCode.ERROR); + assert.strictEqual( + multiIncrSpan.status.message, + 'ERR value is not an integer or out of range' + ); + }); + + it('duration covers create until server response', async () => { + await client.set('another-key', 'another-value'); + const multiClient = client.multi(); + let commands = multiClient.set('key', 'value'); + // wait 10 ms before adding next command + // simulate long operation + await new Promise(resolve => setTimeout(resolve, 10)); + commands = commands.get('another-key'); + const [setKeyReply, otherKeyValue] = await commands.exec(); // ['OK', 'another-value'] + + assert.strictEqual(setKeyReply, 'OK'); // verify we did not screw up the normal functionality + assert.strictEqual(otherKeyValue, 'another-value'); // verify we did not screw up the normal functionality + + const [_setSpan, multiSetSpan, multiGetSpan] = getTestSpans(); + // verify that commands span started when it was added to multi and not when "sent". + // they were called with 10 ms gap between them, so it should be reflected in the span start time + // could be nice feature in the future to capture an event for when it is actually sent + const startTimeDiff = + hrTimeToMilliseconds(multiGetSpan.startTime) - + hrTimeToMilliseconds(multiSetSpan.startTime); + assert.ok( + startTimeDiff >= 9, + `diff of start time should be >= 10 and it's ${startTimeDiff}` + ); + + const endTimeDiff = + hrTimeToMilliseconds(multiGetSpan.endTime) - + hrTimeToMilliseconds(multiSetSpan.endTime); + assert.ok(endTimeDiff < 10); // spans should all end together when multi response arrives from redis server + }); + + it('response hook for multi commands', async () => { + const responseHook: RedisResponseCustomAttributeFunction = ( + span: Span, + cmdName: string, + cmdArgs: Array, + response: unknown + ) => { + span.setAttribute('test.cmd.name', cmdName); + span.setAttribute('test.cmd.args', cmdArgs as string[]); + span.setAttribute('test.db.response', response as string); + }; + instrumentation.setConfig({ responseHook }); + + await client.set('another-key', 'another-value'); + const [setKeyReply, otherKeyValue] = await client + .multi() + .set('key', 'value') + .get('another-key') + .exec(); // ['OK', 'another-value'] + assert.strictEqual(setKeyReply, 'OK'); // verify we did not screw up the normal functionality + assert.strictEqual(otherKeyValue, 'another-value'); // verify we did not screw up the normal functionality + + const [_setSpan, multiSetSpan, multiGetSpan] = getTestSpans(); + + assert.ok(multiSetSpan); + assert.strictEqual(multiSetSpan.attributes['test.cmd.name'], 'SET'); + assert.deepStrictEqual(multiSetSpan.attributes['test.cmd.args'], [ + 'key', + 'value', + ]); + assert.strictEqual(multiSetSpan.attributes['test.db.response'], 'OK'); + + assert.ok(multiGetSpan); + assert.strictEqual(multiGetSpan.attributes['test.cmd.name'], 'GET'); + assert.deepStrictEqual(multiGetSpan.attributes['test.cmd.args'], [ + 'another-key', + ]); + assert.strictEqual( + multiGetSpan.attributes['test.db.response'], + 'another-value' + ); + }); + }); + + describe('config', () => { + describe('dbStatementSerializer', () => { + it('custom dbStatementSerializer', async () => { + const dbStatementSerializer = ( + cmdName: string, + cmdArgs: Array + ) => { + return `${cmdName} ${cmdArgs.join(' ')}`; + }; + + instrumentation.setConfig({ dbStatementSerializer }); + await client.set('key', 'value'); + const [span] = getTestSpans(); + assert.strictEqual( + span.attributes[SemanticAttributes.DB_STATEMENT], + 'SET key value' + ); + }); + + it('dbStatementSerializer throws', async () => { + const dbStatementSerializer = () => { + throw new Error('dbStatementSerializer error'); + }; + + instrumentation.setConfig({ dbStatementSerializer }); + await client.set('key', 'value'); + const [span] = getTestSpans(); + assert.ok(span); + assert.ok(!(SemanticAttributes.DB_STATEMENT in span.attributes)); + }); + }); + + describe('responseHook', () => { + it('valid response hook', async () => { + const responseHook: RedisResponseCustomAttributeFunction = ( + span: Span, + cmdName: string, + cmdArgs: Array, + response: unknown + ) => { + span.setAttribute('test.cmd.name', cmdName); + span.setAttribute('test.cmd.args', cmdArgs as string[]); + span.setAttribute('test.db.response', response as string); + }; + instrumentation.setConfig({ responseHook }); + await client.set('key', 'value'); + const [span] = getTestSpans(); + assert.ok(span); + assert.strictEqual(span.attributes['test.cmd.name'], 'SET'); + assert.deepStrictEqual(span.attributes['test.cmd.args'], [ + 'key', + 'value', + ]); + assert.strictEqual(span.attributes['test.db.response'], 'OK'); + }); + + it('responseHook throws', async () => { + const responseHook = () => { + throw new Error('responseHook error'); + }; + instrumentation.setConfig({ responseHook }); + const res = await client.set('key', 'value'); + assert.strictEqual(res, 'OK'); // package is still functional + const [span] = getTestSpans(); + assert.ok(span); + }); + }); + + describe('requireParentSpan', () => { + it('set to true', async () => { + instrumentation.setConfig({ requireParentSpan: true }); + + // no parent span => no redis span + const res = await client.set('key', 'value'); + assert.strictEqual(res, 'OK'); // verify we did not screw up the normal functionality + assert.ok(getTestSpans().length === 0); + + // has ambient span => redis span + const span = trace.getTracer('test').startSpan('test span'); + await context.with(trace.setSpan(context.active(), span), async () => { + const res = await client.set('key', 'value'); + assert.strictEqual(res, 'OK'); // verify we did not screw up the normal functionality + assert.ok(getTestSpans().length === 1); + }); + span.end(); + }); + }); + }); +}); diff --git a/plugins/node/opentelemetry-instrumentation-redis-4/test/utils.ts b/plugins/node/opentelemetry-instrumentation-redis-4/test/utils.ts new file mode 100644 index 0000000000..c60c703e56 --- /dev/null +++ b/plugins/node/opentelemetry-instrumentation-redis-4/test/utils.ts @@ -0,0 +1,24 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +export const redisTestConfig = { + host: process.env.OPENTELEMETRY_REDIS_HOST || 'localhost', + port: process.env.OPENTELEMETRY_REDIS_PORT || 63790, +}; + +export const redisTestUrl = `redis://${redisTestConfig.host}:${redisTestConfig.port}`; + +export const shouldTestLocal = process.env.RUN_REDIS_TESTS_LOCAL; +export const shouldTest = process.env.RUN_REDIS_TESTS || shouldTestLocal; diff --git a/plugins/node/opentelemetry-instrumentation-redis-4/tsconfig.json b/plugins/node/opentelemetry-instrumentation-redis-4/tsconfig.json new file mode 100644 index 0000000000..28be80d266 --- /dev/null +++ b/plugins/node/opentelemetry-instrumentation-redis-4/tsconfig.json @@ -0,0 +1,11 @@ +{ + "extends": "../../../tsconfig.base", + "compilerOptions": { + "rootDir": ".", + "outDir": "build" + }, + "include": [ + "src/**/*.ts", + "test/**/*.ts" + ] +} From bafee4658ef8d79a909a03ac8b0e8fa3361604d8 Mon Sep 17 00:00:00 2001 From: Amir Blum Date: Sat, 30 Apr 2022 14:23:42 +0300 Subject: [PATCH 06/22] revert(redis): instrumentation-redis back to support only v2 and v3 --- .../package.json | 2 +- .../.tav.yml | 10 +- .../README.md | 3 +- .../package.json | 10 +- .../src/instrumentation.ts | 319 +------------- .../src/types.ts | 8 +- .../src/utils.ts | 5 +- .../{redis_v2_v3.test.ts => redis.test.ts} | 34 +- .../test/redis_v4.test.ts | 409 ------------------ 9 files changed, 36 insertions(+), 764 deletions(-) rename plugins/node/opentelemetry-instrumentation-redis/test/{redis_v2_v3.test.ts => redis.test.ts} (94%) delete mode 100644 plugins/node/opentelemetry-instrumentation-redis/test/redis_v4.test.ts diff --git a/plugins/node/opentelemetry-instrumentation-redis-4/package.json b/plugins/node/opentelemetry-instrumentation-redis-4/package.json index 3193ba5fe1..491c1d658e 100644 --- a/plugins/node/opentelemetry-instrumentation-redis-4/package.json +++ b/plugins/node/opentelemetry-instrumentation-redis-4/package.json @@ -17,7 +17,7 @@ "clean": "rimraf build/*", "lint": "eslint . --ext .ts", "lint:fix": "eslint . --ext .ts --fix", - "precompile": "tsc --version && lerna run version:update --scope @opentelemetry/instrumentation-redis --include-dependencies", + "precompile": "tsc --version && lerna run version:update --scope @opentelemetry/instrumentation-redis-4 --include-dependencies", "prewatch": "npm run precompile", "version:update": "node ../../../scripts/version-update.js", "compile": "tsc -p .", diff --git a/plugins/node/opentelemetry-instrumentation-redis/.tav.yml b/plugins/node/opentelemetry-instrumentation-redis/.tav.yml index 3150bc1c8f..d22ee83fdf 100644 --- a/plugins/node/opentelemetry-instrumentation-redis/.tav.yml +++ b/plugins/node/opentelemetry-instrumentation-redis/.tav.yml @@ -1,12 +1,6 @@ redis: - - jobs: - - versions: "^2.6.0" - commands: npm run test:v2 - - versions: "^3.0.0" - commands: npm run test:v3 - - versions: "^4.0.0" - commands: npm run test:v4 + versions: ^2.6.0 || ^3.0.0 + commands: npm run test # Fix missing `contrib-test-utils` package pretest: npm run --prefix ../../../ lerna:link diff --git a/plugins/node/opentelemetry-instrumentation-redis/README.md b/plugins/node/opentelemetry-instrumentation-redis/README.md index 80d856fb9c..7e3ac9db5e 100644 --- a/plugins/node/opentelemetry-instrumentation-redis/README.md +++ b/plugins/node/opentelemetry-instrumentation-redis/README.md @@ -18,7 +18,8 @@ npm install --save @opentelemetry/instrumentation-redis ### Supported Versions -- `^2.6.0 || ^3.0.0 || ^4.0.0` +This package supports `redis@^2.6.0` and `redis@^3.0.0` +For version `redis@^4.0.0`, please use `@opentelemetry/instrumentation-redis-4` ## Usage diff --git a/plugins/node/opentelemetry-instrumentation-redis/package.json b/plugins/node/opentelemetry-instrumentation-redis/package.json index 32c5cbb2c6..5564202c5d 100644 --- a/plugins/node/opentelemetry-instrumentation-redis/package.json +++ b/plugins/node/opentelemetry-instrumentation-redis/package.json @@ -6,11 +6,8 @@ "types": "build/src/index.d.ts", "repository": "open-telemetry/opentelemetry-js-contrib", "scripts": { - "test": "npm run test:v3", - "test:v2": "nyc ts-mocha -p tsconfig.json 'test/redis_v2_v3.test.ts'", - "test:v3": "nyc ts-mocha -p tsconfig.json 'test/redis_v2_v3.test.ts'", - "test:v4": "nyc ts-mocha -p tsconfig.json --require '@opentelemetry/contrib-test-utils' 'test/redis_v4.test.ts'", - "test:debug": "cross-env RUN_REDIS_TESTS_LOCAL=true ts-mocha --inspect-brk --no-timeouts -p tsconfig.json 'test/redis_v2_v3.test.ts'", + "test": "nyc ts-mocha -p tsconfig.json 'test/**/*.test.ts'", + "test:debug": "cross-env RUN_REDIS_TESTS_LOCAL=true ts-mocha --inspect-brk --no-timeouts -p tsconfig.json 'test/**/*.test.ts'", "test:local": "cross-env RUN_REDIS_TESTS_LOCAL=true npm run test", "test:docker:run": "docker run --rm -d --name otel-redis -p 63790:6379 redis:alpine", "test:docker:stop": "docker stop otel-redis", @@ -37,7 +34,7 @@ "author": "OpenTelemetry Authors", "license": "Apache-2.0", "engines": { - "node": ">=12.0.0" + "node": ">=8.12.0" }, "files": [ "build/src/**/*.js", @@ -56,7 +53,6 @@ "devDependencies": { "@opentelemetry/api": "1.0.2", "@opentelemetry/context-async-hooks": "1.0.1", - "@opentelemetry/core": "1.0.1", "@opentelemetry/contrib-test-utils": "^0.29.0", "@opentelemetry/sdk-trace-base": "1.0.1", "@opentelemetry/sdk-trace-node": "1.0.1", diff --git a/plugins/node/opentelemetry-instrumentation-redis/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-redis/src/instrumentation.ts index 70e086c90d..676a9a374b 100644 --- a/plugins/node/opentelemetry-instrumentation-redis/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-redis/src/instrumentation.ts @@ -14,46 +14,20 @@ * limitations under the License. */ -import { - diag, - trace, - context, - SpanKind, - Span, - SpanStatusCode, -} from '@opentelemetry/api'; +import { diag } from '@opentelemetry/api'; import { isWrapped, InstrumentationBase, InstrumentationNodeModuleDefinition, - InstrumentationNodeModuleFile, } from '@opentelemetry/instrumentation'; import type * as redisTypes from 'redis'; import { - defaultDbStatementSerializer, getTracedCreateClient, getTracedCreateStreamTrace, getTracedInternalSendCommand, } from './utils'; import { RedisInstrumentationConfig } from './types'; import { VERSION } from './version'; -import { - DbSystemValues, - SemanticAttributes, -} from '@opentelemetry/semantic-conventions'; - -const OTEL_OPEN_SPANS = Symbol( - 'opentelemetry.instruemntation.redis.open_spans' -); -const MULTI_COMMAND_OPTIONS = Symbol( - 'opentelemetry.instruemntation.redis.multi_command_options' -); - -interface MutliCommandInfo { - span: Span; - commandName: string; - commandArgs: Array; -} const DEFAULT_CONFIG: RedisInstrumentationConfig = { requireParentSpan: false, @@ -74,118 +48,12 @@ export class RedisInstrumentation extends InstrumentationBase< protected init() { return [ - // @node-redis/client is a new package introduced and consumed by 'redis ^4.0.0' - new InstrumentationNodeModuleDefinition( - '@node-redis/client', - ['^1.0.0'], - (moduleExports: any, moduleVersion?: string) => { - diag.debug( - `Patching @node-redis/client@${moduleVersion} (redis@^4.x.x)` - ); - return moduleExports; - }, - (_moduleExports: any, moduleVersion?: string) => { - diag.debug( - `Unpatching @node-redis/client@${moduleVersion} (redis@^4.x.x)` - ); - }, - [ - new InstrumentationNodeModuleFile( - '@node-redis/client/dist/lib/commander.js', - ['^1.0.0'], - (moduleExports: any) => { - const transformCommandArguments = - moduleExports.transformCommandArguments; - if (!transformCommandArguments) { - this._diag.error( - 'internal instrumentation error, missing transformCommandArguments function' - ); - return moduleExports; - } - - // this is the function that extend a redis client with a list of commands. - // the function patches the commandExecutor to record a span - this._diag.debug('Patching redis commands executor'); - if (isWrapped(moduleExports?.extendWithCommands)) { - this._unwrap(moduleExports, 'extendWithCommands'); - } - this._wrap( - moduleExports, - 'extendWithCommands', - this._getPatchExtendWithCommandsV4(transformCommandArguments) - ); - - return moduleExports; - }, - (moduleExports: any) => { - this._diag.debug('Unpatching redis commands executor'); - if (isWrapped(moduleExports?.extendWithCommands)) { - this._unwrap(moduleExports, 'extendWithCommands'); - } - } - ), - new InstrumentationNodeModuleFile( - '@node-redis/client/dist/lib/client/multi-command.js', - ['^1.0.0'], - (moduleExports: any) => { - this._diag.debug('Patching redis multi commands executor'); - const redisClientMultiCommandPrototype = - moduleExports?.default?.prototype; - if (isWrapped(redisClientMultiCommandPrototype?.exec)) { - this._unwrap(redisClientMultiCommandPrototype, 'exec'); - } - this._wrap( - redisClientMultiCommandPrototype, - 'exec', - this._getPatchMultiCommandsExecV4() - ); - - return moduleExports; - }, - (moduleExports: any) => { - this._diag.debug('Unpatching redis multi commands executor'); - const redisClientMultiCommandPrototype = - moduleExports?.default?.prototype; - if (isWrapped(redisClientMultiCommandPrototype?.exec)) { - this._unwrap(redisClientMultiCommandPrototype, 'exec'); - } - } - ), - new InstrumentationNodeModuleFile( - '@node-redis/client/dist/lib/client/index.js', - ['^1.0.0'], - (moduleExports: any) => { - this._diag.debug('Patching redis client'); - const redisClientPrototype = moduleExports?.default?.prototype; - - if (isWrapped(redisClientPrototype?.multi)) { - this._unwrap(redisClientPrototype, 'multi'); - } - this._wrap( - redisClientPrototype, - 'multi', - this._getPatchRedisClientMultiV4() - ); - - return moduleExports; - }, - (moduleExports: any) => { - this._diag.debug('Unpatching redis client'); - const redisClientPrototype = moduleExports?.default?.prototype; - if (isWrapped(redisClientPrototype?.multi)) { - this._unwrap(redisClientPrototype, 'multi'); - } - } - ), - ] - ), - new InstrumentationNodeModuleDefinition( 'redis', ['^2.6.0', '^3.0.0'], (moduleExports, moduleVersion) => { - this._diag.debug(`Patching redis@${moduleVersion}`); - this._diag.debug('Patching redis.RedisClient.internal_send_command'); + diag.debug(`Patching redis@${moduleVersion}`); + diag.debug('Patching redis.RedisClient.internal_send_command'); if ( isWrapped( moduleExports.RedisClient.prototype['internal_send_command'] @@ -199,7 +67,7 @@ export class RedisInstrumentation extends InstrumentationBase< this._wrap( moduleExports.RedisClient.prototype, 'internal_send_command', - this._getPatchInternalSendCommandV2V3() + this._getPatchInternalSendCommand() ); diag.debug('patching redis.RedisClient.create_stream'); @@ -235,187 +103,10 @@ export class RedisInstrumentation extends InstrumentationBase< ), ]; } - - private _getPatchExtendWithCommandsV4(transformCommandArguments: Function) { - const plugin = this; - return function extendWithCommandsPatchWrapper(original: Function) { - return function extendWithCommandsPatch(this: any, config: any) { - const origExecutor = config.executor; - config.executor = function ( - this: any, - command: any, - args: Array - ) { - const hasNoParentSpan = trace.getSpan(context.active()) === undefined; - if (hasNoParentSpan && plugin._config?.requireParentSpan) { - return origExecutor.apply(this, arguments); - } - - const redisCommandArguments = transformCommandArguments( - command, - args - ).args; - const clientOptions = this.options || this[MULTI_COMMAND_OPTIONS]; - - const commandName = redisCommandArguments[0] as string; // types also allows it to be a Buffer, but in practice it only string - const commandArgs = redisCommandArguments.slice(1); - - const dbStatementSerializer = - plugin._config?.dbStatementSerializer || - defaultDbStatementSerializer; - - const attributes = { - [SemanticAttributes.DB_SYSTEM]: DbSystemValues.REDIS, - [SemanticAttributes.NET_PEER_NAME]: clientOptions?.socket?.host, - [SemanticAttributes.NET_PEER_PORT]: clientOptions?.socket?.port, - [SemanticAttributes.DB_CONNECTION_STRING]: clientOptions?.url, - }; - - try { - const dbStatement = dbStatementSerializer(commandName, commandArgs); - if (dbStatement != null) { - attributes[SemanticAttributes.DB_STATEMENT] = dbStatement; - } - } catch (e) { - plugin._diag.error('dbStatementSerializer throw an exception', e); - } - - const span = plugin.tracer.startSpan( - `${RedisInstrumentation.COMPONENT}-${commandName}`, - { - kind: SpanKind.CLIENT, - attributes, - } - ); - - const res = origExecutor.apply(this, arguments); - if (res.then) { - res.then( - (redisRes: unknown) => { - plugin._endSpanWithResponse( - span, - commandName, - commandArgs, - redisRes, - undefined - ); - }, - (err: any) => { - plugin._endSpanWithResponse( - span, - commandName, - commandArgs, - null, - err - ); - } - ); - } else { - const redisClientMultiCommand = res as { - [OTEL_OPEN_SPANS]?: Array; - }; - redisClientMultiCommand[OTEL_OPEN_SPANS] = - redisClientMultiCommand[OTEL_OPEN_SPANS] || []; - redisClientMultiCommand[OTEL_OPEN_SPANS]!.push({ - span, - commandName, - commandArgs, - }); - } - return res; - }; - return original.apply(this, arguments); - }; - }; - } - - private _getPatchMultiCommandsExecV4() { - const plugin = this; - return function execPatchWrapper(original: Function) { - return function execPatch(this: any) { - const execRes = original.apply(this, arguments); - if (!execRes.then) { - this._diag( - 'got non promise result when patching RedisClientMultiCommand.exec' - ); - return execRes; - } - - execRes.then((redisRes: unknown[]) => { - const openSpans = this[OTEL_OPEN_SPANS]; - if (!openSpans) { - return this._diag.error( - 'cannot find open spans to end for redis multi command' - ); - } - if (redisRes.length !== openSpans.length) { - return this._diag.error( - 'number of multi command spans does not match response from redis' - ); - } - for (let i = 0; i < openSpans.length; i++) { - const { span, commandName, commandArgs } = openSpans[i]; - const currCommandRes = redisRes[i]; - if (currCommandRes instanceof Error) { - plugin._endSpanWithResponse( - span, - commandName, - commandArgs, - null, - currCommandRes - ); - } else { - plugin._endSpanWithResponse( - span, - commandName, - commandArgs, - currCommandRes, - undefined - ); - } - } - }); - return execRes; - }; - }; - } - - private _getPatchRedisClientMultiV4() { - return function multiPatchWrapper(original: Function) { - return function multiPatch(this: any) { - const multiRes = original.apply(this, arguments); - multiRes[MULTI_COMMAND_OPTIONS] = this.options; - return multiRes; - }; - }; - } - - private _endSpanWithResponse( - span: Span, - commandName: string, - commandArgs: string[], - response: unknown, - error: Error | undefined - ) { - if (!error && this._config.responseHook) { - try { - this._config.responseHook(span, commandName, commandArgs, response); - } catch (err) { - this._diag.error('responseHook throw an exception', err); - } - } - if (error) { - span.recordException(error); - span.setStatus({ code: SpanStatusCode.ERROR, message: error?.message }); - } - span.end(); - } - /** * Patch internal_send_command(...) to trace requests - * This is for v2 and v3 only */ - private _getPatchInternalSendCommandV2V3() { + private _getPatchInternalSendCommand() { const tracer = this.tracer; const config = this._config; return function internal_send_command(original: Function) { diff --git a/plugins/node/opentelemetry-instrumentation-redis/src/types.ts b/plugins/node/opentelemetry-instrumentation-redis/src/types.ts index cd9866ed62..245e7611e8 100644 --- a/plugins/node/opentelemetry-instrumentation-redis/src/types.ts +++ b/plugins/node/opentelemetry-instrumentation-redis/src/types.ts @@ -45,8 +45,8 @@ export interface RedisPluginClientTypes { * @returns serialized string that will be used as the db.statement attribute. */ export type DbStatementSerializer = ( - cmdName: string, - cmdArgs: Array + cmdName: RedisCommand['command'], + cmdArgs: RedisCommand['args'] ) => string; /** @@ -61,8 +61,8 @@ export type DbStatementSerializer = ( export interface RedisResponseCustomAttributeFunction { ( span: Span, - cmdName: string, - cmdArgs: Array, + cmdName: RedisCommand['command'], + cmdArgs: RedisCommand['args'], response: unknown ): void; } diff --git a/plugins/node/opentelemetry-instrumentation-redis/src/utils.ts b/plugins/node/opentelemetry-instrumentation-redis/src/utils.ts index 6ff0a09c32..4f74b17a19 100644 --- a/plugins/node/opentelemetry-instrumentation-redis/src/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-redis/src/utils.ts @@ -75,8 +75,7 @@ export const getTracedCreateStreamTrace = ( }; }; -export const defaultDbStatementSerializer: DbStatementSerializer = cmdName => - cmdName; +const defaultDbStatementSerializer: DbStatementSerializer = cmdName => cmdName; export const getTracedInternalSendCommand = ( tracer: Tracer, @@ -119,7 +118,7 @@ export const getTracedInternalSendCommand = ( if (this.options) { span.setAttributes({ [SemanticAttributes.NET_PEER_NAME]: this.options.host, - [SemanticAttributes.NET_PEER_PORT]: parseInt(this.options.port), + [SemanticAttributes.NET_PEER_PORT]: this.options.port, }); } if (this.address) { diff --git a/plugins/node/opentelemetry-instrumentation-redis/test/redis_v2_v3.test.ts b/plugins/node/opentelemetry-instrumentation-redis/test/redis.test.ts similarity index 94% rename from plugins/node/opentelemetry-instrumentation-redis/test/redis_v2_v3.test.ts rename to plugins/node/opentelemetry-instrumentation-redis/test/redis.test.ts index c8fc8f21f0..09842809fc 100644 --- a/plugins/node/opentelemetry-instrumentation-redis/test/redis_v2_v3.test.ts +++ b/plugins/node/opentelemetry-instrumentation-redis/test/redis.test.ts @@ -42,20 +42,21 @@ instrumentation.disable(); import * as redisTypes from 'redis'; import { RedisResponseCustomAttributeFunction } from '../src/types'; -import { - redisTestConfig, - redisTestUrl, - shouldTest, - shouldTestLocal, -} from './utils'; const memoryExporter = new InMemorySpanExporter(); +const CONFIG = { + host: process.env.OPENTELEMETRY_REDIS_HOST || 'localhost', + port: process.env.OPENTELEMETRY_REDIS_PORT || '63790', +}; + +const URL = `redis://${CONFIG.host}:${CONFIG.port}`; + const DEFAULT_ATTRIBUTES = { [SemanticAttributes.DB_SYSTEM]: DbSystemValues.REDIS, - [SemanticAttributes.NET_PEER_NAME]: redisTestConfig.host, - [SemanticAttributes.NET_PEER_PORT]: redisTestConfig.port, - [SemanticAttributes.DB_CONNECTION_STRING]: redisTestUrl, + [SemanticAttributes.NET_PEER_NAME]: CONFIG.host, + [SemanticAttributes.NET_PEER_PORT]: CONFIG.port, + [SemanticAttributes.DB_CONNECTION_STRING]: URL, }; const unsetStatus: SpanStatus = { @@ -66,6 +67,8 @@ describe('redis@2.x', () => { const provider = new NodeTracerProvider(); const tracer = provider.getTracer('external'); let redis: typeof redisTypes; + const shouldTestLocal = process.env.RUN_REDIS_TESTS_LOCAL; + const shouldTest = process.env.RUN_REDIS_TESTS || shouldTestLocal; let contextManager: AsyncHooksContextManager; beforeEach(() => { @@ -116,7 +119,7 @@ describe('redis@2.x', () => { }; context.with(trace.setSpan(context.active(), span), () => { - client = redis.createClient(redisTestUrl); + client = redis.createClient(URL); client.on('ready', readyHandler); client.on('error', errorHandler); }); @@ -155,7 +158,7 @@ describe('redis@2.x', () => { ]; before(done => { - client = redis.createClient(redisTestUrl); + client = redis.createClient(URL); client.on('error', err => { done(err); }); @@ -248,10 +251,7 @@ describe('redis@2.x', () => { }); describe('dbStatementSerializer config', () => { - const dbStatementSerializer = ( - cmdName: string, - cmdArgs: Array - ) => { + const dbStatementSerializer = (cmdName: string, cmdArgs: string[]) => { return Array.isArray(cmdArgs) && cmdArgs.length ? `${cmdName} ${cmdArgs.join(' ')}` : cmdName; @@ -294,7 +294,7 @@ describe('redis@2.x', () => { const responseHook: RedisResponseCustomAttributeFunction = ( span: Span, _cmdName: string, - _cmdArgs: Array, + _cmdArgs: string[], response: unknown ) => { span.setAttribute(dataFieldName, new String(response).toString()); @@ -325,7 +325,7 @@ describe('redis@2.x', () => { const badResponseHook: RedisResponseCustomAttributeFunction = ( _span: Span, _cmdName: string, - _cmdArgs: Array, + _cmdArgs: string[], _response: unknown ) => { throw 'Some kind of error'; diff --git a/plugins/node/opentelemetry-instrumentation-redis/test/redis_v4.test.ts b/plugins/node/opentelemetry-instrumentation-redis/test/redis_v4.test.ts deleted file mode 100644 index 4afeaee7cf..0000000000 --- a/plugins/node/opentelemetry-instrumentation-redis/test/redis_v4.test.ts +++ /dev/null @@ -1,409 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -import { - getTestSpans, - registerInstrumentationTesting, -} from '@opentelemetry/contrib-test-utils'; -import { RedisInstrumentation } from '../src'; -import * as assert from 'assert'; - -import { - redisTestConfig, - redisTestUrl, - shouldTest, - shouldTestLocal, -} from './utils'; -import * as testUtils from '@opentelemetry/contrib-test-utils'; - -const instrumentation = registerInstrumentationTesting( - new RedisInstrumentation() -); - -import { createClient } from 'redis'; -import { - Span, - SpanKind, - SpanStatusCode, - trace, - context, -} from '@opentelemetry/api'; -import { SemanticAttributes } from '@opentelemetry/semantic-conventions'; -import { RedisResponseCustomAttributeFunction } from '../src/types'; -import { hrTimeToMilliseconds } from '@opentelemetry/core'; - -describe('redis@^4.0.0', () => { - before(function () { - // needs to be "function" to have MochaContext "this" context - if (!shouldTest) { - // this.skip() workaround - // https://github.com/mochajs/mocha/issues/2683#issuecomment-375629901 - this.test!.parent!.pending = true; - this.skip(); - } - - if (shouldTestLocal) { - testUtils.startDocker('redis'); - } - }); - - after(() => { - if (shouldTestLocal) { - testUtils.cleanUpDocker('redis'); - } - }); - - let client: any; - - beforeEach(async () => { - client = createClient({ - url: redisTestUrl, - }); - await client.connect(); - }); - - afterEach(async () => { - await client?.disconnect(); - }); - - describe('redis commands', () => { - it('simple set and get', async () => { - await client.set('key', 'value'); - const value = await client.get('key'); - assert.strictEqual(value, 'value'); // verify we did not screw up the normal functionality - - const spans = getTestSpans(); - assert.strictEqual(spans.length, 2); - - const redisSetSpan = spans.find(s => s.name.includes('SET')); - assert.ok(redisSetSpan); - assert.strictEqual(redisSetSpan?.kind, SpanKind.CLIENT); - assert.strictEqual(redisSetSpan?.name, 'redis-SET'); - assert.strictEqual( - redisSetSpan?.attributes[SemanticAttributes.DB_SYSTEM], - 'redis' - ); - assert.strictEqual( - redisSetSpan?.attributes[SemanticAttributes.DB_STATEMENT], - 'SET' - ); - assert.strictEqual( - redisSetSpan?.attributes[SemanticAttributes.NET_PEER_NAME], - redisTestConfig.host - ); - assert.strictEqual( - redisSetSpan?.attributes[SemanticAttributes.NET_PEER_PORT], - redisTestConfig.port - ); - assert.strictEqual( - redisSetSpan?.attributes[SemanticAttributes.DB_CONNECTION_STRING], - redisTestUrl - ); - - const redisGetSpan = spans.find(s => s.name.includes('GET')); - assert.ok(redisGetSpan); - assert.strictEqual(redisGetSpan?.kind, SpanKind.CLIENT); - assert.strictEqual(redisGetSpan?.name, 'redis-GET'); - assert.strictEqual( - redisGetSpan?.attributes[SemanticAttributes.DB_SYSTEM], - 'redis' - ); - assert.strictEqual( - redisGetSpan?.attributes[SemanticAttributes.DB_STATEMENT], - 'GET' - ); - assert.strictEqual( - redisGetSpan?.attributes[SemanticAttributes.NET_PEER_NAME], - redisTestConfig.host - ); - assert.strictEqual( - redisGetSpan?.attributes[SemanticAttributes.NET_PEER_PORT], - redisTestConfig.port - ); - assert.strictEqual( - redisGetSpan?.attributes[SemanticAttributes.DB_CONNECTION_STRING], - redisTestUrl - ); - }); - - it('span with error', async () => { - await client.set('string-key', 'string-value'); - await assert.rejects(async () => await client.incr('string-key')); - - const [_setSpan, incrSpan] = getTestSpans(); - - assert.ok(incrSpan); - assert.strictEqual(incrSpan?.status.code, SpanStatusCode.ERROR); - assert.strictEqual( - incrSpan?.status.message, - 'ERR value is not an integer or out of range' - ); - - const exceptions = incrSpan.events.filter( - event => event.name === 'exception' - ); - assert.strictEqual(exceptions.length, 1); - assert.strictEqual( - exceptions?.[0].attributes?.[SemanticAttributes.EXCEPTION_TYPE], - 'ReplyError' - ); - assert.strictEqual( - exceptions?.[0].attributes?.[SemanticAttributes.EXCEPTION_MESSAGE], - 'ERR value is not an integer or out of range' - ); - }); - }); - - describe('multi (transactions) commands', () => { - it('multi commands', async () => { - await client.set('another-key', 'another-value'); - const [setKeyReply, otherKeyValue] = await client - .multi() - .set('key', 'value') - .get('another-key') - .exec(); // ['OK', 'another-value'] - - assert.strictEqual(setKeyReply, 'OK'); // verify we did not screw up the normal functionality - assert.strictEqual(otherKeyValue, 'another-value'); // verify we did not screw up the normal functionality - - const [setSpan, multiSetSpan, multiGetSpan] = getTestSpans(); - - assert.ok(setSpan); - - assert.ok(multiSetSpan); - assert.strictEqual(multiSetSpan.name, 'redis-SET'); - assert.strictEqual( - multiSetSpan.attributes[SemanticAttributes.DB_STATEMENT], - 'SET' - ); - assert.strictEqual( - multiSetSpan?.attributes[SemanticAttributes.NET_PEER_NAME], - redisTestConfig.host - ); - assert.strictEqual( - multiSetSpan?.attributes[SemanticAttributes.NET_PEER_PORT], - redisTestConfig.port - ); - assert.strictEqual( - multiSetSpan?.attributes[SemanticAttributes.DB_CONNECTION_STRING], - redisTestUrl - ); - - assert.ok(multiGetSpan); - assert.strictEqual(multiGetSpan.name, 'redis-GET'); - assert.strictEqual( - multiGetSpan.attributes[SemanticAttributes.DB_STATEMENT], - 'GET' - ); - assert.strictEqual( - multiGetSpan?.attributes[SemanticAttributes.NET_PEER_NAME], - redisTestConfig.host - ); - assert.strictEqual( - multiGetSpan?.attributes[SemanticAttributes.NET_PEER_PORT], - redisTestConfig.port - ); - assert.strictEqual( - multiGetSpan?.attributes[SemanticAttributes.DB_CONNECTION_STRING], - redisTestUrl - ); - }); - - it('multi command with error', async () => { - const [setReply, incrReply] = await client - .multi() - .set('key', 'value') - .incr('key') - .exec(); // ['OK', 'ReplyError'] - assert.strictEqual(setReply, 'OK'); // verify we did not screw up the normal functionality - assert.ok(incrReply instanceof Error); // verify we did not screw up the normal functionality - - const [multiSetSpan, multiIncrSpan] = getTestSpans(); - - assert.ok(multiSetSpan); - assert.strictEqual(multiSetSpan.status.code, SpanStatusCode.UNSET); - - assert.ok(multiIncrSpan); - assert.strictEqual(multiIncrSpan.status.code, SpanStatusCode.ERROR); - assert.strictEqual( - multiIncrSpan.status.message, - 'ERR value is not an integer or out of range' - ); - }); - - it('duration covers create until server response', async () => { - await client.set('another-key', 'another-value'); - const multiClient = client.multi(); - let commands = multiClient.set('key', 'value'); - // wait 10 ms before adding next command - // simulate long operation - await new Promise(resolve => setTimeout(resolve, 10)); - commands = commands.get('another-key'); - const [setKeyReply, otherKeyValue] = await commands.exec(); // ['OK', 'another-value'] - - assert.strictEqual(setKeyReply, 'OK'); // verify we did not screw up the normal functionality - assert.strictEqual(otherKeyValue, 'another-value'); // verify we did not screw up the normal functionality - - const [_setSpan, multiSetSpan, multiGetSpan] = getTestSpans(); - // verify that commands span started when it was added to multi and not when "sent". - // they were called with 10 ms gap between them, so it should be reflected in the span start time - // could be nice feature in the future to capture an event for when it is actually sent - const startTimeDiff = - hrTimeToMilliseconds(multiGetSpan.startTime) - - hrTimeToMilliseconds(multiSetSpan.startTime); - assert.ok( - startTimeDiff >= 9, - `diff of start time should be >= 10 and it's ${startTimeDiff}` - ); - - const endTimeDiff = - hrTimeToMilliseconds(multiGetSpan.endTime) - - hrTimeToMilliseconds(multiSetSpan.endTime); - assert.ok(endTimeDiff < 10); // spans should all end together when multi response arrives from redis server - }); - - it('response hook for multi commands', async () => { - const responseHook: RedisResponseCustomAttributeFunction = ( - span: Span, - cmdName: string, - cmdArgs: Array, - response: unknown - ) => { - span.setAttribute('test.cmd.name', cmdName); - span.setAttribute('test.cmd.args', cmdArgs as string[]); - span.setAttribute('test.db.response', response as string); - }; - instrumentation.setConfig({ responseHook }); - - await client.set('another-key', 'another-value'); - const [setKeyReply, otherKeyValue] = await client - .multi() - .set('key', 'value') - .get('another-key') - .exec(); // ['OK', 'another-value'] - assert.strictEqual(setKeyReply, 'OK'); // verify we did not screw up the normal functionality - assert.strictEqual(otherKeyValue, 'another-value'); // verify we did not screw up the normal functionality - - const [_setSpan, multiSetSpan, multiGetSpan] = getTestSpans(); - - assert.ok(multiSetSpan); - assert.strictEqual(multiSetSpan.attributes['test.cmd.name'], 'SET'); - assert.deepStrictEqual(multiSetSpan.attributes['test.cmd.args'], [ - 'key', - 'value', - ]); - assert.strictEqual(multiSetSpan.attributes['test.db.response'], 'OK'); - - assert.ok(multiGetSpan); - assert.strictEqual(multiGetSpan.attributes['test.cmd.name'], 'GET'); - assert.deepStrictEqual(multiGetSpan.attributes['test.cmd.args'], [ - 'another-key', - ]); - assert.strictEqual( - multiGetSpan.attributes['test.db.response'], - 'another-value' - ); - }); - }); - - describe('config', () => { - describe('dbStatementSerializer', () => { - it('custom dbStatementSerializer', async () => { - const dbStatementSerializer = ( - cmdName: string, - cmdArgs: Array - ) => { - return `${cmdName} ${cmdArgs.join(' ')}`; - }; - - instrumentation.setConfig({ dbStatementSerializer }); - await client.set('key', 'value'); - const [span] = getTestSpans(); - assert.strictEqual( - span.attributes[SemanticAttributes.DB_STATEMENT], - 'SET key value' - ); - }); - - it('dbStatementSerializer throws', async () => { - const dbStatementSerializer = () => { - throw new Error('dbStatementSerializer error'); - }; - - instrumentation.setConfig({ dbStatementSerializer }); - await client.set('key', 'value'); - const [span] = getTestSpans(); - assert.ok(span); - assert.ok(!(SemanticAttributes.DB_STATEMENT in span.attributes)); - }); - }); - - describe('responseHook', () => { - it('valid response hook', async () => { - const responseHook: RedisResponseCustomAttributeFunction = ( - span: Span, - cmdName: string, - cmdArgs: Array, - response: unknown - ) => { - span.setAttribute('test.cmd.name', cmdName); - span.setAttribute('test.cmd.args', cmdArgs as string[]); - span.setAttribute('test.db.response', response as string); - }; - instrumentation.setConfig({ responseHook }); - await client.set('key', 'value'); - const [span] = getTestSpans(); - assert.ok(span); - assert.strictEqual(span.attributes['test.cmd.name'], 'SET'); - assert.deepStrictEqual(span.attributes['test.cmd.args'], [ - 'key', - 'value', - ]); - assert.strictEqual(span.attributes['test.db.response'], 'OK'); - }); - - it('responseHook throws', async () => { - const responseHook = () => { - throw new Error('responseHook error'); - }; - instrumentation.setConfig({ responseHook }); - const res = await client.set('key', 'value'); - assert.strictEqual(res, 'OK'); // package is still functional - const [span] = getTestSpans(); - assert.ok(span); - }); - }); - - describe('requireParentSpan', () => { - it('set to true', async () => { - instrumentation.setConfig({ requireParentSpan: true }); - - // no parent span => no redis span - const res = await client.set('key', 'value'); - assert.strictEqual(res, 'OK'); // verify we did not screw up the normal functionality - assert.ok(getTestSpans().length === 0); - - // has ambient span => redis span - const span = trace.getTracer('test').startSpan('test span'); - await context.with(trace.setSpan(context.active(), span), async () => { - const res = await client.set('key', 'value'); - assert.strictEqual(res, 'OK'); // verify we did not screw up the normal functionality - assert.ok(getTestSpans().length === 1); - }); - span.end(); - }); - }); - }); -}); From 3c2948276c2a189e13b583d849298f5d4b1bbb09 Mon Sep 17 00:00:00 2001 From: Amir Blum Date: Sat, 30 Apr 2022 14:32:30 +0300 Subject: [PATCH 07/22] fix(auto-instrumentation): add redis v4 instrumentation --- metapackages/auto-instrumentations-node/package.json | 1 + metapackages/auto-instrumentations-node/src/utils.ts | 6 ++++-- metapackages/auto-instrumentations-node/test/utils.test.ts | 3 ++- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/metapackages/auto-instrumentations-node/package.json b/metapackages/auto-instrumentations-node/package.json index 53e1f72755..0189c0afbf 100644 --- a/metapackages/auto-instrumentations-node/package.json +++ b/metapackages/auto-instrumentations-node/package.json @@ -73,6 +73,7 @@ "@opentelemetry/instrumentation-pg": "^0.28.0", "@opentelemetry/instrumentation-pino": "^0.28.0", "@opentelemetry/instrumentation-redis": "^0.29.0", + "@opentelemetry/instrumentation-redis-4": "^0.29.0", "@opentelemetry/instrumentation-restify": "^0.27.2", "@opentelemetry/instrumentation-winston": "^0.27.3" } diff --git a/metapackages/auto-instrumentations-node/src/utils.ts b/metapackages/auto-instrumentations-node/src/utils.ts index 38dd2da472..ba9d64473a 100644 --- a/metapackages/auto-instrumentations-node/src/utils.ts +++ b/metapackages/auto-instrumentations-node/src/utils.ts @@ -41,7 +41,8 @@ import { NestInstrumentation } from '@opentelemetry/instrumentation-nestjs-core' import { NetInstrumentation } from '@opentelemetry/instrumentation-net'; import { PgInstrumentation } from '@opentelemetry/instrumentation-pg'; import { PinoInstrumentation } from '@opentelemetry/instrumentation-pino'; -import { RedisInstrumentation } from '@opentelemetry/instrumentation-redis'; +import { RedisInstrumentation as RedisInstrumentationV2 } from '@opentelemetry/instrumentation-redis'; +import { RedisInstrumentation as RedisInstrumentationV4 } from '@opentelemetry/instrumentation-redis-4'; import { RestifyInstrumentation } from '@opentelemetry/instrumentation-restify'; import { WinstonInstrumentation } from '@opentelemetry/instrumentation-winston'; @@ -72,7 +73,8 @@ const InstrumentationMap = { '@opentelemetry/instrumentation-net': NetInstrumentation, '@opentelemetry/instrumentation-pg': PgInstrumentation, '@opentelemetry/instrumentation-pino': PinoInstrumentation, - '@opentelemetry/instrumentation-redis': RedisInstrumentation, + '@opentelemetry/instrumentation-redis': RedisInstrumentationV2, + '@opentelemetry/instrumentation-redis-4': RedisInstrumentationV4, '@opentelemetry/instrumentation-restify': RestifyInstrumentation, '@opentelemetry/instrumentation-winston': WinstonInstrumentation, }; diff --git a/metapackages/auto-instrumentations-node/test/utils.test.ts b/metapackages/auto-instrumentations-node/test/utils.test.ts index e4b392da9e..aecfd1fd49 100644 --- a/metapackages/auto-instrumentations-node/test/utils.test.ts +++ b/metapackages/auto-instrumentations-node/test/utils.test.ts @@ -51,10 +51,11 @@ describe('utils', () => { '@opentelemetry/instrumentation-pg', '@opentelemetry/instrumentation-pino', '@opentelemetry/instrumentation-redis', + '@opentelemetry/instrumentation-redis-4', '@opentelemetry/instrumentation-restify', '@opentelemetry/instrumentation-winston', ]; - assert.strictEqual(instrumentations.length, 28); + assert.strictEqual(instrumentations.length, 29); for (let i = 0, j = instrumentations.length; i < j; i++) { assert.strictEqual( instrumentations[i].instrumentationName, From d21189a027b85a08f0066991b188a44a0320c231 Mon Sep 17 00:00:00 2001 From: Amir Blum Date: Sat, 30 Apr 2022 15:14:50 +0300 Subject: [PATCH 08/22] chore: lint fix --- .../src/instrumentation.ts | 12 +++--------- .../src/utils.ts | 18 ++++++++++++++++-- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-redis-4/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-redis-4/src/instrumentation.ts index 40a11ae5cf..0b6f2e73b2 100644 --- a/plugins/node/opentelemetry-instrumentation-redis-4/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-redis-4/src/instrumentation.ts @@ -28,9 +28,7 @@ import { InstrumentationNodeModuleDefinition, InstrumentationNodeModuleFile, } from '@opentelemetry/instrumentation'; -import { - defaultDbStatementSerializer, -} from './utils'; +import { defaultDbStatementSerializer } from './utils'; import { RedisInstrumentationConfig } from './types'; import { VERSION } from './version'; import { @@ -55,9 +53,7 @@ const DEFAULT_CONFIG: RedisInstrumentationConfig = { requireParentSpan: false, }; -export class RedisInstrumentation extends InstrumentationBase< - any -> { +export class RedisInstrumentation extends InstrumentationBase { static readonly COMPONENT = 'redis'; constructor(protected override _config: RedisInstrumentationConfig = {}) { @@ -175,7 +171,6 @@ export class RedisInstrumentation extends InstrumentationBase< ), ] ), - ]; } @@ -277,7 +272,7 @@ export class RedisInstrumentation extends InstrumentationBase< return function execPatchWrapper(original: Function) { return function execPatch(this: any) { const execRes = original.apply(this, arguments); - if (typeof(execRes?.then) !== 'function') { + if (typeof execRes?.then !== 'function') { this._diag( 'got non promise result when patching RedisClientMultiCommand.exec' ); @@ -353,5 +348,4 @@ export class RedisInstrumentation extends InstrumentationBase< } span.end(); } - } diff --git a/plugins/node/opentelemetry-instrumentation-redis-4/src/utils.ts b/plugins/node/opentelemetry-instrumentation-redis-4/src/utils.ts index ce7567a08f..eda35eab65 100644 --- a/plugins/node/opentelemetry-instrumentation-redis-4/src/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-redis-4/src/utils.ts @@ -1,5 +1,19 @@ -import { DbStatementSerializer } from "./types"; +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { DbStatementSerializer } from './types'; export const defaultDbStatementSerializer: DbStatementSerializer = cmdName => cmdName; - From 55fae3cb2861c258f8f5a414fabe205740d4f493 Mon Sep 17 00:00:00 2001 From: Amir Blum Date: Sat, 30 Apr 2022 15:54:41 +0300 Subject: [PATCH 09/22] fix(redis): robust promise check --- .../src/instrumentation.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-redis-4/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-redis-4/src/instrumentation.ts index 0b6f2e73b2..7ba7f5f19b 100644 --- a/plugins/node/opentelemetry-instrumentation-redis-4/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-redis-4/src/instrumentation.ts @@ -227,7 +227,7 @@ export class RedisInstrumentation extends InstrumentationBase { ); const res = origExecutor.apply(this, arguments); - if (res.then) { + if (typeof(res?.then) === 'function') { res.then( (redisRes: unknown) => { plugin._endSpanWithResponse( @@ -273,7 +273,7 @@ export class RedisInstrumentation extends InstrumentationBase { return function execPatch(this: any) { const execRes = original.apply(this, arguments); if (typeof execRes?.then !== 'function') { - this._diag( + this._diag.error( 'got non promise result when patching RedisClientMultiCommand.exec' ); return execRes; From c0bebbd3966c151f2f0bf8222105b4ec2fcf9cba Mon Sep 17 00:00:00 2001 From: Amir Blum Date: Sat, 30 Apr 2022 15:58:26 +0300 Subject: [PATCH 10/22] fix(redis): use correct 'this' for diag calls --- .../src/instrumentation.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-redis-4/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-redis-4/src/instrumentation.ts index 7ba7f5f19b..68ba4e62f5 100644 --- a/plugins/node/opentelemetry-instrumentation-redis-4/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-redis-4/src/instrumentation.ts @@ -273,7 +273,7 @@ export class RedisInstrumentation extends InstrumentationBase { return function execPatch(this: any) { const execRes = original.apply(this, arguments); if (typeof execRes?.then !== 'function') { - this._diag.error( + plugin._diag.error( 'got non promise result when patching RedisClientMultiCommand.exec' ); return execRes; @@ -282,12 +282,12 @@ export class RedisInstrumentation extends InstrumentationBase { execRes.then((redisRes: unknown[]) => { const openSpans = this[OTEL_OPEN_SPANS]; if (!openSpans) { - return this._diag.error( + return plugin._diag.error( 'cannot find open spans to end for redis multi command' ); } if (redisRes.length !== openSpans.length) { - return this._diag.error( + return plugin._diag.error( 'number of multi command spans does not match response from redis' ); } From cc228ec887ede53617e3653e116a1dcd1dadf91e Mon Sep 17 00:00:00 2001 From: Amir Blum Date: Sat, 30 Apr 2022 17:41:02 +0300 Subject: [PATCH 11/22] feat(redis-4): instrument generic sendCommand and addCommand --- .../src/instrumentation.ts | 203 +++++++++++------- .../test/redis.test.ts | 86 ++++++-- 2 files changed, 197 insertions(+), 92 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-redis-4/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-redis-4/src/instrumentation.ts index 68ba4e62f5..02799abf6e 100644 --- a/plugins/node/opentelemetry-instrumentation-redis-4/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-redis-4/src/instrumentation.ts @@ -123,6 +123,7 @@ export class RedisInstrumentation extends InstrumentationBase { this._diag.debug('Patching redis multi commands executor'); const redisClientMultiCommandPrototype = moduleExports?.default?.prototype; + if (isWrapped(redisClientMultiCommandPrototype?.exec)) { this._unwrap(redisClientMultiCommandPrototype, 'exec'); } @@ -132,6 +133,15 @@ export class RedisInstrumentation extends InstrumentationBase { this._getPatchMultiCommandsExec() ); + if (isWrapped(redisClientMultiCommandPrototype?.addCommand)) { + this._unwrap(redisClientMultiCommandPrototype, 'addCommand'); + } + this._wrap( + redisClientMultiCommandPrototype, + 'addCommand', + this._getPatchMultiCommandsAddCommand() + ); + return moduleExports; }, (moduleExports: any) => { @@ -141,6 +151,9 @@ export class RedisInstrumentation extends InstrumentationBase { if (isWrapped(redisClientMultiCommandPrototype?.exec)) { this._unwrap(redisClientMultiCommandPrototype, 'exec'); } + if (isWrapped(redisClientMultiCommandPrototype?.addCommand)) { + this._unwrap(redisClientMultiCommandPrototype, 'addCommand'); + } } ), new InstrumentationNodeModuleFile( @@ -159,6 +172,15 @@ export class RedisInstrumentation extends InstrumentationBase { this._getPatchRedisClientMulti() ); + if (isWrapped(redisClientPrototype?.sendCommand)) { + this._unwrap(redisClientPrototype, 'sendCommand'); + } + this._wrap( + redisClientPrototype, + 'sendCommand', + this._getPatchRedisClientSendCommand() + ); + return moduleExports; }, (moduleExports: any) => { @@ -167,6 +189,9 @@ export class RedisInstrumentation extends InstrumentationBase { if (isWrapped(redisClientPrototype?.multi)) { this._unwrap(redisClientPrototype, 'multi'); } + if (isWrapped(redisClientPrototype?.sendCommand)) { + this._unwrap(redisClientPrototype, 'sendCommand'); + } } ), ] @@ -178,89 +203,26 @@ export class RedisInstrumentation extends InstrumentationBase { const plugin = this; return function extendWithCommandsPatchWrapper(original: Function) { return function extendWithCommandsPatch(this: any, config: any) { + if (config?.BaseClass?.name !== 'RedisClient') { + return original.apply(this, arguments); + } + const origExecutor = config.executor; config.executor = function ( this: any, command: any, args: Array ) { - const hasNoParentSpan = trace.getSpan(context.active()) === undefined; - if (hasNoParentSpan && plugin._config?.requireParentSpan) { - return origExecutor.apply(this, arguments); - } - const redisCommandArguments = transformCommandArguments( command, args ).args; - const clientOptions = this.options || this[MULTI_COMMAND_OPTIONS]; - - const commandName = redisCommandArguments[0] as string; // types also allows it to be a Buffer, but in practice it only string - const commandArgs = redisCommandArguments.slice(1); - - const dbStatementSerializer = - plugin._config?.dbStatementSerializer || - defaultDbStatementSerializer; - - const attributes = { - [SemanticAttributes.DB_SYSTEM]: DbSystemValues.REDIS, - [SemanticAttributes.NET_PEER_NAME]: clientOptions?.socket?.host, - [SemanticAttributes.NET_PEER_PORT]: clientOptions?.socket?.port, - [SemanticAttributes.DB_CONNECTION_STRING]: clientOptions?.url, - }; - - try { - const dbStatement = dbStatementSerializer(commandName, commandArgs); - if (dbStatement != null) { - attributes[SemanticAttributes.DB_STATEMENT] = dbStatement; - } - } catch (e) { - plugin._diag.error('dbStatementSerializer throw an exception', e); - } - - const span = plugin.tracer.startSpan( - `${RedisInstrumentation.COMPONENT}-${commandName}`, - { - kind: SpanKind.CLIENT, - attributes, - } + return plugin._traceClientCommand( + origExecutor, + this, + arguments, + redisCommandArguments ); - - const res = origExecutor.apply(this, arguments); - if (typeof(res?.then) === 'function') { - res.then( - (redisRes: unknown) => { - plugin._endSpanWithResponse( - span, - commandName, - commandArgs, - redisRes, - undefined - ); - }, - (err: any) => { - plugin._endSpanWithResponse( - span, - commandName, - commandArgs, - null, - err - ); - } - ); - } else { - const redisClientMultiCommand = res as { - [OTEL_OPEN_SPANS]?: Array; - }; - redisClientMultiCommand[OTEL_OPEN_SPANS] = - redisClientMultiCommand[OTEL_OPEN_SPANS] || []; - redisClientMultiCommand[OTEL_OPEN_SPANS]!.push({ - span, - commandName, - commandArgs, - }); - } - return res; }; return original.apply(this, arguments); }; @@ -318,6 +280,15 @@ export class RedisInstrumentation extends InstrumentationBase { }; } + private _getPatchMultiCommandsAddCommand() { + const plugin = this; + return function addCommandWrapper(original: Function) { + return function addCommandPatch(this: any, args: Array) { + return plugin._traceClientCommand(original, this, arguments, args); + }; + }; + } + private _getPatchRedisClientMulti() { return function multiPatchWrapper(original: Function) { return function multiPatch(this: any) { @@ -328,10 +299,96 @@ export class RedisInstrumentation extends InstrumentationBase { }; } + private _getPatchRedisClientSendCommand() { + const plugin = this; + return function sendCommandWrapper(original: Function) { + return function sendCommandPatch( + this: any, + args: Array + ) { + return plugin._traceClientCommand(original, this, arguments, args); + }; + }; + } + + private _traceClientCommand( + origFunction: Function, + origThis: any, + origArguments: IArguments, + redisCommandArguments: Array + ) { + const hasNoParentSpan = trace.getSpan(context.active()) === undefined; + if (hasNoParentSpan && this._config?.requireParentSpan) { + return origFunction.apply(origThis, origArguments); + } + + const clientOptions = origThis.options || origThis[MULTI_COMMAND_OPTIONS]; + + const commandName = redisCommandArguments[0] as string; // types also allows it to be a Buffer, but in practice it only string + const commandArgs = redisCommandArguments.slice(1); + + const dbStatementSerializer = + this._config?.dbStatementSerializer || defaultDbStatementSerializer; + + const attributes = { + [SemanticAttributes.DB_SYSTEM]: DbSystemValues.REDIS, + [SemanticAttributes.NET_PEER_NAME]: clientOptions?.socket?.host, + [SemanticAttributes.NET_PEER_PORT]: clientOptions?.socket?.port, + [SemanticAttributes.DB_CONNECTION_STRING]: clientOptions?.url, + }; + + try { + const dbStatement = dbStatementSerializer(commandName, commandArgs); + if (dbStatement != null) { + attributes[SemanticAttributes.DB_STATEMENT] = dbStatement; + } + } catch (e) { + this._diag.error('dbStatementSerializer throw an exception', e); + } + + const span = this.tracer.startSpan( + `${RedisInstrumentation.COMPONENT}-${commandName}`, + { + kind: SpanKind.CLIENT, + attributes, + } + ); + + const res = origFunction.apply(origThis, origArguments); + if (typeof res?.then === 'function') { + res.then( + (redisRes: unknown) => { + this._endSpanWithResponse( + span, + commandName, + commandArgs, + redisRes, + undefined + ); + }, + (err: any) => { + this._endSpanWithResponse(span, commandName, commandArgs, null, err); + } + ); + } else { + const redisClientMultiCommand = res as { + [OTEL_OPEN_SPANS]?: Array; + }; + redisClientMultiCommand[OTEL_OPEN_SPANS] = + redisClientMultiCommand[OTEL_OPEN_SPANS] || []; + redisClientMultiCommand[OTEL_OPEN_SPANS]!.push({ + span, + commandName, + commandArgs, + }); + } + return res; + } + private _endSpanWithResponse( span: Span, commandName: string, - commandArgs: string[], + commandArgs: Array, response: unknown, error: Error | undefined ) { diff --git a/plugins/node/opentelemetry-instrumentation-redis-4/test/redis.test.ts b/plugins/node/opentelemetry-instrumentation-redis-4/test/redis.test.ts index 4afeaee7cf..03fee26390 100644 --- a/plugins/node/opentelemetry-instrumentation-redis-4/test/redis.test.ts +++ b/plugins/node/opentelemetry-instrumentation-redis-4/test/redis.test.ts @@ -87,58 +87,79 @@ describe('redis@^4.0.0', () => { const spans = getTestSpans(); assert.strictEqual(spans.length, 2); - const redisSetSpan = spans.find(s => s.name.includes('SET')); - assert.ok(redisSetSpan); - assert.strictEqual(redisSetSpan?.kind, SpanKind.CLIENT); - assert.strictEqual(redisSetSpan?.name, 'redis-SET'); + const setSpan = spans.find(s => s.name.includes('SET')); + assert.ok(setSpan); + assert.strictEqual(setSpan?.kind, SpanKind.CLIENT); + assert.strictEqual(setSpan?.name, 'redis-SET'); assert.strictEqual( - redisSetSpan?.attributes[SemanticAttributes.DB_SYSTEM], + setSpan?.attributes[SemanticAttributes.DB_SYSTEM], 'redis' ); assert.strictEqual( - redisSetSpan?.attributes[SemanticAttributes.DB_STATEMENT], + setSpan?.attributes[SemanticAttributes.DB_STATEMENT], 'SET' ); assert.strictEqual( - redisSetSpan?.attributes[SemanticAttributes.NET_PEER_NAME], + setSpan?.attributes[SemanticAttributes.NET_PEER_NAME], redisTestConfig.host ); assert.strictEqual( - redisSetSpan?.attributes[SemanticAttributes.NET_PEER_PORT], + setSpan?.attributes[SemanticAttributes.NET_PEER_PORT], redisTestConfig.port ); assert.strictEqual( - redisSetSpan?.attributes[SemanticAttributes.DB_CONNECTION_STRING], + setSpan?.attributes[SemanticAttributes.DB_CONNECTION_STRING], redisTestUrl ); - const redisGetSpan = spans.find(s => s.name.includes('GET')); - assert.ok(redisGetSpan); - assert.strictEqual(redisGetSpan?.kind, SpanKind.CLIENT); - assert.strictEqual(redisGetSpan?.name, 'redis-GET'); + const getSpan = spans.find(s => s.name.includes('GET')); + assert.ok(getSpan); + assert.strictEqual(getSpan?.kind, SpanKind.CLIENT); + assert.strictEqual(getSpan?.name, 'redis-GET'); assert.strictEqual( - redisGetSpan?.attributes[SemanticAttributes.DB_SYSTEM], + getSpan?.attributes[SemanticAttributes.DB_SYSTEM], 'redis' ); assert.strictEqual( - redisGetSpan?.attributes[SemanticAttributes.DB_STATEMENT], + getSpan?.attributes[SemanticAttributes.DB_STATEMENT], 'GET' ); assert.strictEqual( - redisGetSpan?.attributes[SemanticAttributes.NET_PEER_NAME], + getSpan?.attributes[SemanticAttributes.NET_PEER_NAME], redisTestConfig.host ); assert.strictEqual( - redisGetSpan?.attributes[SemanticAttributes.NET_PEER_PORT], + getSpan?.attributes[SemanticAttributes.NET_PEER_PORT], redisTestConfig.port ); assert.strictEqual( - redisGetSpan?.attributes[SemanticAttributes.DB_CONNECTION_STRING], + getSpan?.attributes[SemanticAttributes.DB_CONNECTION_STRING], redisTestUrl ); }); - it('span with error', async () => { + it('send general command', async () => { + const res = await client.sendCommand(['SET', 'key', 'value']); + assert.strictEqual(res, 'OK'); // verify we did not screw up the normal functionality + + const [setSpan] = getTestSpans(); + + assert.ok(setSpan); + assert.strictEqual( + setSpan?.attributes[SemanticAttributes.DB_STATEMENT], + 'SET' + ); + assert.strictEqual( + setSpan?.attributes[SemanticAttributes.NET_PEER_NAME], + redisTestConfig.host + ); + assert.strictEqual( + setSpan?.attributes[SemanticAttributes.NET_PEER_PORT], + redisTestConfig.port + ); + }); + + it('command with error', async () => { await client.set('string-key', 'string-value'); await assert.rejects(async () => await client.incr('string-key')); @@ -221,6 +242,33 @@ describe('redis@^4.0.0', () => { ); }); + it('multi command with generic command', async () => { + const [setReply] = await client + .multi() + .addCommand(['SET', 'key', 'value']) + .exec(); + assert.strictEqual(setReply, 'OK'); // verify we did not screw up the normal functionality + + const [multiSetSpan] = getTestSpans(); + assert.ok(multiSetSpan); + assert.strictEqual( + multiSetSpan.attributes[SemanticAttributes.DB_STATEMENT], + 'SET' + ); + assert.strictEqual( + multiSetSpan?.attributes[SemanticAttributes.NET_PEER_NAME], + redisTestConfig.host + ); + assert.strictEqual( + multiSetSpan?.attributes[SemanticAttributes.NET_PEER_PORT], + redisTestConfig.port + ); + assert.strictEqual( + multiSetSpan?.attributes[SemanticAttributes.DB_CONNECTION_STRING], + redisTestUrl + ); + }); + it('multi command with error', async () => { const [setReply, incrReply] = await client .multi() From 5bb764306be553928f701f76654279b85cffbde0 Mon Sep 17 00:00:00 2001 From: Amir Blum Date: Mon, 2 May 2022 14:01:59 +0300 Subject: [PATCH 12/22] fix(redis): indicate supported version in package.json description --- plugins/node/opentelemetry-instrumentation-redis-4/package.json | 2 +- plugins/node/opentelemetry-instrumentation-redis/package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-redis-4/package.json b/plugins/node/opentelemetry-instrumentation-redis-4/package.json index 491c1d658e..b8e94c0dbc 100644 --- a/plugins/node/opentelemetry-instrumentation-redis-4/package.json +++ b/plugins/node/opentelemetry-instrumentation-redis-4/package.json @@ -1,7 +1,7 @@ { "name": "@opentelemetry/instrumentation-redis-4", "version": "0.29.0", - "description": "OpenTelemetry redis automatic instrumentation package.", + "description": "Automatic OpenTelemetry instrumentation for redis package version 4", "main": "build/src/index.js", "types": "build/src/index.d.ts", "repository": "open-telemetry/opentelemetry-js-contrib", diff --git a/plugins/node/opentelemetry-instrumentation-redis/package.json b/plugins/node/opentelemetry-instrumentation-redis/package.json index 5564202c5d..4a95e5c8df 100644 --- a/plugins/node/opentelemetry-instrumentation-redis/package.json +++ b/plugins/node/opentelemetry-instrumentation-redis/package.json @@ -1,7 +1,7 @@ { "name": "@opentelemetry/instrumentation-redis", "version": "0.29.0", - "description": "OpenTelemetry redis automatic instrumentation package.", + "description": "Automatic OpenTelemetry instrumentation for redis package version 2 and 3", "main": "build/src/index.js", "types": "build/src/index.d.ts", "repository": "open-telemetry/opentelemetry-js-contrib", From 56037920fd378dc6561af77cad329eaff2c211a0 Mon Sep 17 00:00:00 2001 From: Amir Blum Date: Mon, 2 May 2022 14:02:56 +0300 Subject: [PATCH 13/22] fix(redis): package name for InstrumentationBase --- .../src/instrumentation.ts | 2 +- .../opentelemetry-instrumentation-redis/src/instrumentation.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-redis-4/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-redis-4/src/instrumentation.ts index 02799abf6e..ef7dae5c68 100644 --- a/plugins/node/opentelemetry-instrumentation-redis-4/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-redis-4/src/instrumentation.ts @@ -57,7 +57,7 @@ export class RedisInstrumentation extends InstrumentationBase { static readonly COMPONENT = 'redis'; constructor(protected override _config: RedisInstrumentationConfig = {}) { - super('@opentelemetry/instrumentation-redis', VERSION, _config); + super('@opentelemetry/instrumentation-redis-4', VERSION, _config); } override setConfig(config: RedisInstrumentationConfig = {}) { diff --git a/plugins/node/opentelemetry-instrumentation-redis/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-redis/src/instrumentation.ts index 676a9a374b..3693f8e136 100644 --- a/plugins/node/opentelemetry-instrumentation-redis/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-redis/src/instrumentation.ts @@ -27,7 +27,7 @@ import { getTracedInternalSendCommand, } from './utils'; import { RedisInstrumentationConfig } from './types'; -import { VERSION } from './version'; +import { VERSION } from 'RedisInstrumentationV2'; const DEFAULT_CONFIG: RedisInstrumentationConfig = { requireParentSpan: false, From 4adedcd0ac6cf91527a6ae8f65754e44b332f2ad Mon Sep 17 00:00:00 2001 From: Amir Blum Date: Mon, 2 May 2022 14:07:10 +0300 Subject: [PATCH 14/22] fix(redis): revert version import change --- .../opentelemetry-instrumentation-redis/src/instrumentation.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/node/opentelemetry-instrumentation-redis/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-redis/src/instrumentation.ts index 3693f8e136..676a9a374b 100644 --- a/plugins/node/opentelemetry-instrumentation-redis/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-redis/src/instrumentation.ts @@ -27,7 +27,7 @@ import { getTracedInternalSendCommand, } from './utils'; import { RedisInstrumentationConfig } from './types'; -import { VERSION } from 'RedisInstrumentationV2'; +import { VERSION } from './version'; const DEFAULT_CONFIG: RedisInstrumentationConfig = { requireParentSpan: false, From 6737a8e29ebe716eea8516a61c5404d16189af38 Mon Sep 17 00:00:00 2001 From: Amir Blum Date: Mon, 2 May 2022 14:32:27 +0300 Subject: [PATCH 15/22] chore(redis): more data in diag error on exception --- .../src/instrumentation.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/plugins/node/opentelemetry-instrumentation-redis-4/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-redis-4/src/instrumentation.ts index ef7dae5c68..2ffbf5bf10 100644 --- a/plugins/node/opentelemetry-instrumentation-redis-4/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-redis-4/src/instrumentation.ts @@ -343,7 +343,9 @@ export class RedisInstrumentation extends InstrumentationBase { attributes[SemanticAttributes.DB_STATEMENT] = dbStatement; } } catch (e) { - this._diag.error('dbStatementSerializer throw an exception', e); + this._diag.error('dbStatementSerializer throw an exception', e, { + commandName, + }); } const span = this.tracer.startSpan( From ba0533f93428fc0034a5fabfa6109d9284cf2fa0 Mon Sep 17 00:00:00 2001 From: Amir Blum Date: Mon, 2 May 2022 14:39:20 +0300 Subject: [PATCH 16/22] fix(redis): remove unused file from PR --- .../test/utils.ts | 24 ------------------- 1 file changed, 24 deletions(-) delete mode 100644 plugins/node/opentelemetry-instrumentation-redis/test/utils.ts diff --git a/plugins/node/opentelemetry-instrumentation-redis/test/utils.ts b/plugins/node/opentelemetry-instrumentation-redis/test/utils.ts deleted file mode 100644 index c60c703e56..0000000000 --- a/plugins/node/opentelemetry-instrumentation-redis/test/utils.ts +++ /dev/null @@ -1,24 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -export const redisTestConfig = { - host: process.env.OPENTELEMETRY_REDIS_HOST || 'localhost', - port: process.env.OPENTELEMETRY_REDIS_PORT || 63790, -}; - -export const redisTestUrl = `redis://${redisTestConfig.host}:${redisTestConfig.port}`; - -export const shouldTestLocal = process.env.RUN_REDIS_TESTS_LOCAL; -export const shouldTest = process.env.RUN_REDIS_TESTS || shouldTestLocal; From a6b75ebe6c8f391b4bb7ef20a74f6a717ee4a82b Mon Sep 17 00:00:00 2001 From: Amir Blum Date: Tue, 10 May 2022 15:12:19 +0300 Subject: [PATCH 17/22] chore(redis-4): upgrade sdk to 0.28.0 --- .../package.json | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-redis-4/package.json b/plugins/node/opentelemetry-instrumentation-redis-4/package.json index b8e94c0dbc..f0d342ece2 100644 --- a/plugins/node/opentelemetry-instrumentation-redis-4/package.json +++ b/plugins/node/opentelemetry-instrumentation-redis-4/package.json @@ -52,11 +52,11 @@ }, "devDependencies": { "@opentelemetry/api": "1.0.0", - "@opentelemetry/context-async-hooks": "1.0.1", - "@opentelemetry/core": "1.0.1", - "@opentelemetry/contrib-test-utils": "^0.29.0", - "@opentelemetry/sdk-trace-base": "1.0.1", - "@opentelemetry/sdk-trace-node": "1.0.1", + "@opentelemetry/context-async-hooks": "1.2.0", + "@opentelemetry/core": "1.2.0", + "@opentelemetry/contrib-test-utils": "0.29.0", + "@opentelemetry/sdk-trace-base": "1.2.0", + "@opentelemetry/sdk-trace-node": "1.2.0", "@types/mocha": "7.0.2", "@types/node": "16.11.21", "cross-env": "7.0.3", @@ -70,7 +70,7 @@ "typescript": "4.3.5" }, "dependencies": { - "@opentelemetry/instrumentation": "^0.27.0", + "@opentelemetry/instrumentation": "^0.28.0", "@opentelemetry/semantic-conventions": "^1.0.0" } } From 7c00d0a33638158a9af489d3210137726da8769a Mon Sep 17 00:00:00 2001 From: Amir Blum Date: Thu, 12 May 2022 11:30:45 +0300 Subject: [PATCH 18/22] fix(redis-4): parse env port to int for CI tests --- plugins/node/opentelemetry-instrumentation-redis-4/package.json | 2 +- .../node/opentelemetry-instrumentation-redis-4/test/utils.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-redis-4/package.json b/plugins/node/opentelemetry-instrumentation-redis-4/package.json index f0d342ece2..f41d215148 100644 --- a/plugins/node/opentelemetry-instrumentation-redis-4/package.json +++ b/plugins/node/opentelemetry-instrumentation-redis-4/package.json @@ -63,7 +63,7 @@ "gts": "3.1.0", "mocha": "7.2.0", "nyc": "15.1.0", - "redis": "4.0.6", + "redis": "4.1.0", "rimraf": "3.0.2", "test-all-versions": "5.0.1", "ts-mocha": "8.0.0", diff --git a/plugins/node/opentelemetry-instrumentation-redis-4/test/utils.ts b/plugins/node/opentelemetry-instrumentation-redis-4/test/utils.ts index c60c703e56..cc0e0a6609 100644 --- a/plugins/node/opentelemetry-instrumentation-redis-4/test/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-redis-4/test/utils.ts @@ -15,7 +15,7 @@ */ export const redisTestConfig = { host: process.env.OPENTELEMETRY_REDIS_HOST || 'localhost', - port: process.env.OPENTELEMETRY_REDIS_PORT || 63790, + port: +(process.env.OPENTELEMETRY_REDIS_PORT || 63790), }; export const redisTestUrl = `redis://${redisTestConfig.host}:${redisTestConfig.port}`; From 2c4243366ceb16af35357c43629950b8024cb37f Mon Sep 17 00:00:00 2001 From: Amir Blum Date: Thu, 12 May 2022 11:34:41 +0300 Subject: [PATCH 19/22] docs(redis-4): update README to follow new style --- plugins/node/opentelemetry-instrumentation-redis-4/README.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-redis-4/README.md b/plugins/node/opentelemetry-instrumentation-redis-4/README.md index 21a5a9c99c..7fa089af4c 100644 --- a/plugins/node/opentelemetry-instrumentation-redis-4/README.md +++ b/plugins/node/opentelemetry-instrumentation-redis-4/README.md @@ -3,10 +3,9 @@ [![NPM Published Version][npm-img]][npm-url] [![Apache License][license-image]][license-image] -This module provides automatic instrumentation for [`redis@^4.0.0`](https://github.com/NodeRedis/node_redis). +This module provides automatic instrumentation for the [`redis@^4.0.0`](https://github.com/NodeRedis/node_redis) module, which may be loaded using the [`@opentelemetry/sdk-trace-node`](https://github.com/open-telemetry/opentelemetry-js/tree/main/packages/opentelemetry-sdk-trace-node) package and is included in the [`@opentelemetry/auto-instrumentations-node`](https://www.npmjs.com/package/@opentelemetry/auto-instrumentations-node) bundle. -For automatic instrumentation see the -[@opentelemetry/sdk-trace-node](https://github.com/open-telemetry/opentelemetry-js/tree/main/packages/opentelemetry-sdk-trace-node) package. +If total installation size is not constrained, it is recommended to use the [`@opentelemetry/auto-instrumentations-node`](https://www.npmjs.com/package/@opentelemetry/auto-instrumentations-node) bundle with [@opentelemetry/sdk-node](`https://www.npmjs.com/package/@opentelemetry/sdk-node`) for the most seamless instrumentation experience. Compatible with OpenTelemetry JS API and SDK `1.0+`. From 541bed89145a6f06868d0af3b8a696a5397a69b8 Mon Sep 17 00:00:00 2001 From: Amir Blum Date: Sat, 14 May 2022 10:26:59 +0300 Subject: [PATCH 20/22] feat(redis-4): add support for redis@^4.1.0 --- .../src/instrumentation.ts | 269 ++++++++++-------- .../test/redis.test.ts | 4 - 2 files changed, 145 insertions(+), 128 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-redis-4/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-redis-4/src/instrumentation.ts index 2ffbf5bf10..9ffe4da639 100644 --- a/plugins/node/opentelemetry-instrumentation-redis-4/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-redis-4/src/instrumentation.ts @@ -65,140 +65,161 @@ export class RedisInstrumentation extends InstrumentationBase { } protected init() { + // @node-redis/client is a new package introduced and consumed by 'redis 4.0.x' + // on redis@4.1.0 it was changed to @redis/client. + // we will instrument both packages return [ - // @node-redis/client is a new package introduced and consumed by 'redis ^4.0.0' - new InstrumentationNodeModuleDefinition( - '@node-redis/client', - ['^1.0.0'], - (moduleExports: any, moduleVersion?: string) => { - diag.debug( - `Patching @node-redis/client@${moduleVersion} (redis@^4.x.x)` + this._getInstrumentationNodeModuleDefinition('@redis/client'), + this._getInstrumentationNodeModuleDefinition('@node-redis/client'), + ]; + } + + private _getInstrumentationNodeModuleDefinition( + basePackageName: string + ): InstrumentationNodeModuleDefinition { + const commanderModuleFile = new InstrumentationNodeModuleFile( + `${basePackageName}/dist/lib/commander.js`, + ['^1.0.0'], + (moduleExports: any, moduleVersion?: string) => { + const transformCommandArguments = + moduleExports.transformCommandArguments; + if (!transformCommandArguments) { + this._diag.error( + 'internal instrumentation error, missing transformCommandArguments function' ); return moduleExports; - }, - (_moduleExports: any, moduleVersion?: string) => { - diag.debug( - `Unpatching @node-redis/client@${moduleVersion} (redis@^4.x.x)` - ); - }, - [ - new InstrumentationNodeModuleFile( - '@node-redis/client/dist/lib/commander.js', - ['^1.0.0'], - (moduleExports: any) => { - const transformCommandArguments = - moduleExports.transformCommandArguments; - if (!transformCommandArguments) { - this._diag.error( - 'internal instrumentation error, missing transformCommandArguments function' - ); - return moduleExports; - } - - // this is the function that extend a redis client with a list of commands. - // the function patches the commandExecutor to record a span - this._diag.debug('Patching redis commands executor'); - if (isWrapped(moduleExports?.extendWithCommands)) { - this._unwrap(moduleExports, 'extendWithCommands'); - } - this._wrap( - moduleExports, - 'extendWithCommands', - this._getPatchExtendWithCommands(transformCommandArguments) - ); + } - return moduleExports; - }, - (moduleExports: any) => { - this._diag.debug('Unpatching redis commands executor'); - if (isWrapped(moduleExports?.extendWithCommands)) { - this._unwrap(moduleExports, 'extendWithCommands'); - } - } - ), - new InstrumentationNodeModuleFile( - '@node-redis/client/dist/lib/client/multi-command.js', - ['^1.0.0'], - (moduleExports: any) => { - this._diag.debug('Patching redis multi commands executor'); - const redisClientMultiCommandPrototype = - moduleExports?.default?.prototype; - - if (isWrapped(redisClientMultiCommandPrototype?.exec)) { - this._unwrap(redisClientMultiCommandPrototype, 'exec'); - } - this._wrap( - redisClientMultiCommandPrototype, - 'exec', - this._getPatchMultiCommandsExec() - ); + // function name and signature changed in redis 4.1.0 from 'extendWithCommands' to 'attachCommands' + // the matching internal package names starts with 1.0.x (for redis 4.0.x) + const functionToPatch = moduleVersion?.startsWith('1.0.') + ? 'extendWithCommands' + : 'attachCommands'; + // this is the function that extend a redis client with a list of commands. + // the function patches the commandExecutor to record a span + this._diag.debug('Patching redis commands executor'); + if (isWrapped(moduleExports?.[functionToPatch])) { + this._unwrap(moduleExports, functionToPatch); + } + this._wrap( + moduleExports, + functionToPatch, + this._getPatchExtendWithCommands(transformCommandArguments) + ); + + return moduleExports; + }, + (moduleExports: any) => { + this._diag.debug('Unpatching redis commands executor'); + if (isWrapped(moduleExports?.extendWithCommands)) { + this._unwrap(moduleExports, 'extendWithCommands'); + } + if (isWrapped(moduleExports?.attachCommands)) { + this._unwrap(moduleExports, 'attachCommands'); + } + } + ); - if (isWrapped(redisClientMultiCommandPrototype?.addCommand)) { - this._unwrap(redisClientMultiCommandPrototype, 'addCommand'); - } - this._wrap( - redisClientMultiCommandPrototype, - 'addCommand', - this._getPatchMultiCommandsAddCommand() - ); + const multiCommanderModule = new InstrumentationNodeModuleFile( + `${basePackageName}/dist/lib/client/multi-command.js`, + ['^1.0.0'], + (moduleExports: any) => { + this._diag.debug('Patching redis multi commands executor'); + const redisClientMultiCommandPrototype = + moduleExports?.default?.prototype; - return moduleExports; - }, - (moduleExports: any) => { - this._diag.debug('Unpatching redis multi commands executor'); - const redisClientMultiCommandPrototype = - moduleExports?.default?.prototype; - if (isWrapped(redisClientMultiCommandPrototype?.exec)) { - this._unwrap(redisClientMultiCommandPrototype, 'exec'); - } - if (isWrapped(redisClientMultiCommandPrototype?.addCommand)) { - this._unwrap(redisClientMultiCommandPrototype, 'addCommand'); - } - } - ), - new InstrumentationNodeModuleFile( - '@node-redis/client/dist/lib/client/index.js', - ['^1.0.0'], - (moduleExports: any) => { - this._diag.debug('Patching redis client'); - const redisClientPrototype = moduleExports?.default?.prototype; - - if (isWrapped(redisClientPrototype?.multi)) { - this._unwrap(redisClientPrototype, 'multi'); - } - this._wrap( - redisClientPrototype, - 'multi', - this._getPatchRedisClientMulti() - ); + if (isWrapped(redisClientMultiCommandPrototype?.exec)) { + this._unwrap(redisClientMultiCommandPrototype, 'exec'); + } + this._wrap( + redisClientMultiCommandPrototype, + 'exec', + this._getPatchMultiCommandsExec() + ); + + if (isWrapped(redisClientMultiCommandPrototype?.addCommand)) { + this._unwrap(redisClientMultiCommandPrototype, 'addCommand'); + } + this._wrap( + redisClientMultiCommandPrototype, + 'addCommand', + this._getPatchMultiCommandsAddCommand() + ); + + return moduleExports; + }, + (moduleExports: any) => { + this._diag.debug('Unpatching redis multi commands executor'); + const redisClientMultiCommandPrototype = + moduleExports?.default?.prototype; + if (isWrapped(redisClientMultiCommandPrototype?.exec)) { + this._unwrap(redisClientMultiCommandPrototype, 'exec'); + } + if (isWrapped(redisClientMultiCommandPrototype?.addCommand)) { + this._unwrap(redisClientMultiCommandPrototype, 'addCommand'); + } + } + ); - if (isWrapped(redisClientPrototype?.sendCommand)) { - this._unwrap(redisClientPrototype, 'sendCommand'); - } - this._wrap( - redisClientPrototype, - 'sendCommand', - this._getPatchRedisClientSendCommand() - ); + const clientIndexModule = new InstrumentationNodeModuleFile( + `${basePackageName}/dist/lib/client/index.js`, + ['^1.0.0'], + (moduleExports: any) => { + this._diag.debug('Patching redis client'); + const redisClientPrototype = moduleExports?.default?.prototype; - return moduleExports; - }, - (moduleExports: any) => { - this._diag.debug('Unpatching redis client'); - const redisClientPrototype = moduleExports?.default?.prototype; - if (isWrapped(redisClientPrototype?.multi)) { - this._unwrap(redisClientPrototype, 'multi'); - } - if (isWrapped(redisClientPrototype?.sendCommand)) { - this._unwrap(redisClientPrototype, 'sendCommand'); - } - } - ), - ] - ), - ]; + if (isWrapped(redisClientPrototype?.multi)) { + this._unwrap(redisClientPrototype, 'multi'); + } + this._wrap( + redisClientPrototype, + 'multi', + this._getPatchRedisClientMulti() + ); + + if (isWrapped(redisClientPrototype?.sendCommand)) { + this._unwrap(redisClientPrototype, 'sendCommand'); + } + this._wrap( + redisClientPrototype, + 'sendCommand', + this._getPatchRedisClientSendCommand() + ); + + return moduleExports; + }, + (moduleExports: any) => { + this._diag.debug('Unpatching redis client'); + const redisClientPrototype = moduleExports?.default?.prototype; + if (isWrapped(redisClientPrototype?.multi)) { + this._unwrap(redisClientPrototype, 'multi'); + } + if (isWrapped(redisClientPrototype?.sendCommand)) { + this._unwrap(redisClientPrototype, 'sendCommand'); + } + } + ); + + return new InstrumentationNodeModuleDefinition( + basePackageName, + ['^1.0.0'], + (moduleExports: any, moduleVersion?: string) => { + diag.debug( + `Patching ${basePackageName}/client@${moduleVersion} (redis@^4.0.0)` + ); + return moduleExports; + }, + (_moduleExports: any, moduleVersion?: string) => { + diag.debug( + `Unpatching ${basePackageName}/client@${moduleVersion} (redis@^4.0.0)` + ); + }, + [commanderModuleFile, multiCommanderModule, clientIndexModule] + ); } + // serves both for redis 4.0.x where function name is extendWithCommands + // and redis ^4.1.0 where function name is attachCommands private _getPatchExtendWithCommands(transformCommandArguments: Function) { const plugin = this; return function extendWithCommandsPatchWrapper(original: Function) { diff --git a/plugins/node/opentelemetry-instrumentation-redis-4/test/redis.test.ts b/plugins/node/opentelemetry-instrumentation-redis-4/test/redis.test.ts index 03fee26390..9701cd130d 100644 --- a/plugins/node/opentelemetry-instrumentation-redis-4/test/redis.test.ts +++ b/plugins/node/opentelemetry-instrumentation-redis-4/test/redis.test.ts @@ -176,10 +176,6 @@ describe('redis@^4.0.0', () => { event => event.name === 'exception' ); assert.strictEqual(exceptions.length, 1); - assert.strictEqual( - exceptions?.[0].attributes?.[SemanticAttributes.EXCEPTION_TYPE], - 'ReplyError' - ); assert.strictEqual( exceptions?.[0].attributes?.[SemanticAttributes.EXCEPTION_MESSAGE], 'ERR value is not an integer or out of range' From 35f2ac99bd464517b56a72b6fde2e01756b5df02 Mon Sep 17 00:00:00 2001 From: Amir Blum Date: Sat, 14 May 2022 11:23:14 +0300 Subject: [PATCH 21/22] ci(redis-4): redis-4 dropped support for node 12 --- .github/workflows/unit-test.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/unit-test.yml b/.github/workflows/unit-test.yml index 687ba18835..689a0f3f65 100644 --- a/.github/workflows/unit-test.yml +++ b/.github/workflows/unit-test.yml @@ -23,6 +23,9 @@ jobs: --ignore @opentelemetry/instrumentation-aws-sdk --ignore @opentelemetry/instrumentation-pino --ignore @opentelemetry/instrumentation-redis-4 + - node: "12" + lerna-extra-args: >- + --ignore @opentelemetry/instrumentation-redis-4 runs-on: ubuntu-latest services: memcached: From e72dd3f001c7124e565edef3c94f8174aa8be4c1 Mon Sep 17 00:00:00 2001 From: Amir Blum Date: Tue, 24 May 2022 12:22:45 +0300 Subject: [PATCH 22/22] docs(redis-4): add instrumentation options to README --- .../auto-instrumentations-node/package.json | 2 +- .../README.md | 30 +++++++++++++++++++ .../package.json | 2 +- 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/metapackages/auto-instrumentations-node/package.json b/metapackages/auto-instrumentations-node/package.json index bbac55d877..cfa7882f5d 100644 --- a/metapackages/auto-instrumentations-node/package.json +++ b/metapackages/auto-instrumentations-node/package.json @@ -73,7 +73,7 @@ "@opentelemetry/instrumentation-pg": "^0.29.0", "@opentelemetry/instrumentation-pino": "^0.29.0", "@opentelemetry/instrumentation-redis": "^0.30.0", - "@opentelemetry/instrumentation-redis-4": "^0.30.0", + "@opentelemetry/instrumentation-redis-4": "^0.29.0", "@opentelemetry/instrumentation-restify": "^0.28.0", "@opentelemetry/instrumentation-winston": "^0.28.0" } diff --git a/plugins/node/opentelemetry-instrumentation-redis-4/README.md b/plugins/node/opentelemetry-instrumentation-redis-4/README.md index 7fa089af4c..7d4870a511 100644 --- a/plugins/node/opentelemetry-instrumentation-redis-4/README.md +++ b/plugins/node/opentelemetry-instrumentation-redis-4/README.md @@ -41,6 +41,36 @@ registerInstrumentations({ }) ``` +### Redis Instrumentation Options + +Redis instrumentation has a few options available to choose from. You can set the following: + +| Options | Type | Description | +| ----------------------- | ------------------------------------------------- | -------------------------------------------------------------------------------------------------------------- | +| `dbStatementSerializer` | `DbStatementSerializer` (function) | Redis instrumentation will serialize the command to the `db.statement` attribute using the specified function. | +| `responseHook` | `RedisResponseCustomAttributeFunction` (function) | Function for adding custom attributes on db response. Receives params: `span, cmdName, cmdArgs, response` | +| `requireParentSpan` | `boolean` | Require parent to create redis span, default when unset is false. | + +#### Custom `db.statement` Serializer + +The instrumentation serializes the command into a Span attribute called +`db.statement`. The default serialization sets the attribute to the command +name, without the command arguments. + +It is also possible to define a custom serialization function. The function +will receive the command name and arguments and must return a string. + +Here is a simple example to serialize the command name and arguments: + +```javascript +const { RedisInstrumentation } = require('@opentelemetry/instrumentation-redis'); +const redisInstrumentation = new RedisInstrumentation({ + dbStatementSerializer: function (cmdName, cmdArgs) { + return [cmdName, ...cmdArgs].join(" "); + }, +}); +``` + ## Useful links - For more information on OpenTelemetry, visit: diff --git a/plugins/node/opentelemetry-instrumentation-redis-4/package.json b/plugins/node/opentelemetry-instrumentation-redis-4/package.json index 9cf4951302..ff5f7ecada 100644 --- a/plugins/node/opentelemetry-instrumentation-redis-4/package.json +++ b/plugins/node/opentelemetry-instrumentation-redis-4/package.json @@ -1,6 +1,6 @@ { "name": "@opentelemetry/instrumentation-redis-4", - "version": "0.30.0", + "version": "0.29.0", "description": "Automatic OpenTelemetry instrumentation for redis package version 4", "main": "build/src/index.js", "types": "build/src/index.d.ts",