Skip to content

Commit 2cbacb5

Browse files
authored
feat(node): Rewrite knex instrumentation to orchestrion tracing channels (#22237)
Migrates the knex integration off the vendored `InstrumentationBase` monkey-patch onto a `node:diagnostics_channel` subscriber whose channels are injected by the orchestrion code transform. The vendored OTel path stays as the fallback when orchestrion isn't injected. closes #20757
1 parent c9716eb commit 2cbacb5

8 files changed

Lines changed: 428 additions & 18 deletions

File tree

dev-packages/node-integration-tests/suites/tracing/knex/mysql2/test.ts

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
import { describe, expect } from 'vitest';
2+
import { isOrchestrionEnabled } from '../../../../utils';
23
import { createEsmAndCjsTests, describeWithDockerCompose } from '../../../../utils/runner';
34

45
describeWithDockerCompose('knex auto instrumentation', { workingDirectory: [__dirname] }, () => {
56
// Update this if another knex version is installed
67
const KNEX_VERSION = '2.5.1';
8+
const ORIGIN = isOrchestrionEnabled() ? 'auto.db.orchestrion.knex' : 'auto.db.otel.knex';
79

810
describe('with `mysql2` client', () => {
911
createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => {
@@ -17,30 +19,30 @@ describeWithDockerCompose('knex auto instrumentation', { workingDirectory: [__di
1719
'db.system': 'mysql2',
1820
'db.name': 'tests',
1921
'db.user': 'root',
20-
'sentry.origin': 'auto.db.otel.knex',
22+
'sentry.origin': ORIGIN,
2123
'sentry.op': 'db',
2224
'net.peer.name': 'localhost',
2325
'net.peer.port': 3307,
2426
}),
2527
status: 'ok',
2628
description:
2729
'create table `User` (`id` int unsigned not null auto_increment primary key, `createdAt` timestamp(3) not null default CURRENT_TIMESTAMP(3), `email` text not null, `name` text not null)',
28-
origin: 'auto.db.otel.knex',
30+
origin: ORIGIN,
2931
}),
3032
expect.objectContaining({
3133
data: expect.objectContaining({
3234
'knex.version': KNEX_VERSION,
3335
'db.system': 'mysql2',
3436
'db.name': 'tests',
3537
'db.user': 'root',
36-
'sentry.origin': 'auto.db.otel.knex',
38+
'sentry.origin': ORIGIN,
3739
'sentry.op': 'db',
3840
'net.peer.name': 'localhost',
3941
'net.peer.port': 3307,
4042
}),
4143
status: 'ok',
4244
description: 'insert into `User` (`email`, `name`) values (?, ?)',
43-
origin: 'auto.db.otel.knex',
45+
origin: ORIGIN,
4446
}),
4547

4648
expect.objectContaining({
@@ -52,12 +54,12 @@ describeWithDockerCompose('knex auto instrumentation', { workingDirectory: [__di
5254
'db.name': 'tests',
5355
'db.statement': 'select * from `User`',
5456
'db.user': 'root',
55-
'sentry.origin': 'auto.db.otel.knex',
57+
'sentry.origin': ORIGIN,
5658
'sentry.op': 'db',
5759
}),
5860
status: 'ok',
5961
description: 'select * from `User`',
60-
origin: 'auto.db.otel.knex',
62+
origin: ORIGIN,
6163
}),
6264
]),
6365
};

dev-packages/node-integration-tests/suites/tracing/knex/pg/test.ts

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
import { describe, expect } from 'vitest';
2+
import { isOrchestrionEnabled } from '../../../../utils';
23
import { createEsmAndCjsTests, describeWithDockerCompose } from '../../../../utils/runner';
34

45
describe('knex auto instrumentation', () => {
56
// Update this if another knex version is installed
67
const KNEX_VERSION = '2.5.1';
8+
const ORIGIN = isOrchestrionEnabled() ? 'auto.db.orchestrion.knex' : 'auto.db.otel.knex';
79

810
describeWithDockerCompose('with `pg` client', { workingDirectory: [__dirname] }, () => {
911
createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => {
@@ -16,30 +18,30 @@ describe('knex auto instrumentation', () => {
1618
'knex.version': KNEX_VERSION,
1719
'db.system': 'postgresql',
1820
'db.name': 'tests',
19-
'sentry.origin': 'auto.db.otel.knex',
21+
'sentry.origin': ORIGIN,
2022
'sentry.op': 'db',
2123
'net.peer.name': 'localhost',
2224
'net.peer.port': 5445,
2325
}),
2426
status: 'ok',
2527
description:
2628
'create table "User" ("id" serial primary key, "createdAt" timestamptz(3) not null default CURRENT_TIMESTAMP(3), "email" text not null, "name" text not null)',
27-
origin: 'auto.db.otel.knex',
29+
origin: ORIGIN,
2830
}),
2931
expect.objectContaining({
3032
data: expect.objectContaining({
3133
'knex.version': KNEX_VERSION,
3234
'db.system': 'postgresql',
3335
'db.name': 'tests',
34-
'sentry.origin': 'auto.db.otel.knex',
36+
'sentry.origin': ORIGIN,
3537
'sentry.op': 'db',
3638
'net.peer.name': 'localhost',
3739
'net.peer.port': 5445,
3840
}),
3941
status: 'ok',
4042
// In the knex-otel spans, the placeholders (e.g., `$1`) are replaced by a `?`.
4143
description: 'insert into "User" ("email", "name") values (?, ?)',
42-
origin: 'auto.db.otel.knex',
44+
origin: ORIGIN,
4345
}),
4446

4547
expect.objectContaining({
@@ -50,12 +52,12 @@ describe('knex auto instrumentation', () => {
5052
'db.system': 'postgresql',
5153
'db.name': 'tests',
5254
'db.statement': 'select * from "User"',
53-
'sentry.origin': 'auto.db.otel.knex',
55+
'sentry.origin': ORIGIN,
5456
'sentry.op': 'db',
5557
}),
5658
status: 'ok',
5759
description: 'select * from "User"',
58-
origin: 'auto.db.otel.knex',
60+
origin: ORIGIN,
5961
}),
6062

6163
expect.objectContaining({
@@ -66,12 +68,12 @@ describe('knex auto instrumentation', () => {
6668
'db.system': 'postgresql',
6769
'db.name': 'tests',
6870
'db.statement': 'select * from "DoesNotExist"',
69-
'sentry.origin': 'auto.db.otel.knex',
71+
'sentry.origin': ORIGIN,
7072
'sentry.op': 'db',
7173
}),
7274
status: 'internal_error',
7375
description: 'select * from "DoesNotExist"',
74-
origin: 'auto.db.otel.knex',
76+
origin: ORIGIN,
7577
}),
7678
]),
7779
};

packages/deno/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ export { denoMysqlIntegration } from './integrations/mysql';
115115
export { denoPostgresIntegration } from './integrations/postgres';
116116
export { denoAmqplibIntegration } from './integrations/amqplib';
117117
export { denoDataloaderIntegration } from './integrations/dataloader';
118+
export { denoKnexIntegration } from './integrations/knex';
118119
export { denoContextIntegration } from './integrations/context';
119120
export { globalHandlersIntegration } from './integrations/globalhandlers';
120121
export { normalizePathsIntegration } from './integrations/normalizepaths';
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import { knexChannelIntegration } from '@sentry/server-utils/orchestrion';
2+
import type { Integration, IntegrationFn } from '@sentry/core';
3+
import { defineIntegration, extendIntegration } from '@sentry/core';
4+
import { setAsyncLocalStorageAsyncContextStrategy } from '../async';
5+
6+
const INTEGRATION_NAME = 'DenoKnex' as const;
7+
8+
/**
9+
* Create spans for `knex` queries under Deno.
10+
*
11+
* `knex` channels are injected by the orchestrion runtime hook at load time.
12+
* The `@sentry/deno/import` loader must be active for this integration to
13+
* record anything.
14+
*
15+
* The channel-subscription logic is shared with the other server runtimes in
16+
* `@sentry/server-utils`. This just installs Deno's
17+
* `AsyncLocalStorage` context strategy (so spans nest under the active
18+
* span and survive knex's internal callback dispatch) before delegating.
19+
*/
20+
const _denoKnexIntegration = (() => {
21+
const inner = knexChannelIntegration();
22+
23+
return extendIntegration(inner, {
24+
name: INTEGRATION_NAME,
25+
setupOnce() {
26+
setAsyncLocalStorageAsyncContextStrategy();
27+
},
28+
});
29+
}) satisfies IntegrationFn;
30+
31+
export const denoKnexIntegration = defineIntegration(_denoKnexIntegration) as () => Integration & {
32+
name: 'DenoKnex';
33+
setupOnce: () => void;
34+
};

packages/node/src/integrations/tracing/knex/index.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { KnexInstrumentation } from './vendored/instrumentation';
22
import type { IntegrationFn } from '@sentry/core';
33
import { defineIntegration } from '@sentry/core';
44
import { generateInstrumentOnce } from '@sentry/node-core';
5+
import { isOrchestrionInjected, knexChannelIntegration } from '@sentry/server-utils/orchestrion';
56

67
const INTEGRATION_NAME = 'Knex' as const;
78

@@ -11,7 +12,14 @@ const _knexIntegration = (() => {
1112
return {
1213
name: INTEGRATION_NAME,
1314
setupOnce() {
14-
instrumentKnex();
15+
// Prefer the diagnostics-channel subscriber when orchestrion injected its channels; otherwise
16+
// fall back to the vendored OTel instrumentation. `isOrchestrionInjected()` is only reliable by
17+
// `setupOnce` (the runtime injection runs during `Sentry.init()`, after integrations are built).
18+
if (isOrchestrionInjected()) {
19+
knexChannelIntegration().setupOnce?.();
20+
} else {
21+
instrumentKnex();
22+
}
1523
},
1624
};
1725
}) satisfies IntegrationFn;

0 commit comments

Comments
 (0)