Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { describe, expect } from 'vitest';
import { isOrchestrionEnabled } from '../../../../utils';
import { createEsmAndCjsTests, describeWithDockerCompose } from '../../../../utils/runner';

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

describe('with `mysql2` client', () => {
createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => {
Expand All @@ -17,30 +19,30 @@ describeWithDockerCompose('knex auto instrumentation', { workingDirectory: [__di
'db.system': 'mysql2',
'db.name': 'tests',
'db.user': 'root',
'sentry.origin': 'auto.db.otel.knex',
'sentry.origin': ORIGIN,
'sentry.op': 'db',
'net.peer.name': 'localhost',
'net.peer.port': 3307,
}),
status: 'ok',
description:
'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)',
origin: 'auto.db.otel.knex',
origin: ORIGIN,
}),
expect.objectContaining({
data: expect.objectContaining({
'knex.version': KNEX_VERSION,
'db.system': 'mysql2',
'db.name': 'tests',
'db.user': 'root',
'sentry.origin': 'auto.db.otel.knex',
'sentry.origin': ORIGIN,
'sentry.op': 'db',
'net.peer.name': 'localhost',
'net.peer.port': 3307,
}),
status: 'ok',
description: 'insert into `User` (`email`, `name`) values (?, ?)',
origin: 'auto.db.otel.knex',
origin: ORIGIN,
}),

expect.objectContaining({
Expand All @@ -52,12 +54,12 @@ describeWithDockerCompose('knex auto instrumentation', { workingDirectory: [__di
'db.name': 'tests',
'db.statement': 'select * from `User`',
'db.user': 'root',
'sentry.origin': 'auto.db.otel.knex',
'sentry.origin': ORIGIN,
'sentry.op': 'db',
}),
status: 'ok',
description: 'select * from `User`',
origin: 'auto.db.otel.knex',
origin: ORIGIN,
}),
]),
};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { describe, expect } from 'vitest';
import { isOrchestrionEnabled } from '../../../../utils';
import { createEsmAndCjsTests, describeWithDockerCompose } from '../../../../utils/runner';

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

describeWithDockerCompose('with `pg` client', { workingDirectory: [__dirname] }, () => {
createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => {
Expand All @@ -16,30 +18,30 @@ describe('knex auto instrumentation', () => {
'knex.version': KNEX_VERSION,
'db.system': 'postgresql',
'db.name': 'tests',
'sentry.origin': 'auto.db.otel.knex',
'sentry.origin': ORIGIN,
'sentry.op': 'db',
'net.peer.name': 'localhost',
'net.peer.port': 5445,
}),
status: 'ok',
description:
'create table "User" ("id" serial primary key, "createdAt" timestamptz(3) not null default CURRENT_TIMESTAMP(3), "email" text not null, "name" text not null)',
origin: 'auto.db.otel.knex',
origin: ORIGIN,
}),
expect.objectContaining({
data: expect.objectContaining({
'knex.version': KNEX_VERSION,
'db.system': 'postgresql',
'db.name': 'tests',
'sentry.origin': 'auto.db.otel.knex',
'sentry.origin': ORIGIN,
'sentry.op': 'db',
'net.peer.name': 'localhost',
'net.peer.port': 5445,
}),
status: 'ok',
// In the knex-otel spans, the placeholders (e.g., `$1`) are replaced by a `?`.
description: 'insert into "User" ("email", "name") values (?, ?)',
origin: 'auto.db.otel.knex',
origin: ORIGIN,
}),

expect.objectContaining({
Expand All @@ -50,12 +52,12 @@ describe('knex auto instrumentation', () => {
'db.system': 'postgresql',
'db.name': 'tests',
'db.statement': 'select * from "User"',
'sentry.origin': 'auto.db.otel.knex',
'sentry.origin': ORIGIN,
'sentry.op': 'db',
}),
status: 'ok',
description: 'select * from "User"',
origin: 'auto.db.otel.knex',
origin: ORIGIN,
}),

expect.objectContaining({
Expand All @@ -66,12 +68,12 @@ describe('knex auto instrumentation', () => {
'db.system': 'postgresql',
'db.name': 'tests',
'db.statement': 'select * from "DoesNotExist"',
'sentry.origin': 'auto.db.otel.knex',
'sentry.origin': ORIGIN,
'sentry.op': 'db',
}),
status: 'internal_error',
description: 'select * from "DoesNotExist"',
origin: 'auto.db.otel.knex',
origin: ORIGIN,
}),
]),
};
Expand Down
1 change: 1 addition & 0 deletions packages/deno/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ export { denoMysqlIntegration } from './integrations/mysql';
export { denoPostgresIntegration } from './integrations/postgres';
export { denoAmqplibIntegration } from './integrations/amqplib';
export { denoDataloaderIntegration } from './integrations/dataloader';
export { denoKnexIntegration } from './integrations/knex';
export { denoContextIntegration } from './integrations/context';
export { globalHandlersIntegration } from './integrations/globalhandlers';
export { normalizePathsIntegration } from './integrations/normalizepaths';
Expand Down
34 changes: 34 additions & 0 deletions packages/deno/src/integrations/knex.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { knexChannelIntegration } from '@sentry/server-utils/orchestrion';
import type { Integration, IntegrationFn } from '@sentry/core';
import { defineIntegration, extendIntegration } from '@sentry/core';
import { setAsyncLocalStorageAsyncContextStrategy } from '../async';

const INTEGRATION_NAME = 'DenoKnex' as const;

/**
* Create spans for `knex` queries under Deno.
*
* `knex` channels are injected by the orchestrion runtime hook at load time.
* The `@sentry/deno/import` loader must be active for this integration to
* record anything.
*
* The channel-subscription logic is shared with the other server runtimes in
* `@sentry/server-utils`. This just installs Deno's
* `AsyncLocalStorage` context strategy (so spans nest under the active
* span and survive knex's internal callback dispatch) before delegating.
*/
const _denoKnexIntegration = (() => {
const inner = knexChannelIntegration();

return extendIntegration(inner, {
name: INTEGRATION_NAME,
setupOnce() {
setAsyncLocalStorageAsyncContextStrategy();
},
});
}) satisfies IntegrationFn;

export const denoKnexIntegration = defineIntegration(_denoKnexIntegration) as () => Integration & {
name: 'DenoKnex';
setupOnce: () => void;
};
10 changes: 9 additions & 1 deletion packages/node/src/integrations/tracing/knex/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { KnexInstrumentation } from './vendored/instrumentation';
import type { IntegrationFn } from '@sentry/core';
import { defineIntegration } from '@sentry/core';
import { generateInstrumentOnce } from '@sentry/node-core';
import { isOrchestrionInjected, knexChannelIntegration } from '@sentry/server-utils/orchestrion';

const INTEGRATION_NAME = 'Knex' as const;

Expand All @@ -11,7 +12,14 @@ const _knexIntegration = (() => {
return {
name: INTEGRATION_NAME,
setupOnce() {
instrumentKnex();
// Prefer the diagnostics-channel subscriber when orchestrion injected its channels; otherwise
// fall back to the vendored OTel instrumentation. `isOrchestrionInjected()` is only reliable by
// `setupOnce` (the runtime injection runs during `Sentry.init()`, after integrations are built).
if (isOrchestrionInjected()) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this needed, I think other integrations (at least the ones I migrated) did not use this pattern?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, because this is not a default integration so it wouldn't be swapped in by the existing mechanism we have.

It is similar to the one in #20749 and NestJS

knexChannelIntegration().setupOnce?.();
Comment thread
cursor[bot] marked this conversation as resolved.
} else {
instrumentKnex();
}
},
};
}) satisfies IntegrationFn;
Expand Down
Loading
Loading