test(node): Streamline lru-memoizer instrumentation

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

@@ -17,51 +17,46 @@
  * - 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[] {
     return [
       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
       ),

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<void>((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');
+    });
+  });
+});