Skip to content

fix(node): Fix preloading of instrumentation #17403

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 13, 2025
Merged

fix(node): Fix preloading of instrumentation #17403

merged 2 commits into from
Aug 13, 2025

Conversation

mydea
Copy link
Member

@mydea mydea commented Aug 13, 2025

Extracted out of #17371

I noticed that we were not fully consistent in instrumentation IDs for integrations that have multiple instrumentation. The intent is that users can provide the integration name (e.g. Http) and it will preload all http instrumentation. To achieve this, I adjusted the preload filter code to look for exact matches as well as startsWith(${name}.id). I also adjusted the test to be more declarative and mock/reset stuff properly (this lead to issues in the linked PR, and should generally be a bit cleaner).

I also updated all instrumentation IDs to follow this pattern. We should be mindful of following this with new instrumentation we add.

@mydea mydea self-assigned this Aug 13, 2025
@mydea mydea requested review from Lms24 and andreiborza August 13, 2025 13:02
Copy link
Member

@Lms24 Lms24 left a comment

Choose a reason for hiding this comment

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

Good catch! (Tbh, I completely forgot that users can preload selectively but makes sense!)

Perhaps this is something we can add to BUGBOT.md to increase the likelihood of a wrong name being caught when we add the next instrumentation?

@mydea mydea merged commit 05af8d0 into develop Aug 13, 2025
242 of 245 checks passed
@mydea mydea deleted the fn/fix-preloading branch August 13, 2025 14:08
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.

2 participants