fix(apm/tracing): update pageload op transaction start timestamps #2773
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Background
When I was testing my react router integration, I found an issue where the duration of pageload transactions in the performance view was different than the duration shown in the span view. Note: The pageload transactions that I am referring to are the one's automatically generated by the
@sentry/apm
or@sentry/tracing
Tracing integration.The transaction duration in the performance view is computed straight from the transaction payload,
end_timestamp
-start_timestamp
of the transaction. Meanwhile, the transaction duration in the span view is actually adjusted with some helper functions in the UI itself. This was to make sure that things were normalized so that strange traces would not be rendered in the UI. See the helpers here: https://github.com/getsentry/sentry/blob/master/src/sentry/static/sentry/app/components/events/interfaces/spans/utils.tsx#L500-L530This meant there were cases where pageload transactions had a computed duration of something like
140 ms
, but the span view would show a duration of300ms
. This also meant that if a user got alerted by a metric alert for average duration, they would see a different number in the individual transaction view (as metric alerts rely on transaction duration computed from snuba -end_timestamp
-start_timestamp
from event payload).As this issue was only occurring in a) automatically created transactions from browser b) pageload transactions, my first instinct was to check the performance spans we add to transactions. It turns out that was causing the issue.
The Issue
The problem was caused by the fact that when we add performance spans (
resource
,script
etc.), we do not adjust transaction start time. When you add certain performance related spans, they might have a start timestamp that is before the transaction's initial start time (they occured beforesentry-tracing-init
). This means that the transaction duration becomes much smaller than the duration of it's child spans.The helpers in the Sentry UI were automatically checking to make sure that if this was the case, that the span UI would render a transaction that had the
start_timestamp
of the earliest child span.In the example below,
sentry-tracing-init
only occurs 5000ms after all the performance related spans. This means that before the change in this PR, the computed duration in from the transaction payload would be5000ms
less than what actually is.Fix
This is fixed by adjusting the performance spans to adjust transaction start time accordingly. See the helper function
_startChild
for more details. This fix was made to both@sentry/apm
and@sentry/tracing
.This was tested with unit tests in
@sentry/tracing
and verified on Sentry master.Notes
It's important to note that these changes may break existing users alerts, because naturally pageload durations will rise as it now accounts for these extra spans.