Skip to content

Commit 2a2df78

Browse files
nicohrubecclaude
andcommitted
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) <noreply@anthropic.com>
1 parent add109f commit 2a2df78

2 files changed

Lines changed: 134 additions & 18 deletions

File tree

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

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -17,51 +17,46 @@
1717
* - Vendored from: https://github.com/open-telemetry/opentelemetry-js-contrib/tree/15ef7506553f631ea4181391e0c5725a56f0d082/packages/instrumentation-lru-memoizer
1818
* - Upstream version: @opentelemetry/instrumentation-lru-memoizer@0.62.0
1919
*/
20-
/* eslint-disable */
2120

2221
import { context } from '@opentelemetry/api';
23-
import {
24-
InstrumentationBase,
25-
InstrumentationConfig,
26-
InstrumentationNodeModuleDefinition,
27-
} from '@opentelemetry/instrumentation';
22+
import { InstrumentationBase, InstrumentationNodeModuleDefinition } from '@opentelemetry/instrumentation';
2823
import { SDK_VERSION } from '@sentry/core';
2924

3025
const PACKAGE_NAME = '@sentry/instrumentation-lru-memoizer';
3126

27+
type Memoizer = (this: unknown, ...args: unknown[]) => unknown;
28+
type LruMemoizerModule = ((this: unknown, ...args: unknown[]) => Memoizer) & { sync: unknown };
29+
3230
export class LruMemoizerInstrumentation extends InstrumentationBase {
33-
constructor(config: InstrumentationConfig = {}) {
34-
super(PACKAGE_NAME, SDK_VERSION, config);
31+
constructor() {
32+
super(PACKAGE_NAME, SDK_VERSION, {});
3533
}
3634

3735
init(): InstrumentationNodeModuleDefinition[] {
3836
return [
3937
new InstrumentationNodeModuleDefinition(
4038
'lru-memoizer',
4139
['>=1.3 <4'],
42-
moduleExports => {
40+
(moduleExports: LruMemoizerModule) => {
4341
// moduleExports is a function which receives an options object,
4442
// and returns a "memoizer" function upon invocation.
4543
// We want to patch this "memoizer's" internal function
46-
const asyncMemoizer = function (this: unknown) {
44+
const asyncMemoizer = function (this: unknown, ...args: unknown[]): unknown {
4745
// This following function is invoked every time the user wants to get a (possible) memoized value
4846
// We replace it with another function in which we bind the current context to the last argument (callback)
49-
const origMemoizer = moduleExports.apply(this, arguments);
50-
return function (this: unknown) {
51-
const modifiedArguments = [...arguments];
47+
const origMemoizer = moduleExports.apply(this, args) as Memoizer;
48+
return function (this: unknown, ...memoizerArgs: unknown[]): unknown {
5249
// last argument is the callback
53-
const origCallback = modifiedArguments.pop();
50+
const origCallback = memoizerArgs.pop();
5451
const callbackWithContext =
5552
typeof origCallback === 'function' ? context.bind(context.active(), origCallback) : origCallback;
56-
modifiedArguments.push(callbackWithContext);
57-
return origMemoizer.apply(this, modifiedArguments);
53+
return origMemoizer.apply(this, [...memoizerArgs, callbackWithContext]);
5854
};
5955
};
6056

6157
// sync function preserves context, but we still need to export it
6258
// as the lru-memoizer package does
63-
asyncMemoizer.sync = moduleExports.sync;
64-
return asyncMemoizer;
59+
return Object.assign(asyncMemoizer, { sync: moduleExports.sync });
6560
},
6661
undefined, // no need to disable as this instrumentation does not create any spans
6762
),
Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
/*
2+
* Tests ported from @opentelemetry/instrumentation-lru-memoizer@0.62.0
3+
* Original source: https://github.com/open-telemetry/opentelemetry-js-contrib/tree/main/packages/instrumentation-lru-memoizer
4+
* Licensed under the Apache License, Version 2.0
5+
*/
6+
7+
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
8+
import * as Sentry from '../../../src';
9+
import { LruMemoizerInstrumentation } from '../../../src/integrations/tracing/lrumemoizer/vendored/instrumentation';
10+
import { cleanupOtel, mockSdkInit } from '../../helpers/mockSdkInit';
11+
12+
type MemoizerCallback = (err: Error | null, result?: string) => void;
13+
type Memoizer = (param: unknown, callback?: MemoizerCallback | null) => void;
14+
type MemoizerModule = ((options: unknown) => Memoizer) & { sync: (options: unknown) => (param: unknown) => string };
15+
16+
describe('lru-memoizer instrumentation', () => {
17+
let instrumentation: LruMemoizerInstrumentation;
18+
19+
beforeEach(() => {
20+
mockSdkInit({ tracesSampleRate: 1 });
21+
instrumentation = new LruMemoizerInstrumentation();
22+
});
23+
24+
afterEach(() => {
25+
instrumentation.disable();
26+
cleanupOtel();
27+
});
28+
29+
// Create a fake `lru-memoizer`.
30+
// The fake queues the instrumented callback so it can be invoked from outside the originating span.
31+
function getMemoizer(): { memoizer: MemoizerModule; queuedCallbacks: MemoizerCallback[] } {
32+
const queuedCallbacks: MemoizerCallback[] = [];
33+
const fakeModule = Object.assign(
34+
(_options: unknown) => (_param: unknown, callback?: MemoizerCallback | null) => {
35+
if (callback) {
36+
queuedCallbacks.push(callback);
37+
}
38+
},
39+
{ sync: (_options: unknown) => (_param: unknown) => 'foo' },
40+
) as MemoizerModule;
41+
42+
const def = instrumentation.getModuleDefinitions()[0];
43+
if (!def?.patch) {
44+
throw new Error('Expected a module definition with a patch function');
45+
}
46+
return { memoizer: def.patch(fakeModule) as MemoizerModule, queuedCallbacks };
47+
}
48+
49+
describe('async', () => {
50+
it('should invoke load callback with original context', async () => {
51+
const { memoizer, queuedCallbacks } = getMemoizer();
52+
const memoizedFoo = memoizer({ max: 10, load: () => {}, hash: () => 'bar' });
53+
54+
await new Promise<void>((resolve, reject) => {
55+
Sentry.startSpan({ name: 'memoized invocation' }, span => {
56+
memoizedFoo({ foo: 'bar' }, err => {
57+
try {
58+
expect(Sentry.getActiveSpan()).toBe(span);
59+
resolve();
60+
} catch (e) {
61+
reject(e as Error);
62+
}
63+
if (err) {
64+
reject(err);
65+
}
66+
});
67+
});
68+
69+
// we invoke the callback from outside of the above span's context.
70+
// however, we expect that the callback is called with the context of the original invocation
71+
queuedCallbacks[0]!(null, 'result');
72+
});
73+
});
74+
75+
it('should invoke callback with right context when serving 2 parallel async requests', () => {
76+
const { memoizer, queuedCallbacks } = getMemoizer();
77+
const memoizedFoo = memoizer({ max: 10, load: () => {}, hash: () => 'bar' });
78+
79+
const observed: Array<{ expected: unknown; actual: unknown }> = [];
80+
81+
Sentry.startSpan({ name: 'first request' }, firstSpan => {
82+
memoizedFoo({ foo: 'bar' }, () => {
83+
observed.push({ expected: firstSpan, actual: Sentry.getActiveSpan() });
84+
});
85+
});
86+
87+
Sentry.startSpan({ name: 'second request' }, secondSpan => {
88+
memoizedFoo({ foo: 'bar' }, () => {
89+
observed.push({ expected: secondSpan, actual: Sentry.getActiveSpan() });
90+
});
91+
});
92+
93+
expect(queuedCallbacks.length).toBe(2);
94+
queuedCallbacks[0]!(null, 'result');
95+
queuedCallbacks[1]!(null, 'result');
96+
97+
expect(observed).toHaveLength(2);
98+
observed.forEach(({ expected, actual }) => {
99+
expect(actual).toBe(expected);
100+
});
101+
});
102+
103+
it('should not throw when last argument is not callback', () => {
104+
const { memoizer } = getMemoizer();
105+
const memoizedFoo = memoizer({ max: 10, load: () => 'foo', hash: () => 'bar' });
106+
107+
// this is not valid but we want to make sure it does not throw or act badly
108+
expect(() => memoizedFoo({ foo: 'bar' }, null)).not.toThrow();
109+
});
110+
});
111+
112+
describe('sync', () => {
113+
it('should not break sync memoizer', () => {
114+
const { memoizer } = getMemoizer();
115+
116+
// the sync memoizer is passed through untouched by the patch
117+
const memoizedFoo = memoizer.sync({ max: 10, load: () => 'foo', hash: () => 'bar' });
118+
expect(memoizedFoo({ foo: 'bar' })).toBe('foo');
119+
});
120+
});
121+
});

0 commit comments

Comments
 (0)