🐛 [RUM-2782] remove buggy redirect timing estimation based on fetchStart #2683
Conversation
BenoitZugmeyer
force-pushed
the
benoit/remove-redirect-estimation-logic
branch
from
d75de18
to
64ef390
Compare
|
/to-staging |
|
🚂 Branch Integration: starting soon, merge in < 10m Commit 64ef390092 will soon be integrated into staging-14. This build is going to start soon! (estimated merge in less than 10m) Use |
Bundles Sizes Evolution
|
|
🚂 Branch Integration: This commit was successfully integrated Commit 64ef390092 has been merged into staging-14 in merge commit 9a182d9551. Check out the triggered pipeline on Gitlab 🦊 |
| const areRedirectionTimingsInOrder = | ||
| !hasRedirection(entry) || areInOrder(entry.startTime, entry.redirectStart, entry.redirectEnd, entry.fetchStart) |
There was a problem hiding this comment.
🥜 nitpick: I think it would be easier to read as:
| const areRedirectionTimingsInOrder = | |
| !hasRedirection(entry) || areInOrder(entry.startTime, entry.redirectStart, entry.redirectEnd, entry.fetchStart) | |
| const areRedirectionTimingsInOrder = hasRedirection(entry) | |
| ? areInOrder(entry.startTime, entry.redirectStart, entry.redirectEnd, entry.fetchStart) | |
| : true |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2683 +/- ##
==========================================
- Coverage 93.38% 93.33% -0.05%
==========================================
Files 239 239
Lines 6937 6931 -6
Branches 1526 1524 -2
==========================================
- Hits 6478 6469 -9
- Misses 459 462 +3 ☔ View full report in Codecov by Sentry. |
|
/to-staging |
|
🚂 Branch Integration: starting soon, merge in < 10m Commit 76ac2e2a62 will soon be integrated into staging-14. This build is going to start soon! (estimated merge in less than 10m) Use |
|
🚂 Branch Integration: This commit was successfully integrated Commit 76ac2e2a62 has been merged into staging-14 in merge commit d84fda558c. Check out the triggered pipeline on Gitlab 🦊 |
Motivation
The assumptions made when implementing #315 are not accurate anymore:
startTimecan be different fromfetchStartfor other reasons than a redirection (ex: when a service worker is involved)Firefox now provide accurate
redirectStartandredirectEndeven for cross-origin resources (as long as we have a correctTiming-Allow-Originheader, same as Chrome)This is a first step to fix #2566
Changes
Remove the redirect timing approximations in RUM resource events.
Testing
I have gone over the contributing documentation.