Skip to content

Commit d8dcc45

Browse files
nicohrubecclaude
andauthored
ref(node): Migrate vendored generic-pool instrumentation to Sentry APIs (#21523)
Migrate the vendored generic-pool instrumentation off the OTel tracer/context APIs to `startSpan`/`startSpanManual` from `@sentry/core`. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent b35c4de commit d8dcc45

5 files changed

Lines changed: 114 additions & 22 deletions

File tree

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
import * as Sentry from '@sentry/node';
2+
import genericPool from 'generic-pool';
3+
4+
// v2 uses the callback API; a `create` error reaches the `acquire` callback and the span is errored.
5+
const Pool = genericPool.Pool;
6+
7+
const pool = new Pool({
8+
name: 'test',
9+
create: callback => callback(new Error('Cannot create resource')),
10+
destroy: () => {},
11+
max: 10,
12+
min: 0,
13+
});
14+
15+
function acquire() {
16+
return new Promise(resolve => {
17+
pool.acquire(() => resolve());
18+
});
19+
}
20+
21+
async function run() {
22+
await Sentry.startSpan(
23+
{
24+
op: 'transaction',
25+
name: 'Test Transaction',
26+
},
27+
async () => {
28+
await acquire();
29+
},
30+
);
31+
}
32+
33+
run();

dev-packages/node-integration-tests/suites/tracing/genericPool-v2/test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,4 +40,30 @@ describe('genericPool v2 auto instrumentation', () => {
4040
},
4141
{ additionalDependencies: { 'generic-pool': '^2.5.0' } },
4242
);
43+
44+
createEsmAndCjsTests(
45+
__dirname,
46+
'scenario-error.mjs',
47+
'instrument.mjs',
48+
(createRunner, test) => {
49+
test('marks the `generic-pool.acquire` span as errored when acquiring fails', async () => {
50+
const EXPECTED_TRANSACTION = {
51+
transaction: 'Test Transaction',
52+
spans: expect.arrayContaining([
53+
expect.objectContaining({
54+
description: 'generic-pool.acquire',
55+
origin: 'auto.db.otel.generic_pool',
56+
data: {
57+
'sentry.origin': 'auto.db.otel.generic_pool',
58+
},
59+
status: 'internal_error',
60+
}),
61+
]),
62+
};
63+
64+
await createRunner().expect({ transaction: EXPECTED_TRANSACTION }).start().completed();
65+
});
66+
},
67+
{ additionalDependencies: { 'generic-pool': '^2.5.0' } },
68+
);
4369
});
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
import * as Sentry from '@sentry/node';
2+
import genericPool from 'generic-pool';
3+
4+
// `create` never resolves, so `acquire` rejects and the span is errored.
5+
const factory = {
6+
create: function () {
7+
return new Promise(() => {});
8+
},
9+
destroy: function () {
10+
return Promise.resolve();
11+
},
12+
};
13+
14+
const myPool = genericPool.createPool(factory, { max: 2, min: 0, acquireTimeoutMillis: 500 });
15+
16+
async function run() {
17+
await Sentry.startSpan(
18+
{
19+
op: 'transaction',
20+
name: 'Test Transaction',
21+
},
22+
async () => {
23+
await myPool.acquire().catch(() => {});
24+
},
25+
);
26+
}
27+
28+
run();

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,4 +34,24 @@ describe('genericPool auto instrumentation', () => {
3434
await createRunner().expect({ transaction: EXPECTED_TRANSACTION }).start().completed();
3535
});
3636
});
37+
38+
createEsmAndCjsTests(__dirname, 'scenario-error.mjs', 'instrument.mjs', (createRunner, test) => {
39+
test('marks the `generic-pool.acquire` span as errored when acquiring fails', async () => {
40+
const EXPECTED_TRANSACTION = {
41+
transaction: 'Test Transaction',
42+
spans: expect.arrayContaining([
43+
expect.objectContaining({
44+
description: 'generic-pool.acquire',
45+
origin: 'auto.db.otel.generic_pool',
46+
data: {
47+
'sentry.origin': 'auto.db.otel.generic_pool',
48+
},
49+
status: 'internal_error',
50+
}),
51+
]),
52+
};
53+
54+
await createRunner().expect({ transaction: EXPECTED_TRANSACTION }).start().completed();
55+
});
56+
});
3757
});

packages/node/src/integrations/tracing/genericPool/vendored/instrumentation.ts

Lines changed: 7 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,9 @@
1919
* - Minor TypeScript strictness adjustments for this repository's compiler settings
2020
*/
2121

22-
import * as api from '@opentelemetry/api';
2322
import type { InstrumentationConfig } from '@opentelemetry/instrumentation';
2423
import { InstrumentationBase, InstrumentationNodeModuleDefinition, isWrapped } from '@opentelemetry/instrumentation';
25-
import { SDK_VERSION } from '@sentry/core';
24+
import { SDK_VERSION, SPAN_STATUS_ERROR, startSpan, startSpanManual } from '@sentry/core';
2625
import type * as genericPool from './generic-pool-types';
2726

2827
const MODULE_NAME = 'generic-pool';
@@ -102,23 +101,9 @@ export class GenericPoolInstrumentation extends InstrumentationBase {
102101
}
103102

104103
private _acquirePatcher(original: AcquireFn) {
105-
const tracer = this.tracer;
106104
return function wrapped_acquire(this: genericPool.Pool<unknown>, ...args: unknown[]) {
107-
const parent = api.context.active();
108-
const span = tracer.startSpan('generic-pool.acquire', {}, parent);
109-
110-
return api.context.with(api.trace.setSpan(parent, span), () => {
111-
return (original.call(this, ...args) as PromiseLike<unknown>).then(
112-
(value: unknown) => {
113-
span.end();
114-
return value;
115-
},
116-
(err: unknown) => {
117-
span.recordException(err as Error);
118-
span.end();
119-
throw err;
120-
},
121-
);
105+
return startSpan({ name: 'generic-pool.acquire' }, () => {
106+
return original.call(this, ...args) as PromiseLike<unknown>;
122107
});
123108
};
124109
}
@@ -134,7 +119,6 @@ export class GenericPoolInstrumentation extends InstrumentationBase {
134119
}
135120

136121
private _acquireWithCallbacksPatcher(original: AcquireFn) {
137-
const tracer = this.tracer;
138122
const isDisabled = (): boolean => this._isDisabled;
139123
return function wrapped_acquire(
140124
this: genericPool.Pool<unknown>,
@@ -145,13 +129,14 @@ export class GenericPoolInstrumentation extends InstrumentationBase {
145129
if (isDisabled()) {
146130
return original.call(this, cb, priority);
147131
}
148-
const parent = api.context.active();
149-
const span = tracer.startSpan('generic-pool.acquire', {}, parent);
150132

151-
return api.context.with(api.trace.setSpan(parent, span), () => {
133+
return startSpanManual({ name: 'generic-pool.acquire' }, span => {
152134
original.call(
153135
this,
154136
(err: unknown, client: unknown) => {
137+
if (err) {
138+
span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' });
139+
}
155140
span.end();
156141
// Not checking whether cb is a function because
157142
// the original code doesn't do that either.

0 commit comments

Comments
 (0)