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`).

Author: logaretm

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();
       }

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',
+            }),
           ]),
         };
 

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;

packages/node/src/integrations/tracing/knex/vendored/constants.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<KnexInstrumentationConfig> {
-  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<InstrumentationConfig> {
+  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<KnexInstrumentation
     );
   }
 
-  private createQueryWrapper(moduleVersion?: string) {
-    const instrumentation = this;
-
+  private _createQueryWrapper(moduleVersion?: string) {
     return function wrapQuery(original: (...args: any[]) => any) {
       return function wrapped_logging_method(this: any, query: any) {
         const config = this.client.config;
@@ -137,83 +123,55 @@ export class KnexInstrumentation extends InstrumentationBase<KnexInstrumentation
           config?.connection?.filename ||
           config?.connection?.database ||
           utils.extractDatabaseFromConnectionString(connectionString);
-        const { maxQueryLength } = instrumentation.getConfig();
 
-        const attributes: api.Attributes = {
+        const attributes: SpanAttributes = {
+          [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: ORIGIN,
           'knex.version': moduleVersion,
+          [ATTR_DB_SYSTEM]: utils.mapSystem(this.client.driverName),
+          [ATTR_DB_SQL_TABLE]: table,
+          [ATTR_DB_OPERATION]: operation,
+          [ATTR_DB_USER]: config?.connection?.user,
+          [ATTR_DB_NAME]: name,
+          [ATTR_NET_PEER_NAME]: config?.connection?.host ?? utils.extractHostFromConnectionString(connectionString),
+          [ATTR_NET_PEER_PORT]: config?.connection?.port ?? utils.extractPortFromConnectionString(connectionString),
+          [ATTR_NET_TRANSPORT]: config?.connection?.filename === ':memory:' ? 'inproc' : undefined,
+          [ATTR_DB_STATEMENT]: utils.limitLength(query?.sql, MAX_QUERY_LENGTH),
         };
-        const transport = config?.connection?.filename === ':memory:' ? 'inproc' : undefined;
-
-        if (instrumentation._semconvStability & SemconvStability.OLD) {
-          Object.assign(attributes, {
-            [ATTR_DB_SYSTEM]: utils.mapSystem(this.client.driverName),
-            [ATTR_DB_SQL_TABLE]: table,
-            [ATTR_DB_OPERATION]: operation,
-            [ATTR_DB_USER]: config?.connection?.user,
-            [ATTR_DB_NAME]: name,
-            [ATTR_NET_PEER_NAME]: config?.connection?.host ?? utils.extractHostFromConnectionString(connectionString),
-            [ATTR_NET_PEER_PORT]: config?.connection?.port ?? utils.extractPortFromConnectionString(connectionString),
-            [ATTR_NET_TRANSPORT]: transport,
-          });
-        }
-        if (instrumentation._semconvStability & SemconvStability.STABLE) {
-          Object.assign(attributes, {
-            [ATTR_DB_SYSTEM_NAME]: utils.mapSystem(this.client.driverName),
-            [ATTR_DB_COLLECTION_NAME]: table,
-            [ATTR_DB_OPERATION_NAME]: operation,
-            [ATTR_DB_NAMESPACE]: name,
-            [ATTR_SERVER_ADDRESS]: config?.connection?.host ?? utils.extractHostFromConnectionString(connectionString),
-            [ATTR_SERVER_PORT]: config?.connection?.port ?? utils.extractPortFromConnectionString(connectionString),
-          });
-        }
-        if (maxQueryLength) {
-          const queryText = utils.limitLength(query?.sql, maxQueryLength);
-          if (instrumentation._semconvStability & SemconvStability.STABLE) {
-            attributes[ATTR_DB_QUERY_TEXT] = queryText;
-          }
-          if (instrumentation._semconvStability & SemconvStability.OLD) {
-            attributes[ATTR_DB_STATEMENT] = queryText;
-          }
-        }
 
+        // The query builder captures the context active when it was created (see `_storeContext`).
+        // We only instrument queries that run as part of an existing trace.
         const parentContext = this.builder[contextSymbol] || api.context.active();
         const parentSpan = api.trace.getSpan(parentContext);
         const hasActiveParent = parentSpan && api.trace.isSpanContextValid(parentSpan.spanContext());
-        if (instrumentation._config.requireParentSpan && !hasActiveParent) {
+        if (!hasActiveParent) {
           return original.bind(this)(...arguments);
         }
 
-        const span = instrumentation.tracer.startSpan(
-          utils.getName(name, operation, table),
-          {
-            kind: api.SpanKind.CLIENT,
-            attributes,
-          },
-          parentContext,
+        const args = arguments;
+        return api.context.with(parentContext, () =>
+          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<KnexInstrumentation
     };
   }
 
-  ensureWrapped(obj: any, methodName: string, wrapper: (original: any) => any) {
+  private _ensureWrapped(obj: any, methodName: string, wrapper: (original: any) => any): void {
     if (isWrapped(obj[methodName])) {
       this._unwrap(obj, methodName);
     }

packages/node/src/integrations/tracing/knex/vendored/instrumentation.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`.