From 94dc9b082c063f13d3dbb675596eaa6694fcc5b3 Mon Sep 17 00:00:00 2001 From: Mark Wolff Date: Wed, 6 Nov 2019 18:12:35 -0800 Subject: [PATCH 01/10] feat(plugin): implement redis plugin --- .../opentelemetry-plugin-redis/package.json | 12 +- .../opentelemetry-plugin-redis/src/enums.ts | 32 ++++ .../opentelemetry-plugin-redis/src/index.ts | 2 + .../opentelemetry-plugin-redis/src/redis.ts | 125 ++++++++++++ .../test/assertionUtils.ts | 76 ++++++++ .../test/redis.test.ts | 179 ++++++++++++++++++ .../test/testUtils.ts | 54 ++++++ 7 files changed, 478 insertions(+), 2 deletions(-) create mode 100644 packages/opentelemetry-plugin-redis/src/enums.ts create mode 100644 packages/opentelemetry-plugin-redis/src/redis.ts create mode 100644 packages/opentelemetry-plugin-redis/test/assertionUtils.ts create mode 100644 packages/opentelemetry-plugin-redis/test/redis.test.ts create mode 100644 packages/opentelemetry-plugin-redis/test/testUtils.ts diff --git a/packages/opentelemetry-plugin-redis/package.json b/packages/opentelemetry-plugin-redis/package.json index 005f06131da..ff5a70f863d 100644 --- a/packages/opentelemetry-plugin-redis/package.json +++ b/packages/opentelemetry-plugin-redis/package.json @@ -8,11 +8,14 @@ "repository": "open-telemetry/opentelemetry-js", "scripts": { "test": "nyc ts-mocha -p tsconfig.json 'test/**/*.ts'", + "test:debug": "ts-mocha --inspect-brk --no-timeouts -p tsconfig.json 'test/**/*.test.ts'", + "test:local": "cross-env RUN_REDIS_TESTS_LOCAL=true yarn test", "tdd": "yarn test -- --watch-extensions ts --watch", "clean": "rimraf build/*", "check": "gts check", "precompile": "tsc --version", "compile": "tsc -p .", + "codecov": "nyc report --reporter=json && codecov -f coverage/*.json -p ../../", "fix": "gts fix", "prepare": "npm run compile" }, @@ -42,10 +45,15 @@ "devDependencies": { "@types/mocha": "^5.2.7", "@types/node": "^12.6.9", + "@types/redis": "^2.8.14", + "@types/shimmer": "^1.0.1", + "@opentelemetry/node": "^0.2.0", + "@opentelemetry/tracing": "^0.2.0", "codecov": "^3.5.0", "gts": "^1.1.0", "mocha": "^6.2.0", "nyc": "^14.1.1", + "redis": "^2.8.0", "rimraf": "^3.0.0", "ts-mocha": "^6.0.0", "ts-node": "^8.3.0", @@ -55,7 +63,7 @@ }, "dependencies": { "@opentelemetry/core": "^0.2.0", - "@opentelemetry/node": "^0.2.0", - "@opentelemetry/types": "^0.2.0" + "@opentelemetry/types": "^0.2.0", + "shimmer": "^1.2.1" } } diff --git a/packages/opentelemetry-plugin-redis/src/enums.ts b/packages/opentelemetry-plugin-redis/src/enums.ts new file mode 100644 index 00000000000..035287a14bb --- /dev/null +++ b/packages/opentelemetry-plugin-redis/src/enums.ts @@ -0,0 +1,32 @@ +/*! + * Copyright 2019, 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 enum AttributeNames { + // required by https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/data-semantic-conventions.md#databases-client-calls + COMPONENT = 'component', + DB_TYPE = 'db.type', + DB_INSTANCE = 'db.instance', + DB_STATEMENT = 'db.statement', + PEER_ADDRESS = 'peer.address', + PEER_HOSTNAME = 'peer.host', + + // optional + DB_USER = 'db.user', + PEER_PORT = 'peer.port', + PEER_IPV4 = 'peer.ipv4', + PEER_IPV6 = 'peer.ipv6', + PEER_SERVICE = 'peer.service', +} diff --git a/packages/opentelemetry-plugin-redis/src/index.ts b/packages/opentelemetry-plugin-redis/src/index.ts index ae225f6b521..4595acb43a2 100644 --- a/packages/opentelemetry-plugin-redis/src/index.ts +++ b/packages/opentelemetry-plugin-redis/src/index.ts @@ -13,3 +13,5 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + +export * from './redis'; diff --git a/packages/opentelemetry-plugin-redis/src/redis.ts b/packages/opentelemetry-plugin-redis/src/redis.ts new file mode 100644 index 00000000000..06232b29884 --- /dev/null +++ b/packages/opentelemetry-plugin-redis/src/redis.ts @@ -0,0 +1,125 @@ +/*! + * Copyright 2019, 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 { BasePlugin } from '@opentelemetry/core'; +import * as redisTypes from 'redis'; +import * as shimmer from 'shimmer'; +import { SpanKind, CanonicalCode } from '@opentelemetry/types'; +import { AttributeNames } from './enums'; + +// exported from +// https://github.com/NodeRedis/node_redis/blob/master/lib/command.js +export interface RedisCommand { + command: string; + args: string[]; + buffer_args: boolean; + callback: Function; + call_on_write: boolean; +} + +interface RedisPluginClientTypes { + options?: { + host: string; + port: string; + }; + + address?: string; +} + +export class RedisPlugin extends BasePlugin { + static readonly COMPONENT = 'redis'; + readonly supportedVersions = ['^2.6.0']; // should be >= 2.6.0 + + constructor(readonly moduleName: string) { + super(); + } + + protected patch() { + this._logger.debug( + 'Patching redis.RedisClient.prototype.internal_send_command' + ); + shimmer.wrap( + this._moduleExports.RedisClient.prototype, + 'internal_send_command', + this._getPatchInternalSendCommand() + ); + return this._moduleExports; + } + + protected unpatch(): void { + if (this._moduleExports) { + shimmer.unwrap( + this._moduleExports.RedisClient.prototype, + 'internal_send_command' + ); + } + } + + /** + * Patch internal_send_command(...) to trace requests + */ + private _getPatchInternalSendCommand() { + const plugin = this; + return function internal_send_command(original: Function) { + return function internal_send_command_trace( + this: redisTypes.RedisClient & RedisPluginClientTypes, + cmd?: RedisCommand + ) { + const parentSpan = plugin._tracer.getCurrentSpan(); + + // New versions of redis (2.4+) use a single options object + // instead of named arguments + if (arguments.length === 1 && typeof cmd === 'object') { + const span = plugin._tracer.startSpan(`redis-${cmd.command}`, { + kind: SpanKind.CLIENT, + parent: parentSpan || undefined, + attributes: { + [AttributeNames.COMPONENT]: RedisPlugin.COMPONENT, + [AttributeNames.DB_STATEMENT]: cmd.command, + }, + }); + + // Set attributes for not explicitly typed RedisPluginClientTypes + if (this.options) { + span.setAttributes({ + [AttributeNames.PEER_HOSTNAME]: this.options.host, + [AttributeNames.PEER_PORT]: this.options.port, + }); + } + if (this.address) { + span.setAttribute( + AttributeNames.PEER_ADDRESS, + `redis://${this.address}` + ); + } + + const originalCallback = arguments[0].callback; + arguments[0].callback = function callback() { + span.setStatus({ + code: CanonicalCode.OK, + }); + span.end(); + return originalCallback.apply(this, arguments); + }; + return original.apply(this, arguments); + } + return original.apply(this, arguments); + }; + }; + } +} + +export const plugin = new RedisPlugin(RedisPlugin.COMPONENT); diff --git a/packages/opentelemetry-plugin-redis/test/assertionUtils.ts b/packages/opentelemetry-plugin-redis/test/assertionUtils.ts new file mode 100644 index 00000000000..fdb517c7a09 --- /dev/null +++ b/packages/opentelemetry-plugin-redis/test/assertionUtils.ts @@ -0,0 +1,76 @@ +/*! + * Copyright 2019, 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 { + SpanKind, + Attributes, + Event, + Span, + Status, +} from '@opentelemetry/types'; +import * as assert from 'assert'; +import { ReadableSpan } from '@opentelemetry/tracing'; +import { + hrTimeToMilliseconds, + hrTimeToMicroseconds, +} from '@opentelemetry/core'; + +export const assertSpan = ( + span: ReadableSpan, + kind: SpanKind, + attributes: Attributes, + events: Event[], + status: Status +) => { + assert.strictEqual(span.spanContext.traceId.length, 32); + assert.strictEqual(span.spanContext.spanId.length, 16); + assert.strictEqual(span.kind, kind); + + assert.ok(span.endTime); + assert.strictEqual(span.links.length, 0); + + assert.ok( + hrTimeToMicroseconds(span.startTime) < hrTimeToMicroseconds(span.endTime) + ); + assert.ok(hrTimeToMilliseconds(span.endTime) > 0); + + // attributes + assert.deepStrictEqual(span.attributes, attributes); + + // events + assert.deepStrictEqual(span.events, events); + + assert.strictEqual(span.status.code, status.code); + if (status.message) { + assert.strictEqual(span.status.message, status.message); + } +}; + +// Check if childSpan was propagated from parentSpan +export const assertPropagation = ( + childSpan: ReadableSpan, + parentSpan: Span +) => { + const targetSpanContext = childSpan.spanContext; + const sourceSpanContext = parentSpan.context(); + assert.strictEqual(targetSpanContext.traceId, sourceSpanContext.traceId); + assert.strictEqual(childSpan.parentSpanId, sourceSpanContext.spanId); + assert.strictEqual( + targetSpanContext.traceFlags, + sourceSpanContext.traceFlags + ); + assert.notStrictEqual(targetSpanContext.spanId, sourceSpanContext.spanId); +}; diff --git a/packages/opentelemetry-plugin-redis/test/redis.test.ts b/packages/opentelemetry-plugin-redis/test/redis.test.ts new file mode 100644 index 00000000000..33ee799fd19 --- /dev/null +++ b/packages/opentelemetry-plugin-redis/test/redis.test.ts @@ -0,0 +1,179 @@ +/*! + * Copyright 2019, 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 * as assert from 'assert'; +import { + InMemorySpanExporter, + SimpleSpanProcessor, +} from '@opentelemetry/tracing'; +import { NodeTracer } from '@opentelemetry/node'; +import { plugin, RedisPlugin } from '../src'; +import * as redisTypes from 'redis'; +import { NoopLogger } from '@opentelemetry/core'; +import * as dockerUtils from './testUtils'; +import * as assertionUtils from './assertionUtils'; +import { SpanKind, Status, CanonicalCode } from '@opentelemetry/types'; +import { AttributeNames } from '../src/enums'; + +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 = { + [AttributeNames.COMPONENT]: RedisPlugin.COMPONENT, + [AttributeNames.PEER_HOSTNAME]: CONFIG.host, + [AttributeNames.PEER_PORT]: CONFIG.port, + [AttributeNames.PEER_ADDRESS]: URL, +}; + +const okStatus: Status = { + code: CanonicalCode.OK, +}; + +describe('redis@2.x', () => { + const tracer = new NodeTracer(); + let redis: typeof redisTypes; + let client: redisTypes.RedisClient; + const shouldTestLocal = process.env.RUN_REDIS_TESTS_LOCAL; + + before(done => { + if (shouldTestLocal) { + dockerUtils.startDocker(); + } + redis = require('redis'); + tracer.addSpanProcessor(new SimpleSpanProcessor(memoryExporter)); + client = redis.createClient(URL); + client.on('error', err => { + done(err); + }); + client.on('ready', () => { + plugin.enable(redis, tracer, new NoopLogger()); + done(); + }); + }); + + beforeEach(done => { + client.set('test', 'data', () => { + memoryExporter.reset(); + done(); + }); + }); + + afterEach(done => { + client.del('hash', () => { + memoryExporter.reset(); + done(); + }); + }); + + after(() => { + if (client) { + client.quit(); + } + if (shouldTestLocal) { + dockerUtils.cleanUpDocker(); + } + }); + + it('should have correct module name', () => { + assert.strictEqual(plugin.moduleName, RedisPlugin.COMPONENT); + }); + + const REDIS_OPERATIONS: Array<{ + description: string; + command: string; + method: (cb: redisTypes.Callback) => unknown; + }> = [ + { + description: 'insert', + command: 'hset', + method: (cb: redisTypes.Callback) => + client.hset('hash', 'random', 'random', cb), + }, + { + description: 'get', + command: 'get', + method: (cb: redisTypes.Callback) => client.get('test', cb), + }, + { + description: 'delete', + command: 'del', + method: (cb: redisTypes.Callback) => client.del('test', cb), + }, + ]; + + describe('Instrumenting query operations', () => { + REDIS_OPERATIONS.forEach(operation => { + it(`should create a child span for ${operation.description}`, done => { + const attributes = { + ...DEFAULT_ATTRIBUTES, + [AttributeNames.DB_STATEMENT]: operation.command, + }; + const span = tracer.startSpan('test span'); + tracer.withSpan(span, () => { + operation.method((err, _result) => { + assert.ifError(err); + assert.strictEqual(memoryExporter.getFinishedSpans().length, 1); + span.end(); + const endedSpans = memoryExporter.getFinishedSpans(); + assert.strictEqual(endedSpans.length, 2); + assert.strictEqual( + endedSpans[0].name, + `redis-${operation.command}` + ); + assertionUtils.assertSpan( + endedSpans[0], + SpanKind.CLIENT, + attributes, + [], + okStatus + ); + assertionUtils.assertPropagation(endedSpans[0], span); + done(); + }); + }); + }); + }); + }); + + describe('Removing instrumentation', () => { + before(() => { + plugin.disable(); + }); + + REDIS_OPERATIONS.forEach(operation => { + it(`should not create a child span for ${operation.description}`, done => { + const span = tracer.startSpan('test span'); + tracer.withSpan(span, () => { + operation.method((err, _) => { + assert.ifError(err); + assert.strictEqual(memoryExporter.getFinishedSpans().length, 0); + span.end(); + const endedSpans = memoryExporter.getFinishedSpans(); + assert.strictEqual(endedSpans.length, 1); + assert.strictEqual(endedSpans[0], span); + done(); + }); + }); + }); + }); + }); +}); diff --git a/packages/opentelemetry-plugin-redis/test/testUtils.ts b/packages/opentelemetry-plugin-redis/test/testUtils.ts new file mode 100644 index 00000000000..a40b0b4aab0 --- /dev/null +++ b/packages/opentelemetry-plugin-redis/test/testUtils.ts @@ -0,0 +1,54 @@ +/*! + * Copyright 2019, 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 * as childProcess from 'child_process'; +export function startDocker() { + const tasks = [ + run('docker run -d -p 63790:6379 --name otjsredis redis:alpine'), + ]; + + for (let i = 0; i < tasks.length; i++) { + const task = tasks[i]; + if (task && task.code !== 0) { + console.error('Failed to start container!'); + console.error(task.output); + return false; + } + } + return true; +} + +export function cleanUpDocker() { + run('docker stop otjsredis'); + run('docker rm otjsredis'); +} + +function run(cmd: string) { + try { + const proc = childProcess.spawnSync(cmd, { + shell: true, + }); + return { + code: proc.status, + output: proc.output + .map(v => String.fromCharCode.apply(null, v as any)) + .join(''), + }; + } catch (e) { + console.log(e); + return; + } +} From 8202ac1a5e3e1bb8c5a5376cb96a7d3c55fc2ec9 Mon Sep 17 00:00:00 2001 From: Mark Wolff Date: Fri, 8 Nov 2019 14:29:12 -0800 Subject: [PATCH 02/10] fix: circle redis testing --- .circleci/config.yml | 3 +++ .../opentelemetry-plugin-redis/test/redis.test.ts | 11 ++++++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 3d72787e934..49d06cbcc60 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -6,12 +6,15 @@ test_env: &test_env POSTGRES_DB: circle_database POSTGRES_HOST: localhost POSTGRES_PORT: 5432 + RUN_REDIS_TESTS: 1 postgres_service: &postgres_service image: circleci/postgres:9.6-alpine environment: # env to pass to CircleCI, specified values must match test_env POSTGRES_USER: postgres POSTGRES_DB: circle_database +redis_service: &redis_service + image: redis node_unit_tests: &node_unit_tests steps: diff --git a/packages/opentelemetry-plugin-redis/test/redis.test.ts b/packages/opentelemetry-plugin-redis/test/redis.test.ts index 33ee799fd19..963ddf00dfb 100644 --- a/packages/opentelemetry-plugin-redis/test/redis.test.ts +++ b/packages/opentelemetry-plugin-redis/test/redis.test.ts @@ -53,11 +53,20 @@ describe('redis@2.x', () => { let redis: typeof redisTypes; let client: redisTypes.RedisClient; const shouldTestLocal = process.env.RUN_REDIS_TESTS_LOCAL; + const shouldTest = process.env.RUN_REDIS_TESTS || shouldTestLocal; + + before(function(done) { + if (!shouldTest) { + // this.skip() workaround + // https://github.com/mochajs/mocha/issues/2683#issuecomment-375629901 + this.test!.parent!.pending = true; + this.skip(); + } - before(done => { if (shouldTestLocal) { dockerUtils.startDocker(); } + redis = require('redis'); tracer.addSpanProcessor(new SimpleSpanProcessor(memoryExporter)); client = redis.createClient(URL); From 632b8c4706fb4b68c4376db608af35a7caf0b2b0 Mon Sep 17 00:00:00 2001 From: Mark Wolff Date: Fri, 8 Nov 2019 14:36:17 -0800 Subject: [PATCH 03/10] fix: set span error status --- .circleci/config.yml | 2 ++ .../opentelemetry-plugin-redis/src/redis.ts | 22 ++++++++++++++----- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 49d06cbcc60..be44a975b5d 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -7,6 +7,8 @@ test_env: &test_env POSTGRES_HOST: localhost POSTGRES_PORT: 5432 RUN_REDIS_TESTS: 1 + OPENTELEMETRY_REDIS_HOST: 'localhost' + OPENTELEMETRY_REDIS_PORT: 6379 postgres_service: &postgres_service image: circleci/postgres:9.6-alpine diff --git a/packages/opentelemetry-plugin-redis/src/redis.ts b/packages/opentelemetry-plugin-redis/src/redis.ts index 06232b29884..f86d4807ca1 100644 --- a/packages/opentelemetry-plugin-redis/src/redis.ts +++ b/packages/opentelemetry-plugin-redis/src/redis.ts @@ -26,7 +26,7 @@ export interface RedisCommand { command: string; args: string[]; buffer_args: boolean; - callback: Function; + callback: redisTypes.Callback; call_on_write: boolean; } @@ -107,13 +107,23 @@ export class RedisPlugin extends BasePlugin { } const originalCallback = arguments[0].callback; - arguments[0].callback = function callback() { - span.setStatus({ - code: CanonicalCode.OK, - }); + (arguments[0] as RedisCommand).callback = function callback( + this: unknown, + err: Error | null, + _reply: T + ) { + if (err) { + span.setStatus({ + code: CanonicalCode.UNKNOWN, + message: err.message, + }); + } else { + span.setStatus({ code: CanonicalCode.OK }); + } + span.end(); return originalCallback.apply(this, arguments); - }; + } as redisTypes.Callback; return original.apply(this, arguments); } return original.apply(this, arguments); From d6665c7b3aff147a9f8db1f4cd78d7139ef1ffef Mon Sep 17 00:00:00 2001 From: Mark Wolff Date: Fri, 8 Nov 2019 14:42:17 -0800 Subject: [PATCH 04/10] fix: run the redis service --- .circleci/config.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.circleci/config.yml b/.circleci/config.yml index be44a975b5d..bc1d48c7f11 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -103,18 +103,21 @@ jobs: - image: node:8 environment: *test_env - *postgres_service + - *redis_service <<: *node_unit_tests node10: docker: - image: node:10 environment: *test_env - *postgres_service + - *redis_service <<: *node_unit_tests node12: docker: - image: node:12 environment: *test_env - *postgres_service + - *redis_service <<: *node_unit_tests node12-browsers: docker: From 6c31a8ab18cef8717aa29f85666f452cb8e3a79b Mon Sep 17 00:00:00 2001 From: Mark Wolff Date: Fri, 8 Nov 2019 14:58:08 -0800 Subject: [PATCH 05/10] fix: linting --- .circleci/config.yml | 10 +++++----- packages/opentelemetry-plugin-redis/test/redis.test.ts | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index bc1d48c7f11..7d299b9fa6f 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -1,6 +1,6 @@ version: 2 -test_env: &test_env +node_test_env: &node_test_env RUN_POSTGRES_TESTS: 1 POSTGRES_USER: postgres POSTGRES_DB: circle_database @@ -12,7 +12,7 @@ test_env: &test_env postgres_service: &postgres_service image: circleci/postgres:9.6-alpine - environment: # env to pass to CircleCI, specified values must match test_env + environment: # env to pass to CircleCI, specified values must match node_test_env POSTGRES_USER: postgres POSTGRES_DB: circle_database redis_service: &redis_service @@ -101,21 +101,21 @@ jobs: node8: docker: - image: node:8 - environment: *test_env + environment: *node_test_env - *postgres_service - *redis_service <<: *node_unit_tests node10: docker: - image: node:10 - environment: *test_env + environment: *node_test_env - *postgres_service - *redis_service <<: *node_unit_tests node12: docker: - image: node:12 - environment: *test_env + environment: *node_test_env - *postgres_service - *redis_service <<: *node_unit_tests diff --git a/packages/opentelemetry-plugin-redis/test/redis.test.ts b/packages/opentelemetry-plugin-redis/test/redis.test.ts index 963ddf00dfb..a0b779b4100 100644 --- a/packages/opentelemetry-plugin-redis/test/redis.test.ts +++ b/packages/opentelemetry-plugin-redis/test/redis.test.ts @@ -66,7 +66,7 @@ describe('redis@2.x', () => { if (shouldTestLocal) { dockerUtils.startDocker(); } - + redis = require('redis'); tracer.addSpanProcessor(new SimpleSpanProcessor(memoryExporter)); client = redis.createClient(URL); From 73d2fbb8bd1b4f8bb3a207ce0858660303ff7fde Mon Sep 17 00:00:00 2001 From: Mark Wolff Date: Mon, 11 Nov 2019 11:46:55 -0800 Subject: [PATCH 06/10] feat: add redis error handling statuses --- .../opentelemetry-plugin-redis/package.json | 3 +- .../opentelemetry-plugin-redis/src/redis.ts | 128 ++++++++--- .../opentelemetry-plugin-redis/src/types.ts | 41 ++++ .../test/redis.test.ts | 217 ++++++++++-------- 4 files changed, 256 insertions(+), 133 deletions(-) create mode 100644 packages/opentelemetry-plugin-redis/src/types.ts diff --git a/packages/opentelemetry-plugin-redis/package.json b/packages/opentelemetry-plugin-redis/package.json index ff5a70f863d..29816ab6948 100644 --- a/packages/opentelemetry-plugin-redis/package.json +++ b/packages/opentelemetry-plugin-redis/package.json @@ -8,7 +8,7 @@ "repository": "open-telemetry/opentelemetry-js", "scripts": { "test": "nyc ts-mocha -p tsconfig.json 'test/**/*.ts'", - "test:debug": "ts-mocha --inspect-brk --no-timeouts -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 yarn test", "tdd": "yarn test -- --watch-extensions ts --watch", "clean": "rimraf build/*", @@ -50,6 +50,7 @@ "@opentelemetry/node": "^0.2.0", "@opentelemetry/tracing": "^0.2.0", "codecov": "^3.5.0", + "cross-env": "^6.0.3", "gts": "^1.1.0", "mocha": "^6.2.0", "nyc": "^14.1.1", diff --git a/packages/opentelemetry-plugin-redis/src/redis.ts b/packages/opentelemetry-plugin-redis/src/redis.ts index f86d4807ca1..cdf80eccb3d 100644 --- a/packages/opentelemetry-plugin-redis/src/redis.ts +++ b/packages/opentelemetry-plugin-redis/src/redis.ts @@ -19,25 +19,12 @@ import * as redisTypes from 'redis'; import * as shimmer from 'shimmer'; import { SpanKind, CanonicalCode } from '@opentelemetry/types'; import { AttributeNames } from './enums'; - -// exported from -// https://github.com/NodeRedis/node_redis/blob/master/lib/command.js -export interface RedisCommand { - command: string; - args: string[]; - buffer_args: boolean; - callback: redisTypes.Callback; - call_on_write: boolean; -} - -interface RedisPluginClientTypes { - options?: { - host: string; - port: string; - }; - - address?: string; -} +import { + RedisPluginStreamTypes, + RedisPluginClientTypes, + RedisCommand, +} from './types'; +import { EventEmitter } from 'events'; export class RedisPlugin extends BasePlugin { static readonly COMPONENT = 'redis'; @@ -48,14 +35,30 @@ export class RedisPlugin extends BasePlugin { } protected patch() { - this._logger.debug( - 'Patching redis.RedisClient.prototype.internal_send_command' - ); - shimmer.wrap( - this._moduleExports.RedisClient.prototype, - 'internal_send_command', - this._getPatchInternalSendCommand() - ); + if (this._moduleExports.RedisClient) { + this._logger.debug( + 'Patching redis.RedisClient.prototype.internal_send_command' + ); + shimmer.wrap( + this._moduleExports.RedisClient.prototype, + 'internal_send_command', + this._getPatchInternalSendCommand() + ); + + this._logger.debug('patching redis.create_stream'); + shimmer.wrap( + this._moduleExports.RedisClient.prototype, + 'create_stream', + this._getPatchCreateStream() + ); + + this._logger.debug('patching redis.createClient'); + shimmer.wrap( + this._moduleExports, + 'createClient', + this._getPatchCreateClient() + ); + } return this._moduleExports; } @@ -65,6 +68,11 @@ export class RedisPlugin extends BasePlugin { this._moduleExports.RedisClient.prototype, 'internal_send_command' ); + shimmer.unwrap( + this._moduleExports.RedisClient.prototype, + 'create_stream' + ); + shimmer.unwrap(this._moduleExports, 'createClient'); } } @@ -106,12 +114,9 @@ export class RedisPlugin extends BasePlugin { ); } - const originalCallback = arguments[0].callback; - (arguments[0] as RedisCommand).callback = function callback( - this: unknown, - err: Error | null, - _reply: T - ) { + let spanEnded = false; + const endSpan = (err?: Error | null) => { + if (spanEnded) return; // never if (err) { span.setStatus({ code: CanonicalCode.UNKNOWN, @@ -120,11 +125,60 @@ export class RedisPlugin extends BasePlugin { } else { span.setStatus({ code: CanonicalCode.OK }); } - span.end(); - return originalCallback.apply(this, arguments); - } as redisTypes.Callback; - return original.apply(this, arguments); + spanEnded = true; + }; + + const originalCallback = arguments[0].callback; + if (originalCallback) { + (arguments[0] as RedisCommand).callback = function callback( + this: unknown, + err: Error | null, + _reply: T + ) { + endSpan(err); + return originalCallback.apply(this, arguments); + }; + } + try { + // Span will be ended in callback + return original.apply(this, arguments); + } catch (rethrow) { + endSpan(rethrow); + throw rethrow; // rethrow after ending span + } + } + + // We don't know how to trace this call, so don't start/stop a span + return original.apply(this, arguments); + }; + }; + } + + private _getPatchCreateClient() { + const plugin = this; + return function createClient(original: Function) { + return function createClientTrace(this: redisTypes.RedisClient) { + const client: redisTypes.RedisClient = original.apply(this, arguments); + return plugin._tracer.bind(client); + }; + }; + } + + private _getPatchCreateStream() { + const plugin = this; + return function createReadStream(original: Function) { + return function create_stream_trace(this: RedisPluginStreamTypes) { + if (!this.stream) { + Object.defineProperty(this, 'stream', { + get() { + return this._patched_redis_stream; + }, + set(val: EventEmitter) { + plugin._tracer.bind(val); + this._patched_redis_stream = val; + }, + }); } return original.apply(this, arguments); }; diff --git a/packages/opentelemetry-plugin-redis/src/types.ts b/packages/opentelemetry-plugin-redis/src/types.ts new file mode 100644 index 00000000000..4014508f49a --- /dev/null +++ b/packages/opentelemetry-plugin-redis/src/types.ts @@ -0,0 +1,41 @@ +/*! + * Copyright 2019, 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 * as redisTypes from 'redis'; +import { EventEmitter } from 'events'; + +// exported from +// https://github.com/NodeRedis/node_redis/blob/master/lib/command.js +export interface RedisCommand { + command: string; + args: string[]; + buffer_args: boolean; + callback: redisTypes.Callback; + call_on_write: boolean; +} + +export interface RedisPluginClientTypes { + options?: { + host: string; + port: string; + }; + + address?: string; +} + +export interface RedisPluginStreamTypes { + stream?: { get(): EventEmitter; set(val: EventEmitter): void }; +} diff --git a/packages/opentelemetry-plugin-redis/test/redis.test.ts b/packages/opentelemetry-plugin-redis/test/redis.test.ts index a0b779b4100..72d6730a69a 100644 --- a/packages/opentelemetry-plugin-redis/test/redis.test.ts +++ b/packages/opentelemetry-plugin-redis/test/redis.test.ts @@ -51,11 +51,11 @@ const okStatus: Status = { describe('redis@2.x', () => { const tracer = new NodeTracer(); let redis: typeof redisTypes; - let client: redisTypes.RedisClient; const shouldTestLocal = process.env.RUN_REDIS_TESTS_LOCAL; const shouldTest = process.env.RUN_REDIS_TESTS || shouldTestLocal; - before(function(done) { + before(function() { + // needs to be "function" to have MochaContext "this" scope if (!shouldTest) { // this.skip() workaround // https://github.com/mochajs/mocha/issues/2683#issuecomment-375629901 @@ -69,34 +69,10 @@ describe('redis@2.x', () => { redis = require('redis'); tracer.addSpanProcessor(new SimpleSpanProcessor(memoryExporter)); - client = redis.createClient(URL); - client.on('error', err => { - done(err); - }); - client.on('ready', () => { - plugin.enable(redis, tracer, new NoopLogger()); - done(); - }); - }); - - beforeEach(done => { - client.set('test', 'data', () => { - memoryExporter.reset(); - done(); - }); - }); - - afterEach(done => { - client.del('hash', () => { - memoryExporter.reset(); - done(); - }); + plugin.enable(redis, tracer, new NoopLogger()); }); after(() => { - if (client) { - client.quit(); - } if (shouldTestLocal) { dockerUtils.cleanUpDocker(); } @@ -106,80 +82,131 @@ describe('redis@2.x', () => { assert.strictEqual(plugin.moduleName, RedisPlugin.COMPONENT); }); - const REDIS_OPERATIONS: Array<{ - description: string; - command: string; - method: (cb: redisTypes.Callback) => unknown; - }> = [ - { - description: 'insert', - command: 'hset', - method: (cb: redisTypes.Callback) => - client.hset('hash', 'random', 'random', cb), - }, - { - description: 'get', - command: 'get', - method: (cb: redisTypes.Callback) => client.get('test', cb), - }, - { - description: 'delete', - command: 'del', - method: (cb: redisTypes.Callback) => client.del('test', cb), - }, - ]; - - describe('Instrumenting query operations', () => { - REDIS_OPERATIONS.forEach(operation => { - it(`should create a child span for ${operation.description}`, done => { - const attributes = { - ...DEFAULT_ATTRIBUTES, - [AttributeNames.DB_STATEMENT]: operation.command, - }; - const span = tracer.startSpan('test span'); - tracer.withSpan(span, () => { - operation.method((err, _result) => { - assert.ifError(err); - assert.strictEqual(memoryExporter.getFinishedSpans().length, 1); - span.end(); - const endedSpans = memoryExporter.getFinishedSpans(); - assert.strictEqual(endedSpans.length, 2); - assert.strictEqual( - endedSpans[0].name, - `redis-${operation.command}` - ); - assertionUtils.assertSpan( - endedSpans[0], - SpanKind.CLIENT, - attributes, - [], - okStatus - ); - assertionUtils.assertPropagation(endedSpans[0], span); - done(); - }); - }); + describe('#createClient()', () => { + it('should propagate the current span to event handlers', done => { + const span = tracer.startSpan('test span'); + let client: redisTypes.RedisClient; + const readyHandler = () => { + assert.strictEqual(tracer.getCurrentSpan(), span); + client.quit(done); + }; + const errorHandler = (err: Error) => { + assert.ifError(err); + client.quit(done); + }; + + tracer.withSpan(span, () => { + client = redis.createClient(URL); + client.on('ready', readyHandler); + client.on('error', errorHandler); }); }); }); - describe('Removing instrumentation', () => { - before(() => { - plugin.disable(); + describe('#send_internal_message()', () => { + let client: redisTypes.RedisClient; + + const REDIS_OPERATIONS: Array<{ + description: string; + command: string; + method: (cb: redisTypes.Callback) => unknown; + }> = [ + { + description: 'insert', + command: 'hset', + method: (cb: redisTypes.Callback) => + client.hset('hash', 'random', 'random', cb), + }, + { + description: 'get', + command: 'get', + method: (cb: redisTypes.Callback) => client.get('test', cb), + }, + { + description: 'delete', + command: 'del', + method: (cb: redisTypes.Callback) => client.del('test', cb), + }, + ]; + + before(done => { + client = redis.createClient(URL); + client.on('error', err => { + done(err); + }); + client.on('ready', done); }); - REDIS_OPERATIONS.forEach(operation => { - it(`should not create a child span for ${operation.description}`, done => { - const span = tracer.startSpan('test span'); - tracer.withSpan(span, () => { - operation.method((err, _) => { - assert.ifError(err); - assert.strictEqual(memoryExporter.getFinishedSpans().length, 0); - span.end(); - const endedSpans = memoryExporter.getFinishedSpans(); - assert.strictEqual(endedSpans.length, 1); - assert.strictEqual(endedSpans[0], span); - done(); + beforeEach(done => { + client.set('test', 'data', () => { + memoryExporter.reset(); + done(); + }); + }); + + after(done => { + client.quit(done); + }); + + afterEach(done => { + client.del('hash', () => { + memoryExporter.reset(); + done(); + }); + }); + + describe('Instrumenting query operations', () => { + REDIS_OPERATIONS.forEach(operation => { + it(`should create a child span for ${operation.description}`, done => { + const attributes = { + ...DEFAULT_ATTRIBUTES, + [AttributeNames.DB_STATEMENT]: operation.command, + }; + const span = tracer.startSpan('test span'); + tracer.withSpan(span, () => { + operation.method((err, _result) => { + assert.ifError(err); + assert.strictEqual(memoryExporter.getFinishedSpans().length, 1); + span.end(); + const endedSpans = memoryExporter.getFinishedSpans(); + assert.strictEqual(endedSpans.length, 2); + assert.strictEqual( + endedSpans[0].name, + `redis-${operation.command}` + ); + assertionUtils.assertSpan( + endedSpans[0], + SpanKind.CLIENT, + attributes, + [], + okStatus + ); + assertionUtils.assertPropagation(endedSpans[0], span); + done(); + }); + }); + }); + }); + }); + + describe('Removing instrumentation', () => { + before(() => { + plugin.disable(); + }); + + REDIS_OPERATIONS.forEach(operation => { + it(`should not create a child span for ${operation.description}`, done => { + const span = tracer.startSpan('test span'); + tracer.withSpan(span, () => { + operation.method((err, _) => { + assert.ifError(err); + assert.strictEqual(memoryExporter.getFinishedSpans().length, 0); + span.end(); + const endedSpans = memoryExporter.getFinishedSpans(); + assert.strictEqual(endedSpans.length, 1); + assert.strictEqual(endedSpans[0], span); + done(); + }); }); }); }); From 391326b01f6dd189fb807b0a36359e86dd34c2c9 Mon Sep 17 00:00:00 2001 From: Mark Wolff Date: Mon, 11 Nov 2019 17:13:23 -0800 Subject: [PATCH 07/10] fix: pr comments --- packages/opentelemetry-plugin-redis/README.md | 29 +++++++++++++++++-- .../opentelemetry-plugin-redis/package.json | 1 - .../opentelemetry-plugin-redis/src/redis.ts | 2 +- 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/packages/opentelemetry-plugin-redis/README.md b/packages/opentelemetry-plugin-redis/README.md index 3509b21de35..266435a421d 100644 --- a/packages/opentelemetry-plugin-redis/README.md +++ b/packages/opentelemetry-plugin-redis/README.md @@ -4,7 +4,7 @@ [![devDependencies][devDependencies-image]][devDependencies-url] [![Apache License][license-image]][license-image] -This module provides automatic instrumentation for [`redis`](https://github.com/NodeRedis/node_redis). +This module provides automatic instrumentation for [`redis@^2.6.0`](https://github.com/NodeRedis/node_redis). For automatic instrumentation see the [@opentelemetry/node](https://github.com/open-telemetry/opentelemetry-js/tree/master/packages/opentelemetry-node) package. @@ -15,14 +15,37 @@ For automatic instrumentation see the npm install --save @opentelemetry/plugin-redis ``` +### Supported Versions + - `>=2.6.0` + ## Usage +OptenTelemetry 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 plugin (**redis** in this case), specify it in the Node Tracer's configuration +```js +const { NodeTracer } = require('@opentelemetry/node'); + +const tracer = new NodeTracer({ + plugins: { + redis: { + enabled: true, + // You may use a package name or absolute path to the file. + path: '@opentelemetry/plugin-redis', + } + } +}); ``` -const opentelemetry = require('@opentelemetry/plugin-redis'); -// TODO: DEMONSTRATE API +To load all the [supported plugins](https://github.com/open-telemetry/opentelemetry-js#plugins), use below approach. Each plugin is only loaded when the module that it patches is loaded; in other words, there is no computational overhead for listing plugins for unused modules. +```javascript +const { NodeTracer } = require('@opentelemetry/node'); + +const tracer = new NodeTracer(); ``` + + ## Useful links - For more information on OpenTelemetry, visit: - For more about OpenTelemetry JavaScript: diff --git a/packages/opentelemetry-plugin-redis/package.json b/packages/opentelemetry-plugin-redis/package.json index d7c99c155c2..61b3a9bfa23 100644 --- a/packages/opentelemetry-plugin-redis/package.json +++ b/packages/opentelemetry-plugin-redis/package.json @@ -2,7 +2,6 @@ "name": "@opentelemetry/plugin-redis", "version": "0.2.0", "description": "OpenTelemetry redis automatic instrumentation package.", - "private": true, "main": "build/src/index.js", "types": "build/src/index.d.ts", "repository": "open-telemetry/opentelemetry-js", diff --git a/packages/opentelemetry-plugin-redis/src/redis.ts b/packages/opentelemetry-plugin-redis/src/redis.ts index cdf80eccb3d..7da4154de32 100644 --- a/packages/opentelemetry-plugin-redis/src/redis.ts +++ b/packages/opentelemetry-plugin-redis/src/redis.ts @@ -91,7 +91,7 @@ export class RedisPlugin extends BasePlugin { // New versions of redis (2.4+) use a single options object // instead of named arguments if (arguments.length === 1 && typeof cmd === 'object') { - const span = plugin._tracer.startSpan(`redis-${cmd.command}`, { + const span = plugin._tracer.startSpan(`${RedisPlugin.COMPONENT}-${cmd.command}`, { kind: SpanKind.CLIENT, parent: parentSpan || undefined, attributes: { From 0821b41f5c3a070c279d46edb6f16d19489668d3 Mon Sep 17 00:00:00 2001 From: Mark Wolff Date: Tue, 12 Nov 2019 11:53:27 -0800 Subject: [PATCH 08/10] fix: redis linting --- .../opentelemetry-plugin-redis/src/redis.ts | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/packages/opentelemetry-plugin-redis/src/redis.ts b/packages/opentelemetry-plugin-redis/src/redis.ts index 7da4154de32..bc86bd31884 100644 --- a/packages/opentelemetry-plugin-redis/src/redis.ts +++ b/packages/opentelemetry-plugin-redis/src/redis.ts @@ -91,14 +91,17 @@ export class RedisPlugin extends BasePlugin { // New versions of redis (2.4+) use a single options object // instead of named arguments if (arguments.length === 1 && typeof cmd === 'object') { - const span = plugin._tracer.startSpan(`${RedisPlugin.COMPONENT}-${cmd.command}`, { - kind: SpanKind.CLIENT, - parent: parentSpan || undefined, - attributes: { - [AttributeNames.COMPONENT]: RedisPlugin.COMPONENT, - [AttributeNames.DB_STATEMENT]: cmd.command, - }, - }); + const span = plugin._tracer.startSpan( + `${RedisPlugin.COMPONENT}-${cmd.command}`, + { + kind: SpanKind.CLIENT, + parent: parentSpan || undefined, + attributes: { + [AttributeNames.COMPONENT]: RedisPlugin.COMPONENT, + [AttributeNames.DB_STATEMENT]: cmd.command, + }, + } + ); // Set attributes for not explicitly typed RedisPluginClientTypes if (this.options) { From 8654a71e77c53c57c50f8e564373dc585112f226 Mon Sep 17 00:00:00 2001 From: Mark Wolff Date: Wed, 13 Nov 2019 17:41:49 -0800 Subject: [PATCH 09/10] refactor: move patches to utils for clarity --- .../opentelemetry-plugin-redis/src/redis.ts | 112 ++-------------- .../opentelemetry-plugin-redis/src/utils.ts | 120 ++++++++++++++++++ 2 files changed, 129 insertions(+), 103 deletions(-) create mode 100644 packages/opentelemetry-plugin-redis/src/utils.ts diff --git a/packages/opentelemetry-plugin-redis/src/redis.ts b/packages/opentelemetry-plugin-redis/src/redis.ts index bc86bd31884..eec2ef3cba4 100644 --- a/packages/opentelemetry-plugin-redis/src/redis.ts +++ b/packages/opentelemetry-plugin-redis/src/redis.ts @@ -17,18 +17,12 @@ import { BasePlugin } from '@opentelemetry/core'; import * as redisTypes from 'redis'; import * as shimmer from 'shimmer'; -import { SpanKind, CanonicalCode } from '@opentelemetry/types'; -import { AttributeNames } from './enums'; -import { - RedisPluginStreamTypes, - RedisPluginClientTypes, - RedisCommand, -} from './types'; -import { EventEmitter } from 'events'; +import { getTracedCreateClient, getTracedCreateStreamTrace, getTracedInternalSendCommand } from './utils'; + export class RedisPlugin extends BasePlugin { static readonly COMPONENT = 'redis'; - readonly supportedVersions = ['^2.6.0']; // should be >= 2.6.0 + readonly supportedVersions = ['^2.6.0']; // equivalent to >= 2.6.0 <3 constructor(readonly moduleName: string) { super(); @@ -80,111 +74,23 @@ export class RedisPlugin extends BasePlugin { * Patch internal_send_command(...) to trace requests */ private _getPatchInternalSendCommand() { - const plugin = this; + const tracer = this._tracer return function internal_send_command(original: Function) { - return function internal_send_command_trace( - this: redisTypes.RedisClient & RedisPluginClientTypes, - cmd?: RedisCommand - ) { - const parentSpan = plugin._tracer.getCurrentSpan(); - - // New versions of redis (2.4+) use a single options object - // instead of named arguments - if (arguments.length === 1 && typeof cmd === 'object') { - const span = plugin._tracer.startSpan( - `${RedisPlugin.COMPONENT}-${cmd.command}`, - { - kind: SpanKind.CLIENT, - parent: parentSpan || undefined, - attributes: { - [AttributeNames.COMPONENT]: RedisPlugin.COMPONENT, - [AttributeNames.DB_STATEMENT]: cmd.command, - }, - } - ); - - // Set attributes for not explicitly typed RedisPluginClientTypes - if (this.options) { - span.setAttributes({ - [AttributeNames.PEER_HOSTNAME]: this.options.host, - [AttributeNames.PEER_PORT]: this.options.port, - }); - } - if (this.address) { - span.setAttribute( - AttributeNames.PEER_ADDRESS, - `redis://${this.address}` - ); - } - - let spanEnded = false; - const endSpan = (err?: Error | null) => { - if (spanEnded) return; // never - if (err) { - span.setStatus({ - code: CanonicalCode.UNKNOWN, - message: err.message, - }); - } else { - span.setStatus({ code: CanonicalCode.OK }); - } - span.end(); - spanEnded = true; - }; - - const originalCallback = arguments[0].callback; - if (originalCallback) { - (arguments[0] as RedisCommand).callback = function callback( - this: unknown, - err: Error | null, - _reply: T - ) { - endSpan(err); - return originalCallback.apply(this, arguments); - }; - } - try { - // Span will be ended in callback - return original.apply(this, arguments); - } catch (rethrow) { - endSpan(rethrow); - throw rethrow; // rethrow after ending span - } - } - - // We don't know how to trace this call, so don't start/stop a span - return original.apply(this, arguments); - }; + return getTracedInternalSendCommand(tracer, original); }; } private _getPatchCreateClient() { - const plugin = this; + const tracer = this._tracer; return function createClient(original: Function) { - return function createClientTrace(this: redisTypes.RedisClient) { - const client: redisTypes.RedisClient = original.apply(this, arguments); - return plugin._tracer.bind(client); - }; + return getTracedCreateClient(tracer, original); }; } private _getPatchCreateStream() { - const plugin = this; + const tracer = this._tracer; return function createReadStream(original: Function) { - return function create_stream_trace(this: RedisPluginStreamTypes) { - if (!this.stream) { - Object.defineProperty(this, 'stream', { - get() { - return this._patched_redis_stream; - }, - set(val: EventEmitter) { - plugin._tracer.bind(val); - this._patched_redis_stream = val; - }, - }); - } - return original.apply(this, arguments); - }; + return getTracedCreateStreamTrace(tracer, original); }; } } diff --git a/packages/opentelemetry-plugin-redis/src/utils.ts b/packages/opentelemetry-plugin-redis/src/utils.ts new file mode 100644 index 00000000000..4d59d826c74 --- /dev/null +++ b/packages/opentelemetry-plugin-redis/src/utils.ts @@ -0,0 +1,120 @@ +/*! + * Copyright 2019, 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 * as redisTypes from 'redis'; +import { Tracer, SpanKind, Span, CanonicalCode } from '@opentelemetry/types'; +import { RedisPluginStreamTypes, RedisPluginClientTypes, RedisCommand } from './types'; +import { EventEmitter } from 'events'; +import { RedisPlugin } from './redis'; +import { AttributeNames } from './enums'; + +const endSpan = (span: Span, err?: Error | null) => { + if (err) { + span.setStatus({ + code: CanonicalCode.UNKNOWN, + message: err.message, + }); + } else { + span.setStatus({ code: CanonicalCode.OK }); + } + span.end(); +}; + +export const getTracedCreateClient = (tracer: Tracer, original: Function) => { + return function createClientTrace(this: redisTypes.RedisClient) { + const client: redisTypes.RedisClient = original.apply(this, arguments); + return tracer.bind(client); + }; +} + +export const getTracedCreateStreamTrace = (tracer: Tracer, original: Function) => { + return function create_stream_trace(this: RedisPluginStreamTypes) { + if (!this.stream) { + Object.defineProperty(this, 'stream', { + get() { + return this._patched_redis_stream; + }, + set(val: EventEmitter) { + tracer.bind(val); + this._patched_redis_stream = val; + }, + }); + } + return original.apply(this, arguments); + }; +} + +export const getTracedInternalSendCommand = (tracer: Tracer, original: Function) => { + + return function internal_send_command_trace( + this: redisTypes.RedisClient & RedisPluginClientTypes, + cmd?: RedisCommand + ) { + const parentSpan = tracer.getCurrentSpan(); + + // New versions of redis (2.4+) use a single options object + // instead of named arguments + if (arguments.length === 1 && typeof cmd === 'object') { + const span = tracer.startSpan( + `${RedisPlugin.COMPONENT}-${cmd.command}`, + { + kind: SpanKind.CLIENT, + parent: parentSpan || undefined, + attributes: { + [AttributeNames.COMPONENT]: RedisPlugin.COMPONENT, + [AttributeNames.DB_STATEMENT]: cmd.command, + }, + } + ); + + // Set attributes for not explicitly typed RedisPluginClientTypes + if (this.options) { + span.setAttributes({ + [AttributeNames.PEER_HOSTNAME]: this.options.host, + [AttributeNames.PEER_PORT]: this.options.port, + }); + } + if (this.address) { + span.setAttribute( + AttributeNames.PEER_ADDRESS, + `redis://${this.address}` + ); + } + + const originalCallback = arguments[0].callback; + if (originalCallback) { + (arguments[0] as RedisCommand).callback = function callback( + this: unknown, + err: Error | null, + _reply: T + ) { + endSpan(span, err); + return originalCallback.apply(this, arguments); + }; + } + try { + // Span will be ended in callback + return original.apply(this, arguments); + } catch (rethrow) { + endSpan(span, rethrow); + throw rethrow; // rethrow after ending span + } + } + + // We don't know how to trace this call, so don't start/stop a span + return original.apply(this, arguments); + }; +} From 9386c42040f4531ebc28d114e7769c5a6c3282b5 Mon Sep 17 00:00:00 2001 From: Mark Wolff Date: Wed, 13 Nov 2019 17:46:15 -0800 Subject: [PATCH 10/10] fix: linting --- .../opentelemetry-plugin-redis/src/redis.ts | 9 ++-- .../opentelemetry-plugin-redis/src/utils.ts | 44 +++++++++++-------- 2 files changed, 31 insertions(+), 22 deletions(-) diff --git a/packages/opentelemetry-plugin-redis/src/redis.ts b/packages/opentelemetry-plugin-redis/src/redis.ts index eec2ef3cba4..5d4cb833f89 100644 --- a/packages/opentelemetry-plugin-redis/src/redis.ts +++ b/packages/opentelemetry-plugin-redis/src/redis.ts @@ -17,8 +17,11 @@ import { BasePlugin } from '@opentelemetry/core'; import * as redisTypes from 'redis'; import * as shimmer from 'shimmer'; -import { getTracedCreateClient, getTracedCreateStreamTrace, getTracedInternalSendCommand } from './utils'; - +import { + getTracedCreateClient, + getTracedCreateStreamTrace, + getTracedInternalSendCommand, +} from './utils'; export class RedisPlugin extends BasePlugin { static readonly COMPONENT = 'redis'; @@ -74,7 +77,7 @@ export class RedisPlugin extends BasePlugin { * Patch internal_send_command(...) to trace requests */ private _getPatchInternalSendCommand() { - const tracer = this._tracer + const tracer = this._tracer; return function internal_send_command(original: Function) { return getTracedInternalSendCommand(tracer, original); }; diff --git a/packages/opentelemetry-plugin-redis/src/utils.ts b/packages/opentelemetry-plugin-redis/src/utils.ts index 4d59d826c74..0be83cf6446 100644 --- a/packages/opentelemetry-plugin-redis/src/utils.ts +++ b/packages/opentelemetry-plugin-redis/src/utils.ts @@ -14,9 +14,13 @@ * limitations under the License. */ -import * as redisTypes from 'redis'; +import * as redisTypes from 'redis'; import { Tracer, SpanKind, Span, CanonicalCode } from '@opentelemetry/types'; -import { RedisPluginStreamTypes, RedisPluginClientTypes, RedisCommand } from './types'; +import { + RedisPluginStreamTypes, + RedisPluginClientTypes, + RedisCommand, +} from './types'; import { EventEmitter } from 'events'; import { RedisPlugin } from './redis'; import { AttributeNames } from './enums'; @@ -38,9 +42,12 @@ export const getTracedCreateClient = (tracer: Tracer, original: Function) => { const client: redisTypes.RedisClient = original.apply(this, arguments); return tracer.bind(client); }; -} +}; -export const getTracedCreateStreamTrace = (tracer: Tracer, original: Function) => { +export const getTracedCreateStreamTrace = ( + tracer: Tracer, + original: Function +) => { return function create_stream_trace(this: RedisPluginStreamTypes) { if (!this.stream) { Object.defineProperty(this, 'stream', { @@ -55,10 +62,12 @@ export const getTracedCreateStreamTrace = (tracer: Tracer, original: Function) = } return original.apply(this, arguments); }; -} - -export const getTracedInternalSendCommand = (tracer: Tracer, original: Function) => { +}; +export const getTracedInternalSendCommand = ( + tracer: Tracer, + original: Function +) => { return function internal_send_command_trace( this: redisTypes.RedisClient & RedisPluginClientTypes, cmd?: RedisCommand @@ -68,17 +77,14 @@ export const getTracedInternalSendCommand = (tracer: Tracer, original: Function) // New versions of redis (2.4+) use a single options object // instead of named arguments if (arguments.length === 1 && typeof cmd === 'object') { - const span = tracer.startSpan( - `${RedisPlugin.COMPONENT}-${cmd.command}`, - { - kind: SpanKind.CLIENT, - parent: parentSpan || undefined, - attributes: { - [AttributeNames.COMPONENT]: RedisPlugin.COMPONENT, - [AttributeNames.DB_STATEMENT]: cmd.command, - }, - } - ); + const span = tracer.startSpan(`${RedisPlugin.COMPONENT}-${cmd.command}`, { + kind: SpanKind.CLIENT, + parent: parentSpan || undefined, + attributes: { + [AttributeNames.COMPONENT]: RedisPlugin.COMPONENT, + [AttributeNames.DB_STATEMENT]: cmd.command, + }, + }); // Set attributes for not explicitly typed RedisPluginClientTypes if (this.options) { @@ -117,4 +123,4 @@ export const getTracedInternalSendCommand = (tracer: Tracer, original: Function) // We don't know how to trace this call, so don't start/stop a span return original.apply(this, arguments); }; -} +};