Skip to content

Conversation

DrewCorlin
Copy link
Contributor

@DrewCorlin DrewCorlin commented Sep 8, 2025

Which problem is this PR solving?

Return version-specific module instrumentations from version-agnostic instrumentation, to enable consumers of this library to get access to the instrumentations that are being applied without having to know this instrumentation is really just a wrapper for 2 others.

Short description of the changes

I'm the author of https://github.com/DrewCorlin/opentelemetry-node-bundler-plugins. This library depends on getModuleDefinitions() to know what modules to patch (in it's case to patch them during the bundling of an app). Updating to the latest version to support instrumentation of redis@v5 uncovered this.

Rather than special casing this in my plugin I thought it made sense to have this module behave consistently with the rest.

Also let me know if I should remove the link to my plugin here. I thought it was useful context, but understand if links to other projects that depend on this aren't desired.

@DrewCorlin DrewCorlin requested a review from a team as a code owner September 8, 2025 01:00
@github-actions github-actions bot requested a review from blumamir September 8, 2025 01:00
@DrewCorlin DrewCorlin changed the title Return underlying modules from redis instrumentation fix: Return underlying modules from redis instrumentation Sep 8, 2025
@trentm
Copy link
Contributor

trentm commented Sep 10, 2025

https://github.com/DrewCorlin/opentelemetry-node-bundler-plugins

👀 Nice!

Copy link

codecov bot commented Sep 10, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.44%. Comparing base (d60ff3a) to head (e294d78).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3025   +/-   ##
=======================================
  Coverage   91.44%   91.44%           
=======================================
  Files         146      146           
  Lines        8192     8194    +2     
  Branches     1846     1846           
=======================================
+ Hits         7491     7493    +2     
  Misses        701      701           
Flag Coverage Δ
instrumentation-redis 85.34% <100.00%> (+0.07%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
packages/instrumentation-redis/src/redis.ts 78.04% <100.00%> (+1.12%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@trentm trentm enabled auto-merge (squash) September 11, 2025 00:00
@trentm trentm changed the title fix: Return underlying modules from redis instrumentation fix(instrumentation-redis): Return underlying modules from redis instrumentation Sep 11, 2025
@trentm trentm merged commit 976dcd2 into open-telemetry:main Sep 11, 2025
17 checks passed
@opentelemetrybot
Copy link
Contributor

Thank you for your contribution @DrewCorlin! 🎉 We would like to hear from you about your experience contributing to OpenTelemetry by taking a few minutes to fill out this survey.

@dyladan dyladan mentioned this pull request Sep 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants