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

feature(plugin): implement postgres plugin #417

Merged
merged 25 commits into from
Oct 30, 2019
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
0212dea
feat(pg): implement postgres plugin
Oct 9, 2019
62ecb39
fix: linting
Oct 9, 2019
f8d885b
fix: docker starting not locally
Oct 9, 2019
3ed0cf3
Merge branch 'master' into markwolff/impl-pg-plugin
Oct 9, 2019
9b90fde
fix: compile errors from merge
Oct 9, 2019
c14b33f
fix: linting
Oct 9, 2019
dd1a7d3
refactor: use helper functions for span building
Oct 15, 2019
7a854fe
fix: add callback patching to end span
Oct 16, 2019
eafdb3d
fix: add required attributes, address comments
Oct 18, 2019
7504ba7
fix: lint errors
Oct 18, 2019
9316ff5
Merge branch 'master' into markwolff/impl-pg-plugin
Oct 18, 2019
bfb19d3
refactor: start named spans in query handlers
Oct 21, 2019
64a2a8b
fix: linting errors
Oct 21, 2019
10fd6ca
Merge branch 'master' into markwolff/impl-pg-plugin
Oct 21, 2019
d81e4b0
fix: circleci config, make pg helpers nonexported
Oct 21, 2019
297e699
Merge branch 'markwolff/impl-pg-plugin' of github.com:markwolff/opent…
Oct 21, 2019
208d3a9
fix: linting
Oct 21, 2019
f986476
docs: add supported versions
Oct 21, 2019
644f5f1
fix: pass PG env to spawned container
Oct 21, 2019
b3a83b4
fix: remove hardcoded shouldTest
markwolff Oct 23, 2019
ccefe01
Merge branch 'master' into markwolff/impl-pg-plugin
mayurkale22 Oct 23, 2019
fefb04e
Merge branch 'master' into markwolff/impl-pg-plugin
mayurkale22 Oct 28, 2019
4e24474
test: add span tests for pg driver errors
markwolff Oct 28, 2019
0279784
chore: remove hardcode shouldTest
markwolff Oct 29, 2019
bbdc02d
Merge branch 'master' into markwolff/impl-pg-plugin
mayurkale22 Oct 29, 2019
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
version: 2

test_env: &test_env
TEST_POSTGRES: 1
Copy link
Member

Choose a reason for hiding this comment

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

This can be more descriptive, something like RUN_POSTGRES_TESTS WDYT?


postgres_service: &postgres_service
image: postgres:alpine

node_unit_tests: &node_unit_tests
steps:
- checkout
Expand Down Expand Up @@ -71,18 +77,26 @@ jobs:
node8:
docker:
- image: node:8
environment: *test_env
- *postgres_service
<<: *node_unit_tests
node10:
docker:
- image: node:10
environment: *test_env
- *postgres_service
<<: *node_unit_tests
node11:
docker:
- image: node:11
environment: *test_env
- *postgres_service
<<: *node_unit_tests
node12:
docker:
- image: node:12
environment: *test_env
- *postgres_service
<<: *node_unit_tests
node12-browsers:
docker:
Expand Down
12 changes: 9 additions & 3 deletions packages/opentelemetry-plugin-postgres/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
"types": "build/src/index.d.ts",
"repository": "open-telemetry/opentelemetry-js",
"scripts": {
"test": "nyc ts-mocha -p tsconfig.json 'test/**/*.ts'",
"test": "nyc ts-mocha -p tsconfig.json 'test/**/*.test.ts'",
"test:debug": "ts-mocha --inspect-brk --no-timeouts -p tsconfig.json 'test/**/*.test.ts'",
"tdd": "yarn test -- --watch-extensions ts --watch",
"clean": "rimraf build/*",
"check": "gts check",
Expand Down Expand Up @@ -43,11 +44,14 @@
"devDependencies": {
"@types/mocha": "^5.2.7",
"@types/node": "^12.6.9",
"@types/pg": "^7.11.2",
"@types/shimmer": "^1.0.1",
"codecov": "^3.5.0",
"gts": "^1.1.0",
"gts": "^1.0.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

Why use the earlier version of gts?

Copy link
Member

Choose a reason for hiding this comment

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

AFAIK most of the places we are using 1.0.0, (I think) this change is to make sure we are consistent everywhere. We can update the latest version (1.1.0) in separate PR.

"mocha": "^6.2.0",
"nyc": "^14.1.1",
"rimraf": "^3.0.0",
"pg": "^7.12.1",
"tslint-microsoft-contrib": "^6.2.0",
"tslint-consistent-codestyle": "^1.15.1",
"ts-mocha": "^6.0.0",
Expand All @@ -57,6 +61,8 @@
"dependencies": {
"@opentelemetry/core": "^0.1.0",
"@opentelemetry/node": "^0.1.0",
"@opentelemetry/types": "^0.1.0"
"@opentelemetry/tracing": "^0.1.0",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"@opentelemetry/tracing": "^0.1.0",
"@opentelemetry/tracing": "^0.1.1",

"@opentelemetry/types": "^0.1.0",
"shimmer": "^1.2.1"
}
}
36 changes: 36 additions & 0 deletions packages/opentelemetry-plugin-postgres/src/enums.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*!
* 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
Contributor

Choose a reason for hiding this comment

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

Is this a standard label in the spec?

Copy link
Member

Choose a reason for hiding this comment

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

https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/data-semantic-conventions.md#databases-client-calls

Looks like the spec calls for:

Attribute name Notes and examples Required?
component Database driver name or database name (when known) "JDBI", "jdbc", "odbc", "postgreSQL". Yes
db.type Database type. For any SQL database, "sql". For others, the lower-case database category, e.g. "cassandra", "hbase", or "redis". Yes
db.instance Database instance name. E.g., In java, if the jdbc.url="jdbc:mysql://db.example.com:3306/customers", the instance name is "customers". Yes
db.statement A database statement for the given database type. Note, that the value may be sanitized to exclude sensitive information. E.g., for db.type="sql", "SELECT * FROM wuser_table"; for db.type="redis", "SET mykey 'WuValue'". Yes
db.user Username for accessing database. E.g., "readonly_user" or "reporting_user" No

For database client calls, peer information can be populated and interpreted as
follows:

Attribute name Notes and examples Required
peer.address JDBC substring like "mysql://db.example.com:3306" Yes
peer.hostname Remote hostname. "db.example.com" Yes
peer.ipv4 Remote IPv4 address as a .-separated tuple. E.g., "127.0.0.1" No
peer.ipv6 Remote IPv6 address as a string of colon-separated 4-char hex tuples. E.g., "2001:0db8:85a3:0000:0000:8a2e:0370:7334" No
peer.port Remote port. E.g., 80 (integer) No
peer.service Remote service name. Can be database friendly name or "db.instance" No

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',

// PG specific -- not specified by spec
PG_VALUES = 'pg.values',
PG_PLAN = 'pg.plan',
}
2 changes: 2 additions & 0 deletions packages/opentelemetry-plugin-postgres/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 './pg';
178 changes: 178 additions & 0 deletions packages/opentelemetry-plugin-postgres/src/pg.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,178 @@
/*!
* 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 { SpanKind, CanonicalCode } from '@opentelemetry/types';
import { AttributeNames } from './enums';
import {
PostgresPluginOptions,
PgClientExtended,
PgPluginQueryConfig,
PostgresCallback,
} from './types';
import * as pgTypes from 'pg';
import * as shimmer from 'shimmer';
import * as utils from './utils';

export class PostgresPlugin extends BasePlugin<typeof pgTypes> {
protected _config: PostgresPluginOptions;

static readonly COMPONENT = 'pg';
static readonly DB_TYPE = 'sql';

static readonly BASE_SPAN_NAME = PostgresPlugin.COMPONENT + '.query';

readonly supportedVersions = ['^7.12.1'];

constructor(readonly moduleName: string) {
super();
this._config = {};
}

protected patch(): typeof pgTypes {
if (this._moduleExports.Client.prototype.query) {
shimmer.wrap(
this._moduleExports.Client.prototype,
'query',
this._getClientQueryPatch() as never
);
}
return this._moduleExports;
}

protected unpatch(): void {
if (this._moduleExports.Client.prototype.query) {
shimmer.unwrap(this._moduleExports.Client.prototype, 'query');
}
}

private _getClientQueryPatch() {
const plugin = this;
return (original: typeof pgTypes.Client.prototype.query) => {
plugin._logger.debug(
`Patching ${PostgresPlugin.COMPONENT}.Client.prototype.query`
);
return function query(
this: pgTypes.Client & PgClientExtended,
...args: unknown[]
) {
let callbackProvided = false;
const span = plugin._pgStartSpan(this);

// Handle different client.query(...) signatures
if (typeof args[0] === 'string') {
if (args.length > 1 && args[1] instanceof Array) {
utils._handleParameterizedQuery.call(this, span, ...args);
markwolff marked this conversation as resolved.
Show resolved Hide resolved
} else {
utils._handleTextQuery.call(this, span, ...args);
}
} else if (typeof args[0] === 'object') {
utils._handleConfigQuery.call(this, span, ...args);
}

// Bind callback to parent span
if (args.length > 0) {
const parentSpan = plugin._tracer.getCurrentSpan();
if (typeof args[args.length - 1] === 'function') {
// Patch ParameterQuery callback
args[args.length - 1] = utils._patchCallback(span, args[
args.length - 1
] as PostgresCallback);
// If a parent span exists, bind the callback
if (parentSpan) {
args[args.length - 1] = plugin._tracer.bind(
args[args.length - 1]
);
}
callbackProvided = true;
} else if (
typeof (args[0] as PgPluginQueryConfig).callback === 'function'
) {
// Patch ConfigQuery callback
let callback = utils._patchCallback(
span,
(args[0] as PgPluginQueryConfig).callback!
);
// If a parent span existed, bind the callback
if (parentSpan) {
callback = plugin._tracer.bind(callback);
}

// Copy the callback instead of writing to args.callback so that we don't modify user's
// original callback reference
args[0] = { ...args[0], callback };
callbackProvided = true;
}
}

// Perform the original query
const result: unknown = original.apply(this, args as never);

// Bind promise to parent span and end the span
if (result instanceof Promise) {
return result
.then((result: unknown) => {
// Return a pass-along promise which ends the span and then goes to user's orig resolvers
return new Promise((resolve, _) => {
span.setStatus({ code: CanonicalCode.OK });
span.end();
resolve(result);
});
})
.catch((error: Error) => {
return new Promise((_, reject) => {
span.setStatus({ code: CanonicalCode.UNKNOWN });
Copy link
Member

Choose a reason for hiding this comment

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

Should we set something like span.setStatus({ code: CanonicalCode.UNKNOWN, message: error.message });

span.end();
reject(error);
});
});
}

// If a promise was not returned and no callback is provided, we recieved invalid args
if (!callbackProvided) {
span.setStatus({
code: CanonicalCode.INVALID_ARGUMENT,
message: 'Invalid query provided to the driver',
});
span.end();
}

// else returns void
return result; // void
};
};
}

// Private helper function to start a span
private _pgStartSpan(client: pgTypes.Client & PgClientExtended) {
const jdbcString = utils._getJDBCString(client.connectionParameters);
return this._tracer.startSpan(PostgresPlugin.BASE_SPAN_NAME, {
kind: SpanKind.CLIENT,
parent: this._tracer.getCurrentSpan() || undefined,
attributes: {
[AttributeNames.COMPONENT]: PostgresPlugin.COMPONENT, // required
[AttributeNames.DB_INSTANCE]: client.connectionParameters.database, // required
[AttributeNames.DB_TYPE]: PostgresPlugin.DB_TYPE, // required
[AttributeNames.PEER_ADDRESS]: jdbcString, // required
[AttributeNames.PEER_HOSTNAME]: client.connectionParameters.host, // required
[AttributeNames.PEER_PORT]: client.connectionParameters.port,
[AttributeNames.DB_USER]: client.connectionParameters.user,
},
});
}
}

export const plugin = new PostgresPlugin(PostgresPlugin.COMPONENT);
38 changes: 38 additions & 0 deletions packages/opentelemetry-plugin-postgres/src/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*!
* 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 pgTypes from 'pg';

export interface PostgresPluginOptions {}

export type PostgresCallback = (err: Error, res: object) => unknown;

// These are not included in @types/pg, so manually define them.
// https://github.com/brianc/node-postgres/blob/fde5ec586e49258dfc4a2fcd861fcdecb4794fc3/lib/client.js#L25
export interface PgClientConnectionParams {
database: string;
host: string;
port: number;
user: string;
}

export interface PgClientExtended {
connectionParameters: PgClientConnectionParams;
}

export interface PgPluginQueryConfig extends pgTypes.QueryConfig {
callback?: PostgresCallback;
}
Loading