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,17 +1,20 @@
import { afterAll, expect } from 'vitest';
import { isOrchestrionEnabled } from '../../../utils';
import { cleanupChildProcesses, createEsmAndCjsTests, describeWithDockerCompose } from '../../../utils/runner';

describeWithDockerCompose('tedious auto instrumentation', { workingDirectory: [__dirname] }, () => {
const ORIGIN = isOrchestrionEnabled() ? 'auto.db.orchestrion.tedious' : 'auto.db.otel.tedious';

afterAll(() => {
cleanupChildProcesses();
});

const dbSpan = (overrides: Record<string, unknown>) =>
expect.objectContaining({
op: 'db',
origin: 'auto.db.otel.tedious',
origin: ORIGIN,
data: expect.objectContaining({
'sentry.origin': 'auto.db.otel.tedious',
'sentry.origin': ORIGIN,
'sentry.op': 'db',
'db.system': 'mssql',
'db.name': 'master',
Expand All @@ -33,7 +36,7 @@ describeWithDockerCompose('tedious auto instrumentation', { workingDirectory: [_
expect.objectContaining({
description: 'execBulkLoad test_bulk master',
op: 'db',
origin: 'auto.db.otel.tedious',
origin: ORIGIN,
status: 'ok',
data: expect.objectContaining({ 'db.sql.table': 'test_bulk' }),
}),
Expand Down
260 changes: 260 additions & 0 deletions packages/server-utils/src/integrations/tracing-channel/tedious.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,260 @@
// The `@sentry/conventions` db/net attribute keys are deprecated (superseded by newer semconv), but we
// emit them deliberately to preserve parity with what `@opentelemetry/instrumentation-tedious` produced.
/* oxlint-disable typescript/no-deprecated */

import { EventEmitter } from 'node:events';
import * as diagnosticsChannel from 'node:diagnostics_channel';
import type { IntegrationFn, SpanAttributes } from '@sentry/core';
import {
debug,
defineIntegration,
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
SPAN_KIND,
SPAN_STATUS_ERROR,
startInactiveSpan,
waitForTracingChannelBinding,
} from '@sentry/core';
import {
DB_NAME,
DB_STATEMENT,
DB_SYSTEM,
DB_USER,
NET_PEER_NAME,
NET_PEER_PORT,
} from '@sentry/conventions/attributes';
import { DEBUG_BUILD } from '../../debug-build';
import { CHANNELS } from '../../orchestrion/channels';

// NOTE: this uses the same name as the OTel integration by design. When orchestrion injection is active,
// `_init` swaps the OTel `Tedious` integration out of the defaults and appends this one (matched by name).
const INTEGRATION_NAME = 'Tedious' as const;
const ORIGIN = 'auto.db.orchestrion.tedious';

// OTel db/net semantic-convention values/keys not exported by `@sentry/conventions`, inlined to match
// what `@opentelemetry/instrumentation-tedious` emitted.
const DB_SYSTEM_VALUE_MSSQL = 'mssql';
const ATTR_DB_SQL_TABLE = 'db.sql.table';

// Tracks the connection's active database (updated on `databaseChange`), read into `db.name` when a query
// runs. Mirrors the `CURRENT_DATABASE` symbol the vendored OTel instrumentation stashed on the connection.
const currentDatabaseSymbol = Symbol('sentry.orchestrion.tedious.current-database');

type UnknownFunction = (...args: unknown[]) => unknown;

interface TediousConnectionConfig {
server?: string;
userName?: string;
authentication?: { options?: { userName?: string } };
options?: { database?: string; port?: number };
}

interface TediousConnection extends EventEmitter {
config?: TediousConnectionConfig;
[currentDatabaseSymbol]?: string;
}

interface TediousRequest extends EventEmitter {
sqlTextOrProcedure?: string;
callback?: UnknownFunction;
table?: string;
parametersByName?: Record<string, { value?: unknown } | undefined>;
}

/** Context orchestrion attaches to the query channels (wrapping the `Connection` request methods). */
interface TediousQueryChannelContext {
// `arguments[0]` is the `Request` (or `BulkLoad` for `execBulkLoad`), both `EventEmitter`s.
arguments: [TediousRequest?, ...unknown[]];
self?: TediousConnection;
moduleVersion?: string;
}

/** Context orchestrion attaches to the `Connection.connect` channel. */
interface TediousConnectChannelContext {
arguments: unknown[];
self?: TediousConnection;
}

// Used both to seed the initial database and as the `databaseChange` listener, where `this` is the
// connection (a non-arrow listener). Keeping one shared reference lets `removeListener` find it again.
function setDatabase(this: TediousConnection, databaseName: string | undefined): void {
Object.defineProperty(this, currentDatabaseSymbol, { value: databaseName, writable: true, configurable: true });
}

// The `end` cleanup listener, where `this` is the connection (a non-arrow listener). Named (like
// `setDatabase`) so repeated `connect` calls can `removeListener` it rather than accumulate anonymous ones.
function removeDatabaseListener(this: TediousConnection): void {
this.removeListener('databaseChange', setDatabase);
}

function subscribeConnect(): void {
diagnosticsChannel.tracingChannel(CHANNELS.TEDIOUS_CONNECT).start.subscribe(message => {
const connection = (message as TediousConnectChannelContext).self;
if (!connection) {
return;
}

setDatabase.call(connection, connection.config?.options?.database);

// Remove first in case `connect` runs more than once on the same connection, so neither listener
// accumulates across reconnects.
connection.removeListener('databaseChange', setDatabase);
connection.on('databaseChange', setDatabase);
connection.removeListener('end', removeDatabaseListener);
connection.once('end', removeDatabaseListener);
});
}

function subscribeQuery(channelName: string, operation: string): void {
diagnosticsChannel.tracingChannel(channelName).start.subscribe(message => {
const data = message as TediousQueryChannelContext;
const connection = data.self;
const request = data.arguments[0];

// The vendored instrumentation only traced when the first argument is an `EventEmitter` (a `Request`
// or `BulkLoad`); anything else is left untouched.
if (!connection || !(request instanceof EventEmitter)) {
return;
}

let procCount = 0;
let statementCount = 0;
const incrementStatementCount = (): void => {
statementCount++;
};
const incrementProcCount = (): void => {
procCount++;
};

const databaseName = connection[currentDatabaseSymbol];
const sql = extractSql(request);

const attributes: SpanAttributes = {
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: ORIGIN,
[DB_SYSTEM]: DB_SYSTEM_VALUE_MSSQL,
[DB_NAME]: databaseName,
// `>=4` uses the `authentication` object; older versions expose `userName` directly.
[DB_USER]: connection.config?.userName ?? connection.config?.authentication?.options?.userName,
[DB_STATEMENT]: sql,
[ATTR_DB_SQL_TABLE]: request.table,
[NET_PEER_NAME]: connection.config?.server,
[NET_PEER_PORT]: connection.config?.options?.port,
};

const span = startInactiveSpan({
name: getSpanName(operation, databaseName, sql, request.table),
kind: SPAN_KIND.CLIENT,
attributes,
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Missing db span op attribute

Medium Severity

startInactiveSpan sets sentry.origin but never sets a span op (op: 'db' or SEMANTIC_ATTRIBUTE_SENTRY_OP). Sibling orchestrion DB integrations set this explicitly so spans stay correct outside the Node OTel inference path. Flagged because the PR review guidelines require both origin and op on every startSpan API call.

Fix in Cursor Fix in Web

Triggered by project rule: PR Review Guidelines for Cursor Bot

Reviewed by Cursor Bugbot for commit 9a144dd. Configure here.

Comment on lines +143 to +147

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: The startInactiveSpan call for the tedious integration is missing the op: 'db' field, which is present in other database integrations like mysql and postgres.
Severity: MEDIUM

Suggested Fix

Add op: 'db' to the options object passed to the startInactiveSpan function in tedious.ts, similar to how it is done in mysql.ts and postgres.ts. This will ensure consistency across database integrations.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: packages/server-utils/src/integrations/tracing-channel/tedious.ts#L143-L147

Potential issue: In `tedious.ts`, the call to `startInactiveSpan` does not include the
`op: 'db'` field in its options. This is inconsistent with other database integrations
like `mysql.ts` and `postgres.ts`, which explicitly set this field. The associated tests
expect the `op` field to be present on the generated spans. Its absence will cause test
failures and may affect how these spans are processed or displayed, as they will be
missing a key piece of contextual information that identifies them as database
operations.


const endSpan = once((err?: { message?: string }): void => {
request.removeListener('done', incrementStatementCount);
request.removeListener('doneInProc', incrementStatementCount);
request.removeListener('doneProc', incrementProcCount);
request.removeListener('error', endSpan);
connection.removeListener('end', endSpan);

span.setAttribute('tedious.procedure_count', procCount);
span.setAttribute('tedious.statement_count', statementCount);
if (err) {
span.setStatus({ code: SPAN_STATUS_ERROR, message: err.message });
}

span.end();
});

request.on('done', incrementStatementCount);
request.on('doneInProc', incrementStatementCount);
request.on('doneProc', incrementProcCount);
request.once('error', endSpan);
connection.on('end', endSpan);

// tedious invokes `request.callback` when the request settles (passing the error, if any). Wrapping it
// here (at `start`, before the method body dispatches) is the completion signal. A failed non-preparing
// request reports its error only through this callback, not via an `'error'` event.
if (typeof request.callback === 'function') {
const originalCallback = request.callback;
request.callback = function (this: unknown, ...args: unknown[]): unknown {
endSpan(args[0] as { message?: string } | undefined);

return originalCallback.apply(this, args);
};
}
});
}

function extractSql(request: TediousRequest): string | undefined {
// Required for <11.0.9: the SQL for a prepared statement is carried in the `stmt` parameter.
if (request.sqlTextOrProcedure === 'sp_prepare' && request.parametersByName?.stmt?.value != null) {
const value = request.parametersByName.stmt.value;

return typeof value === 'string' ? value : undefined;
}

return request.sqlTextOrProcedure;
}

/**
* The span name is a low-cardinality label for the operation; the SDK's db-span inference later renames
* the span description off `db.statement` when present. Mirrors the vendored OTel `getSpanName`.
*/
function getSpanName(
operation: string,
db: string | undefined,
sql: string | undefined,
bulkLoadTable: string | undefined,
): string {
if (operation === 'execBulkLoad' && bulkLoadTable && db) {
return `${operation} ${bulkLoadTable} ${db}`;
}
if (operation === 'callProcedure') {
// `sql` refers to the procedure name for `callProcedure`.
return db ? `${operation} ${sql} ${db}` : `${operation} ${sql}`;
}
// Avoid `sql` in the general case because of its high cardinality.
return db ? `${operation} ${db}` : operation;
}

function once<Args extends unknown[]>(fn: (...args: Args) => void): (...args: Args) => void {
let called = false;

return (...args: Args): void => {
if (called) {
return;
}
called = true;
fn(...args);
};
}

const _tediousChannelIntegration = (() => {
return {
name: INTEGRATION_NAME,
setupOnce() {
// `tracingChannel` is unavailable before Node 18.19 so do nothing in that case.
if (!diagnosticsChannel.tracingChannel) {
return;
}

DEBUG_BUILD && debug.log(`[orchestrion:tedious] subscribing to channel "${CHANNELS.TEDIOUS_EXEC_SQL}"`);

waitForTracingChannelBinding(() => {
subscribeConnect();
subscribeQuery(CHANNELS.TEDIOUS_EXEC_SQL, 'execSql');
subscribeQuery(CHANNELS.TEDIOUS_EXEC_SQL_BATCH, 'execSqlBatch');
subscribeQuery(CHANNELS.TEDIOUS_CALL_PROCEDURE, 'callProcedure');
subscribeQuery(CHANNELS.TEDIOUS_EXEC_BULK_LOAD, 'execBulkLoad');
subscribeQuery(CHANNELS.TEDIOUS_PREPARE, 'prepare');
subscribeQuery(CHANNELS.TEDIOUS_EXECUTE, 'execute');
});
},
};
}) satisfies IntegrationFn;

/**
* EXPERIMENTAL - orchestrion-driven tedious integration.
*
* Subscribes to the `orchestrion:tedious:*` diagnostics_channels that the orchestrion code transform
* injects into tedious's `Connection` request methods (each traced as one db span) and `Connection.connect`
* (active-database bookkeeping). Requires the orchestrion runtime hook or bundler plugin to be active.
*/
export const tediousChannelIntegration = defineIntegration(_tediousChannelIntegration);
31 changes: 28 additions & 3 deletions packages/server-utils/src/orchestrion/config/tedious.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,31 @@
import type { InstrumentationConfig } from '@apm-js-collab/code-transformer';

// TODO: Stub for the `tedious` orchestrion integration (ports `@opentelemetry/instrumentation-tedious`).
export const tediousConfig: InstrumentationConfig[] = [];
const MODULE_NAME = 'tedious';

export const tediousChannels = {} as const;
// `Connection` has lived in `lib/connection.js` across the whole supported range (matches the vendored
// OTel `supportedVersions`). Orchestrion never matches a file that doesn't exist, so a single entry is
// safe even for versions that shipped extra layouts.
const FILE_PATH = 'lib/connection.js';
const VERSION_RANGE = '>=1.11.0 <20';

// `Connection` methods that dispatch a request (each traced as one db span) plus `connect`, which the
// subscriber wraps for bookkeeping only (tracking the connection's active database, read into `db.name`).
// All return synchronously; the request completes later via its callback/events, so the subscriber owns
// span-ending rather than the channel lifecycle.
const METHODS = ['connect', 'execSql', 'execSqlBatch', 'callProcedure', 'execBulkLoad', 'prepare', 'execute'] as const;

export const tediousConfig: InstrumentationConfig[] = METHODS.map(methodName => ({
channelName: methodName,
module: { name: MODULE_NAME, versionRange: VERSION_RANGE, filePath: FILE_PATH },
functionQuery: { className: 'Connection', methodName, kind: 'Sync' },
}));

export const tediousChannels = {
TEDIOUS_CONNECT: 'orchestrion:tedious:connect',
TEDIOUS_EXEC_SQL: 'orchestrion:tedious:execSql',
TEDIOUS_EXEC_SQL_BATCH: 'orchestrion:tedious:execSqlBatch',
TEDIOUS_CALL_PROCEDURE: 'orchestrion:tedious:callProcedure',
TEDIOUS_EXEC_BULK_LOAD: 'orchestrion:tedious:execBulkLoad',
TEDIOUS_PREPARE: 'orchestrion:tedious:prepare',
TEDIOUS_EXECUTE: 'orchestrion:tedious:execute',
} as const;
3 changes: 3 additions & 0 deletions packages/server-utils/src/orchestrion/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { mysqlChannelIntegration } from '../integrations/tracing-channel/mysql';
import { openaiChannelIntegration } from '../integrations/tracing-channel/openai';
import { postgresChannelIntegration } from '../integrations/tracing-channel/postgres';
import { postgresJsChannelIntegration } from '../integrations/tracing-channel/postgres-js';
import { tediousChannelIntegration } from '../integrations/tracing-channel/tedious';
import { vercelAiChannelIntegration } from '../integrations/tracing-channel/vercel-ai';
import { expressChannelIntegration } from '../integrations/tracing-channel/express';

Expand All @@ -35,6 +36,7 @@ export {
openaiChannelIntegration,
postgresChannelIntegration,
postgresJsChannelIntegration,
tediousChannelIntegration,
vercelAiChannelIntegration,
expressChannelIntegration,
};
Expand Down Expand Up @@ -79,4 +81,5 @@ export const channelIntegrations = {
expressIntegration: expressChannelIntegration,
graphqlIntegration: graphqlDiagnosticsChannelIntegration,
kafkajsIntegration: kafkajsChannelIntegration,
tediousIntegration: tediousChannelIntegration,
} as const;
Loading