-
-
Couldn't load subscription status.
- Fork 1.7k
fix(node): Ensure isolation scope is correctly cloned for non-recording spans #11503
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(node): Ensure isolation scope is correctly cloned for non-recording spans #11503
Conversation
0072aa9 to
4478007
Compare
size-limit report 📦
|
| }, | ||
| spanNameHook(info, defaultName) { | ||
| if (getIsolationScope() === getDefaultIsolationScope()) { | ||
| logger.warn('Isolation scope is still default isolation scope - skipping setting transactionName'); |
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.
l: should we add DEBUG_BUILD && here?
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.
very nice catch! That's actually a huge fix, great find!
…ng spans (getsentry#11503) Our Http instrumentation didn't clone the isolation scope correctly when the request span is not recording. We early returned if the span was not a server kind span. However, non-recording spans apparently have no span kind, so we wrongfully early returned in the check for non-recording spans.
Noticed that our Http instrumentation doesn't clone the isolation scope correctly when the request span is not recording.
The reason is that we early returned if the span was not a server kind span. However, non-recording spans apparently have no span kind, so we wrongfully early returned in the check for non-recording spans.
This probably led to pollution of the default isolation scope. Added tests to demonstrate the fixed behaviour.