-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(browser): Send additional LCP timing info #14372
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
Conversation
size-limit report 📦
|
❌ 1 Tests Failed:
View the top 1 failed tests by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
d2b7830 to
d0e0bac
Compare
12e36fa to
a90c628
Compare
|
The failing tests are just NextJS canary tests (which we know currently fail but that's unrelated to this PR) and the test flake detector which times out after an hour. So this PR is ready for review. |
| span.setAttribute('lcp.url', _lcpEntry.url.trim().slice(0, 200)); | ||
| } | ||
|
|
||
| if (_lcpEntry.loadTime != null) { |
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.
Is != on purpose or do we want !== here? (also for renderTime)
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.
Yes, I only want to add the attribute it loadTime is not null or undefined. != null is a shorthand way of checking for that (and a bit more size efficient than !== undefined)
This PR adds
loadTimeandrenderTimeattributes from the LCP performance entry to thepageloadspan. Besides being valuable information for users, this also helps us detect if LCP is potentially unreliable:Timing-Allow-Originheader was not sent in the resource response, therenderTimewill be0.Background information: The
web-vitalslibrary takes thestartTimeattribute of the LCP entry as the LCP value.startTimeis set to:renderTimeifrenderTime > 0loadTimeotherwise