From f0dc26ddd00e93d4567c92b9e5f75422e73b374a Mon Sep 17 00:00:00 2001 From: Ryan Eberhardt Date: Fri, 16 Sep 2022 16:04:34 -0700 Subject: [PATCH 1/7] feat(pg): add requireParentTrace option --- .../src/instrumentation.ts | 57 +++++++++--------- .../src/types.ts | 3 + .../src/utils.ts | 58 ++++++++++++++----- .../test/pg.test.ts | 29 ++++++++++ 4 files changed, 102 insertions(+), 45 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts index c5bbad195e..c267065548 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts @@ -24,7 +24,6 @@ import { diag, trace, Span, - SpanKind, SpanStatusCode, } from '@opentelemetry/api'; import * as pgTypes from 'pg'; @@ -46,6 +45,7 @@ import { DbSystemValues, } from '@opentelemetry/semantic-conventions'; import { VERSION } from './version'; +import { startSpan } from './utils'; const PG_POOL_COMPONENT = 'pg-pool'; @@ -129,21 +129,15 @@ export class PgInstrumentation extends InstrumentationBase { this: pgTypes.Client, callback?: PgErrorCallback ) { - const span = plugin.tracer.startSpan( - `${PgInstrumentation.COMPONENT}.connect`, - { - kind: SpanKind.CLIENT, - attributes: { - [SemanticAttributes.DB_SYSTEM]: DbSystemValues.POSTGRESQL, - [SemanticAttributes.DB_NAME]: this.database, - [SemanticAttributes.NET_PEER_NAME]: this.host, - [SemanticAttributes.DB_CONNECTION_STRING]: - utils.getConnectionString(this), - [SemanticAttributes.NET_PEER_PORT]: this.port, - [SemanticAttributes.DB_USER]: this.user, - }, - } - ); + const span = startSpan(plugin.tracer, plugin.getConfig(), `${PgInstrumentation.COMPONENT}.connect`, { + [SemanticAttributes.DB_SYSTEM]: DbSystemValues.POSTGRESQL, + [SemanticAttributes.DB_NAME]: this.database, + [SemanticAttributes.NET_PEER_NAME]: this.host, + [SemanticAttributes.DB_CONNECTION_STRING]: + utils.getConnectionString(this), + [SemanticAttributes.NET_PEER_PORT]: this.port, + [SemanticAttributes.DB_USER]: this.user, + }); if (callback) { const parentSpan = trace.getSpan(context.active()); @@ -187,7 +181,12 @@ export class PgInstrumentation extends InstrumentationBase { params ); } else { - span = utils.handleTextQuery.call(this, plugin.tracer, query); + span = utils.handleTextQuery.call( + this, + plugin.tracer, + plugin.getConfig() as PgInstrumentationConfig, + query + ); } } else if (typeof args[0] === 'object') { const queryConfig = args[0] as NormalizedQueryConfig; @@ -201,6 +200,7 @@ export class PgInstrumentation extends InstrumentationBase { return utils.handleInvalidQuery.call( this, plugin.tracer, + plugin.getConfig() as PgInstrumentationConfig, original, ...args ); @@ -285,19 +285,16 @@ export class PgInstrumentation extends InstrumentationBase { return function connect(this: PgPoolExtended, callback?: PgPoolCallback) { const connString = utils.getConnectionString(this.options); // setup span - const span = plugin.tracer.startSpan(`${PG_POOL_COMPONENT}.connect`, { - kind: SpanKind.CLIENT, - attributes: { - [SemanticAttributes.DB_SYSTEM]: DbSystemValues.POSTGRESQL, - [SemanticAttributes.DB_NAME]: this.options.database, // required - [SemanticAttributes.NET_PEER_NAME]: this.options.host, // required - [SemanticAttributes.DB_CONNECTION_STRING]: connString, // required - [SemanticAttributes.NET_PEER_PORT]: this.options.port, - [SemanticAttributes.DB_USER]: this.options.user, - [AttributeNames.IDLE_TIMEOUT_MILLIS]: - this.options.idleTimeoutMillis, - [AttributeNames.MAX_CLIENT]: this.options.maxClient, - }, + const span = startSpan(plugin.tracer, plugin.getConfig(), `${PG_POOL_COMPONENT}.connect`, { + [SemanticAttributes.DB_SYSTEM]: DbSystemValues.POSTGRESQL, + [SemanticAttributes.DB_NAME]: this.options.database, // required + [SemanticAttributes.NET_PEER_NAME]: this.options.host, // required + [SemanticAttributes.DB_CONNECTION_STRING]: connString, // required + [SemanticAttributes.NET_PEER_PORT]: this.options.port, + [SemanticAttributes.DB_USER]: this.options.user, + [AttributeNames.IDLE_TIMEOUT_MILLIS]: + this.options.idleTimeoutMillis, + [AttributeNames.MAX_CLIENT]: this.options.maxClient, }); if (callback) { diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/types.ts b/plugins/node/opentelemetry-instrumentation-pg/src/types.ts index 19cdef646a..f2d8b3bdd3 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/types.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/types.ts @@ -40,6 +40,9 @@ export interface PgInstrumentationConfig extends InstrumentationConfig { * @default undefined */ responseHook?: PgInstrumentationExecutionResponseHook; + + /** Require that is a parent span to create new spans. Defaults to false. */ + requireParentSpan?: boolean; } export type PostgresCallback = (err: Error, res: object) => unknown; diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts index 86b060dd26..0c4a8ffa1a 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts @@ -15,11 +15,15 @@ */ import { + context, + trace, Span, SpanStatusCode, Tracer, SpanKind, diag, + INVALID_SPAN_CONTEXT, + Attributes, } from '@opentelemetry/api'; import { AttributeNames } from './enums/AttributeNames'; import { @@ -58,19 +62,41 @@ export function getConnectionString(params: PgClientConnectionParams) { return `postgresql://${host}:${port}/${database}`; } -// Private helper function to start a span -function pgStartSpan(tracer: Tracer, client: PgClientExtended, name: string) { - const jdbcString = getConnectionString(client.connectionParameters); +export function startSpan( + tracer: Tracer, + instrumentationConfig: PgInstrumentationConfig, + name: string, + attributes: Attributes, +): Span { + // If a parent span is required but not present, use a noop span to propagate + // context without recording it. Adapted from opentelemetry-instrumentation-http: + // https://github.com/open-telemetry/opentelemetry-js/blob/597ea98e58a4f68bcd9aec5fd283852efe444cd6/experimental/packages/opentelemetry-instrumentation-http/src/http.ts#L660 + const currentSpan = trace.getSpan(context.active()); + if (instrumentationConfig.requireParentSpan && currentSpan === undefined) { + return trace.wrapSpanContext(INVALID_SPAN_CONTEXT); + } + return tracer.startSpan(name, { kind: SpanKind.CLIENT, - attributes: { - [SemanticAttributes.DB_NAME]: client.connectionParameters.database, // required - [SemanticAttributes.DB_SYSTEM]: DbSystemValues.POSTGRESQL, // required - [SemanticAttributes.DB_CONNECTION_STRING]: jdbcString, // required - [SemanticAttributes.NET_PEER_NAME]: client.connectionParameters.host, // required - [SemanticAttributes.NET_PEER_PORT]: client.connectionParameters.port, - [SemanticAttributes.DB_USER]: client.connectionParameters.user, - }, + attributes, + }); +} + +// Private helper function to start a span +function pgStartSpan( + client: PgClientExtended, + tracer: Tracer, + instrumentationConfig: PgInstrumentationConfig, + name: string, +) { + const jdbcString = getConnectionString(client.connectionParameters); + return startSpan(tracer, instrumentationConfig, name, { + [SemanticAttributes.DB_NAME]: client.connectionParameters.database, // required + [SemanticAttributes.DB_SYSTEM]: DbSystemValues.POSTGRESQL, // required + [SemanticAttributes.DB_CONNECTION_STRING]: jdbcString, // required + [SemanticAttributes.NET_PEER_NAME]: client.connectionParameters.host, // required + [SemanticAttributes.NET_PEER_PORT]: client.connectionParameters.port, + [SemanticAttributes.DB_USER]: client.connectionParameters.user, }); } @@ -84,7 +110,7 @@ export function handleConfigQuery( // Set child span name const queryCommand = getCommandFromText(queryConfig.name || queryConfig.text); const name = PgInstrumentation.BASE_SPAN_NAME + ':' + queryCommand; - const span = pgStartSpan(tracer, this, name); + const span = pgStartSpan(this, tracer, instrumentationConfig, name); // Set attributes if (queryConfig.text) { @@ -118,7 +144,7 @@ export function handleParameterizedQuery( // Set child span name const queryCommand = getCommandFromText(query); const name = PgInstrumentation.BASE_SPAN_NAME + ':' + queryCommand; - const span = pgStartSpan(tracer, this, name); + const span = pgStartSpan(this, tracer, instrumentationConfig, name); // Set attributes span.setAttribute(SemanticAttributes.DB_STATEMENT, query); @@ -133,12 +159,13 @@ export function handleParameterizedQuery( export function handleTextQuery( this: PgClientExtended, tracer: Tracer, + instrumentationConfig: PgInstrumentationConfig, query: string ) { // Set child span name const queryCommand = getCommandFromText(query); const name = PgInstrumentation.BASE_SPAN_NAME + ':' + queryCommand; - const span = pgStartSpan(tracer, this, name); + const span = pgStartSpan(this, tracer, instrumentationConfig, name); // Set attributes span.setAttribute(SemanticAttributes.DB_STATEMENT, query); @@ -153,11 +180,12 @@ export function handleTextQuery( export function handleInvalidQuery( this: PgClientExtended, tracer: Tracer, + instrumentationConfig: PgInstrumentationConfig, originalQuery: typeof pgTypes.Client.prototype.query, ...args: unknown[] ) { let result; - const span = pgStartSpan(tracer, this, PgInstrumentation.BASE_SPAN_NAME); + const span = pgStartSpan(this, tracer, instrumentationConfig, PgInstrumentation.BASE_SPAN_NAME); try { result = originalQuery.apply(this, args as never); } catch (e) { diff --git a/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts b/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts index d966eda8d8..6a430d7a94 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts @@ -640,4 +640,33 @@ describe('pg', () => { }); }); }); + + describe('Instrumentation with requireParentSpan', () => { + beforeEach(() => { + const config: PgInstrumentationConfig = { + requireParentSpan: true, + }; + instrumentation.setConfig(config); + memoryExporter.reset(); + }); + + it('should not generate traces for connect() when requireParentSpan=true', async () => { + const connClient = new postgres.Client(CONFIG); + await connClient.connect(); + const spans = memoryExporter.getFinishedSpans(); + assert.strictEqual(spans.length, 0); + await connClient.end(); + }); + + it('should not generate traces for client.query(text, callback) when requireParentSpan=true', done => { + client.query('SELECT NOW()', (err, res) => { + assert.strictEqual(err, null); + assert.ok(res); + const spans = memoryExporter.getFinishedSpans(); + console.log(spans); + assert.strictEqual(spans.length, 0); + done(); + }); + }); + }); }); From 5dd5036e83d89ec812a9ff5c79a3bb8305d6e678 Mon Sep 17 00:00:00 2001 From: Ryan Eberhardt Date: Thu, 29 Sep 2022 01:25:52 -0700 Subject: [PATCH 2/7] Remove accidental console.log --- plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts b/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts index 6a430d7a94..34235ae633 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts @@ -663,7 +663,6 @@ describe('pg', () => { assert.strictEqual(err, null); assert.ok(res); const spans = memoryExporter.getFinishedSpans(); - console.log(spans); assert.strictEqual(spans.length, 0); done(); }); From caa66b683fcfce087dd2fa6b6c4bb36ef913f17a Mon Sep 17 00:00:00 2001 From: Ryan Eberhardt Date: Thu, 29 Sep 2022 01:27:44 -0700 Subject: [PATCH 3/7] Fix up config type casts --- .../src/instrumentation.ts | 22 +++++++++++++------ .../test/pg-pool.test.ts | 6 ++--- .../test/pg.test.ts | 10 ++++----- 3 files changed, 21 insertions(+), 17 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts index c267065548..bc84f5bfda 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts @@ -122,6 +122,14 @@ export class PgInstrumentation extends InstrumentationBase { return [modulePG, modulePGPool]; } + override setConfig(config: PgInstrumentationConfig = {}) { + this._config = Object.assign({}, config); + } + + override getConfig(): PgInstrumentationConfig { + return this._config as PgInstrumentationConfig; + } + private _getClientConnectPatch() { const plugin = this; return (original: PgClientConnect) => { @@ -176,7 +184,7 @@ export class PgInstrumentation extends InstrumentationBase { span = utils.handleParameterizedQuery.call( this, plugin.tracer, - plugin.getConfig() as PgInstrumentationConfig, + plugin.getConfig(), query, params ); @@ -184,7 +192,7 @@ export class PgInstrumentation extends InstrumentationBase { span = utils.handleTextQuery.call( this, plugin.tracer, - plugin.getConfig() as PgInstrumentationConfig, + plugin.getConfig(), query ); } @@ -193,14 +201,14 @@ export class PgInstrumentation extends InstrumentationBase { span = utils.handleConfigQuery.call( this, plugin.tracer, - plugin.getConfig() as PgInstrumentationConfig, + plugin.getConfig(), queryConfig ); } else { return utils.handleInvalidQuery.call( this, plugin.tracer, - plugin.getConfig() as PgInstrumentationConfig, + plugin.getConfig(), original, ...args ); @@ -212,7 +220,7 @@ export class PgInstrumentation extends InstrumentationBase { if (typeof args[args.length - 1] === 'function') { // Patch ParameterQuery callback args[args.length - 1] = utils.patchCallback( - plugin.getConfig() as PgInstrumentationConfig, + plugin.getConfig(), span, args[args.length - 1] as PostgresCallback ); @@ -228,7 +236,7 @@ export class PgInstrumentation extends InstrumentationBase { ) { // Patch ConfigQuery callback let callback = utils.patchCallback( - plugin.getConfig() as PgInstrumentationConfig, + plugin.getConfig(), span, (args[0] as NormalizedQueryConfig).callback! ); @@ -253,7 +261,7 @@ export class PgInstrumentation extends InstrumentationBase { // Return a pass-along promise which ends the span and then goes to user's orig resolvers return new Promise(resolve => { utils.handleExecutionResult( - plugin.getConfig() as PgInstrumentationConfig, + plugin.getConfig(), span, result ); diff --git a/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts b/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts index 7cb29e87d6..84b3a3ed7e 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts @@ -340,7 +340,7 @@ describe('pg-pool', () => { }; beforeEach(async () => { - const config: PgInstrumentationConfig = { + create({ enhancedDatabaseReporting: true, responseHook: ( span: Span, @@ -350,9 +350,7 @@ describe('pg-pool', () => { dataAttributeName, JSON.stringify({ rowCount: responseInfo?.data.rowCount }) ), - }; - - create(config); + }); }); it('should attach response hook data to resulting spans for query with callback ', done => { diff --git a/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts b/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts index 34235ae633..e2977319fa 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts @@ -482,7 +482,7 @@ describe('pg', () => { [dataAttributeName]: '{"rowCount":1}', }; beforeEach(async () => { - const config: PgInstrumentationConfig = { + create({ enhancedDatabaseReporting: true, responseHook: ( span: Span, @@ -492,8 +492,7 @@ describe('pg', () => { dataAttributeName, JSON.stringify({ rowCount: responseInfo?.data.rowCount }) ), - }; - create(config); + }); }); it('should attach response hook data to resulting spans for query with callback ', done => { @@ -643,10 +642,9 @@ describe('pg', () => { describe('Instrumentation with requireParentSpan', () => { beforeEach(() => { - const config: PgInstrumentationConfig = { + instrumentation.setConfig({ requireParentSpan: true, - }; - instrumentation.setConfig(config); + }); memoryExporter.reset(); }); From 700baa0c42b084a5ee580f78c32c76e90e158507 Mon Sep 17 00:00:00 2001 From: Ryan Eberhardt Date: Thu, 29 Sep 2022 01:46:14 -0700 Subject: [PATCH 4/7] Split tests into corresponding blocks --- .../test/pg.test.ts | 25 ++++++++----------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts b/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts index e2977319fa..d43eb7537f 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts @@ -279,6 +279,16 @@ describe('pg', () => { testUtils.assertPropagation(connectSpan, span); }); }); + + it('should not generate traces when requireParentSpan=true is specified', async () => { + instrumentation.setConfig({ + requireParentSpan: true, + }); + memoryExporter.reset(); + await connClient.connect(); + const spans = memoryExporter.getFinishedSpans(); + assert.strictEqual(spans.length, 0); + }); }); describe('#client.query(...)', () => { @@ -638,25 +648,12 @@ describe('pg', () => { client.query('SELECT NOW()').then(queryHandler); }); }); - }); - describe('Instrumentation with requireParentSpan', () => { - beforeEach(() => { + it('should not generate traces for client.query() when requireParentSpan=true is specified', done => { instrumentation.setConfig({ requireParentSpan: true, }); memoryExporter.reset(); - }); - - it('should not generate traces for connect() when requireParentSpan=true', async () => { - const connClient = new postgres.Client(CONFIG); - await connClient.connect(); - const spans = memoryExporter.getFinishedSpans(); - assert.strictEqual(spans.length, 0); - await connClient.end(); - }); - - it('should not generate traces for client.query(text, callback) when requireParentSpan=true', done => { client.query('SELECT NOW()', (err, res) => { assert.strictEqual(err, null); assert.ok(res); From 82fcd51b73b752417000a0dd12a9df8ca451f90d Mon Sep 17 00:00:00 2001 From: Ryan Eberhardt Date: Thu, 29 Sep 2022 02:15:54 -0700 Subject: [PATCH 5/7] Add tests for new util --- .../src/utils.ts | 12 ++-- .../test/utils.test.ts | 57 ++++++++++++++++++- 2 files changed, 62 insertions(+), 7 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts index 0c4a8ffa1a..72cd553703 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts @@ -82,8 +82,8 @@ export function startSpan( }); } -// Private helper function to start a span -function pgStartSpan( +// Private helper function to start a span after a connection has been established +function startQuerySpan( client: PgClientExtended, tracer: Tracer, instrumentationConfig: PgInstrumentationConfig, @@ -110,7 +110,7 @@ export function handleConfigQuery( // Set child span name const queryCommand = getCommandFromText(queryConfig.name || queryConfig.text); const name = PgInstrumentation.BASE_SPAN_NAME + ':' + queryCommand; - const span = pgStartSpan(this, tracer, instrumentationConfig, name); + const span = startQuerySpan(this, tracer, instrumentationConfig, name); // Set attributes if (queryConfig.text) { @@ -144,7 +144,7 @@ export function handleParameterizedQuery( // Set child span name const queryCommand = getCommandFromText(query); const name = PgInstrumentation.BASE_SPAN_NAME + ':' + queryCommand; - const span = pgStartSpan(this, tracer, instrumentationConfig, name); + const span = startQuerySpan(this, tracer, instrumentationConfig, name); // Set attributes span.setAttribute(SemanticAttributes.DB_STATEMENT, query); @@ -165,7 +165,7 @@ export function handleTextQuery( // Set child span name const queryCommand = getCommandFromText(query); const name = PgInstrumentation.BASE_SPAN_NAME + ':' + queryCommand; - const span = pgStartSpan(this, tracer, instrumentationConfig, name); + const span = startQuerySpan(this, tracer, instrumentationConfig, name); // Set attributes span.setAttribute(SemanticAttributes.DB_STATEMENT, query); @@ -185,7 +185,7 @@ export function handleInvalidQuery( ...args: unknown[] ) { let result; - const span = pgStartSpan(this, tracer, instrumentationConfig, PgInstrumentation.BASE_SPAN_NAME); + const span = startQuerySpan(this, tracer, instrumentationConfig, PgInstrumentation.BASE_SPAN_NAME); try { result = originalQuery.apply(this, args as never); } catch (e) { diff --git a/plugins/node/opentelemetry-instrumentation-pg/test/utils.test.ts b/plugins/node/opentelemetry-instrumentation-pg/test/utils.test.ts index 2735ad0281..fd46571db1 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/test/utils.test.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/test/utils.test.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { context } from '@opentelemetry/api'; +import { context, INVALID_SPAN_CONTEXT, trace } from '@opentelemetry/api'; import { AsyncHooksContextManager } from '@opentelemetry/context-async-hooks'; import { InstrumentationConfig } from '@opentelemetry/instrumentation'; import { @@ -68,6 +68,61 @@ describe('utils.ts', () => { context.disable(); }); + describe('.startSpan()', () => { + it('starts real span when requireParentSpan=false', async () => { + const span = utils.startSpan( + tracer, + instrumentationConfig, + 'spanName', + { key: 'value' }, + ); + span.end(); + + const readableSpan = getLatestSpan(); + + assert.strictEqual(readableSpan.name, 'spanName'); + assert.strictEqual(readableSpan.attributes['key'], 'value'); + assert.notDeepStrictEqual(readableSpan.spanContext, INVALID_SPAN_CONTEXT); + }); + + it('starts real span when requireParentSpan=true and there is a parent span', async () => { + const parent = tracer.startSpan('parentSpan'); + context.with(trace.setSpan(context.active(), parent), () => { + const childSpan = utils.startSpan( + tracer, + { + ...instrumentationConfig, + requireParentSpan: true, + }, + 'childSpan', + { key: 'value' }, + ); + childSpan.end(); + + const readableSpan = getLatestSpan(); + assert.strictEqual(readableSpan.name, 'childSpan'); + assert.strictEqual(readableSpan.attributes['key'], 'value'); + assert.notDeepStrictEqual(readableSpan.spanContext, INVALID_SPAN_CONTEXT); + }); + }); + + it('creates placeholder span when requireParentSpan=true and there is no parent span', async () => { + const span = utils.startSpan( + tracer, + { + ...instrumentationConfig, + requireParentSpan: true, + }, + 'spanName', + { key: 'value' }, + ); + span.end(); + + const readableSpan = getLatestSpan(); + assert.strictEqual(readableSpan, undefined); + }); + }); + describe('.handleConfigQuery()', () => { const queryConfig: NormalizedQueryConfig = { text: 'SELECT $1::text', From a01de6907f59936ac4bca17b7aebfefdd6cf9e3a Mon Sep 17 00:00:00 2001 From: ryan-codaio <91277838+ryan-codaio@users.noreply.github.com> Date: Thu, 29 Sep 2022 02:16:50 -0700 Subject: [PATCH 6/7] Update config type docstring Co-authored-by: Henri Normak --- plugins/node/opentelemetry-instrumentation-pg/src/types.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/types.ts b/plugins/node/opentelemetry-instrumentation-pg/src/types.ts index f2d8b3bdd3..73ad89b061 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/types.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/types.ts @@ -41,7 +41,11 @@ export interface PgInstrumentationConfig extends InstrumentationConfig { */ responseHook?: PgInstrumentationExecutionResponseHook; - /** Require that is a parent span to create new spans. Defaults to false. */ + /** + * If true, requires a parent span to create new spans. + * + * @default false + */ requireParentSpan?: boolean; } From c19d06d653535cf58e4fc06372a0c5df9e827ab8 Mon Sep 17 00:00:00 2001 From: Ryan Eberhardt Date: Thu, 29 Sep 2022 15:48:56 -0700 Subject: [PATCH 7/7] Fix lint errors --- .../src/instrumentation.ts | 64 +++++++++---------- .../src/utils.ts | 11 +++- .../test/utils.test.ts | 18 +++--- 3 files changed, 49 insertions(+), 44 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts index bc84f5bfda..ebdcf2f602 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts @@ -19,13 +19,7 @@ import { InstrumentationNodeModuleDefinition, } from '@opentelemetry/instrumentation'; -import { - context, - diag, - trace, - Span, - SpanStatusCode, -} from '@opentelemetry/api'; +import { context, diag, trace, Span, SpanStatusCode } from '@opentelemetry/api'; import * as pgTypes from 'pg'; import * as pgPoolTypes from 'pg-pool'; import { @@ -137,15 +131,20 @@ export class PgInstrumentation extends InstrumentationBase { this: pgTypes.Client, callback?: PgErrorCallback ) { - const span = startSpan(plugin.tracer, plugin.getConfig(), `${PgInstrumentation.COMPONENT}.connect`, { - [SemanticAttributes.DB_SYSTEM]: DbSystemValues.POSTGRESQL, - [SemanticAttributes.DB_NAME]: this.database, - [SemanticAttributes.NET_PEER_NAME]: this.host, - [SemanticAttributes.DB_CONNECTION_STRING]: - utils.getConnectionString(this), - [SemanticAttributes.NET_PEER_PORT]: this.port, - [SemanticAttributes.DB_USER]: this.user, - }); + const span = startSpan( + plugin.tracer, + plugin.getConfig(), + `${PgInstrumentation.COMPONENT}.connect`, + { + [SemanticAttributes.DB_SYSTEM]: DbSystemValues.POSTGRESQL, + [SemanticAttributes.DB_NAME]: this.database, + [SemanticAttributes.NET_PEER_NAME]: this.host, + [SemanticAttributes.DB_CONNECTION_STRING]: + utils.getConnectionString(this), + [SemanticAttributes.NET_PEER_PORT]: this.port, + [SemanticAttributes.DB_USER]: this.user, + } + ); if (callback) { const parentSpan = trace.getSpan(context.active()); @@ -260,11 +259,7 @@ export class PgInstrumentation extends InstrumentationBase { .then((result: unknown) => { // Return a pass-along promise which ends the span and then goes to user's orig resolvers return new Promise(resolve => { - utils.handleExecutionResult( - plugin.getConfig(), - span, - result - ); + utils.handleExecutionResult(plugin.getConfig(), span, result); span.end(); resolve(result); }); @@ -293,17 +288,22 @@ export class PgInstrumentation extends InstrumentationBase { return function connect(this: PgPoolExtended, callback?: PgPoolCallback) { const connString = utils.getConnectionString(this.options); // setup span - const span = startSpan(plugin.tracer, plugin.getConfig(), `${PG_POOL_COMPONENT}.connect`, { - [SemanticAttributes.DB_SYSTEM]: DbSystemValues.POSTGRESQL, - [SemanticAttributes.DB_NAME]: this.options.database, // required - [SemanticAttributes.NET_PEER_NAME]: this.options.host, // required - [SemanticAttributes.DB_CONNECTION_STRING]: connString, // required - [SemanticAttributes.NET_PEER_PORT]: this.options.port, - [SemanticAttributes.DB_USER]: this.options.user, - [AttributeNames.IDLE_TIMEOUT_MILLIS]: - this.options.idleTimeoutMillis, - [AttributeNames.MAX_CLIENT]: this.options.maxClient, - }); + const span = startSpan( + plugin.tracer, + plugin.getConfig(), + `${PG_POOL_COMPONENT}.connect`, + { + [SemanticAttributes.DB_SYSTEM]: DbSystemValues.POSTGRESQL, + [SemanticAttributes.DB_NAME]: this.options.database, // required + [SemanticAttributes.NET_PEER_NAME]: this.options.host, // required + [SemanticAttributes.DB_CONNECTION_STRING]: connString, // required + [SemanticAttributes.NET_PEER_PORT]: this.options.port, + [SemanticAttributes.DB_USER]: this.options.user, + [AttributeNames.IDLE_TIMEOUT_MILLIS]: + this.options.idleTimeoutMillis, + [AttributeNames.MAX_CLIENT]: this.options.maxClient, + } + ); if (callback) { const parentSpan = trace.getSpan(context.active()); diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts index 72cd553703..5b0711e302 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts @@ -66,7 +66,7 @@ export function startSpan( tracer: Tracer, instrumentationConfig: PgInstrumentationConfig, name: string, - attributes: Attributes, + attributes: Attributes ): Span { // If a parent span is required but not present, use a noop span to propagate // context without recording it. Adapted from opentelemetry-instrumentation-http: @@ -87,7 +87,7 @@ function startQuerySpan( client: PgClientExtended, tracer: Tracer, instrumentationConfig: PgInstrumentationConfig, - name: string, + name: string ) { const jdbcString = getConnectionString(client.connectionParameters); return startSpan(tracer, instrumentationConfig, name, { @@ -185,7 +185,12 @@ export function handleInvalidQuery( ...args: unknown[] ) { let result; - const span = startQuerySpan(this, tracer, instrumentationConfig, PgInstrumentation.BASE_SPAN_NAME); + const span = startQuerySpan( + this, + tracer, + instrumentationConfig, + PgInstrumentation.BASE_SPAN_NAME + ); try { result = originalQuery.apply(this, args as never); } catch (e) { diff --git a/plugins/node/opentelemetry-instrumentation-pg/test/utils.test.ts b/plugins/node/opentelemetry-instrumentation-pg/test/utils.test.ts index fd46571db1..e1b628f4fe 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/test/utils.test.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/test/utils.test.ts @@ -70,12 +70,9 @@ describe('utils.ts', () => { describe('.startSpan()', () => { it('starts real span when requireParentSpan=false', async () => { - const span = utils.startSpan( - tracer, - instrumentationConfig, - 'spanName', - { key: 'value' }, - ); + const span = utils.startSpan(tracer, instrumentationConfig, 'spanName', { + key: 'value', + }); span.end(); const readableSpan = getLatestSpan(); @@ -95,14 +92,17 @@ describe('utils.ts', () => { requireParentSpan: true, }, 'childSpan', - { key: 'value' }, + { key: 'value' } ); childSpan.end(); const readableSpan = getLatestSpan(); assert.strictEqual(readableSpan.name, 'childSpan'); assert.strictEqual(readableSpan.attributes['key'], 'value'); - assert.notDeepStrictEqual(readableSpan.spanContext, INVALID_SPAN_CONTEXT); + assert.notDeepStrictEqual( + readableSpan.spanContext, + INVALID_SPAN_CONTEXT + ); }); }); @@ -114,7 +114,7 @@ describe('utils.ts', () => { requireParentSpan: true, }, 'spanName', - { key: 'value' }, + { key: 'value' } ); span.end();