-
Notifications
You must be signed in to change notification settings - Fork 542
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
fix: redis instrumentation loses context when using callbacks #580
fix: redis instrumentation loses context when using callbacks #580
Conversation
return context.with(originalContext, () => { | ||
return originalCallback.apply(callbackThis, arguments); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can also just do return context.with(originalContext, originalCallback, callbackThis, ...arguments)
https://open-telemetry.github.io/opentelemetry-js-api/classes/contextapi.html#with
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This way you avoid creating an additional function call
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wasn't aware of that one. thanks @dyladan , fixed :)
Codecov Report
@@ Coverage Diff @@
## main #580 +/- ##
=======================================
Coverage 94.94% 94.94%
=======================================
Files 195 195
Lines 11630 11637 +7
Branches 1115 1115
=======================================
+ Hits 11042 11049 +7
Misses 588 588
|
Which problem is this PR solving?
Short description of the changes