-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
fix(core): continueTrace doesn't propagate given trace ID if active span exists #18328
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
Changes from 6 commits
d55a3c3
45b74b7
e7e83c6
21713dc
459766e
c8e32bb
661a11b
499a58e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ import { | |
| getGlobalScope, | ||
| getIsolationScope, | ||
| getMainCarrier, | ||
| getTraceData, | ||
| Scope, | ||
| SEMANTIC_ATTRIBUTE_SENTRY_OP, | ||
| SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, | ||
|
|
@@ -2021,6 +2022,39 @@ describe('continueTrace', () => { | |
| }); | ||
| }); | ||
| }); | ||
|
|
||
| it('works inside an active span', () => { | ||
| const client = new TestClient( | ||
| getDefaultTestClientOptions({ | ||
| dsn: 'https://username@domain/123', | ||
| tracesSampleRate: 1, | ||
| }), | ||
| ); | ||
| setCurrentClient(client); | ||
| client.init(); | ||
|
|
||
| const sentryTrace = '12312012123120121231201212312012-1121201211212012-1'; | ||
| const sentryTraceId = '12312012123120121231201212312012'; | ||
| const sentryBaggage = 'sentry-org_id=123'; | ||
|
|
||
| startSpan({ name: 'outer' }, () => { | ||
| continueTrace( | ||
| { | ||
| sentryTrace: sentryTrace, | ||
| baggage: sentryBaggage, | ||
| }, | ||
| () => { | ||
| const traceDataInContinuedTrace = getTraceData(); | ||
| const traceIdInContinuedTrace = traceDataInContinuedTrace['sentry-trace']?.split('-')[0]; | ||
| const traceIdInCurrentScope = getCurrentScope().getPropagationContext().traceId; | ||
|
|
||
| expect(getActiveSpan()).toBeUndefined(); | ||
| expect(traceIdInContinuedTrace).toBe(sentryTraceId); | ||
| expect(traceIdInCurrentScope).toBe(sentryTraceId); | ||
| }, | ||
| ); | ||
| }); | ||
| }); | ||
|
Comment on lines
+2056
to
+2057
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. m: Let's also add a second test that shows that when you now start another span within the
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sure, added! |
||
| }); | ||
|
|
||
| describe('getActiveSpan', () => { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -186,7 +186,7 @@ describe('sentryHandle', () => { | |
| resolve: async _ => { | ||
| // simulating a nested load call: | ||
| await sentryHandle()({ | ||
| event: mockEvent({ route: { id: 'api/users/details/[id]', isSubRequest: true } }), | ||
| event: mockEvent({ route: { id: 'api/users/details/[id]' }, isSubRequest: true }), | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good catch, and very interesting that this passed beforehand 🤔 👀
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah it is! didn't investigate further though |
||
| resolve: resolve(type, isError), | ||
| }); | ||
| return mockResponse; | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
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: let's find a slightly more descriptive description. Feel free to go with something else but suggestion: