Skip to content

ref(node): Streamline lru-memoizer instrumentation - #21350

Merged
nicohrubec merged 2 commits into
developfrom
nh/streamline-lrumemoizer
Jun 8, 2026
Merged

ref(node): Streamline lru-memoizer instrumentation#21350
nicohrubec merged 2 commits into
developfrom
nh/streamline-lrumemoizer

Conversation

@nicohrubec

@nicohrubecnicohrubec commented Jun 5, 2026

Copy link
Copy Markdown
Member

Streamlines the vendored @opentelemetry/instrumentation-lru-memoizer:

  • Ported the upstream OTel unit tests for the instrumentation but using a fake lru memoizer instead of real module.
  • Removed the unused config constructor param (the SDK always constructs it with no config).

Fixes#20735

@nicohrubec
nicohrubecforce-pushed the nh/streamline-lrumemoizer branch 3 times, most recently from edab9dc to 2a2df78CompareJune 8, 2026 08:25
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>
@nicohrubec
nicohrubecforce-pushed the nh/streamline-lrumemoizer branch from 2a2df78 to 811b01cCompareJune 8, 2026 08:41
@nicohrubec
nicohrubec marked this pull request as ready for review June 8, 2026 08:43
@nicohrubec
nicohrubec requested a review from a team as a code ownerJune 8, 2026 08:43
@nicohrubec
nicohrubec requested review from JPeer264 and andreiborza and removed request for a teamJune 8, 2026 08:43
@nicohrubecnicohrubec changed the title test(node): Streamline lru-memoizer instrumentationref(node): Streamline lru-memoizer instrumentationJun 8, 2026
@@ -0,0 +1,112 @@
/*

@nicohrubecnicohrubecJun 8, 2026

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

mhm on second thought: since we are gonna rewrite the instrumentation to orchestrion soon, maybe porting the unit tests might actually be too close to the implementation to make sense? as in we probably will have to rewrite these then anyways when porting to orchestrion?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The tests themselves test instrumentation details right? I think it's fine, we'll only have to swap out the patching mechanism I guess, unless I'm missing something.

@andreiborzaandreiborza left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@nicohrubec
nicohrubec merged commit d645349 into developJun 8, 2026
174 of 175 checks passed
@nicohrubec
nicohrubec deleted the nh/streamline-lrumemoizer branch June 8, 2026 12:20
Sign up for freeto 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.

Streamline @opentelemetry/instrumentation-lru-memoizer

2 participants

@nicohrubec@andreiborza