diff --git a/dev-packages/node-integration-tests/suites/tracing/genericPool-v2/instrument.mjs b/dev-packages/node-integration-tests/suites/tracing/genericPool-v2/instrument.mjs new file mode 100644 index 000000000000..46a27dd03b74 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/genericPool-v2/instrument.mjs @@ -0,0 +1,9 @@ +import * as Sentry from '@sentry/node'; +import { loggingTransport } from '@sentry-internal/node-integration-tests'; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracesSampleRate: 1.0, + transport: loggingTransport, +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/genericPool-v2/scenario.mjs b/dev-packages/node-integration-tests/suites/tracing/genericPool-v2/scenario.mjs new file mode 100644 index 000000000000..86062a9847e7 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/genericPool-v2/scenario.mjs @@ -0,0 +1,42 @@ +import * as Sentry from '@sentry/node'; +import genericPool from 'generic-pool'; + +// generic-pool v2 uses the callback-based API: `new Pool(factory)` with `factory.create(callback)` +// and `pool.acquire((err, client) => ...)`. +const Pool = genericPool.Pool; + +const pool = new Pool({ + name: 'test', + create: callback => callback(null, { id: Math.random() }), + destroy: () => {}, + max: 10, + min: 2, +}); + +function acquire() { + return new Promise((resolve, reject) => { + pool.acquire((err, client) => { + if (err) { + reject(err); + return; + } + pool.release(client); + resolve(); + }); + }); +} + +async function run() { + await Sentry.startSpan( + { + op: 'transaction', + name: 'Test Transaction', + }, + async () => { + await acquire(); + 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 new file mode 100644 index 000000000000..4f4bf4019139 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/genericPool-v2/test.ts @@ -0,0 +1,43 @@ +import { afterAll, describe, expect } from 'vitest'; +import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../utils/runner'; + +describe('genericPool v2 auto instrumentation', () => { + afterAll(() => { + cleanupChildProcesses(); + }); + + createEsmAndCjsTests( + __dirname, + 'scenario.mjs', + 'instrument.mjs', + (createRunner, test) => { + test('should auto-instrument `generic-pool` v2 when calling pool.acquire()', 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: 'ok', + }), + + expect.objectContaining({ + description: 'generic-pool.acquire', + origin: 'auto.db.otel.generic_pool', + data: { + 'sentry.origin': 'auto.db.otel.generic_pool', + }, + status: 'ok', + }), + ]), + }; + + 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/test.ts b/dev-packages/node-integration-tests/suites/tracing/genericPool/test.ts index dd18c456e958..ef37e4eeff73 100644 --- a/dev-packages/node-integration-tests/suites/tracing/genericPool/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/genericPool/test.ts @@ -12,7 +12,7 @@ describe('genericPool auto instrumentation', () => { transaction: 'Test Transaction', spans: expect.arrayContaining([ expect.objectContaining({ - description: expect.stringMatching(/^generic-pool\.ac?quire/), + description: 'generic-pool.acquire', origin: 'auto.db.otel.generic_pool', data: { 'sentry.origin': 'auto.db.otel.generic_pool', @@ -21,7 +21,7 @@ describe('genericPool auto instrumentation', () => { }), expect.objectContaining({ - description: expect.stringMatching(/^generic-pool\.ac?quire/), + description: 'generic-pool.acquire', origin: 'auto.db.otel.generic_pool', data: { 'sentry.origin': 'auto.db.otel.generic_pool', diff --git a/packages/node/src/integrations/tracing/genericPool/index.ts b/packages/node/src/integrations/tracing/genericPool/index.ts index a9b7e94d56d9..0a704f192bc3 100644 --- a/packages/node/src/integrations/tracing/genericPool/index.ts +++ b/packages/node/src/integrations/tracing/genericPool/index.ts @@ -21,12 +21,8 @@ const _genericPoolIntegration = (() => { instrumentationWrappedCallback?.(() => client.on('spanStart', span => { const spanJSON = spanToJSON(span); - const spanDescription = spanJSON.description; - - // typo in emitted span for version <= 0.38.0 of @opentelemetry/instrumentation-generic-pool - const isGenericPoolSpan = - spanDescription === 'generic-pool.aquire' || spanDescription === 'generic-pool.acquire'; + const isGenericPoolSpan = spanDescription === 'generic-pool.acquire'; if (isGenericPoolSpan) { span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto.db.otel.generic_pool'); diff --git a/packages/node/src/integrations/tracing/genericPool/vendored/instrumentation.ts b/packages/node/src/integrations/tracing/genericPool/vendored/instrumentation.ts index aa1615669e5a..7779b966c7b7 100644 --- a/packages/node/src/integrations/tracing/genericPool/vendored/instrumentation.ts +++ b/packages/node/src/integrations/tracing/genericPool/vendored/instrumentation.ts @@ -18,23 +18,24 @@ * - Upstream version: @opentelemetry/instrumentation-generic-pool@0.61.0 * - Minor TypeScript strictness adjustments for this repository's compiler settings */ -/* eslint-disable */ import * as api from '@opentelemetry/api'; -import { - InstrumentationBase, - InstrumentationConfig, - InstrumentationNodeModuleDefinition, - isWrapped, -} from '@opentelemetry/instrumentation'; - -import type * as genericPool from './generic-pool-types'; - +import type { InstrumentationConfig } from '@opentelemetry/instrumentation'; +import { InstrumentationBase, InstrumentationNodeModuleDefinition, isWrapped } from '@opentelemetry/instrumentation'; import { SDK_VERSION } from '@sentry/core'; +import type * as genericPool from './generic-pool-types'; const MODULE_NAME = 'generic-pool'; const PACKAGE_NAME = '@sentry/instrumentation-generic-pool'; +type AcquireFn = (this: unknown, ...args: unknown[]) => unknown; +interface PoolConstructor { + prototype: { acquire: AcquireFn }; +} +interface GenericPoolModule { + Pool: PoolConstructor; +} + export class GenericPoolInstrumentation extends InstrumentationBase { // only used for v2 - v2.3) private _isDisabled = false; @@ -48,16 +49,16 @@ export class GenericPoolInstrumentation extends InstrumentationBase { new InstrumentationNodeModuleDefinition( MODULE_NAME, ['>=3.0.0 <4'], - moduleExports => { - const Pool: any = moduleExports.Pool; + (moduleExports: GenericPoolModule) => { + const Pool = moduleExports.Pool; if (isWrapped(Pool.prototype.acquire)) { this._unwrap(Pool.prototype, 'acquire'); } this._wrap(Pool.prototype, 'acquire', this._acquirePatcher.bind(this)); return moduleExports; }, - moduleExports => { - const Pool: any = moduleExports.Pool; + (moduleExports: GenericPoolModule) => { + const Pool = moduleExports.Pool; this._unwrap(Pool.prototype, 'acquire'); return moduleExports; }, @@ -65,16 +66,16 @@ export class GenericPoolInstrumentation extends InstrumentationBase { new InstrumentationNodeModuleDefinition( MODULE_NAME, ['>=2.4.0 <3'], - moduleExports => { - const Pool: any = moduleExports.Pool; + (moduleExports: GenericPoolModule) => { + const Pool = moduleExports.Pool; if (isWrapped(Pool.prototype.acquire)) { this._unwrap(Pool.prototype, 'acquire'); } this._wrap(Pool.prototype, 'acquire', this._acquireWithCallbacksPatcher.bind(this)); return moduleExports; }, - moduleExports => { - const Pool: any = moduleExports.Pool; + (moduleExports: GenericPoolModule) => { + const Pool = moduleExports.Pool; this._unwrap(Pool.prototype, 'acquire'); return moduleExports; }, @@ -82,7 +83,7 @@ export class GenericPoolInstrumentation extends InstrumentationBase { new InstrumentationNodeModuleDefinition( MODULE_NAME, ['>=2.0.0 <2.4'], - moduleExports => { + (moduleExports: GenericPoolModule) => { this._isDisabled = false; if (isWrapped(moduleExports.Pool)) { this._unwrap(moduleExports, 'Pool'); @@ -90,7 +91,7 @@ export class GenericPoolInstrumentation extends InstrumentationBase { this._wrap(moduleExports, 'Pool', this._poolWrapper.bind(this)); return moduleExports; }, - moduleExports => { + (moduleExports: GenericPoolModule) => { // since the object is created on the fly every time, we need to use // a boolean switch here to disable the instrumentation this._isDisabled = true; @@ -100,14 +101,15 @@ export class GenericPoolInstrumentation extends InstrumentationBase { ]; } - private _acquirePatcher(original: genericPool.Pool['acquire']) { + private _acquirePatcher(original: AcquireFn) { + // eslint-disable-next-line @typescript-eslint/no-this-alias const instrumentation = this; - return function wrapped_acquire(this: genericPool.Pool, ...args: any[]) { + return function wrapped_acquire(this: genericPool.Pool, ...args: unknown[]) { const parent = api.context.active(); const span = instrumentation.tracer.startSpan('generic-pool.acquire', {}, parent); return api.context.with(api.trace.setSpan(parent, span), () => { - return original.call(this, ...args).then( + return (original.call(this, ...args) as PromiseLike).then( (value: unknown) => { span.end(); return value; @@ -122,18 +124,24 @@ export class GenericPoolInstrumentation extends InstrumentationBase { }; } - private _poolWrapper(original: any) { + private _poolWrapper(original: (this: unknown, ...args: unknown[]) => { acquire: AcquireFn }) { + // eslint-disable-next-line @typescript-eslint/no-this-alias const instrumentation = this; - return function wrapped_pool(this: any) { - const pool = original.apply(this, arguments); + return function wrapped_pool(this: unknown, ...args: unknown[]) { + const pool = original.apply(this, args); instrumentation._wrap(pool, 'acquire', instrumentation._acquireWithCallbacksPatcher.bind(instrumentation)); return pool; }; } - private _acquireWithCallbacksPatcher(original: any) { + private _acquireWithCallbacksPatcher(original: AcquireFn) { + // eslint-disable-next-line @typescript-eslint/no-this-alias const instrumentation = this; - return function wrapped_acquire(this: genericPool.Pool, cb: Function, priority: number) { + return function wrapped_acquire( + this: genericPool.Pool, + cb: (err: unknown, client: unknown) => unknown, + priority: number, + ) { // only used for v2 - v2.3 if (instrumentation._isDisabled) { return original.call(this, cb, priority); @@ -148,8 +156,9 @@ export class GenericPoolInstrumentation extends InstrumentationBase { span.end(); // Not checking whether cb is a function because // the original code doesn't do that either. + // The callback's return value is unused by generic-pool, so we don't return it. if (cb) { - return cb(err, client); + cb(err, client); } }, priority, diff --git a/packages/node/test/integrations/tracing/genericPool.test.ts b/packages/node/test/integrations/tracing/genericPool.test.ts new file mode 100644 index 000000000000..7c15939a5432 --- /dev/null +++ b/packages/node/test/integrations/tracing/genericPool.test.ts @@ -0,0 +1,68 @@ +/* + * Tests ported from @opentelemetry/instrumentation-generic-pool@0.61.0 + * Original source: https://github.com/open-telemetry/opentelemetry-js-contrib/tree/main/packages/instrumentation-generic-pool + * Licensed under the Apache License, Version 2.0 + */ + +import { context, trace } from '@opentelemetry/api'; +import { BasicTracerProvider, InMemorySpanExporter, SimpleSpanProcessor } from '@opentelemetry/sdk-trace-base'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { GenericPoolInstrumentation } from '../../../src/integrations/tracing/genericPool/vendored/instrumentation'; +import { cleanupOtel, mockSdkInit } from '../../helpers/mockSdkInit'; + +// Create a fake `generic-pool` module. +function createPoolModule(): { Pool: new () => { acquire: () => Promise } } { + class FakePool { + public acquire(): Promise { + return Promise.resolve('client'); + } + } + return { Pool: FakePool }; +} + +describe('GenericPoolInstrumentation', () => { + const memoryExporter = new InMemorySpanExporter(); + const provider = new BasicTracerProvider({ spanProcessors: [new SimpleSpanProcessor(memoryExporter)] }); + + let instrumentation: GenericPoolInstrumentation; + + beforeEach(() => { + mockSdkInit({ tracesSampleRate: 1 }); + instrumentation = new GenericPoolInstrumentation(); + instrumentation.setTracerProvider(provider); + memoryExporter.reset(); + }); + + afterEach(() => { + instrumentation.disable(); + cleanupOtel(); + }); + + it('should attach it to the parent span', async () => { + const moduleExports = createPoolModule(); + const def = instrumentation.getModuleDefinitions()[0]!; + def.patch!(moduleExports); + + const parent = provider.getTracer('test').startSpan('parent'); + await context.with(trace.setSpan(context.active(), parent), async () => { + await new moduleExports.Pool().acquire(); + }); + parent.end(); + + const acquireSpan = memoryExporter.getFinishedSpans().find(span => span.name === 'generic-pool.acquire'); + expect(acquireSpan).toBeDefined(); + expect(acquireSpan!.parentSpanContext?.spanId).toBe(parent.spanContext().spanId); + }); + + it('should not create anything if disabled', async () => { + const moduleExports = createPoolModule(); + const def = instrumentation.getModuleDefinitions()[0]!; + // Patch then unpatch to mimic the instrumentation being disabled. + def.patch!(moduleExports); + def.unpatch!(moduleExports); + + await new moduleExports.Pool().acquire(); + + expect(memoryExporter.getFinishedSpans()).toHaveLength(0); + }); +});