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.

Sign up for GitHub

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

Upgrade web-vitals to 3.3.1 and switch to attribution build #115

Merged
merged 37 commits into from
Apr 19, 2023
Merged

Upgrade web-vitals to 3.3.1 and switch to attribution build #115

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
a341315
Upgrade web-vitals to 3.3.1
tunetheweb Apr 12, 2023
530a228
Upgrade src version
tunetheweb Apr 12, 2023
9a7f95d
Switch to attribution build
tunetheweb Apr 13, 2023
aba8d54
Add LCP breakdowns
tunetheweb Apr 13, 2023
41db69f
Fix Math.max
tunetheweb Apr 13, 2023
2d94edc
Fixes
tunetheweb Apr 13, 2023
55d87a2
Fix INP
tunetheweb Apr 13, 2023
87f4d3b
Tab
tunetheweb Apr 13, 2023
672d572
Fixes
tunetheweb Apr 13, 2023
e8074a2
Fix bug
tunetheweb Apr 13, 2023
8287b4f
Debug
tunetheweb Apr 13, 2023
f08879c
Restore LCP breakdowns
tunetheweb Apr 13, 2023
f0194c7
Remove LCP element
tunetheweb Apr 13, 2023
6ff84fb
Clean up
tunetheweb Apr 13, 2023
1fd9965
More breakdowns
tunetheweb Apr 13, 2023
3b533e3
Add total LCP time
tunetheweb Apr 13, 2023
f93ba16
Rename console table loggin
tunetheweb Apr 13, 2023
04939f7
Rename INP in table to make obvious not one of the breakdowns
tunetheweb Apr 13, 2023
a0ed05f
Fix capitalisation
tunetheweb Apr 13, 2023
991ff04
Re-enable reportAllChanges
tunetheweb Apr 13, 2023
0b7eb72
Rework CLS table
tunetheweb Apr 13, 2023
8ae60ae
Switch INP to attribution entry to ensure we use longest one
tunetheweb Apr 13, 2023
b0976dc
Consistency
tunetheweb Apr 13, 2023
7af7193
Missed a reportAllChanges
tunetheweb Apr 13, 2023
42c1862
Remove unnecessary columns to display
tunetheweb Apr 13, 2023
2353ffa
Add entry name to table
tunetheweb Apr 13, 2023
94d1311
Fix LCP performance.measure
tunetheweb Apr 14, 2023
91ded44
Review feedback
tunetheweb Apr 15, 2023
7649895
More CLS breakdown data
tunetheweb Apr 15, 2023
7716a8e
Fix shift plurals
tunetheweb Apr 17, 2023
21333b4
Yet more review feedback
tunetheweb Apr 17, 2023
930250c
Add "element"
tunetheweb Apr 17, 2023
64fd4f0
Prefer localName
tunetheweb Apr 17, 2023
130a173
Add rating
tunetheweb Apr 17, 2023
c2f37e6
Gilbert feedback
tunetheweb Apr 18, 2023
15fd43a
Add elements column to all tables
tunetheweb Apr 18, 2023
d6f3b74
Add element number
tunetheweb Apr 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,15 @@
"license": "Apache-2.0",
"private": true,
"scripts": {
"lint": "npx eslint src --fix"
"lint": "npx eslint src --fix",
"build": "npm install; cp node_modules/web-vitals/dist/web-vitals.attribution.js src/browser_action/web-vitals.js"
},
"devDependencies": {
"babel-eslint": "^10.1.0",
"eslint": "^6.8.0",
"eslint-config-google": "^0.14.0"
},
"dependencies": {
"web-vitals": "^3.0.0-beta.0"
"web-vitals": "^3.3.1"
}
}
174 changes: 134 additions & 40 deletions src/browser_action/vitals.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,13 @@
let latestCLS = {};
let enableLogging = localStorage.getItem('web-vitals-extension-debug')=='TRUE';
let enableUserTiming = localStorage.getItem('web-vitals-extension-user-timing')=='TRUE';
let enableUserTimingConsole = localStorage.getItem('web-vitals-extension-user-timing-console')=='TRUE';
tunetheweb marked this conversation as resolved.
Show resolved Hide resolved

// Core Web Vitals thresholds
const LCP_THRESHOLD = 2500;
const FID_THRESHOLD = 100;
const INP_THRESHOLD = 200;
const CLS_THRESHOLD = 0.1;
const LCP_THRESHOLD = webVitals.LCPThresholds[0];
const FID_THRESHOLD = webVitals.FIDThresholds[0];
const INP_THRESHOLD = webVitals.INPThresholds[0];
const CLS_THRESHOLD = webVitals.CLSThresholds[0];

// CLS update frequency
const DEBOUNCE_DELAY = 500;
Expand Down Expand Up @@ -118,8 +119,9 @@
enableOverlay: false,
debug: false,
userTiming: false,
userTimingConsole: false,
}, ({
enableOverlay, debug, userTiming,
enableOverlay, debug, userTiming, userTimingConsole,
}) => {
if (enableOverlay === true && overlayClosedForSession == false) {
// Overlay
Expand Down Expand Up @@ -162,13 +164,20 @@
localStorage.removeItem('web-vitals-extension-debug');
enableLogging = false;
}
if (debug) {
if (userTiming) {
tunetheweb marked this conversation as resolved.
Show resolved Hide resolved
localStorage.setItem('web-vitals-extension-user-timing', 'TRUE');
enableUserTiming = true;
} else {
localStorage.removeItem('web-vitals-extension-user-timing');
enableUserTiming = false;
}
if (userTimingConsole) {
localStorage.setItem('web-vitals-extension-user-timing-console', 'TRUE');
enableUserTimingConsole = true;
} else {
localStorage.removeItem('web-vitals-extension-user-timing-console');
enableUserTimingConsole = false;
}
});
}

Expand Down Expand Up @@ -205,10 +214,10 @@
return;
}
if (enableLogging) {
console.log('[Web Vitals]', body.name, body.value.toFixed(2), body);
console.log('[Web Vitals Extension]', body.name, body.value.toFixed(2), body);
}
if (enableUserTiming) {
addUserTimings(body);
if (enableUserTiming || enableUserTimingConsole) {
addUserTimings(body, enableUserTiming, enableUserTimingConsole);
}
badgeMetrics[metricName].value = body.value;
badgeMetrics.location = getURL();
Expand All @@ -224,17 +233,62 @@
);
}

function addUserTimings(metric) {
function addUserTimings(metric, enableUserTiming, enableUserTimingConsole) {
switch (metric.name) {
case "LCP":
tunetheweb marked this conversation as resolved.
Show resolved Hide resolved
// LCP has a loadTime/renderTime (startTime), but not a duration.
// Could visualize relative to timeOrigin, or from loadTime -> renderTime.
// Skip for now.
if (metric.attribution) {
// Set the start time to the later of the actual start time or the activationStart (for prerender) or 0
const startTime = Math.max(metric.attribution.navigationEntry?.startTime, metric.attribution.navigationEntry?.activationStart) || 0;
// Add the performance marks for the Performance Panel
if (enableUserTiming) {
performance.measure(`[Web Vitals Extension] LCP.timeToFirstByte`, {
start: startTime,
duration: metric.attribution.timeToFirstByte,
});
performance.measure(`[Web Vitals Extension] LCP.resourceLoadDelay`, {
start: startTime + metric.attribution.timeToFirstByte,
duration: metric.attribution.resourceLoadDelay,
});
performance.measure(`[Web Vitals Extension] LCP.resourceLoadTime`, {
start: startTime + metric.attribution.timeToFirstByte + metric.attribution.resourceLoadDelay,
duration: metric.attribution.resourceLoadTime,
});
performance.measure(`[Web Vitals Extension] LCP.elmentRenderDelay`, {
duration: metric.attribution.elementRenderDelay,
end: metric.value
});
}
// Add a nice console output
if (enableUserTimingConsole) {
console.table(
[
{
'LCP Breakdown': 'timeToFirstByte',
'Time (ms)': metric.attribution.timeToFirstByte.toFixed(2),
},
{
'LCP Breakdown': 'resourceLoadDelay',
'Time (ms)': metric.attribution.resourceLoadDelay.toFixed(2),
},
{
'LCP Breakdown': 'resourceLoadTime',
'Time (ms)': metric.attribution.resourceLoadTime.toFixed(2),
},
{
'LCP Breakdown': 'elementRenderDelay',
'Time (ms)': metric.attribution.elementRenderDelay.toFixed(2),
}
],
['LCP Breakdown', 'Time (ms)']
)
}
}
break;
case "CLS":
// CLS has a startTime, but not a duration.
// Could visualize the time between rendering tasks (Commit-to-Commit).
// Skip for now.
if (enableUserTimingConsole) {
// Add a nice console output of all the shifts
console.table(metric.entries, ['entryType', 'hadRecentInput', 'value'])
}
break;
case "INP":
if (metric.entries.length > 0) {
Expand All @@ -246,31 +300,71 @@
const presentationTime = inpEntry.startTime + inpEntry.duration;
const adjustedPresentationTime = Math.max(inpEntry.processingEnd + 4, presentationTime);

performance.measure(`[Web Vitals] INP.duration (${inpEntry.name})`, {
start: inpEntry.startTime,
end: presentationTime,
});
performance.measure(`[Web Vitals] INP.inputDelay (${inpEntry.name})`, {
start: inpEntry.startTime,
end: inpEntry.processingStart,
});
performance.measure(`[Web Vitals] INP.processingTime (${inpEntry.name})`, {
start: inpEntry.processingStart,
end: inpEntry.processingEnd,
});
performance.measure(`[Web Vitals] INP.presentationDelay (${inpEntry.name})`, {
start: inpEntry.processingEnd,
end: adjustedPresentationTime,
});
if (enableUserTiming) {
performance.measure(`[Web Vitals Extension] INP.duration (${inpEntry.name})`, {
Copy link
Member

Choose a reason for hiding this comment

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

Aside: any reason these INP sub-parts aren't a standard part of web-vitals attribution?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a good question!

Maybe @mmocny can confirm but maybe because it was less mature back then? Or because they are directly referenceable from the Entry itself it wasn't as important to add to the web-vitals build, as that differs slightly from LCP which needs the nav entry for TTFB and then the LCP resource time entry for the other parts so a bit more complicated.

Anyway, as you know Michael's been iterating on this recently so I imagine these sub-parts could change when he adds his "live breakdown" piece after this PR is merged. But still think this is a good first step and good to include for the reasons given above.

But do agree we should add whatever we finally land on to the web-vitals attribution build so other users can grab them easily - at which point we can reference them here too. It wouldn't really simplify the performance.measure (which needs both entries anyway) but could simplify the table code slightly (which only needs the duration).

Copy link
Member

Choose a reason for hiding this comment

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

We did want to add something to web-vitals attribution, but I decided to wait until we knew better exactly what we want to recommend measuring so as not to break compat eventually.

A long time ago we recommended just doing what we do here in the extension (split a single event into the three components: input-delay, processing-duration, presentation-delay)-- but as Barry says you can do that easily from a single entry.

In practice, processing-duration is better to SUM(all-events) in a single Animation Frame, rather than just looking at a single event. But when you do that, you end up having to re-define input-delay and presentation-delay to be "delay until first event" and "delay after last event".

Finally, presentation-delay can be split into "rendering-duration" (on main) and "presentation-delay" (off main).

All these things become easier (or possible, even) with Long Animation Frame (LoAF) reporting, and so that's where I've been iterating.

(The direction tooling is going is to "squash" multiple interactions in a single frame down to a single frame latency-- so we'll probably want to evolve this in the extension as well. I'll try a stab at updating INP user timings after updating INP logs.)

start: inpEntry.startTime,
end: presentationTime,
});
performance.measure(`[Web Vitals Extension] INP.inputDelay (${inpEntry.name})`, {
start: inpEntry.startTime,
end: inpEntry.processingStart,
});
performance.measure(`[Web Vitals Extension] INP.processingTime (${inpEntry.name})`, {
start: inpEntry.processingStart,
end: inpEntry.processingEnd,
});
performance.measure(`[Web Vitals Extension] INP.presentationDelay (${inpEntry.name})`, {
start: inpEntry.processingEnd,
end: adjustedPresentationTime,
});
}
if (enableUserTimingConsole) {
// Add a nice console output
console.table(
[
{
'INP Breakdown': 'duration',
'Time (ms)': (presentationTime - inpEntry.startTime).toFixed(2),
},
{
'INP Breakdown': 'inputDelay',
'Time (ms)': (inpEntry.processingStart - inpEntry.startTime).toFixed(2),
},
{
'INP Breakdown': 'processingTime',
'Time (ms)': (inpEntry.processingEnd - inpEntry.processingStart).toFixed(2),
},
{
'INP Breakdown': 'presentationDelay',
'Time (ms)': (adjustedPresentationTime - inpEntry.processingEnd).toFixed(2),
}
],
['INP Breakdown', 'Time (ms)']
)
}
}
break;
case "FID":
if (metric.entries.length > 0) {
const fidEntry = metric.entries[0]
performance.measure(`[Web Vitals Extension] FID (${fidEntry.name})`, {
start: fidEntry.startTime,
end: fidEntry.processingStart,
});
if (enableUserTiming) {
performance.measure(`[Web Vitals Extension] FID (${fidEntry.name})`, {
start: fidEntry.startTime,
end: fidEntry.processingStart,
});
}
if (enableUserTimingConsole) {
// Add a nice console output
console.table(
[
{
'FID Breakdown': 'inputDelay',
'Time (ms)': (metric.value).toFixed(2),
},
],
['FID Breakdown', 'Time (ms)']
)
}
}
}
}
Expand Down Expand Up @@ -306,20 +400,20 @@
if (self._hasInstalledPerfMetrics) return;
self._hasInstalledPerfMetrics = true;

webVitals.getCLS((metric) => {
webVitals.onCLS((metric) => {
// As CLS values can fire frequently in the case
// of animations or highly-dynamic content, we
// debounce the broadcast of the metric.
latestCLS = metric;
debouncedCLSBroadcast();
}, true);
webVitals.getLCP((metric) => {
webVitals.onLCP((metric) => {
broadcastMetricsUpdates('lcp', metric);
}, true);
webVitals.getFID((metric) => {
webVitals.onFID((metric) => {
broadcastMetricsUpdates('fid', metric);
}, true);
webVitals.getINP((metric) => {
webVitals.onINP((metric) => {
broadcastMetricsUpdates('inp', metric);
}, true);
}
Expand Down
Loading