diff --git a/dev-packages/node-integration-tests/suites/tracing/postgres/scenario-connect-error.mjs b/dev-packages/node-integration-tests/suites/tracing/postgres/scenario-connect-error.mjs new file mode 100644 index 000000000000..1eb5469145c5 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/postgres/scenario-connect-error.mjs @@ -0,0 +1,22 @@ +import * as Sentry from '@sentry/node'; +import { Client } from 'pg'; + +// Nothing is listening on this port, so `connect()` fails and the connect span +// must be recorded with an errored status. +const client = new Client({ port: 5499, user: 'test', password: 'test', database: 'tests' }); + +async function run() { + await Sentry.startSpan( + { + name: 'Test Transaction', + op: 'transaction', + }, + async () => { + await client.connect().catch(() => { + // expected: connection refused + }); + }, + ); +} + +run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/postgres/scenario-no-parent.mjs b/dev-packages/node-integration-tests/suites/tracing/postgres/scenario-no-parent.mjs new file mode 100644 index 000000000000..67c91affdc45 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/postgres/scenario-no-parent.mjs @@ -0,0 +1,26 @@ +import * as Sentry from '@sentry/node'; +import { Client } from 'pg'; + +const client = new Client({ port: 5494, user: 'test', password: 'test', database: 'tests' }); + +async function run() { + // No active span here: `requireParentSpan` means the connect and this query + // must NOT produce spans. + await client.connect(); + await client.query('SELECT 1 AS unparented'); + + // With an active span, the query is instrumented as a child span. + await Sentry.startSpan( + { + name: 'Test Transaction', + op: 'transaction', + }, + async () => { + await client.query('SELECT 2 AS parented'); + }, + ); + + await client.end(); +} + +run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/postgres/scenario-pool.mjs b/dev-packages/node-integration-tests/suites/tracing/postgres/scenario-pool.mjs new file mode 100644 index 000000000000..b45d41086ed5 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/postgres/scenario-pool.mjs @@ -0,0 +1,30 @@ +import * as Sentry from '@sentry/node'; +import pg from 'pg'; + +const { Pool } = pg; + +// The connection string carries credentials so we can assert they are masked +// out of the `db.connection_string` span attribute. +const pool = new Pool({ connectionString: 'postgresql://test:test@localhost:5494/tests' }); + +async function run() { + await Sentry.startSpan( + { + name: 'Test Transaction', + op: 'transaction', + }, + async () => { + try { + // Callback-style query exercises the callback-patching path (the + // promise-based scenarios never hit it). + await new Promise((resolve, reject) => { + pool.query('SELECT 1 AS foo', (err, res) => (err ? reject(err) : resolve(res))); + }); + } finally { + await pool.end(); + } + }, + ); +} + +run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/postgres/scenario.mjs b/dev-packages/node-integration-tests/suites/tracing/postgres/scenario.mjs index 351309e05010..2c9142eaf1bb 100644 --- a/dev-packages/node-integration-tests/suites/tracing/postgres/scenario.mjs +++ b/dev-packages/node-integration-tests/suites/tracing/postgres/scenario.mjs @@ -23,6 +23,18 @@ async function run() { await client.query('INSERT INTO "User" ("email", "name") VALUES ($1, $2)', ['tim', 'tim@domain.com']); await client.query('SELECT * FROM "User"'); + + // A named (prepared) query records its name as the `db.postgresql.plan` attribute + await client.query({ + name: 'select-user-by-email', + text: 'SELECT * FROM "User" WHERE "email" = $1', + values: ['tim'], + }); + + // A failing query should still produce an errored span + await client.query('SELECT * FROM "does_not_exist_table"').catch(() => { + // swallow: we only care about the span it produces + }); } finally { await client.end(); } diff --git a/dev-packages/node-integration-tests/suites/tracing/postgres/test.ts b/dev-packages/node-integration-tests/suites/tracing/postgres/test.ts index e9ed24371d8c..0a801aab4046 100644 --- a/dev-packages/node-integration-tests/suites/tracing/postgres/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/postgres/test.ts @@ -48,6 +48,33 @@ describe('postgres auto instrumentation', () => { status: 'ok', origin: 'auto.db.otel.postgres', }), + expect.objectContaining({ + data: expect.objectContaining({ + 'db.system': 'postgresql', + 'db.name': 'tests', + 'db.statement': 'SELECT * FROM "User" WHERE "email" = $1', + 'db.postgresql.plan': 'select-user-by-email', + 'sentry.origin': 'auto.db.otel.postgres', + 'sentry.op': 'db', + }), + description: 'SELECT * FROM "User" WHERE "email" = $1', + op: 'db', + status: 'ok', + origin: 'auto.db.otel.postgres', + }), + expect.objectContaining({ + data: expect.objectContaining({ + 'db.system': 'postgresql', + 'db.name': 'tests', + 'db.statement': 'SELECT * FROM "does_not_exist_table"', + 'sentry.origin': 'auto.db.otel.postgres', + 'sentry.op': 'db', + }), + description: 'SELECT * FROM "does_not_exist_table"', + op: 'db', + status: 'internal_error', + origin: 'auto.db.otel.postgres', + }), ]), }; @@ -114,6 +141,124 @@ describe('postgres auto instrumentation', () => { }); }); + describe('pool', () => { + const EXPECTED_TRANSACTION = { + transaction: 'Test Transaction', + spans: expect.arrayContaining([ + // Pool connect span: no origin is set on connect spans, so it defaults + // to 'manual', and the connection-string credentials are masked out. + expect.objectContaining({ + data: expect.objectContaining({ + 'db.system': 'postgresql', + 'db.name': 'tests', + 'db.connection_string': 'postgresql://localhost:5494/tests', + 'sentry.op': 'db', + }), + description: 'pg-pool.connect', + op: 'db', + status: 'ok', + origin: 'manual', + }), + // Callback-style query (no awaited promise returned to the caller). + expect.objectContaining({ + data: expect.objectContaining({ + 'db.system': 'postgresql', + 'db.name': 'tests', + 'db.statement': 'SELECT 1 AS foo', + 'sentry.origin': 'auto.db.otel.postgres', + 'sentry.op': 'db', + }), + description: 'SELECT 1 AS foo', + op: 'db', + status: 'ok', + origin: 'auto.db.otel.postgres', + }), + ]), + }; + + createEsmAndCjsTests(__dirname, 'scenario-pool.mjs', 'instrument.mjs', (createTestRunner, test) => { + test( + 'auto-instruments `pg.Pool`, masks connection-string credentials, and handles callback-style queries', + { timeout: 90_000 }, + async () => { + await createTestRunner() + .withDockerCompose({ + workingDirectory: [__dirname], + }) + .expect({ transaction: EXPECTED_TRANSACTION }) + .start() + .completed(); + }, + ); + }); + }); + + describe('connect error', () => { + const EXPECTED_TRANSACTION = { + transaction: 'Test Transaction', + spans: expect.arrayContaining([ + expect.objectContaining({ + data: expect.objectContaining({ + 'db.system': 'postgresql', + 'db.name': 'tests', + 'sentry.op': 'db', + }), + description: 'pg.connect', + op: 'db', + status: 'internal_error', + origin: 'manual', + }), + ]), + }; + + // No DB needed: the scenario connects to a port where nothing is listening. + createEsmAndCjsTests(__dirname, 'scenario-connect-error.mjs', 'instrument.mjs', (createTestRunner, test) => { + test('records an errored connect span when the connection fails', { timeout: 90_000 }, async () => { + await createTestRunner().expect({ transaction: EXPECTED_TRANSACTION }).start().completed(); + }); + }); + }); + + describe('requireParentSpan', () => { + createEsmAndCjsTests(__dirname, 'scenario-no-parent.mjs', 'instrument.mjs', (createTestRunner, test) => { + test('does not instrument queries or connects without an active parent span', { timeout: 90_000 }, async () => { + await createTestRunner() + .withDockerCompose({ + workingDirectory: [__dirname], + }) + .expect({ + transaction: txn => { + const descriptions = txn.spans?.map(span => span.description) ?? []; + // The unparented connect + query must not have produced spans + expect(descriptions).not.toContain('SELECT 1 AS unparented'); + expect(descriptions.find(name => name?.includes('connect'))).toBeUndefined(); + // Only the parented query is instrumented + expect(txn).toMatchObject({ + transaction: 'Test Transaction', + spans: expect.arrayContaining([ + expect.objectContaining({ + data: expect.objectContaining({ + 'db.system': 'postgresql', + 'db.name': 'tests', + 'db.statement': 'SELECT 2 AS parented', + 'sentry.origin': 'auto.db.otel.postgres', + 'sentry.op': 'db', + }), + description: 'SELECT 2 AS parented', + op: 'db', + status: 'ok', + origin: 'auto.db.otel.postgres', + }), + ]), + }); + }, + }) + .start() + .completed(); + }); + }); + }); + conditionalTest({ max: 25 })('pg-native', () => { const EXPECTED_TRANSACTION = { transaction: 'Test Transaction', diff --git a/packages/node/src/integrations/tracing/postgres/index.ts b/packages/node/src/integrations/tracing/postgres/index.ts index 18a8c823b161..a4581c33e2b9 100644 --- a/packages/node/src/integrations/tracing/postgres/index.ts +++ b/packages/node/src/integrations/tracing/postgres/index.ts @@ -1,7 +1,7 @@ import { PgInstrumentation } from './vendored/instrumentation'; import type { IntegrationFn } from '@sentry/core'; import { defineIntegration } from '@sentry/core'; -import { addOriginToSpan, generateInstrumentOnce } from '@sentry/node-core'; +import { generateInstrumentOnce } from '@sentry/node-core'; interface PostgresIntegrationOptions { ignoreConnectSpans?: boolean; @@ -13,10 +13,6 @@ export const instrumentPostgres = generateInstrumentOnce( INTEGRATION_NAME, PgInstrumentation, (options?: PostgresIntegrationOptions) => ({ - requireParentSpan: true, - requestHook(span) { - addOriginToSpan(span, 'auto.db.otel.postgres'); - }, ignoreConnectSpans: options?.ignoreConnectSpans ?? false, }), ); diff --git a/packages/node/src/integrations/tracing/postgres/vendored/enums/AttributeNames.ts b/packages/node/src/integrations/tracing/postgres/vendored/enums/AttributeNames.ts index 753e7d328808..3796cc23ff03 100644 --- a/packages/node/src/integrations/tracing/postgres/vendored/enums/AttributeNames.ts +++ b/packages/node/src/integrations/tracing/postgres/vendored/enums/AttributeNames.ts @@ -17,11 +17,9 @@ * - Vendored from: https://github.com/open-telemetry/opentelemetry-js-contrib/tree/15ef7506553f631ea4181391e0c5725a56f0d082/packages/instrumentation-pg * - Upstream version: @opentelemetry/instrumentation-pg@0.70.0 */ -/* eslint-disable */ // Postgresql specific attributes not covered by semantic conventions export enum AttributeNames { - PG_VALUES = 'db.postgresql.values', PG_PLAN = 'db.postgresql.plan', IDLE_TIMEOUT_MILLIS = 'db.postgresql.idle.timeout.millis', MAX_CLIENT = 'db.postgresql.max.client', diff --git a/packages/node/src/integrations/tracing/postgres/vendored/enums/SpanNames.ts b/packages/node/src/integrations/tracing/postgres/vendored/enums/SpanNames.ts index c7d61b383d57..8d3de763c17d 100644 --- a/packages/node/src/integrations/tracing/postgres/vendored/enums/SpanNames.ts +++ b/packages/node/src/integrations/tracing/postgres/vendored/enums/SpanNames.ts @@ -17,7 +17,6 @@ * - Vendored from: https://github.com/open-telemetry/opentelemetry-js-contrib/tree/15ef7506553f631ea4181391e0c5725a56f0d082/packages/instrumentation-pg * - Upstream version: @opentelemetry/instrumentation-pg@0.70.0 */ -/* eslint-disable */ // Contains span names produced by instrumentation export enum SpanNames { diff --git a/packages/node/src/integrations/tracing/postgres/vendored/instrumentation.ts b/packages/node/src/integrations/tracing/postgres/vendored/instrumentation.ts index 96b8b4a24dad..6b3181d501bd 100644 --- a/packages/node/src/integrations/tracing/postgres/vendored/instrumentation.ts +++ b/packages/node/src/integrations/tracing/postgres/vendored/instrumentation.ts @@ -17,114 +17,54 @@ * - Vendored from: https://github.com/open-telemetry/opentelemetry-js-contrib/tree/15ef7506553f631ea4181391e0c5725a56f0d082/packages/instrumentation-pg * - Upstream version: @opentelemetry/instrumentation-pg@0.70.0 * - Types from `pg` and `pg-pool` packages inlined as simplified interfaces - * - Minor TypeScript strictness adjustments + * - Refactored to use Sentry's span APIs instead of OpenTelemetry tracing APIs + * - Dropped the OTel metrics (no MeterProvider is wired up), the `SemconvStability` + * dual-emission, and config the SDK never passes (request/response hooks, + * enhancedDatabaseReporting, addSqlCommenterCommentToQueries) */ -/* eslint-disable */ - -import { - isWrapped, - InstrumentationBase, - InstrumentationNodeModuleDefinition, - safeExecuteInTheMiddle, - SemconvStability, - semconvStabilityFromStr, -} from '@opentelemetry/instrumentation'; + +import { SpanKind } from '@opentelemetry/api'; +import { InstrumentationBase, InstrumentationNodeModuleDefinition, isWrapped } from '@opentelemetry/instrumentation'; +import type { Span } from '@sentry/core'; +import { getActiveSpan, SDK_VERSION, SPAN_STATUS_ERROR, startInactiveSpan, withActiveSpan } from '@sentry/core'; import { InstrumentationNodeModuleFile } from '../../InstrumentationNodeModuleFile'; -import { - context, - trace, - Span, - SpanStatusCode, - SpanKind, - Histogram, - ValueType, - Attributes, - UpDownCounter, -} from '@opentelemetry/api'; -import type { PgClient } from './pg-types'; -import { +import { SpanNames } from './enums/SpanNames'; +import type { PgClientConnect, PgClientExtended, - PostgresCallback, - PgPoolExtended, PgPoolCallback, - EVENT_LISTENERS_SET, + PgPoolExtended, + PostgresCallback, } from './internal-types'; -import { PgInstrumentationConfig } from './types'; +import type { PgInstrumentationConfig } from './types'; import * as utils from './utils'; -import { addSqlCommenterComment } from '../../utils/sql-common'; -import { SDK_VERSION, timestampInSeconds } from '@sentry/core'; + const PACKAGE_NAME = '@sentry/instrumentation-pg'; -import { SpanNames } from './enums/SpanNames'; -import { - ATTR_ERROR_TYPE, - ATTR_SERVER_PORT, - ATTR_SERVER_ADDRESS, - ATTR_DB_NAMESPACE, - ATTR_DB_OPERATION_NAME, - ATTR_DB_SYSTEM_NAME, - METRIC_DB_CLIENT_OPERATION_DURATION, -} from '@opentelemetry/semantic-conventions'; -import { - METRIC_DB_CLIENT_CONNECTION_COUNT, - METRIC_DB_CLIENT_CONNECTION_PENDING_REQUESTS, - ATTR_DB_SYSTEM, - DB_SYSTEM_VALUE_POSTGRESQL, -} from './semconv'; - -function extractModuleExports(module: any) { + +function extractModuleExports(module: any): any { return module[Symbol.toStringTag] === 'Module' ? module.default // ESM : module; // CommonJS } +/** + * Binds `callback` to `parentSpan`, so that the span is active again whenever the + * (deferred) callback runs. This mirrors the upstream `context.bind(context.active(), callback)`: + * `pg` invokes callbacks outside of the original async scope (e.g. for pooled connections), so we + * re-establish the trace context to keep spans created inside the callback correctly parented. + */ +function bindCallbackToSpan any>(parentSpan: Span, callback: T): T { + return function (this: unknown, ...args: any[]): unknown { + return withActiveSpan(parentSpan, () => callback.apply(this, args)); + } as T; +} + export class PgInstrumentation extends InstrumentationBase { - declare private _operationDuration: Histogram; - declare private _connectionsCount: UpDownCounter; - declare private _connectionPendingRequests: UpDownCounter; - // Pool events connect, acquire, release and remove can be called - // multiple times without changing the values of total, idle and waiting - // connections. The _connectionsCounter is used to keep track of latest - // values and only update the metrics _connectionsCount and _connectionPendingRequests - // when the value change. - private _connectionsCounter: utils.poolConnectionsCounter = { - used: 0, - idle: 0, - pending: 0, - }; - private _semconvStability: SemconvStability; - - constructor(config: PgInstrumentationConfig = {}) { + public constructor(config: PgInstrumentationConfig = {}) { super(PACKAGE_NAME, SDK_VERSION, config); - this._semconvStability = semconvStabilityFromStr('database', process.env.OTEL_SEMCONV_STABILITY_OPT_IN); } - override _updateMetricInstruments() { - this._operationDuration = this.meter.createHistogram(METRIC_DB_CLIENT_OPERATION_DURATION, { - description: 'Duration of database client operations.', - unit: 's', - valueType: ValueType.DOUBLE, - advice: { - explicitBucketBoundaries: [0.001, 0.005, 0.01, 0.05, 0.1, 0.5, 1, 5, 10], - }, - }); - - this._connectionsCounter = { - idle: 0, - pending: 0, - used: 0, - }; - this._connectionsCount = this.meter.createUpDownCounter(METRIC_DB_CLIENT_CONNECTION_COUNT, { - description: 'The number of connections that are currently in state described by the state attribute.', - unit: '{connection}', - }); - this._connectionPendingRequests = this.meter.createUpDownCounter(METRIC_DB_CLIENT_CONNECTION_PENDING_REQUESTS, { - description: 'The number of current pending requests for an open connection.', - unit: '{connection}', - }); - } - - protected init() { + protected init(): InstrumentationNodeModuleDefinition[] { const SUPPORTED_PG_VERSIONS = ['>=8.0.3 <9']; const SUPPORTED_PG_POOL_VERSIONS = ['>=2.0.0 <4']; @@ -182,7 +122,7 @@ export class PgInstrumentation extends InstrumentationBase { - return function connect(this: PgClient, callback?: Function) { - const config = plugin.getConfig(); - - if (utils.shouldSkipInstrumentation(config) || config.ignoreConnectSpans) { + return function connect(this: PgClientExtended, callback?: (...args: unknown[]) => void) { + if (utils.shouldSkipInstrumentation() || plugin.getConfig().ignoreConnectSpans) { return original.call(this, callback); } - const span = plugin.tracer.startSpan(SpanNames.CONNECT, { + const span = startInactiveSpan({ + name: SpanNames.CONNECT, kind: SpanKind.CLIENT, - attributes: utils.getSemanticAttributesFromConnection(this, plugin._semconvStability), + attributes: utils.getSemanticAttributesFromConnection(this), }); - if (callback) { - const parentSpan = trace.getSpan(context.active()); - callback = utils.patchClientConnectCallback(span, callback); + let cb = callback; + if (cb) { + const parentSpan = getActiveSpan(); + cb = utils.patchClientConnectCallback(span, cb); if (parentSpan) { - callback = context.bind(context.active(), callback); + cb = bindCallbackToSpan(parentSpan, cb); } } - const connectResult: unknown = context.with(trace.setSpan(context.active(), span), () => { - return original.call(this, callback); + const connectResult: unknown = withActiveSpan(span, () => { + return original.call(this, cb); }); return handleConnectResult(span, connectResult); @@ -250,41 +190,13 @@ export class PgInstrumentation extends InstrumentationBase { - if (key in attributes) { - metricsAttributes[key] = attributes[key]; - } - }); - - const durationSeconds = timestampInSeconds() - startTime; - this._operationDuration.record(durationSeconds, metricsAttributes); - } - private _getClientQueryPatch() { - const plugin = this; return (original: (...args: any[]) => any) => { this._diag.debug('Patching pg.Client.prototype.query'); return function query(this: PgClientExtended, ...args: unknown[]) { - if (utils.shouldSkipInstrumentation(plugin.getConfig())) { + if (utils.shouldSkipInstrumentation()) { return original.apply(this, args as never); } - const startTime = timestampInSeconds(); // client.query(text, cb?), client.query(text, values, cb?), and // client.query(configObj, cb?) are all valid signatures. We construct @@ -297,12 +209,9 @@ export class PgInstrumentation extends InstrumentationBase { - plugin.recordOperationDuration(attributes, startTime); - }; - - const instrumentationConfig = plugin.getConfig(); - - const span = utils.handleConfigQuery.call( - this, - plugin.tracer, - instrumentationConfig, - plugin._semconvStability, - queryConfig, - ); - - // Modify query text w/ a tracing comment before invoking original for - // tracing, but only if args[0] has one of our expected shapes. - if (instrumentationConfig.addSqlCommenterCommentToQueries) { - if (firstArgIsString) { - args[0] = addSqlCommenterComment(span, arg0); - } else if (firstArgIsQueryObjectWithText && !('name' in arg0)) { - // In the case of a query object, we need to ensure there's no name field - // as this indicates a prepared query, where the comment would remain the same - // for every invocation and contain an outdated trace context. - args[0] = { - ...arg0, - text: addSqlCommenterComment(span, arg0.text), - }; - } - } + const span = utils.handleConfigQuery.call(this, queryConfig); // Bind callback (if any) to parent span (if any) if (args.length > 0) { - const parentSpan = trace.getSpan(context.active()); + const parentSpan = getActiveSpan(); if (typeof args[args.length - 1] === 'function') { // Patch ParameterQuery callback - args[args.length - 1] = utils.patchCallback( - instrumentationConfig, - span, - args[args.length - 1] as PostgresCallback, // nb: not type safe. - attributes, - recordDuration, - ); + 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] = context.bind(context.active(), args[args.length - 1]); + args[args.length - 1] = bindCallbackToSpan(parentSpan, args[args.length - 1] as PostgresCallback); } } else if (typeof queryConfig?.callback === 'function') { // Patch ConfigQuery callback - let callback = utils.patchCallback( - plugin.getConfig(), - span, - queryConfig.callback as PostgresCallback, // nb: not type safe. - attributes, - recordDuration, - ); + let callback = utils.patchCallback(span, queryConfig.callback as PostgresCallback); // If a parent span existed, bind the callback if (parentSpan) { - callback = context.bind(context.active(), callback); + callback = bindCallbackToSpan(parentSpan, callback); } (args[0] as { callback?: PostgresCallback }).callback = callback; } } - const { requestHook } = instrumentationConfig; - if (typeof requestHook === 'function' && queryConfig) { - safeExecuteInTheMiddle( - () => { - // pick keys to expose explicitly, so we're not leaking pg package - // internals that are subject to change - const { database, host, port, user } = this.connectionParameters; - const connection = { database, host, port, user }; - - requestHook(span, { - connection, - query: { - text: queryConfig.text, - // nb: if `client.query` is called with illegal arguments - // (e.g., if `queryConfig.values` is passed explicitly, but a - // non-array is given), then the type casts will be wrong. But - // we leave it up to the queryHook to handle that, and we - // catch and swallow any errors it throws. The other options - // are all worse. E.g., we could leave `queryConfig.values` - // and `queryConfig.name` as `unknown`, but then the hook body - // would be forced to validate (or cast) them before using - // them, which seems incredibly cumbersome given that these - // casts will be correct 99.9% of the time -- and pg.query - // will immediately throw during development in the other .1% - // of cases. Alternatively, we could simply skip calling the - // hook when `values` or `name` don't have the expected type, - // but that would add unnecessary validation overhead to every - // hook invocation and possibly be even more confusing/unexpected. - values: queryConfig.values as unknown[], - name: queryConfig.name as string | undefined, - }, - }); - }, - err => { - if (err) { - plugin._diag.error('Error running query hook', err); - } - }, - true, - ); - } - let result: unknown; try { result = original.apply(this, args as never); } catch (e: unknown) { - if (e instanceof Error) { - span.recordException(utils.sanitizedErrorMessage(e)); - } - span.setStatus({ - code: SpanStatusCode.ERROR, - message: utils.getErrorMessage(e), - }); + span.setStatus({ code: SPAN_STATUS_ERROR, message: utils.getErrorMessage(e) }); span.end(); throw e; } @@ -452,27 +262,13 @@ 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(), span, result); - recordDuration(); - span.end(); - resolve(result); - }); + span.end(); + return result; }) .catch((error: Error) => { - return new Promise((_, reject) => { - if (error instanceof Error) { - span.recordException(utils.sanitizedErrorMessage(error)); - } - span.setStatus({ - code: SpanStatusCode.ERROR, - message: error.message, - }); - recordDuration(); - span.end(); - reject(error); - }); + span.setStatus({ code: SPAN_STATUS_ERROR, message: error.message }); + span.end(); + return Promise.reject(error); }); } @@ -482,86 +278,32 @@ export class PgInstrumentation extends InstrumentationBase { - this._connectionsCounter = utils.updateCounter( - poolName, - pgPool, - this._connectionsCount, - this._connectionPendingRequests, - this._connectionsCounter, - ); - }); - - pgPool.on('acquire', () => { - this._connectionsCounter = utils.updateCounter( - poolName, - pgPool, - this._connectionsCount, - this._connectionPendingRequests, - this._connectionsCounter, - ); - }); - - pgPool.on('remove', () => { - this._connectionsCounter = utils.updateCounter( - poolName, - pgPool, - this._connectionsCount, - this._connectionPendingRequests, - this._connectionsCounter, - ); - }); - - pgPool.on('release' as any, () => { - this._connectionsCounter = utils.updateCounter( - poolName, - pgPool, - this._connectionsCount, - this._connectionPendingRequests, - this._connectionsCounter, - ); - }); - pgPool[EVENT_LISTENERS_SET] = true; - } - private _getPoolConnectPatch() { const plugin = this; return (originalConnect: (...args: any[]) => any) => { return function connect(this: PgPoolExtended, callback?: PgPoolCallback) { - const config = plugin.getConfig(); - - if (utils.shouldSkipInstrumentation(config)) { - return originalConnect.call(this, callback as any); - } - - // Still set up event listeners for metrics even when skipping spans - plugin._setPoolConnectEventListeners(this); - - if (config.ignoreConnectSpans) { + if (utils.shouldSkipInstrumentation() || plugin.getConfig().ignoreConnectSpans) { return originalConnect.call(this, callback as any); } - // setup span - const span = plugin.tracer.startSpan(SpanNames.POOL_CONNECT, { + const span = startInactiveSpan({ + name: SpanNames.POOL_CONNECT, kind: SpanKind.CLIENT, - attributes: utils.getSemanticAttributesFromPoolConnection(this.options, plugin._semconvStability), + attributes: utils.getSemanticAttributesFromPoolConnection(this.options), }); - if (callback) { - const parentSpan = trace.getSpan(context.active()); - callback = utils.patchCallbackPGPool(span, callback) as PgPoolCallback; + let cb = callback; + if (cb) { + const parentSpan = getActiveSpan(); + cb = utils.patchCallbackPGPool(span, cb); // If a parent span exists, bind the callback if (parentSpan) { - callback = context.bind(context.active(), callback); + cb = bindCallbackToSpan(parentSpan, cb); } } - const connectResult: unknown = context.with(trace.setSpan(context.active(), span), () => { - return originalConnect.call(this, callback as any); + const connectResult: unknown = withActiveSpan(span, () => { + return originalConnect.call(this, cb as any); }); return handleConnectResult(span, connectResult); @@ -570,29 +312,22 @@ export class PgInstrumentation extends InstrumentationBase; - return context.bind( - context.active(), - connectResultPromise - .then(result => { - span.end(); - return result; - }) - .catch((error: unknown) => { - if (error instanceof Error) { - span.recordException(utils.sanitizedErrorMessage(error)); - } - span.setStatus({ - code: SpanStatusCode.ERROR, - message: utils.getErrorMessage(error), - }); - span.end(); - return Promise.reject(error); - }), - ); + // The caller's continuation after `await client.connect()` keeps its trace context via the + // SDK's async context propagation, so we don't need to re-bind the returned promise. + return connectResultPromise + .then(result => { + span.end(); + return result; + }) + .catch((error: unknown) => { + span.setStatus({ code: SPAN_STATUS_ERROR, message: utils.getErrorMessage(error) }); + span.end(); + return Promise.reject(error); + }); } diff --git a/packages/node/src/integrations/tracing/postgres/vendored/internal-types.ts b/packages/node/src/integrations/tracing/postgres/vendored/internal-types.ts index 181c85052555..c8d640c69d0f 100644 --- a/packages/node/src/integrations/tracing/postgres/vendored/internal-types.ts +++ b/packages/node/src/integrations/tracing/postgres/vendored/internal-types.ts @@ -18,7 +18,6 @@ * - Upstream version: @opentelemetry/instrumentation-pg@0.70.0 * - Types from `pg` and `pg-pool` packages inlined as simplified interfaces */ -/* eslint-disable */ import type { PgClient } from './pg-types'; import type { PgPool } from './pg-pool-types'; @@ -64,11 +63,8 @@ export interface PgPoolOptionsParams { user: string; } -export const EVENT_LISTENERS_SET = Symbol('opentelemetry.instrumentation.pg.eventListenersSet'); - export interface PgPoolExtended extends PgPool { options: PgPoolOptionsParams; - [EVENT_LISTENERS_SET]?: boolean; // flag to identify if the event listeners for instrumentation have been set } export type PgClientConnect = (callback?: Function) => Promise | void; diff --git a/packages/node/src/integrations/tracing/postgres/vendored/pg-pool-types.ts b/packages/node/src/integrations/tracing/postgres/vendored/pg-pool-types.ts index d388d823c41d..4bf1ea6a112a 100644 --- a/packages/node/src/integrations/tracing/postgres/vendored/pg-pool-types.ts +++ b/packages/node/src/integrations/tracing/postgres/vendored/pg-pool-types.ts @@ -2,7 +2,6 @@ * Simplified type definitions inlined from the `pg-pool` package. * PoolClient and PoolConfig are replaced with structural equivalents. */ -/* eslint-disable */ import type { PgPoolClient } from './pg-types'; diff --git a/packages/node/src/integrations/tracing/postgres/vendored/pg-types.ts b/packages/node/src/integrations/tracing/postgres/vendored/pg-types.ts index 0d51e09e4f81..181449405947 100644 --- a/packages/node/src/integrations/tracing/postgres/vendored/pg-types.ts +++ b/packages/node/src/integrations/tracing/postgres/vendored/pg-types.ts @@ -3,7 +3,6 @@ * Deep type trees (Connection, Submittable, QueryConfig generics, FieldDef, * NoticeMessage, ClientConfig) are replaced with structural equivalents. */ -/* eslint-disable */ export interface FieldDef { name: string; diff --git a/packages/node/src/integrations/tracing/postgres/vendored/semconv.ts b/packages/node/src/integrations/tracing/postgres/vendored/semconv.ts index 0c4434f078a9..e8f672a0f9e0 100644 --- a/packages/node/src/integrations/tracing/postgres/vendored/semconv.ts +++ b/packages/node/src/integrations/tracing/postgres/vendored/semconv.ts @@ -16,8 +16,8 @@ * NOTICE from the Sentry authors: * - Vendored from: https://github.com/open-telemetry/opentelemetry-js-contrib/tree/15ef7506553f631ea4181391e0c5725a56f0d082/packages/instrumentation-pg * - Upstream version: @opentelemetry/instrumentation-pg@0.70.0 + * - Trimmed to the (old) semantic conventions the SDK actually emits */ -/* eslint-disable */ /* * This file contains a copy of unstable semantic convention definitions @@ -25,31 +25,11 @@ * @see https://github.com/open-telemetry/opentelemetry-js/tree/main/semantic-conventions#unstable-semconv */ -/** - * The name of the connection pool; unique within the instrumented application. In case the connection pool implementation doesn't provide a name, instrumentation **SHOULD** use a combination of parameters that would make the name unique, for example, combining attributes `server.address`, `server.port`, and `db.namespace`, formatted as `server.address:server.port/db.namespace`. Instrumentations that generate connection pool name following different patterns **SHOULD** document it. - * - * @example myDataSource - * - * @experimental This attribute is experimental and is subject to breaking changes in minor releases of `@opentelemetry/semantic-conventions`. - */ -export const ATTR_DB_CLIENT_CONNECTION_POOL_NAME = 'db.client.connection.pool.name' as const; - -/** - * The state of a connection in the pool - * - * @example idle - * - * @experimental This attribute is experimental and is subject to breaking changes in minor releases of `@opentelemetry/semantic-conventions`. - */ -export const ATTR_DB_CLIENT_CONNECTION_STATE = 'db.client.connection.state' as const; - /** * Deprecated, use `server.address`, `server.port` attributes instead. * * @example "Server=(localdb)\\v11.0;Integrated Security=true;" * - * @experimental This attribute is experimental and is subject to breaking changes in minor releases of `@opentelemetry/semantic-conventions`. - * * @deprecated Replaced by `server.address` and `server.port`. */ export const ATTR_DB_CONNECTION_STRING = 'db.connection_string' as const; @@ -60,8 +40,6 @@ export const ATTR_DB_CONNECTION_STRING = 'db.connection_string' as const; * @example customers * @example main * - * @experimental This attribute is experimental and is subject to breaking changes in minor releases of `@opentelemetry/semantic-conventions`. - * * @deprecated Replaced by `db.namespace`. */ export const ATTR_DB_NAME = 'db.name' as const; @@ -72,8 +50,6 @@ export const ATTR_DB_NAME = 'db.name' as const; * @example SELECT * FROM wuser_table * @example SET mykey "WuValue" * - * @experimental This attribute is experimental and is subject to breaking changes in minor releases of `@opentelemetry/semantic-conventions`. - * * @deprecated Replaced by `db.query.text`. */ export const ATTR_DB_STATEMENT = 'db.statement' as const; @@ -81,8 +57,6 @@ export const ATTR_DB_STATEMENT = 'db.statement' as const; /** * Deprecated, use `db.system.name` instead. * - * @experimental This attribute is experimental and is subject to breaking changes in minor releases of `@opentelemetry/semantic-conventions`. - * * @deprecated Replaced by `db.system.name`. */ export const ATTR_DB_SYSTEM = 'db.system' as const; @@ -93,8 +67,6 @@ export const ATTR_DB_SYSTEM = 'db.system' as const; * @example readonly_user * @example reporting_user * - * @experimental This attribute is experimental and is subject to breaking changes in minor releases of `@opentelemetry/semantic-conventions`. - * * @deprecated Removed, no replacement at this time. */ export const ATTR_DB_USER = 'db.user' as const; @@ -104,8 +76,6 @@ export const ATTR_DB_USER = 'db.user' as const; * * @example example.com * - * @experimental This attribute is experimental and is subject to breaking changes in minor releases of `@opentelemetry/semantic-conventions`. - * * @deprecated Replaced by `server.address` on client spans and `client.address` on server spans. */ export const ATTR_NET_PEER_NAME = 'net.peer.name' as const; @@ -115,37 +85,11 @@ export const ATTR_NET_PEER_NAME = 'net.peer.name' as const; * * @example 8080 * - * @experimental This attribute is experimental and is subject to breaking changes in minor releases of `@opentelemetry/semantic-conventions`. - * * @deprecated Replaced by `server.port` on client spans and `client.port` on server spans. */ export const ATTR_NET_PEER_PORT = 'net.peer.port' as const; -/** - * Enum value "idle" for attribute {@link ATTR_DB_CLIENT_CONNECTION_STATE}. - */ -export const DB_CLIENT_CONNECTION_STATE_VALUE_IDLE = 'idle' as const; - -/** - * Enum value "used" for attribute {@link ATTR_DB_CLIENT_CONNECTION_STATE}. - */ -export const DB_CLIENT_CONNECTION_STATE_VALUE_USED = 'used' as const; - /** * Enum value "postgresql" for attribute {@link ATTR_DB_SYSTEM}. */ export const DB_SYSTEM_VALUE_POSTGRESQL = 'postgresql' as const; - -/** - * The number of connections that are currently in state described by the `state` attribute - * - * @experimental This metric is experimental and is subject to breaking changes in minor releases of `@opentelemetry/semantic-conventions`. - */ -export const METRIC_DB_CLIENT_CONNECTION_COUNT = 'db.client.connection.count' as const; - -/** - * The number of current pending requests for an open connection - * - * @experimental This metric is experimental and is subject to breaking changes in minor releases of `@opentelemetry/semantic-conventions`. - */ -export const METRIC_DB_CLIENT_CONNECTION_PENDING_REQUESTS = 'db.client.connection.pending_requests' as const; diff --git a/packages/node/src/integrations/tracing/postgres/vendored/types.ts b/packages/node/src/integrations/tracing/postgres/vendored/types.ts index 05fffa56336b..d5fdba6084d5 100644 --- a/packages/node/src/integrations/tracing/postgres/vendored/types.ts +++ b/packages/node/src/integrations/tracing/postgres/vendored/types.ts @@ -16,79 +16,15 @@ * NOTICE from the Sentry authors: * - Vendored from: https://github.com/open-telemetry/opentelemetry-js-contrib/tree/15ef7506553f631ea4181391e0c5725a56f0d082/packages/instrumentation-pg * - Upstream version: @opentelemetry/instrumentation-pg@0.70.0 - * - Types from `pg` package inlined as simplified interfaces + * - Trimmed to the config the SDK actually passes */ -/* eslint-disable */ -import type { QueryResult, QueryArrayResult } from './pg-types'; -import type * as api from '@opentelemetry/api'; -import { InstrumentationConfig } from '@opentelemetry/instrumentation'; - -export interface PgResponseHookInformation { - data: QueryResult | QueryArrayResult; -} - -export interface PgInstrumentationExecutionResponseHook { - (span: api.Span, responseInfo: PgResponseHookInformation): void; -} - -export interface PgRequestHookInformation { - query: { - text: string; - name?: string; - values?: unknown[]; - }; - connection: { - database?: string; - host?: string; - port?: number; - user?: string; - }; -} - -export interface PgInstrumentationExecutionRequestHook { - (span: api.Span, queryInfo: PgRequestHookInformation): void; -} +import type { InstrumentationConfig } from '@opentelemetry/instrumentation'; export interface PgInstrumentationConfig extends InstrumentationConfig { - /** - * If true, an attribute containing the query's parameters will be attached - * the spans generated to represent the query. - */ - enhancedDatabaseReporting?: boolean; - - /** - * Hook that allows adding custom span attributes or updating the - * span's name based on the data about the query to execute. - * - * @default undefined - */ - requestHook?: PgInstrumentationExecutionRequestHook; - - /** - * Hook that allows adding custom span attributes based on the data - * returned from "query" Pg actions. - * - * @default undefined - */ - responseHook?: PgInstrumentationExecutionResponseHook; - - /** - * If true, requires a parent span to create new spans. - * - * @default false - */ - requireParentSpan?: boolean; - - /** - * If true, queries are modified to also include a comment with - * the tracing context, following the {@link https://github.com/open-telemetry/opentelemetry-sqlcommenter sqlcommenter} format - */ - addSqlCommenterCommentToQueries?: boolean; - /** * If true, `pg.connect` and `pg-pool.connect` spans will not be created. - * Query spans and pool metrics are still recorded. + * Query spans are still recorded. * * @default false */ diff --git a/packages/node/src/integrations/tracing/postgres/vendored/utils.ts b/packages/node/src/integrations/tracing/postgres/vendored/utils.ts index a22036674243..203da8fb2e07 100644 --- a/packages/node/src/integrations/tracing/postgres/vendored/utils.ts +++ b/packages/node/src/integrations/tracing/postgres/vendored/utils.ts @@ -17,57 +17,35 @@ * - Vendored from: https://github.com/open-telemetry/opentelemetry-js-contrib/tree/15ef7506553f631ea4181391e0c5725a56f0d082/packages/instrumentation-pg * - Upstream version: @opentelemetry/instrumentation-pg@0.70.0 * - Types from `pg` package inlined as simplified interfaces - * - Minor TypeScript strictness adjustments + * - Refactored to use Sentry's span APIs instead of OpenTelemetry tracing APIs */ -/* eslint-disable */ -import { - context, - trace, - Span, - SpanStatusCode, - Tracer, - SpanKind, - diag, - UpDownCounter, - Attributes, -} from '@opentelemetry/api'; +import { SpanKind } from '@opentelemetry/api'; +import type { Span, SpanAttributes } from '@sentry/core'; +import { getActiveSpan, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SPAN_STATUS_ERROR, startInactiveSpan } from '@sentry/core'; import { AttributeNames } from './enums/AttributeNames'; -import { - ATTR_ERROR_TYPE, - ATTR_DB_SYSTEM_NAME, - ATTR_DB_NAMESPACE, - ATTR_SERVER_ADDRESS, - ATTR_SERVER_PORT, - ATTR_DB_QUERY_TEXT, - DB_SYSTEM_NAME_VALUE_POSTGRESQL, -} from '@opentelemetry/semantic-conventions'; -import { - ATTR_DB_CLIENT_CONNECTION_POOL_NAME, - ATTR_DB_CLIENT_CONNECTION_STATE, - DB_CLIENT_CONNECTION_STATE_VALUE_USED, - DB_CLIENT_CONNECTION_STATE_VALUE_IDLE, - ATTR_DB_SYSTEM, - ATTR_DB_NAME, - ATTR_DB_USER, - DB_SYSTEM_VALUE_POSTGRESQL, - ATTR_DB_CONNECTION_STRING, - ATTR_NET_PEER_PORT, - ATTR_NET_PEER_NAME, - ATTR_DB_STATEMENT, -} from './semconv'; -import { +import { SpanNames } from './enums/SpanNames'; +import type { PgClientExtended, - PostgresCallback, + PgParsedConnectionParams, PgPoolCallback, PgPoolExtended, - PgParsedConnectionParams, PgPoolOptionsParams, + PostgresCallback, } from './internal-types'; -import { PgInstrumentationConfig } from './types'; -import type { PgClient, QueryResult, QueryArrayResult } from './pg-types'; -import { safeExecuteInTheMiddle, SemconvStability } from '@opentelemetry/instrumentation'; -import { SpanNames } from './enums/SpanNames'; +import type { PgClient } from './pg-types'; +import { + ATTR_DB_CONNECTION_STRING, + ATTR_DB_NAME, + ATTR_DB_STATEMENT, + ATTR_DB_SYSTEM, + ATTR_DB_USER, + ATTR_NET_PEER_NAME, + ATTR_NET_PEER_PORT, + DB_SYSTEM_VALUE_POSTGRESQL, +} from './semconv'; + +export const ORIGIN = 'auto.db.otel.postgres'; /** * Helper function to get a low cardinality span name from whatever info we have @@ -88,7 +66,7 @@ import { SpanNames } from './enums/SpanNames'; * `client.query()`. This will be undefined if `client.query()` was called * with invalid arguments. */ -export function getQuerySpanName(dbName: string | undefined, queryConfig?: { text: string; name?: unknown }) { +export function getQuerySpanName(dbName: string | undefined, queryConfig?: { text: string; name?: unknown }): string { // 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. @@ -104,7 +82,7 @@ export function getQuerySpanName(dbName: string | undefined, queryConfig?: { tex return `${SpanNames.QUERY_PREFIX}:${command}${dbName ? ` ${dbName}` : ''}`; } -export function parseNormalizedOperationName(queryText: string) { +export function parseNormalizedOperationName(queryText: string): string { // Trim the query text to handle leading/trailing whitespace const trimmedQuery = queryText.trim(); const indexOfFirstSpace = trimmedQuery.indexOf(' '); @@ -125,13 +103,13 @@ export function parseAndMaskConnectionString(connectionString: string): string { url.password = ''; return url.toString(); - } catch (e) { + } catch { // If parsing fails, return a generic connection string return 'postgresql://localhost:5432/'; } } -export function getConnectionString(params: PgParsedConnectionParams | PgPoolOptionsParams) { +export function getConnectionString(params: PgParsedConnectionParams | PgPoolOptionsParams): string { if ('connectionString' in params && params.connectionString) { return parseAndMaskConnectionString(params.connectionString); } @@ -152,96 +130,63 @@ function getPort(port: number | undefined): number | undefined { return undefined; } -export function getSemanticAttributesFromConnection( - params: PgParsedConnectionParams, - semconvStability: SemconvStability, -) { - let attributes: Attributes = {}; - - if (semconvStability & SemconvStability.OLD) { - attributes = { - ...attributes, - [ATTR_DB_SYSTEM]: DB_SYSTEM_VALUE_POSTGRESQL, - [ATTR_DB_NAME]: params.database, - [ATTR_DB_CONNECTION_STRING]: getConnectionString(params), - [ATTR_DB_USER]: params.user, - [ATTR_NET_PEER_NAME]: params.host, // required - [ATTR_NET_PEER_PORT]: getPort(params.port), - }; - } - if (semconvStability & SemconvStability.STABLE) { - attributes = { - ...attributes, - [ATTR_DB_SYSTEM_NAME]: DB_SYSTEM_NAME_VALUE_POSTGRESQL, - [ATTR_DB_NAMESPACE]: params.namespace, - [ATTR_SERVER_ADDRESS]: params.host, - [ATTR_SERVER_PORT]: getPort(params.port), - }; - } - - return attributes; +export function getSemanticAttributesFromConnection(params: PgParsedConnectionParams): SpanAttributes { + return { + [ATTR_DB_SYSTEM]: DB_SYSTEM_VALUE_POSTGRESQL, + [ATTR_DB_NAME]: params.database, + [ATTR_DB_CONNECTION_STRING]: getConnectionString(params), + [ATTR_DB_USER]: params.user, + [ATTR_NET_PEER_NAME]: params.host, // required + [ATTR_NET_PEER_PORT]: getPort(params.port), + }; } -export function getSemanticAttributesFromPoolConnection( - params: PgPoolOptionsParams, - semconvStability: SemconvStability, -) { +export function getSemanticAttributesFromPoolConnection(params: PgPoolOptionsParams): SpanAttributes { let url: URL | undefined; try { url = params.connectionString ? new URL(params.connectionString) : undefined; - } catch (e) { + } catch { url = undefined; } - let attributes: Attributes = { + + return { [AttributeNames.IDLE_TIMEOUT_MILLIS]: params.idleTimeoutMillis, [AttributeNames.MAX_CLIENT]: params.maxClient, + [ATTR_DB_SYSTEM]: DB_SYSTEM_VALUE_POSTGRESQL, + [ATTR_DB_NAME]: url?.pathname.slice(1) ?? params.database, + [ATTR_DB_CONNECTION_STRING]: getConnectionString(params), + [ATTR_NET_PEER_NAME]: url?.hostname ?? params.host, + [ATTR_NET_PEER_PORT]: Number(url?.port) || getPort(params.port), + [ATTR_DB_USER]: url?.username ?? params.user, }; - - if (semconvStability & SemconvStability.OLD) { - attributes = { - ...attributes, - [ATTR_DB_SYSTEM]: DB_SYSTEM_VALUE_POSTGRESQL, - [ATTR_DB_NAME]: url?.pathname.slice(1) ?? params.database, - [ATTR_DB_CONNECTION_STRING]: getConnectionString(params), - [ATTR_NET_PEER_NAME]: url?.hostname ?? params.host, - [ATTR_NET_PEER_PORT]: Number(url?.port) || getPort(params.port), - [ATTR_DB_USER]: url?.username ?? params.user, - }; - } - if (semconvStability & SemconvStability.STABLE) { - attributes = { - ...attributes, - [ATTR_DB_SYSTEM_NAME]: DB_SYSTEM_NAME_VALUE_POSTGRESQL, - [ATTR_DB_NAMESPACE]: params.namespace, - [ATTR_SERVER_ADDRESS]: url?.hostname ?? params.host, - [ATTR_SERVER_PORT]: Number(url?.port) || getPort(params.port), - }; - } - - return attributes; } -export function shouldSkipInstrumentation(instrumentationConfig: PgInstrumentationConfig) { - return instrumentationConfig.requireParentSpan === true && trace.getSpan(context.active()) === undefined; +/** + * The SDK always requires a parent span (it sets `requireParentSpan: true`), so + * we only instrument when there is an active span to parent the new span under. + */ +export function shouldSkipInstrumentation(): boolean { + return getActiveSpan() === undefined; } -// Create a span from our normalized queryConfig object, +// Create an (inactive) span from our normalized queryConfig object, // or return a basic span if no queryConfig was given/could be created. export function handleConfigQuery( this: PgClientExtended, - tracer: Tracer, - instrumentationConfig: PgInstrumentationConfig, - semconvStability: SemconvStability, queryConfig?: { text: string; values?: unknown; name?: unknown }, -) { +): Span { // Create child span. const { connectionParameters } = this; const dbName = connectionParameters.database; const spanName = getQuerySpanName(dbName, queryConfig); - const span = tracer.startSpan(spanName, { + const span = startInactiveSpan({ + name: spanName, kind: SpanKind.CLIENT, - attributes: getSemanticAttributesFromConnection(connectionParameters, semconvStability), + attributes: { + ...getSemanticAttributesFromConnection(connectionParameters), + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: ORIGIN, + }, }); if (!queryConfig) { @@ -250,35 +195,7 @@ export function handleConfigQuery( // Set attributes if (queryConfig.text) { - if (semconvStability & SemconvStability.OLD) { - span.setAttribute(ATTR_DB_STATEMENT, queryConfig.text); - } - if (semconvStability & SemconvStability.STABLE) { - span.setAttribute(ATTR_DB_QUERY_TEXT, queryConfig.text); - } - } - - if (instrumentationConfig.enhancedDatabaseReporting && Array.isArray(queryConfig.values)) { - try { - const convertedValues = queryConfig.values.map(value => { - if (value == null) { - return 'null'; - } else if (value instanceof Buffer) { - return value.toString(); - } else if (typeof value === 'object') { - if (typeof value.toPostgres === 'function') { - return value.toPostgres(); - } - return JSON.stringify(value); - } else { - //string, number - return value.toString(); - } - }); - span.setAttribute(AttributeNames.PG_VALUES, convertedValues); - } catch (e) { - diag.error('failed to stringify ', queryConfig.values, e); - } + span.setAttribute(ATTR_DB_STATEMENT, queryConfig.text); } // Set plan name attribute, if present @@ -289,130 +206,34 @@ export function handleConfigQuery( return span; } -export function handleExecutionResult( - config: PgInstrumentationConfig, - span: Span, - pgResult: QueryResult | QueryArrayResult | unknown, -) { - if (typeof config.responseHook === 'function') { - safeExecuteInTheMiddle( - () => { - config.responseHook!(span, { - data: pgResult as QueryResult | QueryArrayResult, - }); - }, - err => { - if (err) { - diag.error('Error running response hook', err); - } - }, - true, - ); - } -} - -export function patchCallback( - instrumentationConfig: PgInstrumentationConfig, - span: Span, - cb: PostgresCallback, - attributes: Attributes, - recordDuration: { (): void }, -): PostgresCallback { +export function patchCallback(span: Span, cb: PostgresCallback): PostgresCallback { return function patchedCallback(this: PgClientExtended, err: Error, res: object) { if (err) { - if (Object.prototype.hasOwnProperty.call(err, 'code')) { - attributes[ATTR_ERROR_TYPE] = (err as any)['code']; - } - if (err instanceof Error) { - span.recordException(sanitizedErrorMessage(err)); - } - span.setStatus({ - code: SpanStatusCode.ERROR, - message: err.message, - }); - } else { - handleExecutionResult(instrumentationConfig, span, res); + span.setStatus({ code: SPAN_STATUS_ERROR, message: err.message }); } - - recordDuration(); span.end(); cb.call(this, err, res); }; } -export function getPoolName(pool: PgPoolOptionsParams): string { - let poolName = ''; - poolName += (pool?.host ? `${pool.host}` : 'unknown_host') + ':'; - poolName += (pool?.port ? `${pool.port}` : 'unknown_port') + '/'; - poolName += pool?.database ? `${pool.database}` : 'unknown_database'; - - return poolName.trim(); -} - -export interface poolConnectionsCounter { - used: number; - idle: number; - pending: number; -} - -export function updateCounter( - poolName: string, - pool: PgPoolExtended, - connectionCount: UpDownCounter, - connectionPendingRequests: UpDownCounter, - latestCounter: poolConnectionsCounter, -): poolConnectionsCounter { - const all = pool.totalCount; - const pending = pool.waitingCount; - const idle = pool.idleCount; - const used = all - idle; - - connectionCount.add(used - latestCounter.used, { - [ATTR_DB_CLIENT_CONNECTION_STATE]: DB_CLIENT_CONNECTION_STATE_VALUE_USED, - [ATTR_DB_CLIENT_CONNECTION_POOL_NAME]: poolName, - }); - - connectionCount.add(idle - latestCounter.idle, { - [ATTR_DB_CLIENT_CONNECTION_STATE]: DB_CLIENT_CONNECTION_STATE_VALUE_IDLE, - [ATTR_DB_CLIENT_CONNECTION_POOL_NAME]: poolName, - }); - - connectionPendingRequests.add(pending - latestCounter.pending, { - [ATTR_DB_CLIENT_CONNECTION_POOL_NAME]: poolName, - }); - - return { used: used, idle: idle, pending: pending }; -} - export function patchCallbackPGPool(span: Span, cb: PgPoolCallback): PgPoolCallback { return function patchedCallback(this: PgPoolExtended, err: Error, res: object, done: any) { if (err) { - if (err instanceof Error) { - span.recordException(sanitizedErrorMessage(err)); - } - span.setStatus({ - code: SpanStatusCode.ERROR, - message: err.message, - }); + span.setStatus({ code: SPAN_STATUS_ERROR, message: err.message }); } span.end(); cb.call(this, err, res, done); }; } -export function patchClientConnectCallback(span: Span, cb: Function): Function { - return function patchedClientConnectCallback(this: PgClient, err: Error) { - if (err) { - if (err instanceof Error) { - span.recordException(sanitizedErrorMessage(err)); - } - span.setStatus({ - code: SpanStatusCode.ERROR, - message: err.message, - }); +export function patchClientConnectCallback(span: Span, cb: (...args: unknown[]) => void): (...args: unknown[]) => void { + return function patchedClientConnectCallback(this: PgClient, ...args: unknown[]) { + const err = args[0]; + if (err instanceof Error) { + span.setStatus({ code: SPAN_STATUS_ERROR, message: err.message }); } span.end(); - cb.apply(this, arguments); + cb.apply(this, args); }; } @@ -421,7 +242,7 @@ export function patchClientConnectCallback(span: Span, cb: Function): Function { * defensive, to recognize the fact that, in JS, any kind of value (even * primitives) can be thrown. */ -export function getErrorMessage(e: unknown) { +export function getErrorMessage(e: unknown): string | undefined { return typeof e === 'object' && e !== null && 'message' in e ? String((e as { message?: unknown }).message) : undefined; @@ -435,14 +256,3 @@ export type ObjectWithText = { text: string; [k: string]: unknown; }; - -/** - * Generates a sanitized message for the error. - * Only includes the error type and PostgreSQL error code, omitting any sensitive details. - */ -export function sanitizedErrorMessage(error: unknown): string { - const name = (error as any)?.name ?? 'PostgreSQLError'; - const code = (error as any)?.code ?? 'UNKNOWN'; - - return `PostgreSQL error of type '${name}' occurred (code: ${code})`; -} diff --git a/packages/node/test/integrations/tracing/postgres.test.ts b/packages/node/test/integrations/tracing/postgres.test.ts deleted file mode 100644 index bdb9a07a6594..000000000000 --- a/packages/node/test/integrations/tracing/postgres.test.ts +++ /dev/null @@ -1,93 +0,0 @@ -import { PgInstrumentation } from '../../../src/integrations/tracing/postgres/vendored/instrumentation'; -import { INSTRUMENTED } from '@sentry/node-core'; -import { beforeEach, describe, expect, it, type MockInstance, vi } from 'vitest'; -import { instrumentPostgres, postgresIntegration } from '../../../src/integrations/tracing/postgres'; - -vi.mock('../../../src/integrations/tracing/postgres/vendored/instrumentation'); - -describe('postgres integration', () => { - beforeEach(() => { - vi.clearAllMocks(); - delete INSTRUMENTED.Postgres; - - (PgInstrumentation as unknown as MockInstance).mockImplementation(() => ({ - setTracerProvider: () => undefined, - setMeterProvider: () => undefined, - getConfig: () => ({}), - setConfig: () => ({}), - enable: () => undefined, - })); - }); - - it('has a name and setupOnce method', () => { - const integration = postgresIntegration(); - expect(integration.name).toBe('Postgres'); - expect(typeof integration.setupOnce).toBe('function'); - }); - - it('passes ignoreConnectSpans: true to PgInstrumentation when set on integration', () => { - postgresIntegration({ ignoreConnectSpans: true }).setupOnce!(); - - expect(PgInstrumentation).toHaveBeenCalledTimes(1); - expect(PgInstrumentation).toHaveBeenCalledWith({ - requireParentSpan: true, - requestHook: expect.any(Function), - ignoreConnectSpans: true, - }); - }); - - it('passes ignoreConnectSpans: false to PgInstrumentation by default', () => { - postgresIntegration().setupOnce!(); - - expect(PgInstrumentation).toHaveBeenCalledTimes(1); - expect(PgInstrumentation).toHaveBeenCalledWith({ - requireParentSpan: true, - requestHook: expect.any(Function), - ignoreConnectSpans: false, - }); - }); - - it('instrumentPostgres receives ignoreConnectSpans option', () => { - instrumentPostgres({ ignoreConnectSpans: true }); - - expect(PgInstrumentation).toHaveBeenCalledTimes(1); - expect(PgInstrumentation).toHaveBeenCalledWith({ - requireParentSpan: true, - requestHook: expect.any(Function), - ignoreConnectSpans: true, - }); - }); - - it('second call to instrumentPostgres passes full config to setConfig, not raw user options', () => { - const mockSetConfig = vi.fn(); - (PgInstrumentation as unknown as MockInstance).mockImplementation(() => ({ - setTracerProvider: () => undefined, - setMeterProvider: () => undefined, - getConfig: () => ({}), - setConfig: mockSetConfig, - enable: () => undefined, - })); - - instrumentPostgres({ ignoreConnectSpans: true }); - expect(PgInstrumentation).toHaveBeenCalledWith( - expect.objectContaining({ - requireParentSpan: true, - ignoreConnectSpans: true, - requestHook: expect.any(Function), - }), - ); - - mockSetConfig.mockClear(); - instrumentPostgres({ ignoreConnectSpans: false }); - - expect(PgInstrumentation).toHaveBeenCalledTimes(1); - expect(mockSetConfig).toHaveBeenCalledTimes(1); - expect(mockSetConfig).toHaveBeenCalledWith( - expect.objectContaining({ - requireParentSpan: true, - ignoreConnectSpans: false, - requestHook: expect.any(Function), - }), - ); - }); -});