Uh oh!
There was an error while loading. Please reload this page.
ref(node): Streamline generic-pool - #21363
Merged
Merged
Conversation
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…cases Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Drop the unused PoolConstructor call signature and the _poolWrapper cast (type the factory return instead). Behavior unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…l unit tests Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
nicohrubec
marked this pull request as ready for review
June 9, 2026 10:16
nicohrubec
requested review from
JPeer264 and andreiborza
and removed request for
a teamJune 9, 2026 10:16
andreiborza
approved these changes
Jun 9, 2026
| transaction: 'Test Transaction', | ||
| spans: expect.arrayContaining([ | ||
| expect.objectContaining({ | ||
| description: expect.stringMatching(/^generic-pool\.ac?quire/), |
Member
There was a problem hiding this comment.
nit: is the ? necessary because they have a typo somewhere?
MemberAuthor
There was a problem hiding this comment.
this was just copy pasted from the other tests. this was necessary because in some old otel version there was a typo so we also had some special handling to deal with both potential span names (with and without typo). we can remove this now and simplify the assertions
| }), | ||
| expect.objectContaining({ | ||
| description: expect.stringMatching(/^generic-pool\.ac?quire/), |
The `generic-pool.aquire` typo only came from @opentelemetry/instrumentation-generic-pool <= 0.38.0; the vendored instrumentation (0.61.0) always emits `generic-pool.acquire`, so simplify the origin check and the test span matchers. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
Contributor
size-limit report 📦
|
andreiborza
approved these changes
Jun 9, 2026
andreiborza
left a comment
Member
There was a problem hiding this comment.
I'd keep v2 around, it's still downloaded quite a bit (~100k downloads in the last 7 days roughly).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Streamlines the vendored
@opentelemetry/instrumentation-generic-pool:generic-poolv2 integration suite (genericPool-v2) alongside the existing v3 one, so both major versions are exercised. Ported unit tests from upstream.This one has three supported version ranges with different instrumentation paths (
2.0 - <2.4,2.4 - <3.0,>3.0). The only path that is still heavily used is>3.0based on npm downloads. In the future we can think about dropping v2 support, which would allow us to slim this instrumentation down quite a bit.Closes#20727