fix(angular): Ensure navigations always create a transaction #10646
Conversation
| "overrides": { | ||
| "@sentry/browser": "latest || *", | ||
| "@sentry/core": "latest || *", | ||
| "@sentry/utils": "latest || *" | ||
| } |
There was a problem hiding this comment.
Noticed that this seems to be required to ensure the local/verdaccio version of the dependencies of @sentry/angular-ivy are taken instead of the ones uploaded to NPM.
| fullyParallel: false, | ||
| workers: 1, | ||
| /* Fail the build on CI if you accidentally left test.only in the source code. */ |
There was a problem hiding this comment.
Just to avoid bleeds of events due to the proxy. 5 tests shouldn't make much difference here.
| { | ||
| path: 'redirect1', | ||
| redirectTo: '/redirect2', | ||
| }, | ||
| { | ||
| path: 'redirect2', | ||
| redirectTo: '/redirect3', | ||
| }, | ||
| { | ||
| path: 'redirect3', | ||
| redirectTo: '/users/456', | ||
| }, |
There was a problem hiding this comment.
Although these are 3 redirects, we'll only get one navigation root span and one routing span for them. I think this is fine (3 routing spans would have been nice to have but whatever)
|
I reworked the logic a bit to account for the edge case when the pageload root span already finished before the first In this case (i.e. no active pageload root span), we'll now immediately start a navigation root span, bypassing the "wait for the first emit" behaviour. |
This PR addresses the problematic check of
getActiveSpan()in our Angular SDK'sTraceServiceraised in #10634.In certain situations (e.g. very quick navigations after the initial pageload) the check of
getActiveSpan()could lead to navigations not getting their own transaction, as the pageload transaction was still active at that time.I also checked the behaviour of multiple subsequent redirects and here, we only create one navigation transaction for all redirects. The reason for this is that the Angular router doesn't fire individual events for the redirects but only one for the final redirect. IMHO this is fine and should be expected behaviour.
ref #10634