From dafd1dd8fb69de08f1ac874b463ece514445441a Mon Sep 17 00:00:00 2001 From: Samuron Date: Mon, 18 Mar 2024 13:25:35 +0200 Subject: [PATCH 1/3] perf(instrumentation-pg): reduce temp objects allocations --- .../src/instrumentation.ts | 44 ++++++------------- .../src/utils.ts | 6 +-- 2 files changed, 16 insertions(+), 34 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts index 1fa8f58d3d..2494d38c5d 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts @@ -39,19 +39,11 @@ import { import { PgInstrumentationConfig } from './types'; import * as utils from './utils'; import { AttributeNames } from './enums/AttributeNames'; -import { - SemanticAttributes, - DbSystemValues, -} from '@opentelemetry/semantic-conventions'; import { addSqlCommenterComment } from '@opentelemetry/sql-common'; import { VERSION } from './version'; -const PG_POOL_COMPONENT = 'pg-pool'; - export class PgInstrumentation extends InstrumentationBase { - static readonly COMPONENT = 'pg'; - - static readonly BASE_SPAN_NAME = PgInstrumentation.COMPONENT + '.query'; + static readonly BASE_SPAN_NAME = 'pg.query'; constructor(config: PgInstrumentationConfig = {}) { super( @@ -149,16 +141,10 @@ export class PgInstrumentation extends InstrumentationBase { return original.call(this, callback); } - const span = plugin.tracer.startSpan( - `${PgInstrumentation.COMPONENT}.connect`, - { - kind: SpanKind.CLIENT, - attributes: { - [SemanticAttributes.DB_SYSTEM]: DbSystemValues.POSTGRESQL, - ...utils.getSemanticAttributesFromConnection(this), - }, - } - ); + const span = plugin.tracer.startSpan('pg.connect', { + kind: SpanKind.CLIENT, + attributes: utils.getSemanticAttributesFromConnection(this), + }); if (callback) { const parentSpan = trace.getSpan(context.active()); @@ -183,9 +169,7 @@ export class PgInstrumentation extends InstrumentationBase { private _getClientQueryPatch() { const plugin = this; return (original: typeof pgTypes.Client.prototype.query) => { - this._diag.debug( - `Patching ${PgInstrumentation.COMPONENT}.Client.prototype.query` - ); + this._diag.debug('Patching pg.Client.prototype.query'); return function query(this: PgClientExtended, ...args: unknown[]) { if (utils.shouldSkipInstrumentation(plugin.getConfig())) { return original.apply(this, args as never); @@ -367,17 +351,17 @@ export class PgInstrumentation extends InstrumentationBase { } // setup span - const span = plugin.tracer.startSpan(`${PG_POOL_COMPONENT}.connect`, { + const span = plugin.tracer.startSpan('pg-pool.connect', { kind: SpanKind.CLIENT, - attributes: { - [SemanticAttributes.DB_SYSTEM]: DbSystemValues.POSTGRESQL, - ...utils.getSemanticAttributesFromConnection(this.options), - [AttributeNames.IDLE_TIMEOUT_MILLIS]: - this.options.idleTimeoutMillis, - [AttributeNames.MAX_CLIENT]: this.options.maxClient, - }, + attributes: utils.getSemanticAttributesFromConnection(this.options), }); + span.setAttribute( + AttributeNames.IDLE_TIMEOUT_MILLIS, + this.options.idleTimeoutMillis + ); + span.setAttribute(AttributeNames.MAX_CLIENT, this.options.maxClient); + if (callback) { const parentSpan = trace.getSpan(context.active()); callback = utils.patchCallbackPGPool( diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts index bcf8e4efb7..c4e3094943 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts @@ -109,6 +109,7 @@ export function getSemanticAttributesFromConnection( params: PgParsedConnectionParams ) { return { + [SemanticAttributes.DB_SYSTEM]: DbSystemValues.POSTGRESQL, [SemanticAttributes.DB_NAME]: params.database, // required [SemanticAttributes.DB_CONNECTION_STRING]: getConnectionString(params), // required [SemanticAttributes.NET_PEER_NAME]: params.host, // required @@ -141,10 +142,7 @@ export function handleConfigQuery( const spanName = getQuerySpanName(dbName, queryConfig); const span = tracer.startSpan(spanName, { kind: SpanKind.CLIENT, - attributes: { - [SemanticAttributes.DB_SYSTEM]: DbSystemValues.POSTGRESQL, // required - ...getSemanticAttributesFromConnection(connectionParameters), - }, + attributes: getSemanticAttributesFromConnection(connectionParameters), }); if (!queryConfig) { From 8e7f718efe4c300e181c272a91fa7984c5121f06 Mon Sep 17 00:00:00 2001 From: Samuron Date: Wed, 20 Mar 2024 17:58:24 +0200 Subject: [PATCH 2/3] fix: review comments --- .../src/enums/SpanNames.ts | 21 ++++++++++++++++++ .../src/index.ts | 1 + .../src/instrumentation.ts | 16 ++++---------- .../src/utils.ts | 22 ++++++++++++++----- 4 files changed, 43 insertions(+), 17 deletions(-) create mode 100644 plugins/node/opentelemetry-instrumentation-pg/src/enums/SpanNames.ts diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/enums/SpanNames.ts b/plugins/node/opentelemetry-instrumentation-pg/src/enums/SpanNames.ts new file mode 100644 index 0000000000..c505d642d0 --- /dev/null +++ b/plugins/node/opentelemetry-instrumentation-pg/src/enums/SpanNames.ts @@ -0,0 +1,21 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +// Contains span names produced by instrumentation +export enum SpanNames { + QUERY_PREFIX = 'pg.query', + CONNECT = 'pg.connect', + POOL_CONNECT = 'pg-pool.connect', +} diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/index.ts b/plugins/node/opentelemetry-instrumentation-pg/src/index.ts index 13de301c07..7e321b6a75 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/index.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/index.ts @@ -17,3 +17,4 @@ export * from './instrumentation'; export * from './types'; export * from './enums/AttributeNames'; +export * from './enums/SpanNames'; diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts index 2494d38c5d..abe4e65c5e 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts @@ -38,13 +38,11 @@ import { } from './internal-types'; import { PgInstrumentationConfig } from './types'; import * as utils from './utils'; -import { AttributeNames } from './enums/AttributeNames'; import { addSqlCommenterComment } from '@opentelemetry/sql-common'; import { VERSION } from './version'; +import { SpanNames } from './enums/SpanNames'; export class PgInstrumentation extends InstrumentationBase { - static readonly BASE_SPAN_NAME = 'pg.query'; - constructor(config: PgInstrumentationConfig = {}) { super( '@opentelemetry/instrumentation-pg', @@ -141,7 +139,7 @@ export class PgInstrumentation extends InstrumentationBase { return original.call(this, callback); } - const span = plugin.tracer.startSpan('pg.connect', { + const span = plugin.tracer.startSpan(SpanNames.CONNECT, { kind: SpanKind.CLIENT, attributes: utils.getSemanticAttributesFromConnection(this), }); @@ -351,17 +349,11 @@ export class PgInstrumentation extends InstrumentationBase { } // setup span - const span = plugin.tracer.startSpan('pg-pool.connect', { + const span = plugin.tracer.startSpan(SpanNames.POOL_CONNECT, { kind: SpanKind.CLIENT, - attributes: utils.getSemanticAttributesFromConnection(this.options), + attributes: utils.getSemanticAttributesFromPool(this.options), }); - span.setAttribute( - AttributeNames.IDLE_TIMEOUT_MILLIS, - this.options.idleTimeoutMillis - ); - span.setAttribute(AttributeNames.MAX_CLIENT, this.options.maxClient); - if (callback) { const parentSpan = trace.getSpan(context.active()); callback = utils.patchCallbackPGPool( diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts index c4e3094943..77f690dffd 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts @@ -34,11 +34,12 @@ import { PgPoolCallback, PgPoolExtended, PgParsedConnectionParams, + PgPoolOptionsParams, } from './internal-types'; import { PgInstrumentationConfig } from './types'; import type * as pgTypes from 'pg'; -import { PgInstrumentation } from './'; import { safeExecuteInTheMiddle } from '@opentelemetry/instrumentation'; +import { SpanNames } from './enums/SpanNames'; /** * Helper function to get a low cardinality span name from whatever info we have @@ -66,7 +67,7 @@ export function getQuerySpanName( // NB: when the query config is invalid, we omit the dbName too, so that // someone (or some tool) reading the span name doesn't misinterpret the // dbName as being a prepared statement or sql commit name. - if (!queryConfig) return PgInstrumentation.BASE_SPAN_NAME; + if (!queryConfig) return SpanNames.QUERY_PREFIX; // Either the name of a prepared statement; or an attempted parse // of the SQL command, normalized to uppercase; or unknown. @@ -75,9 +76,7 @@ export function getQuerySpanName( ? queryConfig.name : parseNormalizedOperationName(queryConfig.text); - return `${PgInstrumentation.BASE_SPAN_NAME}:${command}${ - dbName ? ` ${dbName}` : '' - }`; + return `${SpanNames.QUERY_PREFIX}:${command}${dbName ? ` ${dbName}` : ''}`; } function parseNormalizedOperationName(queryText: string) { @@ -118,6 +117,19 @@ export function getSemanticAttributesFromConnection( }; } +export function getSemanticAttributesFromPool(params: PgPoolOptionsParams) { + return { + [SemanticAttributes.DB_SYSTEM]: DbSystemValues.POSTGRESQL, + [SemanticAttributes.DB_NAME]: params.database, // required + [SemanticAttributes.DB_CONNECTION_STRING]: getConnectionString(params), // required + [SemanticAttributes.NET_PEER_NAME]: params.host, // required + [SemanticAttributes.NET_PEER_PORT]: getPort(params.port), + [SemanticAttributes.DB_USER]: params.user, + [AttributeNames.IDLE_TIMEOUT_MILLIS]: params.idleTimeoutMillis, + [AttributeNames.MAX_CLIENT]: params.maxClient, + }; +} + export function shouldSkipInstrumentation( instrumentationConfig: PgInstrumentationConfig ) { From c86b2115b4ba0881a61649676602bd69aada1bd4 Mon Sep 17 00:00:00 2001 From: Samuron Date: Wed, 20 Mar 2024 18:48:22 +0200 Subject: [PATCH 3/3] fix: do not export span names from module --- plugins/node/opentelemetry-instrumentation-pg/src/index.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/index.ts b/plugins/node/opentelemetry-instrumentation-pg/src/index.ts index 7e321b6a75..13de301c07 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/index.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/index.ts @@ -17,4 +17,3 @@ export * from './instrumentation'; export * from './types'; export * from './enums/AttributeNames'; -export * from './enums/SpanNames';