Skip to content

Commit b35c4de

Browse files
nicohrubecclaude
andauthored
test(node): Replace lru-memoizer fake unit test with integration coverage (#21437)
Move the fake-based lru-memoizer unit test to integration tests using the real package. Context propagation is asserted via a span attribute. The test resolves the memoized load from outside the span and records whether the callback still ran in its originating span's context, instead of relying on a thrown error that the runner might miss once the expected transactions arrive. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 0d3f1b1 commit b35c4de

4 files changed

Lines changed: 112 additions & 131 deletions

File tree

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
import * as Sentry from '@sentry/node';
2+
import memoizer from 'lru-memoizer';
3+
4+
const run = async () => {
5+
// lru-memoizer's only job is to bind the active context onto each memoized callback, so the
6+
// callback runs in its originating span's context whenever the (possibly async) load resolves.
7+
// To test that: `load` captures its callback without resolving, so memoized calls stay queued.
8+
// We register one call per span, then resolve the load from outside both spans (see below) -
9+
// if binding works, each queued callback still sees its own span active.
10+
let memoizerLoadCallback;
11+
const memoizedFoo = memoizer({
12+
load: (_param, callback) => {
13+
memoizerLoadCallback = callback;
14+
},
15+
hash: () => 'bar',
16+
});
17+
18+
// Concurrent calls with the same key share one in-flight load, so each span's callback is queued
19+
// against it. We record on each transaction whether its callback kept its own span's context.
20+
const first = Sentry.startSpan(
21+
{ op: 'first' },
22+
firstSpan =>
23+
new Promise(resolve => {
24+
memoizedFoo({ foo: 'bar' }, () => {
25+
firstSpan.setAttribute(
26+
'memoized.context_preserved',
27+
Sentry.getActiveSpan()?.spanContext().spanId === firstSpan.spanContext().spanId,
28+
);
29+
resolve();
30+
});
31+
}),
32+
);
33+
34+
const second = Sentry.startSpan(
35+
{ op: 'second' },
36+
secondSpan =>
37+
new Promise(resolve => {
38+
memoizedFoo({ foo: 'bar' }, () => {
39+
secondSpan.setAttribute(
40+
'memoized.context_preserved',
41+
Sentry.getActiveSpan()?.spanContext().spanId === secondSpan.spanContext().spanId,
42+
);
43+
resolve();
44+
});
45+
}),
46+
);
47+
48+
// Fire the single load outside both spans, resolving both queued waiters.
49+
memoizerLoadCallback();
50+
51+
await Promise.all([first, second]);
52+
};
53+
54+
run();

dev-packages/node-integration-tests/suites/tracing/lru-memoizer/scenario.mjs

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,20 @@ import * as Sentry from '@sentry/node';
22
import memoizer from 'lru-memoizer';
33

44
const run = async () => {
5+
// The sync memoizer is passed through untouched by the instrumentation.
6+
const memoizedSync = memoizer.sync({ load: () => 'foo', hash: () => 'bar' });
7+
if (memoizedSync({ foo: 'bar' }) !== 'foo') {
8+
throw new Error('Sync memoizer should return the loaded value');
9+
}
10+
11+
// A non-function last argument must be passed through without throwing.
12+
const memoizedNoCallback = memoizer({ load: () => {}, hash: () => 'bar' });
13+
memoizedNoCallback({ foo: 'bar' }, null);
14+
15+
// lru-memoizer's only job is to bind the active context onto the memoized callback, so it runs in
16+
// its originating span's context whenever the (possibly async) load resolves. To test that, `load`
17+
// captures its callback without resolving (the call stays queued), and we resolve it later from
18+
// outside the span.
519
// Test ported from the OTEL implementation:
620
// https://github.com/open-telemetry/opentelemetry-js-contrib/blob/0d6ebded313bb75b5a0e7a6422206c922daf3943/plugins/node/instrumentation-lru-memoizer/test/index.test.ts#L28
721
let memoizerLoadCallback;
@@ -12,24 +26,26 @@ const run = async () => {
1226
hash: () => 'bar',
1327
});
1428

15-
Sentry.startSpan({ op: 'run' }, async span => {
16-
const outerSpanContext = span.spanContext();
17-
18-
memoizedFoo({ foo: 'bar' }, () => {
19-
const innerContext = Sentry.getActiveSpan().spanContext();
20-
21-
// The span context should be the same as the outer span
22-
// Throwing an error here will cause the test to fail
23-
if (outerSpanContext !== innerContext) {
24-
throw new Error('Outer and inner span context should match');
25-
}
26-
});
29+
// Keep the span open until the memoized callback fires, recording on the transaction whether the
30+
// callback ran in the originating span's context.
31+
const spanFinished = Sentry.startSpan(
32+
{ op: 'run' },
33+
span =>
34+
new Promise(resolve => {
35+
memoizedFoo({ foo: 'bar' }, () => {
36+
span.setAttribute(
37+
'memoized.context_preserved',
38+
Sentry.getActiveSpan()?.spanContext().spanId === span.spanContext().spanId,
39+
);
40+
resolve();
41+
});
42+
}),
43+
);
2744

28-
span.end();
29-
});
30-
31-
// Invoking the load callback outside the span
45+
// Fire the load outside the span, so the assertion above proves the context was restored.
3246
memoizerLoadCallback();
47+
48+
await spanFinished;
3349
};
3450

3551
run();

dev-packages/node-integration-tests/suites/tracing/lru-memoizer/test.ts

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,6 @@ describe('lru-memoizer', () => {
1313
(createTestRunner, test) => {
1414
test('keeps outer context inside the memoized inner functions', async () => {
1515
await createTestRunner()
16-
// We expect only one transaction and nothing else.
17-
// A failed test will result in an error event being sent to Sentry.
18-
// Which will fail this suite.
1916
.expect({
2017
transaction: {
2118
transaction: '<unknown>',
@@ -25,6 +22,7 @@ describe('lru-memoizer', () => {
2522
data: expect.objectContaining({
2623
'sentry.op': 'run',
2724
'sentry.origin': 'manual',
25+
'memoized.context_preserved': true,
2826
}),
2927
}),
3028
},
@@ -36,4 +34,29 @@ describe('lru-memoizer', () => {
3634
},
3735
{ failsOnEsm: true },
3836
);
37+
38+
createEsmAndCjsTests(
39+
__dirname,
40+
'scenario-parallel.mjs',
41+
'instrument.mjs',
42+
(createTestRunner, test) => {
43+
test('keeps each span context across parallel memoized requests', async () => {
44+
// Each parallel request emits a transaction whose callback must have run in its own context.
45+
// Two identical expectations keep this order-independent.
46+
const expectation = {
47+
transaction: {
48+
contexts: {
49+
trace: expect.objectContaining({
50+
op: expect.stringMatching(/^(first|second)$/),
51+
data: expect.objectContaining({ 'memoized.context_preserved': true }),
52+
}),
53+
},
54+
},
55+
};
56+
57+
await createTestRunner().expect(expectation).expect(expectation).start().completed();
58+
});
59+
},
60+
{ failsOnEsm: true },
61+
);
3962
});

packages/node/test/integrations/tracing/lrumemoizer.test.ts

Lines changed: 0 additions & 112 deletions
This file was deleted.

0 commit comments

Comments
 (0)