Skip to content
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

feat(cwv): get rid of third party dependency to GoogleChrome/web-vitals #158

Closed
wants to merge 17 commits into from

Conversation

ekremney
Copy link
Member

@ekremney ekremney commented Apr 18, 2024

This PR moves the Core Web Vitals (CWV) collection logic from the third-party dependency GoogleChrome/web-vitals to the inlined rum-enhancer.

To maintain consistency, the data collection logic for CWV metrics remains unchanged.

Here's an explanation of how CWV metrics are collected:

TTFB (Time to First Byte): Calculated using the navigation entry from the Performance API by subtracting activationStart from responseStart. (see What is TTFB?)

FCP (First Contentful Paint): Calculated using the paint entry from the Performance API. For the first-contentful-paint type paint entry, FCP is the value obtained by subtracting the activationStart time from the first-contentful-paint start time. (see Measure FCP in Javascript)

If the page is loaded from the back-forward cache, FCP is found by subtracting the back-forward cache event time from performance.now().

LCP (Largest Contentful Paint): Calculated using the largest-contentful-paint entry from the Performance API. Like FCP, LCP is the value obtained by subtracting the activationStart time from the 'large-contentful-paint' start time. (see Measure LCP in Javascript)

If the page is loaded from the back-forward cache, LCP is found by subtracting the back-forward cache event time from performance.now().

CLS (Cumulative Layout Shift): Calculated using the layout-shift entry from the Performance API. For each layout-shift event, the values are accumulated to determine the "cumulative" layout shift. (see Measure CLS in Javascript)

If the page is loaded from the back-forward cache, CLS is reported as 0 (zero).

INP (Interaction to Next Paint): Calculated using PerformanceEventTiming entries from the Performance API by tracking and processing user interaction entries on a web page. The code listens for interaction events (like clicks or key presses) and stores details about these interactions in longestInteractionList and longestInteractionMap. It updates the list with the ten longest interactions based on latency (duration). The code then estimates the 98th percentile of the longest interactions to determine the INP value, reflecting the responsiveness of the web page.

Important Considerations:

  1. Since parts of the Performance API are not implemented in all browsers, data collection is not possible in all browsers. In particular, Safari on iOS does not implement many interesting Performance API entries other than navigation entry, making it impossible to collect CWVs from Safari on iOS. Since all other browsers on iOS are essentially skins on top of Safari iOS, it is currently not possible to collect CWV from mobile iOS devices.

  2. Different values for a metric can be reported (in an increasing fashion) as the measurement changes throughout the lifespan of a visit. For example, new CLS values can be reported for a page if dynamically loaded content introduces new layout shifts. In cases where there is more than one measurement per type, the maximum value should be used.

Related Issues

#206

Thanks for contributing!

Copy link

This PR will trigger a minor release when merged.

@ekremney ekremney changed the title feat: cwv-lcp is measured manually feat: cwv is measured manually Apr 18, 2024
@ekremney ekremney changed the title feat: cwv is measured manually feat(cwv): metrics to be measured manually Apr 18, 2024
@ekremney ekremney changed the title feat(cwv): metrics to be measured manually feat(cwv): get rid of third party dependency to chrome/web-vitals May 27, 2024
@ekremney ekremney changed the title feat(cwv): get rid of third party dependency to chrome/web-vitals feat(cwv): get rid of third party dependency to GoogleChrome/web-vitals May 27, 2024
trieloff
trieloff previously approved these changes May 27, 2024
src/index.js Outdated Show resolved Hide resolved
src/index.js Outdated Show resolved Hide resolved
};
};

const initInteractionCountPolyfill = () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What browser versions need this polyfill? Can we do without it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code in web-vitals checks if 'interactionCount' in performance exists, if yes it doesn't use the polyfill. However, I couldn't find anything related to performance.interactionCount.

closest thing I could've find was performance.eventCounts which is not supported by Safari.

I will open an issue in the original repo and ask about this

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so, a chrome developer kindly informed me that performance.interactionCount has only been implemented by Chrome but it is behind a feature flag now. They expect to release it soon. Other browsers doesn't even have that; so yes, we need the polyfill. To quote them:

Chrome has implemented this feature, but it is still behind a feature flag in Stable (or via experimental web platform features). We expect to ship it in stable.

eventCounts is not possible to use as a way to estimate interactionCount (and its the reason Event Timing added interactionCount). (You can also find lots of discussion in the Web Perf WG for this feature).

The single biggest flaw of eventCount is that it does not differentiate when e.g. pointer events actually become a scroll, or how keyboard events might trigger clicks, or how accessibility or IME interfaces might dispatch events in different orders...

You could try to approximate interactionCount but using something like the count of clicks - count of pointerCancel, and an earlier version of web-vitals.js actually tried that (especially since this is supported on Firefox). Unfortunately it was off and unreliable on all browsers.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok

src/index.js Outdated
});
};

sampleRUM.onFCP = (cb) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CLS should only be reported only if FCP is reported. According to the web-vitals, this is the current behavior of the crux: https://github.com/GoogleChrome/web-vitals/blob/main/src/onCLS.ts#L60

we don't have to collect it, but we need to measure it

@ekremney ekremney marked this pull request as ready for review May 27, 2024 16:10
@kptdobe
Copy link
Contributor

kptdobe commented May 30, 2024

Something that the original library has which is missing here is... tests!

@kptdobe kptdobe mentioned this pull request May 30, 2024
@trieloff
Copy link
Contributor

@ekremney feature flag rework has been merged.

@ekremney
Copy link
Member Author

ekremney commented Jun 6, 2024

@trieloff here's the changes:

  1. the new measurement is called cwv2. Now we have cwv and cwv2
  2. cwv2 is behind feature flag for bacom

Screenshot 2024-06-06 at 16 11 34

Copy link

codecov bot commented Jun 11, 2024

Codecov Report

Attention: Patch coverage is 78.97436% with 82 lines in your changes missing coverage. Please review.

Project coverage is 67.54%. Comparing base (9c156a3) to head (9c1a535).
Report is 37 commits behind head on 1.x.

Files with missing lines Patch % Lines
src/index.js 78.97% 82 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##              1.x     #158       +/-   ##
===========================================
+ Coverage   36.90%   67.54%   +30.64%     
===========================================
  Files           1        1               
  Lines         271      647      +376     
===========================================
+ Hits          100      437      +337     
- Misses        171      210       +39     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

trieloff
trieloff previously approved these changes Jun 14, 2024
@ekremney
Copy link
Member Author

ekremney commented Dec 3, 2024

closing this PR as the work here will be discontinued

@ekremney ekremney closed this Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants