From 2146bfa587382b3969f03916050cb65f1c45ad0b Mon Sep 17 00:00:00 2001 From: Nicolas Hrubec Date: Mon, 15 Jun 2026 10:35:17 +0200 Subject: [PATCH] feat(node): Migrate vendored generic-pool instrumentation to Sentry APIs Replace the OTel tracer/context span lifecycle in the vendored generic-pool instrumentation with `startSpan`/`startSpanManual` from `@sentry/core`, and set an error status on failed acquires. Adds error-path integration tests for both the v3 (promise) and v2 (callback) paths. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../tracing/genericPool-v2/scenario-error.mjs | 33 +++++++++++++++++++ .../suites/tracing/genericPool-v2/test.ts | 26 +++++++++++++++ .../tracing/genericPool/scenario-error.mjs | 28 ++++++++++++++++ .../suites/tracing/genericPool/test.ts | 20 +++++++++++ .../genericPool/vendored/instrumentation.ts | 29 ++++------------ 5 files changed, 114 insertions(+), 22 deletions(-) create mode 100644 dev-packages/node-integration-tests/suites/tracing/genericPool-v2/scenario-error.mjs create mode 100644 dev-packages/node-integration-tests/suites/tracing/genericPool/scenario-error.mjs diff --git a/dev-packages/node-integration-tests/suites/tracing/genericPool-v2/scenario-error.mjs b/dev-packages/node-integration-tests/suites/tracing/genericPool-v2/scenario-error.mjs new file mode 100644 index 000000000000..0020f94d30c8 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/genericPool-v2/scenario-error.mjs @@ -0,0 +1,33 @@ +import * as Sentry from '@sentry/node'; +import genericPool from 'generic-pool'; + +// v2 uses the callback API; a `create` error reaches the `acquire` callback and the span is errored. +const Pool = genericPool.Pool; + +const pool = new Pool({ + name: 'test', + create: callback => callback(new Error('Cannot create resource')), + destroy: () => {}, + max: 10, + min: 0, +}); + +function acquire() { + return new Promise(resolve => { + pool.acquire(() => resolve()); + }); +} + +async function run() { + await Sentry.startSpan( + { + op: 'transaction', + name: 'Test Transaction', + }, + async () => { + await acquire(); + }, + ); +} + +run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/genericPool-v2/test.ts b/dev-packages/node-integration-tests/suites/tracing/genericPool-v2/test.ts index 4f4bf4019139..ede616d629e1 100644 --- a/dev-packages/node-integration-tests/suites/tracing/genericPool-v2/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/genericPool-v2/test.ts @@ -40,4 +40,30 @@ describe('genericPool v2 auto instrumentation', () => { }, { additionalDependencies: { 'generic-pool': '^2.5.0' } }, ); + + createEsmAndCjsTests( + __dirname, + 'scenario-error.mjs', + 'instrument.mjs', + (createRunner, test) => { + test('marks the `generic-pool.acquire` span as errored when acquiring fails', async () => { + const EXPECTED_TRANSACTION = { + transaction: 'Test Transaction', + spans: expect.arrayContaining([ + expect.objectContaining({ + description: 'generic-pool.acquire', + origin: 'auto.db.otel.generic_pool', + data: { + 'sentry.origin': 'auto.db.otel.generic_pool', + }, + status: 'internal_error', + }), + ]), + }; + + await createRunner().expect({ transaction: EXPECTED_TRANSACTION }).start().completed(); + }); + }, + { additionalDependencies: { 'generic-pool': '^2.5.0' } }, + ); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/genericPool/scenario-error.mjs b/dev-packages/node-integration-tests/suites/tracing/genericPool/scenario-error.mjs new file mode 100644 index 000000000000..d9a8843e9f0c --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/genericPool/scenario-error.mjs @@ -0,0 +1,28 @@ +import * as Sentry from '@sentry/node'; +import genericPool from 'generic-pool'; + +// `create` never resolves, so `acquire` rejects and the span is errored. +const factory = { + create: function () { + return new Promise(() => {}); + }, + destroy: function () { + return Promise.resolve(); + }, +}; + +const myPool = genericPool.createPool(factory, { max: 2, min: 0, acquireTimeoutMillis: 500 }); + +async function run() { + await Sentry.startSpan( + { + op: 'transaction', + name: 'Test Transaction', + }, + async () => { + await myPool.acquire().catch(() => {}); + }, + ); +} + +run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/genericPool/test.ts b/dev-packages/node-integration-tests/suites/tracing/genericPool/test.ts index ef37e4eeff73..d5ccd13362e6 100644 --- a/dev-packages/node-integration-tests/suites/tracing/genericPool/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/genericPool/test.ts @@ -34,4 +34,24 @@ describe('genericPool auto instrumentation', () => { await createRunner().expect({ transaction: EXPECTED_TRANSACTION }).start().completed(); }); }); + + createEsmAndCjsTests(__dirname, 'scenario-error.mjs', 'instrument.mjs', (createRunner, test) => { + test('marks the `generic-pool.acquire` span as errored when acquiring fails', async () => { + const EXPECTED_TRANSACTION = { + transaction: 'Test Transaction', + spans: expect.arrayContaining([ + expect.objectContaining({ + description: 'generic-pool.acquire', + origin: 'auto.db.otel.generic_pool', + data: { + 'sentry.origin': 'auto.db.otel.generic_pool', + }, + status: 'internal_error', + }), + ]), + }; + + await createRunner().expect({ transaction: EXPECTED_TRANSACTION }).start().completed(); + }); + }); }); diff --git a/packages/node/src/integrations/tracing/genericPool/vendored/instrumentation.ts b/packages/node/src/integrations/tracing/genericPool/vendored/instrumentation.ts index 6bf28fd4c83e..34b6566e63cb 100644 --- a/packages/node/src/integrations/tracing/genericPool/vendored/instrumentation.ts +++ b/packages/node/src/integrations/tracing/genericPool/vendored/instrumentation.ts @@ -19,10 +19,9 @@ * - Minor TypeScript strictness adjustments for this repository's compiler settings */ -import * as api from '@opentelemetry/api'; import type { InstrumentationConfig } from '@opentelemetry/instrumentation'; import { InstrumentationBase, InstrumentationNodeModuleDefinition, isWrapped } from '@opentelemetry/instrumentation'; -import { SDK_VERSION } from '@sentry/core'; +import { SDK_VERSION, SPAN_STATUS_ERROR, startSpan, startSpanManual } from '@sentry/core'; import type * as genericPool from './generic-pool-types'; const MODULE_NAME = 'generic-pool'; @@ -102,23 +101,9 @@ export class GenericPoolInstrumentation extends InstrumentationBase { } private _acquirePatcher(original: AcquireFn) { - const tracer = this.tracer; return function wrapped_acquire(this: genericPool.Pool, ...args: unknown[]) { - const parent = api.context.active(); - const span = tracer.startSpan('generic-pool.acquire', {}, parent); - - return api.context.with(api.trace.setSpan(parent, span), () => { - return (original.call(this, ...args) as PromiseLike).then( - (value: unknown) => { - span.end(); - return value; - }, - (err: unknown) => { - span.recordException(err as Error); - span.end(); - throw err; - }, - ); + return startSpan({ name: 'generic-pool.acquire' }, () => { + return original.call(this, ...args) as PromiseLike; }); }; } @@ -134,7 +119,6 @@ export class GenericPoolInstrumentation extends InstrumentationBase { } private _acquireWithCallbacksPatcher(original: AcquireFn) { - const tracer = this.tracer; const isDisabled = (): boolean => this._isDisabled; return function wrapped_acquire( this: genericPool.Pool, @@ -145,13 +129,14 @@ export class GenericPoolInstrumentation extends InstrumentationBase { if (isDisabled()) { return original.call(this, cb, priority); } - const parent = api.context.active(); - const span = tracer.startSpan('generic-pool.acquire', {}, parent); - return api.context.with(api.trace.setSpan(parent, span), () => { + return startSpanManual({ name: 'generic-pool.acquire' }, span => { original.call( this, (err: unknown, client: unknown) => { + if (err) { + span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); + } span.end(); // Not checking whether cb is a function because // the original code doesn't do that either.