Skip to content

feat(server-utils): Add lru-memoizer diagnostics-channel integration#21786

Merged
nicohrubec merged 4 commits into
developfrom
nh/orchestrion-lru-memoizer
Jun 29, 2026
Merged

feat(server-utils): Add lru-memoizer diagnostics-channel integration#21786
nicohrubec merged 4 commits into
developfrom
nh/orchestrion-lru-memoizer

Conversation

@nicohrubec

@nicohrubec nicohrubec commented Jun 25, 2026

Copy link
Copy Markdown
Member

Adds an orchestrion-based lru-memoizer integration replacing the equivalent otel integration if enabled. This currently supports >=2.1.0, which should be sufficient I think since <2.1.0 is barely used anymore.

Closes #20759

@nicohrubec nicohrubec force-pushed the nh/orchestrion-lru-memoizer branch from 02dfa93 to 86b7148 Compare June 25, 2026 12:59
Adds an experimental orchestrion (diagnostics-channel) integration for lru-memoizer, active only when `experimentalUseDiagnosticsChannelInjection()` is opted into. It rebinds the memoized callback to the caller's active span (the channel equivalent of the OTel `context.bind`) and creates no spans. Purely additive — the vendored OTel integration stays the default and is filtered out via `replacedOtelIntegrationNames` only when opted in.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 783398f. Configure here.

integrations: [mysqlChannelIntegration()],
replacedOtelIntegrationNames: ['Mysql'],
integrations: [mysqlChannelIntegration(), lruMemoizerChannelIntegration()],
replacedOtelIntegrationNames: ['Mysql', 'LruMemoizer'],

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opt-in drops older lru-memoizer

Medium Severity

Calling experimentalUseDiagnosticsChannelInjection() removes the OpenTelemetry LruMemoizer integration, but orchestrion injection only targets lru-memoizer >=2.1.0. Apps on 1.3.x2.0.x that opt in lose callback context binding with no channel replacement.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 783398f. Configure here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<2.1.0 is barely used anymore so I think this should be fine: https://www.npmjs.com/package/lru-memoizer?activeTab=versions

@nicohrubec nicohrubec marked this pull request as ready for review June 26, 2026 12:19
@nicohrubec nicohrubec requested a review from a team as a code owner June 26, 2026 12:19
@nicohrubec nicohrubec requested review from JPeer264, andreiborza and mydea and removed request for a team June 26, 2026 12:19
Comment thread packages/server-utils/src/integrations/tracing-channel/lru-memoizer.ts Outdated
@nicohrubec nicohrubec requested a review from logaretm June 26, 2026 14:02
…hannel subscriber

Aligns the lru-memoizer diagnostics-channel subscriber with the mysql one: capture `getCurrentScope()` and re-run the memoized callback via `withScope(scope, …)` instead of `withActiveSpan(getActiveSpan(), …)`.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@nicohrubec nicohrubec requested a review from logaretm June 26, 2026 19:07
Comment thread packages/node/src/sdk/experimentalUseDiagnosticsChannelInjection.ts
@nicohrubec nicohrubec merged commit b76d21e into develop Jun 29, 2026
557 of 560 checks passed
@nicohrubec nicohrubec deleted the nh/orchestrion-lru-memoizer branch June 29, 2026 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Rewrite @opentelemetry/instrumentation-lru-memoizer to orchestrion

2 participants