From edab9dc906e62498a139769816dd8d6b1a5eba42 Mon Sep 17 00:00:00 2001 From: Nicolas Hrubec Date: Fri, 5 Jun 2026 15:42:06 +0200 Subject: [PATCH] test(node): Streamline lru-memoizer instrumentation Vendor the upstream OTel unit tests and clean out the dead config passthrough. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../lrumemoizer/vendored/instrumentation.ts | 31 ++--- .../vendored/instrumentation.test.ts | 121 ++++++++++++++++++ 2 files changed, 134 insertions(+), 18 deletions(-) create mode 100644 packages/node/test/integrations/tracing/lrumemoizer/vendored/instrumentation.test.ts diff --git a/packages/node/src/integrations/tracing/lrumemoizer/vendored/instrumentation.ts b/packages/node/src/integrations/tracing/lrumemoizer/vendored/instrumentation.ts index 1f24c1b570d1..884df24a7489 100644 --- a/packages/node/src/integrations/tracing/lrumemoizer/vendored/instrumentation.ts +++ b/packages/node/src/integrations/tracing/lrumemoizer/vendored/instrumentation.ts @@ -17,21 +17,19 @@ * - Vendored from: https://github.com/open-telemetry/opentelemetry-js-contrib/tree/15ef7506553f631ea4181391e0c5725a56f0d082/packages/instrumentation-lru-memoizer * - Upstream version: @opentelemetry/instrumentation-lru-memoizer@0.62.0 */ -/* eslint-disable */ import { context } from '@opentelemetry/api'; -import { - InstrumentationBase, - InstrumentationConfig, - InstrumentationNodeModuleDefinition, -} from '@opentelemetry/instrumentation'; +import { InstrumentationBase, InstrumentationNodeModuleDefinition } from '@opentelemetry/instrumentation'; import { SDK_VERSION } from '@sentry/core'; const PACKAGE_NAME = '@sentry/instrumentation-lru-memoizer'; +type Memoizer = (this: unknown, ...args: unknown[]) => unknown; +type LruMemoizerModule = ((this: unknown, ...args: unknown[]) => Memoizer) & { sync: unknown }; + export class LruMemoizerInstrumentation extends InstrumentationBase { - constructor(config: InstrumentationConfig = {}) { - super(PACKAGE_NAME, SDK_VERSION, config); + constructor() { + super(PACKAGE_NAME, SDK_VERSION, {}); } init(): InstrumentationNodeModuleDefinition[] { @@ -39,29 +37,26 @@ export class LruMemoizerInstrumentation extends InstrumentationBase { new InstrumentationNodeModuleDefinition( 'lru-memoizer', ['>=1.3 <4'], - moduleExports => { + (moduleExports: LruMemoizerModule) => { // moduleExports is a function which receives an options object, // and returns a "memoizer" function upon invocation. // We want to patch this "memoizer's" internal function - const asyncMemoizer = function (this: unknown) { + const asyncMemoizer = function (this: unknown, ...args: unknown[]): unknown { // This following function is invoked every time the user wants to get a (possible) memoized value // We replace it with another function in which we bind the current context to the last argument (callback) - const origMemoizer = moduleExports.apply(this, arguments); - return function (this: unknown) { - const modifiedArguments = [...arguments]; + const origMemoizer = moduleExports.apply(this, args) as Memoizer; + return function (this: unknown, ...memoizerArgs: unknown[]): unknown { // last argument is the callback - const origCallback = modifiedArguments.pop(); + const origCallback = memoizerArgs.pop(); const callbackWithContext = typeof origCallback === 'function' ? context.bind(context.active(), origCallback) : origCallback; - modifiedArguments.push(callbackWithContext); - return origMemoizer.apply(this, modifiedArguments); + return origMemoizer.apply(this, [...memoizerArgs, callbackWithContext]); }; }; // sync function preserves context, but we still need to export it // as the lru-memoizer package does - asyncMemoizer.sync = moduleExports.sync; - return asyncMemoizer; + return Object.assign(asyncMemoizer, { sync: moduleExports.sync }); }, undefined, // no need to disable as this instrumentation does not create any spans ), diff --git a/packages/node/test/integrations/tracing/lrumemoizer/vendored/instrumentation.test.ts b/packages/node/test/integrations/tracing/lrumemoizer/vendored/instrumentation.test.ts new file mode 100644 index 000000000000..45a9c6187e92 --- /dev/null +++ b/packages/node/test/integrations/tracing/lrumemoizer/vendored/instrumentation.test.ts @@ -0,0 +1,121 @@ +/* + * Tests ported from @opentelemetry/instrumentation-lru-memoizer@0.62.0 + * Original source: https://github.com/open-telemetry/opentelemetry-js-contrib/tree/main/packages/instrumentation-lru-memoizer + * Licensed under the Apache License, Version 2.0 + */ + +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import * as Sentry from '../../../../../src'; +import { LruMemoizerInstrumentation } from '../../../../../src/integrations/tracing/lrumemoizer/vendored/instrumentation'; +import { cleanupOtel, mockSdkInit } from '../../../../helpers/mockSdkInit'; + +type MemoizerCallback = (err: Error | null, result?: string) => void; +type Memoizer = (param: unknown, callback?: MemoizerCallback | null) => void; +type MemoizerModule = ((options: unknown) => Memoizer) & { sync: (options: unknown) => (param: unknown) => string }; + +describe('lru-memoizer instrumentation', () => { + let instrumentation: LruMemoizerInstrumentation; + + beforeEach(() => { + mockSdkInit({ tracesSampleRate: 1 }); + instrumentation = new LruMemoizerInstrumentation(); + }); + + afterEach(() => { + instrumentation.disable(); + cleanupOtel(); + }); + + // Create a fake `lru-memoizer`. + // The fake queues the instrumented callback so it can be invoked from outside the originating span. + function getMemoizer(): { memoizer: MemoizerModule; queuedCallbacks: MemoizerCallback[] } { + const queuedCallbacks: MemoizerCallback[] = []; + const fakeModule = Object.assign( + (_options: unknown) => (_param: unknown, callback?: MemoizerCallback | null) => { + if (callback) { + queuedCallbacks.push(callback); + } + }, + { sync: (_options: unknown) => (_param: unknown) => 'foo' }, + ) as MemoizerModule; + + const def = instrumentation.getModuleDefinitions()[0]; + if (!def?.patch) { + throw new Error('Expected a module definition with a patch function'); + } + return { memoizer: def.patch(fakeModule) as MemoizerModule, queuedCallbacks }; + } + + describe('async', () => { + it('should invoke load callback with original context', async () => { + const { memoizer, queuedCallbacks } = getMemoizer(); + const memoizedFoo = memoizer({ max: 10, load: () => {}, hash: () => 'bar' }); + + await new Promise((resolve, reject) => { + Sentry.startSpan({ name: 'memoized invocation' }, span => { + memoizedFoo({ foo: 'bar' }, err => { + try { + expect(Sentry.getActiveSpan()).toBe(span); + resolve(); + } catch (e) { + reject(e as Error); + } + if (err) { + reject(err); + } + }); + }); + + // we invoke the callback from outside of the above span's context. + // however, we expect that the callback is called with the context of the original invocation + queuedCallbacks[0]!(null, 'result'); + }); + }); + + it('should invoke callback with right context when serving 2 parallel async requests', () => { + const { memoizer, queuedCallbacks } = getMemoizer(); + const memoizedFoo = memoizer({ max: 10, load: () => {}, hash: () => 'bar' }); + + const observed: Array<{ expected: unknown; actual: unknown }> = []; + + Sentry.startSpan({ name: 'first request' }, firstSpan => { + memoizedFoo({ foo: 'bar' }, () => { + observed.push({ expected: firstSpan, actual: Sentry.getActiveSpan() }); + }); + }); + + Sentry.startSpan({ name: 'second request' }, secondSpan => { + memoizedFoo({ foo: 'bar' }, () => { + observed.push({ expected: secondSpan, actual: Sentry.getActiveSpan() }); + }); + }); + + expect(queuedCallbacks.length).toBe(2); + queuedCallbacks[0]!(null, 'result'); + queuedCallbacks[1]!(null, 'result'); + + expect(observed).toHaveLength(2); + observed.forEach(({ expected, actual }) => { + expect(actual).toBe(expected); + }); + }); + + it('should not throw when last argument is not callback', () => { + const { memoizer } = getMemoizer(); + const memoizedFoo = memoizer({ max: 10, load: () => 'foo', hash: () => 'bar' }); + + // this is not valid but we want to make sure it does not throw or act badly + expect(() => memoizedFoo({ foo: 'bar' }, null)).not.toThrow(); + }); + }); + + describe('sync', () => { + it('should not break sync memoizer', () => { + const { memoizer } = getMemoizer(); + + // the sync memoizer is passed through untouched by the patch + const memoizedFoo = memoizer.sync({ max: 10, load: () => 'foo', hash: () => 'bar' }); + expect(memoizedFoo({ foo: 'bar' })).toBe('foo'); + }); + }); +});