From a32abea3daa78662ebfe3a23b14e2e19a22ee67d Mon Sep 17 00:00:00 2001 From: Abdelrahman Awad Date: Mon, 15 Jun 2026 16:25:34 -0400 Subject: [PATCH] ref(node): Streamline knex instrumentation Refactors the vendored knex instrumentation off the OpenTelemetry tracing APIs onto Sentry's span APIs, mirroring the mongoose (#21481) and mysql2 (#21509) streamlines. - Replace `tracer.startSpan` + manual `context.with`/`.then`/`.catch` with `startSpan`. `Runner.query` returns a real, already-executing Promise, so `startSpan` can safely await it and auto-end the span while keeping it active so the underlying `pg`/`mysql2` driver spans nest correctly. - Preserve the build-time `contextSymbol` parent + require-parent-span behavior (only instrument queries that run within an existing trace). - Bake the `auto.db.otel.knex` origin into the span attributes and drop the `spanStart` hook from `index.ts`. - Drop the env-gated `OTEL_SEMCONV_STABILITY_OPT_IN` dual-emission; only the OLD semantic conventions (`db.system`, `db.statement`, ...) were ever emitted. Behavior change: the unsupported stable-semconv opt-in is no longer honored. - Hardcode `requireParentSpan`/`maxQueryLength` (the integration only ever used the defaults), delete the now-dead `types.ts`/`constants.ts` and `otelExceptionFromKnexError`, drop OTel `recordException`, and remove the blanket `eslint-disable`. - Extend the `pg` integration suite with a failing query to cover the error path (`status: internal_error`). --- .../suites/tracing/knex/pg/scenario.mjs | 5 + .../suites/tracing/knex/pg/test.ts | 16 ++ .../src/integrations/tracing/knex/index.ts | 27 +-- .../tracing/knex/vendored/constants.ts | 31 --- .../tracing/knex/vendored/instrumentation.ts | 194 +++++++----------- .../tracing/knex/vendored/semconv.ts | 1 - .../tracing/knex/vendored/types.ts | 29 --- .../tracing/knex/vendored/utils.ts | 22 +- 8 files changed, 103 insertions(+), 222 deletions(-) delete mode 100644 packages/node/src/integrations/tracing/knex/vendored/constants.ts delete mode 100644 packages/node/src/integrations/tracing/knex/vendored/types.ts diff --git a/dev-packages/node-integration-tests/suites/tracing/knex/pg/scenario.mjs b/dev-packages/node-integration-tests/suites/tracing/knex/pg/scenario.mjs index d534d1ad9b01..e8f21b377e13 100644 --- a/dev-packages/node-integration-tests/suites/tracing/knex/pg/scenario.mjs +++ b/dev-packages/node-integration-tests/suites/tracing/knex/pg/scenario.mjs @@ -29,6 +29,11 @@ async function run() { await pgClient('User').insert({ name: 'bob', email: 'bob@domain.com' }); await pgClient('User').select('*'); + + // Trigger a failing query to capture the error span (table does not exist). + await pgClient('DoesNotExist') + .select('*') + .catch(() => {}); } finally { await pgClient.destroy(); } diff --git a/dev-packages/node-integration-tests/suites/tracing/knex/pg/test.ts b/dev-packages/node-integration-tests/suites/tracing/knex/pg/test.ts index 63af7b3628b5..f2defdada93f 100644 --- a/dev-packages/node-integration-tests/suites/tracing/knex/pg/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/knex/pg/test.ts @@ -57,6 +57,22 @@ describe('knex auto instrumentation', () => { description: 'select * from "User"', origin: 'auto.db.otel.knex', }), + + expect.objectContaining({ + data: expect.objectContaining({ + 'knex.version': KNEX_VERSION, + 'db.operation': 'select', + 'db.sql.table': 'DoesNotExist', + 'db.system': 'postgresql', + 'db.name': 'tests', + 'db.statement': 'select * from "DoesNotExist"', + 'sentry.origin': 'auto.db.otel.knex', + 'sentry.op': 'db', + }), + status: 'internal_error', + description: 'select * from "DoesNotExist"', + origin: 'auto.db.otel.knex', + }), ]), }; diff --git a/packages/node/src/integrations/tracing/knex/index.ts b/packages/node/src/integrations/tracing/knex/index.ts index 9698da0813fe..fdb44e9b8a02 100644 --- a/packages/node/src/integrations/tracing/knex/index.ts +++ b/packages/node/src/integrations/tracing/knex/index.ts @@ -1,36 +1,17 @@ import { KnexInstrumentation } from './vendored/instrumentation'; import type { IntegrationFn } from '@sentry/core'; -import { defineIntegration, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, spanToJSON } from '@sentry/core'; -import { generateInstrumentOnce, instrumentWhenWrapped } from '@sentry/node-core'; +import { defineIntegration } from '@sentry/core'; +import { generateInstrumentOnce } from '@sentry/node-core'; const INTEGRATION_NAME = 'Knex'; -export const instrumentKnex = generateInstrumentOnce( - INTEGRATION_NAME, - () => new KnexInstrumentation({ requireParentSpan: true }), -); +export const instrumentKnex = generateInstrumentOnce(INTEGRATION_NAME, () => new KnexInstrumentation()); const _knexIntegration = (() => { - let instrumentationWrappedCallback: undefined | ((callback: () => void) => void); - return { name: INTEGRATION_NAME, setupOnce() { - const instrumentation = instrumentKnex(); - instrumentationWrappedCallback = instrumentWhenWrapped(instrumentation); - }, - - setup(client) { - instrumentationWrappedCallback?.(() => - client.on('spanStart', span => { - const { data } = spanToJSON(span); - // knex.version is always set in the span data - // https://github.com/open-telemetry/opentelemetry-js-contrib/blob/0309caeafc44ac9cb13a3345b790b01b76d0497d/plugins/node/opentelemetry-instrumentation-knex/src/instrumentation.ts#L138 - if ('knex.version' in data) { - span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto.db.otel.knex'); - } - }), - ); + instrumentKnex(); }, }; }) satisfies IntegrationFn; diff --git a/packages/node/src/integrations/tracing/knex/vendored/constants.ts b/packages/node/src/integrations/tracing/knex/vendored/constants.ts deleted file mode 100644 index e7e8655c1810..000000000000 --- a/packages/node/src/integrations/tracing/knex/vendored/constants.ts +++ /dev/null @@ -1,31 +0,0 @@ -/* - * 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. - * - * NOTICE from the Sentry authors: - * - Vendored from: https://github.com/open-telemetry/opentelemetry-js-contrib/tree/15ef7506553f631ea4181391e0c5725a56f0d082/packages/instrumentation-knex - * - Upstream version: @opentelemetry/instrumentation-knex@0.62.0 - */ -/* eslint-disable */ - -export const MODULE_NAME = 'knex'; -export const SUPPORTED_VERSIONS = [ - // use "lib/execution" for runner.js, "lib" for client.js as basepath, latest tested 0.95.6 - '>=0.22.0 <4', - // use "lib" as basepath - '>=0.10.0 <0.18.0', - '>=0.19.0 <0.22.0', - // use "src" as basepath - '>=0.18.0 <0.19.0', -]; diff --git a/packages/node/src/integrations/tracing/knex/vendored/instrumentation.ts b/packages/node/src/integrations/tracing/knex/vendored/instrumentation.ts index dafbc5c40161..7aaa8a7b61be 100644 --- a/packages/node/src/integrations/tracing/knex/vendored/instrumentation.ts +++ b/packages/node/src/integrations/tracing/knex/vendored/instrumentation.ts @@ -17,31 +17,15 @@ * - Vendored from: https://github.com/open-telemetry/opentelemetry-js-contrib/tree/15ef7506553f631ea4181391e0c5725a56f0d082/packages/instrumentation-knex * - Upstream version: @opentelemetry/instrumentation-knex@0.62.0 * - Minor TypeScript strictness adjustments for this repository's compiler settings + * - Refactored to use Sentry's span APIs instead of OpenTelemetry tracing APIs */ -/* eslint-disable */ import * as api from '@opentelemetry/api'; -import { SDK_VERSION } from '@sentry/core'; -import * as constants from './constants'; -import { - InstrumentationBase, - InstrumentationNodeModuleDefinition, - isWrapped, - SemconvStability, - semconvStabilityFromStr, -} from '@opentelemetry/instrumentation'; +import type { InstrumentationConfig } from '@opentelemetry/instrumentation'; +import { InstrumentationBase, InstrumentationNodeModuleDefinition, isWrapped } from '@opentelemetry/instrumentation'; +import type { SpanAttributes } from '@sentry/core'; +import { SDK_VERSION, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SPAN_STATUS_ERROR, startSpan } from '@sentry/core'; import { InstrumentationNodeModuleFile } from '../../InstrumentationNodeModuleFile'; -import * as utils from './utils'; -import { KnexInstrumentationConfig } from './types'; -import { - ATTR_DB_COLLECTION_NAME, - ATTR_DB_NAMESPACE, - ATTR_DB_OPERATION_NAME, - ATTR_DB_QUERY_TEXT, - ATTR_DB_SYSTEM_NAME, - ATTR_SERVER_ADDRESS, - ATTR_SERVER_PORT, -} from '@opentelemetry/semantic-conventions'; import { ATTR_DB_NAME, ATTR_DB_OPERATION, @@ -53,65 +37,69 @@ import { ATTR_NET_PEER_PORT, ATTR_NET_TRANSPORT, } from './semconv'; +import * as utils from './utils'; const PACKAGE_NAME = '@sentry/instrumentation-knex'; +const ORIGIN = 'auto.db.otel.knex'; + +const MODULE_NAME = 'knex'; +const SUPPORTED_VERSIONS = [ + // use "lib/execution" for runner.js, "lib" for client.js as basepath, latest tested 0.95.6 + '>=0.22.0 <4', + // use "lib" as basepath + '>=0.10.0 <0.18.0', + '>=0.19.0 <0.22.0', + // use "src" as basepath + '>=0.18.0 <0.19.0', +]; + +// Max length of the query text captured in the `db.statement` attribute; ".." is appended when truncated. +const MAX_QUERY_LENGTH = 1022; const contextSymbol = Symbol('opentelemetry.instrumentation-knex.context'); -const DEFAULT_CONFIG: KnexInstrumentationConfig = { - maxQueryLength: 1022, - requireParentSpan: false, -}; - -export class KnexInstrumentation extends InstrumentationBase { - private _semconvStability: SemconvStability; - - constructor(config: KnexInstrumentationConfig = {}) { - super(PACKAGE_NAME, SDK_VERSION, { ...DEFAULT_CONFIG, ...config }); - this._semconvStability = semconvStabilityFromStr('database', process.env.OTEL_SEMCONV_STABILITY_OPT_IN); +export class KnexInstrumentation extends InstrumentationBase { + public constructor(config: InstrumentationConfig = {}) { + super(PACKAGE_NAME, SDK_VERSION, config); } - override setConfig(config: KnexInstrumentationConfig = {}) { - super.setConfig({ ...DEFAULT_CONFIG, ...config }); - } - - init() { - const module = new InstrumentationNodeModuleDefinition(constants.MODULE_NAME, constants.SUPPORTED_VERSIONS); + public init(): InstrumentationNodeModuleDefinition { + const module = new InstrumentationNodeModuleDefinition(MODULE_NAME, SUPPORTED_VERSIONS); module.files.push( - this.getClientNodeModuleFileInstrumentation('src'), - this.getClientNodeModuleFileInstrumentation('lib'), - this.getRunnerNodeModuleFileInstrumentation('src'), - this.getRunnerNodeModuleFileInstrumentation('lib'), - this.getRunnerNodeModuleFileInstrumentation('lib/execution'), + this._getClientNodeModuleFileInstrumentation('src'), + this._getClientNodeModuleFileInstrumentation('lib'), + this._getRunnerNodeModuleFileInstrumentation('src'), + this._getRunnerNodeModuleFileInstrumentation('lib'), + this._getRunnerNodeModuleFileInstrumentation('lib/execution'), ); return module; } - private getRunnerNodeModuleFileInstrumentation(basePath: string) { + private _getRunnerNodeModuleFileInstrumentation(basePath: string): InstrumentationNodeModuleFile { return new InstrumentationNodeModuleFile( `knex/${basePath}/runner.js`, - constants.SUPPORTED_VERSIONS, + SUPPORTED_VERSIONS, (Runner: any, moduleVersion?: string) => { - this.ensureWrapped(Runner.prototype, 'query', this.createQueryWrapper(moduleVersion)); + this._ensureWrapped(Runner.prototype, 'query', this._createQueryWrapper(moduleVersion)); return Runner; }, - (Runner: any, _moduleVersion?: string) => { + (Runner: any) => { this._unwrap(Runner.prototype, 'query'); return Runner; }, ); } - private getClientNodeModuleFileInstrumentation(basePath: string) { + private _getClientNodeModuleFileInstrumentation(basePath: string): InstrumentationNodeModuleFile { return new InstrumentationNodeModuleFile( `knex/${basePath}/client.js`, - constants.SUPPORTED_VERSIONS, + SUPPORTED_VERSIONS, (Client: any) => { - this.ensureWrapped(Client.prototype, 'queryBuilder', this.storeContext.bind(this)); - this.ensureWrapped(Client.prototype, 'schemaBuilder', this.storeContext.bind(this)); - this.ensureWrapped(Client.prototype, 'raw', this.storeContext.bind(this)); + this._ensureWrapped(Client.prototype, 'queryBuilder', this._storeContext.bind(this)); + this._ensureWrapped(Client.prototype, 'schemaBuilder', this._storeContext.bind(this)); + this._ensureWrapped(Client.prototype, 'raw', this._storeContext.bind(this)); return Client; }, (Client: any) => { @@ -123,9 +111,7 @@ export class KnexInstrumentation extends InstrumentationBase any) { return function wrapped_logging_method(this: any, query: any) { const config = this.client.config; @@ -137,83 +123,55 @@ export class KnexInstrumentation extends InstrumentationBase + startSpan( + { + name: utils.getName(name, operation, table), + kind: api.SpanKind.CLIENT, + attributes, + }, + span => + // `Runner.query` returns a real, already-executing Promise, so it is safe to let + // `startSpan` await it and auto-end the span. + original.apply(this, args).catch((err: any) => { + const formatter = utils.getFormatter(this); + const fullQuery = formatter(query.sql, query.bindings || []); + const message = err.message.replace(`${fullQuery} - `, ''); + span.setStatus({ code: SPAN_STATUS_ERROR, message }); + throw err; + }), + ), ); - const spanContext = api.trace.setSpan(api.context.active(), span); - - return api.context - .with(spanContext, original, this, ...arguments) - .then((result: unknown) => { - span.end(); - return result; - }) - .catch((err: any) => { - const formatter = utils.getFormatter(this); - const fullQuery = formatter(query.sql, query.bindings || []); - const message = err.message.replace(fullQuery + ' - ', ''); - const exc = utils.otelExceptionFromKnexError(err, message); - span.recordException(exc); - span.setStatus({ code: api.SpanStatusCode.ERROR, message }); - span.end(); - throw err; - }); }; }; } - private storeContext(original: Function) { + private _storeContext(original: (...args: any[]) => any) { return function wrapped_logging_method(this: any) { const builder = original.apply(this, arguments); Object.defineProperty(builder, contextSymbol, { @@ -223,7 +181,7 @@ export class KnexInstrumentation extends InstrumentationBase any) { + private _ensureWrapped(obj: any, methodName: string, wrapper: (original: any) => any): void { if (isWrapped(obj[methodName])) { this._unwrap(obj, methodName); } diff --git a/packages/node/src/integrations/tracing/knex/vendored/semconv.ts b/packages/node/src/integrations/tracing/knex/vendored/semconv.ts index c5a0f0369efb..32bc382489e5 100644 --- a/packages/node/src/integrations/tracing/knex/vendored/semconv.ts +++ b/packages/node/src/integrations/tracing/knex/vendored/semconv.ts @@ -17,7 +17,6 @@ * - Vendored from: https://github.com/open-telemetry/opentelemetry-js-contrib/tree/15ef7506553f631ea4181391e0c5725a56f0d082/packages/instrumentation-knex * - Upstream version: @opentelemetry/instrumentation-knex@0.62.0 */ -/* eslint-disable */ /** * @deprecated Replaced by `db.namespace`. diff --git a/packages/node/src/integrations/tracing/knex/vendored/types.ts b/packages/node/src/integrations/tracing/knex/vendored/types.ts deleted file mode 100644 index eff2ca5f2d1b..000000000000 --- a/packages/node/src/integrations/tracing/knex/vendored/types.ts +++ /dev/null @@ -1,29 +0,0 @@ -/* - * 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. - * - * NOTICE from the Sentry authors: - * - Vendored from: https://github.com/open-telemetry/opentelemetry-js-contrib/tree/15ef7506553f631ea4181391e0c5725a56f0d082/packages/instrumentation-knex - * - Upstream version: @opentelemetry/instrumentation-knex@0.62.0 - */ -/* eslint-disable */ - -import { InstrumentationConfig } from '@opentelemetry/instrumentation'; - -export interface KnexInstrumentationConfig extends InstrumentationConfig { - /** max query length in db.statement attribute ".." is added to the end when query is truncated */ - maxQueryLength?: number; - /** only create spans if part of an existing trace */ - requireParentSpan?: boolean; -} diff --git a/packages/node/src/integrations/tracing/knex/vendored/utils.ts b/packages/node/src/integrations/tracing/knex/vendored/utils.ts index da55f118fd66..e494349b4402 100644 --- a/packages/node/src/integrations/tracing/knex/vendored/utils.ts +++ b/packages/node/src/integrations/tracing/knex/vendored/utils.ts @@ -16,17 +16,12 @@ * NOTICE from the Sentry authors: * - Vendored from: https://github.com/open-telemetry/opentelemetry-js-contrib/tree/15ef7506553f631ea4181391e0c5725a56f0d082/packages/instrumentation-knex * - Upstream version: @opentelemetry/instrumentation-knex@0.62.0 + * - Refactored to use Sentry's span APIs instead of OpenTelemetry tracing APIs */ -/* eslint-disable */ -import { Exception } from '@opentelemetry/api'; import { DB_SYSTEM_NAME_VALUE_POSTGRESQL } from '@opentelemetry/semantic-conventions'; import { DB_SYSTEM_NAME_VALUE_SQLITE } from './semconv'; -type KnexError = Error & { - code?: string; -}; - export const getFormatter = (runner: any) => { if (runner) { if (runner.client) { @@ -43,19 +38,6 @@ export const getFormatter = (runner: any) => { return () => ''; }; -export function otelExceptionFromKnexError(err: KnexError, message: string): Exception { - if (!(err && err instanceof Error)) { - return err; - } - - return { - message, - code: err.code, - stack: err.stack, - name: err.name, - }; -} - const systemMap = new Map([ ['sqlite3', DB_SYSTEM_NAME_VALUE_SQLITE], ['pg', DB_SYSTEM_NAME_VALUE_POSTGRESQL], @@ -77,7 +59,7 @@ export const getName = (db: string, operation?: string, table?: string) => { export const limitLength = (str: string, maxLength: number) => { if (typeof str === 'string' && typeof maxLength === 'number' && 0 < maxLength && maxLength < str.length) { - return str.substring(0, maxLength) + '..'; + return `${str.substring(0, maxLength)}..`; } return str; };