Skip to content

Commit fb7616a

Browse files
nicohrubecclaude
nicohrubec
and
claude
committed
ref(node): Streamline tedious instrumentation
Move vendored tedious span creation onto Sentry's span APIs (startInactiveSpan/withActiveSpan), fold the origin into creation, drop the spanStart hook + semconv-stability branching, and expand+un-skip the integration suite to cover all patched methods + the error path. Adds a getSpanName unit test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent e408422 commit fb7616a

6 files changed

Lines changed: 167 additions & 150 deletions

File tree

‎dev-packages/node-integration-tests/suites/tracing/tedious/scenario.mjs‎

Lines changed: 84 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import * as Sentry from '@sentry/node';
2-
import { Connection, Request } from 'tedious';
2+
import { Connection, Request, TYPES } from 'tedious';
33

44
const config = {
55
server: '127.0.0.1',
@@ -13,44 +13,99 @@ const config = {
1313
options: {
1414
port: 1433,
1515
encrypt: false,
16+
rowCollectionOnRequestCompletion: true,
1617
},
1718
};
1819

19-
const connection = new Connection(config);
20+
const PROCEDURE_NAME = '[dbo].[test_proced]';
21+
const PREPARED_TABLE = '[dbo].[test_prepared]';
22+
const BULK_TABLE = 'test_bulk';
2023

21-
function executeAllStatements(span) {
22-
executeStatement('SELECT 1 + 1 AS solution', () => {
23-
executeStatement('SELECT GETDATE()', () => {
24-
span.end();
25-
connection.close();
24+
function connect() {
25+
return new Promise((resolve, reject) => {
26+
const connection = new Connection(config);
27+
connection.on('connect', err => (err ? reject(err) : resolve(connection)));
28+
if (connection.state !== connection.STATE.CONNECTING) {
29+
connection.connect();
30+
}
31+
});
32+
}
33+
34+
function query(connection, sql, method = 'execSql') {
35+
return new Promise((resolve, reject) => {
36+
const request = new Request(sql, err => (err ? reject(err) : resolve()));
37+
connection[method](request);
38+
});
39+
}
40+
41+
function callProcedure(connection) {
42+
return new Promise((resolve, reject) => {
43+
const request = new Request(PROCEDURE_NAME, err => (err ? reject(err) : resolve()));
44+
request.addParameter('inputVal', TYPES.VarChar, 'hello world');
45+
request.addOutputParameter('outputCount', TYPES.Int);
46+
connection.callProcedure(request);
47+
});
48+
}
49+
50+
function prepare(connection) {
51+
return new Promise((resolve, reject) => {
52+
const request = new Request(`INSERT INTO ${PREPARED_TABLE} VALUES (@val1, @val2)`, err => {
53+
if (err) reject(err);
2654
});
55+
request.addParameter('val1', TYPES.Int);
56+
request.addParameter('val2', TYPES.Int);
57+
request.on('prepared', () => resolve(request));
58+
connection.prepare(request);
2759
});
2860
}
2961

30-
function executeStatement(query, callback) {
31-
const request = new Request(query, err => {
32-
if (err) {
33-
throw err;
34-
}
35-
callback();
62+
function execute(connection, request) {
63+
return new Promise((resolve, reject) => {
64+
request.on('error', reject);
65+
request.on('requestCompleted', () => resolve());
66+
connection.execute(request, { val1: 1, val2: 2 });
3667
});
68+
}
3769

38-
connection.execSql(request);
70+
function bulkLoad(connection) {
71+
return new Promise((resolve, reject) => {
72+
const request = connection.newBulkLoad(BULK_TABLE, { keepNulls: true }, err => (err ? reject(err) : resolve()));
73+
request.addColumn('c1', TYPES.Int, { nullable: true });
74+
request.addColumn('c2', TYPES.NVarChar, { length: 50, nullable: true });
75+
connection.execBulkLoad(request, [{ c1: 1 }, { c1: 2, c2: 'hello' }]);
76+
});
3977
}
4078

41-
connection.connect(err => {
42-
if (err) {
43-
throw err;
44-
}
79+
async function run() {
80+
const connection = await connect();
4581

46-
Sentry.startSpanManual(
47-
{
48-
op: 'transaction',
49-
name: 'Test Transaction',
50-
},
51-
span => {
52-
// span must be ended manually after all queries
53-
executeAllStatements(span);
54-
},
55-
);
56-
});
82+
await Sentry.startSpan({ name: 'Test Transaction' }, async () => {
83+
await query(connection, 'SELECT 1 + 1 AS solution');
84+
await query(connection, 'SELECT 42; SELECT 42;', 'execSqlBatch');
85+
86+
await query(connection, 'select !').catch(() => {});
87+
88+
await query(
89+
connection,
90+
`CREATE OR ALTER PROCEDURE ${PROCEDURE_NAME} @inputVal varchar(30), @outputCount int OUTPUT AS set @outputCount = LEN(@inputVal);`,
91+
);
92+
await callProcedure(connection);
93+
94+
await query(
95+
connection,
96+
`if object_id('${PREPARED_TABLE}') is null CREATE TABLE ${PREPARED_TABLE} (c1 int, c2 int)`,
97+
);
98+
const prepared = await prepare(connection);
99+
await execute(connection, prepared);
100+
101+
await query(
102+
connection,
103+
`if object_id('[dbo].[${BULK_TABLE}]') is null CREATE TABLE [dbo].[${BULK_TABLE}] (c1 int, c2 varchar(30))`,
104+
);
105+
await bulkLoad(connection);
106+
});
107+
108+
connection.close();
109+
}
110+
111+
run();

‎dev-packages/node-integration-tests/suites/tracing/tedious/test.ts‎

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,41 +1,41 @@
11
import { afterAll, describe, expect } from 'vitest';
22
import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../utils/runner';
33

4-
describe.skip('tedious auto instrumentation', { timeout: 75_000 }, () => {
4+
describe('tedious auto instrumentation', { timeout: 90_000 }, () => {
55
afterAll(() => {
66
cleanupChildProcesses();
77
});
88

9+
const dbSpan = (overrides: Record<string, unknown>) =>
10+
expect.objectContaining({
11+
op: 'db',
12+
origin: 'auto.db.otel.tedious',
13+
data: expect.objectContaining({
14+
'sentry.origin': 'auto.db.otel.tedious',
15+
'sentry.op': 'db',
16+
'db.system': 'mssql',
17+
'db.name': 'master',
18+
'db.user': 'sa',
19+
'net.peer.name': '127.0.0.1',
20+
'net.peer.port': 1433,
21+
}),
22+
...overrides,
23+
});
24+
925
const EXPECTED_TRANSACTION = {
1026
transaction: 'Test Transaction',
1127
spans: expect.arrayContaining([
28+
dbSpan({ description: 'SELECT 1 + 1 AS solution', status: 'ok' }),
29+
dbSpan({ description: 'SELECT 42; SELECT 42;', status: 'ok' }),
30+
dbSpan({ description: 'select !', status: 'internal_error' }),
31+
dbSpan({ description: '[dbo].[test_proced]', status: 'ok' }),
32+
dbSpan({ description: 'INSERT INTO [dbo].[test_prepared] VALUES (@val1, @val2)', status: 'ok' }),
1233
expect.objectContaining({
13-
description: 'SELECT GETDATE()',
14-
data: expect.objectContaining({
15-
'sentry.origin': 'auto.db.otel.tedious',
16-
'sentry.op': 'db',
17-
'db.name': 'master',
18-
'db.statement': 'SELECT GETDATE()',
19-
'db.system': 'mssql',
20-
'db.user': 'sa',
21-
'net.peer.name': '127.0.0.1',
22-
'net.peer.port': 1433,
23-
}),
24-
status: 'ok',
25-
}),
26-
expect.objectContaining({
27-
description: 'SELECT 1 + 1 AS solution',
28-
data: expect.objectContaining({
29-
'sentry.origin': 'auto.db.otel.tedious',
30-
'sentry.op': 'db',
31-
'db.name': 'master',
32-
'db.statement': 'SELECT 1 + 1 AS solution',
33-
'db.system': 'mssql',
34-
'db.user': 'sa',
35-
'net.peer.name': '127.0.0.1',
36-
'net.peer.port': 1433,
37-
}),
34+
description: 'execBulkLoad test_bulk master',
35+
op: 'db',
36+
origin: 'auto.db.otel.tedious',
3837
status: 'ok',
38+
data: expect.objectContaining({ 'db.sql.table': 'test_bulk' }),
3939
}),
4040
]),
4141
};

‎packages/node/src/integrations/tracing/tedious/index.ts‎

Lines changed: 3 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,46 +1,17 @@
11
import { TediousInstrumentation } from './vendored/instrumentation';
22
import type { IntegrationFn } from '@sentry/core';
3-
import { defineIntegration, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, spanToJSON } from '@sentry/core';
4-
import { generateInstrumentOnce, instrumentWhenWrapped } from '@sentry/node-core';
5-
6-
const TEDIUS_INSTRUMENTED_METHODS = new Set([
7-
'callProcedure',
8-
'execSql',
9-
'execSqlBatch',
10-
'execBulkLoad',
11-
'prepare',
12-
'execute',
13-
]);
3+
import { defineIntegration } from '@sentry/core';
4+
import { generateInstrumentOnce } from '@sentry/node-core';
145

156
const INTEGRATION_NAME = 'Tedious';
167

178
export const instrumentTedious = generateInstrumentOnce(INTEGRATION_NAME, () => new TediousInstrumentation({}));
189

1910
const _tediousIntegration = (() => {
20-
let instrumentationWrappedCallback: undefined | ((callback: () => void) => void);
21-
2211
return {
2312
name: INTEGRATION_NAME,
2413
setupOnce() {
25-
const instrumentation = instrumentTedious();
26-
instrumentationWrappedCallback = instrumentWhenWrapped(instrumentation);
27-
},
28-
29-
setup(client) {
30-
instrumentationWrappedCallback?.(() =>
31-
client.on('spanStart', span => {
32-
const { description, data } = spanToJSON(span);
33-
// Tedius integration always set a span name and `db.system` attribute to `mssql`.
34-
if (!description || data['db.system'] !== 'mssql') {
35-
return;
36-
}
37-
38-
const operation = description.split(' ')[0] || '';
39-
if (TEDIUS_INSTRUMENTED_METHODS.has(operation)) {
40-
span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto.db.otel.tedious');
41-
}
42-
}),
43-
);
14+
instrumentTedious();
4415
},
4516
};
4617
}) satisfies IntegrationFn;

0 commit comments

Comments
 (0)