Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(plugin): implement redis plugin #503

Merged
merged 16 commits into from
Nov 19, 2019
Merged
18 changes: 13 additions & 5 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
@@ -1,17 +1,22 @@
version: 2

test_env: &test_env
node_test_env: &node_test_env
RUN_POSTGRES_TESTS: 1
POSTGRES_USER: postgres
POSTGRES_DB: circle_database
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
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
image: redis

node_unit_tests: &node_unit_tests
steps:
Expand Down Expand Up @@ -99,20 +104,23 @@ 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
node12-browsers:
docker:
Expand Down
29 changes: 26 additions & 3 deletions packages/opentelemetry-plugin-redis/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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();
```

<!-- See [examples/redis](https://github.com/open-telemetry/opentelemetry-js/tree/master/examples/redis) for a short example. -->

## Useful links
- For more information on OpenTelemetry, visit: <https://opentelemetry.io/>
- For more about OpenTelemetry JavaScript: <https://github.com/open-telemetry/opentelemetry-js>
Expand Down
14 changes: 11 additions & 3 deletions packages/opentelemetry-plugin-redis/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,19 @@
"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",
"scripts": {
"test": "nyc ts-mocha -p tsconfig.json 'test/**/*.ts'",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we should remove "private": true to make this package available for the next release?

Copy link
Member Author

@markwolff markwolff Nov 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. I removed it from this plugin. I think we should also remove it from other "ready" plugins (separate PR). Seems to just be postgres and http2 currently

"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/*",
"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"
},
Expand Down Expand Up @@ -42,10 +44,16 @@
"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",
"cross-env": "^6.0.3",
"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",
Expand All @@ -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"
}
}
32 changes: 32 additions & 0 deletions packages/opentelemetry-plugin-redis/src/enums.ts
Original file line number Diff line number Diff line change
@@ -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',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should move these common attributes to either core or base package, this is being used in almost all the plugins. WDYT?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM. I'll create an issue for it.

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',
}
2 changes: 2 additions & 0 deletions packages/opentelemetry-plugin-redis/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,5 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/

export * from './redis';
192 changes: 192 additions & 0 deletions packages/opentelemetry-plugin-redis/src/redis.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,192 @@
/*!
* 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';
import {
RedisPluginStreamTypes,
RedisPluginClientTypes,
RedisCommand,
} from './types';
import { EventEmitter } from 'events';

export class RedisPlugin extends BasePlugin<typeof redisTypes> {
static readonly COMPONENT = 'redis';
readonly supportedVersions = ['^2.6.0']; // should be >= 2.6.0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please include supported version in README.md?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 391326b


constructor(readonly moduleName: string) {
super();
}

protected patch() {
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;
}

protected unpatch(): void {
if (this._moduleExports) {
shimmer.unwrap(
this._moduleExports.RedisClient.prototype,
'internal_send_command'
);
shimmer.unwrap(
this._moduleExports.RedisClient.prototype,
'create_stream'
);
shimmer.unwrap(this._moduleExports, 'createClient');
}
}

/**
* 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(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to move this function out of the class and pass in plugin as an argument (would need to wrap with an arrow function)? That could reducer the indentation of it

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this what you had in mind? 8654a71

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) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be extracted as a helper function at the top level of the module?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. I moved it out. I had to get rid of the spanEnded check, but if the span is being ended more than once, its probably a bug that we should log out anyways.

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<T>(
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);
};
};
}
}

export const plugin = new RedisPlugin(RedisPlugin.COMPONENT);
Loading