fix(browser): Avoid recording long task spans starting before their parent span #14183
Conversation
| continue; | ||
| } | ||
|
|
||
| startAndEndSpan(parent, startTime, startTime + duration, { |
There was a problem hiding this comment.
small refactor: I switched from manually starting and ending the span to using the startAndEndSpan helper which does just that. However, the helper would also adjust the navigation span start time stamp if the stamp was started beforehand.
I medium-strongly feel we should not adjust this start time stamp b/c the long task might have nothing to do with the navigation but it would look like it otherwise.
If reviewers prefer the other way around (i.e. actually record the span and adjust the transaction start time), I'm also fine with that.
size-limit report 📦
|
| @@ -40,7 +40,7 @@ module.exports = [ | |||
| path: 'packages/browser/build/npm/esm/index.js', | |||
| import: createImport('init', 'browserTracingIntegration'), | |||
| gzip: true, | |||
| limit: '36 KB', | |||
| limit: '36.5 KB', | |||
There was a problem hiding this comment.
we're just ~15 bytes over the previous limit but I figured I'd give the limit a slightly bigger bump as we usually do
…ore their parent span (#14186) - Check for start time of parent navigation span and don't start long animation frame span if its start timestamp is earlier than the navigation start time stamp - Refactor span starting logic to use common helper function to compensate the bundle size increase - Add regression test that failed previously - Improve regression test from #14183 to avoid flakes and improve the in-test navigation
This PR fixes inconsistent behaviour in our browserTracingIntegration, where we'd previously add
ui.long-taskspans to an ongoing (navigation) transaction, even if the long task started before the navigation transaction started.Most other browserTracingIntegration spans (resource, timing, measure/mark spans) are already dropped/not started if their start time stamp is earlier than the navigation start time stamp. This happens in one central location but it does not apply to long-task spans because they have to be handled separately.
This PR specifically:
Note: We have similar behaviour for long animation frame spans. I'll probably open another PR to adjust the logic there as well.