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

Add web vital metrics #836

Merged
merged 19 commits into from Mar 29, 2023
Merged

Add web vital metrics #836

merged 19 commits into from Mar 29, 2023

Conversation

ankur22
Copy link
Collaborator

@ankur22 ankur22 commented Mar 27, 2023

What & Why

This adds the core functionality to get k6-browser exposing web vital metrics out of the box for all users. This means that users will be able to work with industry standard metrics to measure their frontend, and based on those results alone be able to identify areas of their site that requires work.:

It's worth noting that web vitals do not measure the aesthetics of the website. If an element is out of place after a code change, then the only way of measuring this is to manually track those changes and encode them in the test script (using the coordinates of the element), or comparing screenshots of the site before and after the code change.

How

The changes can be summarised to:

  1. Adding the web vital metric library into the k6-browser binary.
  2. Implementing the page.evaluateOnNewDocument method so that web vital metric can be initialised on all new pages.
  3. Binding a method to the window object in the DOM so that the web vital metrics can be sent to k6-browser for processing.
  4. Registering the six new web vital metrics in k6-browser.
  5. Removing the use of the existing first contentful paint metric, which comes via a CDP event. The js webvital library calculates this metric and also a rating. We want to keep the source of web vital metrics the same so that the values are "consistent" with each. This is a breaking change since it will affect anyone parsing the output summary.

Two existing unmerged PRs/branches were used to help with this solution:

Test

To test the change, you should be able to run any test. I've used fillform.js from the examples directory and this is the specific result you are looking for after the test run has completed (the values may be slightly different:

     webvital_cumulative_layout_shift..........: avg=0        min=0        med=0        max=0        p(90)=0        p(95)=0       
     webvital_cumulative_layout_shift_good.....: 1       0.147673/s
     webvital_first_contentful_paint...........: avg=292.6ms  min=148.4ms  med=243ms    max=486.39ms p(90)=437.71ms p(95)=462.05ms
     webvital_first_contentful_paint_good......: 3       0.44302/s
     webvital_first_input_delay................: avg=500µs    min=299.99µs med=500µs    max=700µs    p(90)=660µs    p(95)=680µs   
     webvital_first_input_delay_good...........: 2       0.295346/s
     webvital_interaction_to_next_paint........: avg=16ms     min=16ms     med=16ms     max=16ms     p(90)=16ms     p(95)=16ms    
     webvital_interaction_to_next_paint_good...: 1       0.147673/s
     webvital_largest_content_paint............: avg=317.4ms  min=148.4ms  med=317.4ms  max=486.39ms p(90)=452.59ms p(95)=469.49ms
     webvital_largest_content_paint_good.......: 2       0.295346/s
     webvital_time_to_first_byte...............: avg=215.4ms  min=105.3ms  med=207.89ms max=333ms    p(90)=307.97ms p(95)=320.48ms
     webvital_time_to_first_byte_good..........: 3       0.44302/s

Closes: #690

ankur22 and others added 7 commits March 23, 2023 16:38
We're going to bundle the web vital js library from the internet
instead of downloading them on each test run. This will ensure that
users who run tests in a locked down environment can still work with
web vitals, and it will avoid any CORS related issues. The downside
is that we will need to manually update this library, and users who
do not update to latest k6 binary will be working with an outdated
version of the library.
This will reduce the complexity of the method.
This will evaluate the script on the current page. BrowserContext
relies on page.evaluateOnNewDocument when a new init script is added
so that they are applied to all existing pages.

For now, the preference is not to work with the worldName param in the
AddScriptToEvaluateOnNewDocument CDP command. For now we don't want
the script to be isolated to a particular named world.

The script identifier that is returned from a successful CDP command
to add the init script, is ignored. When we need it (e.g. for removing
init scripts) we should use it then.

Co-authored-by: Daniel Jiménez <daniel.jimenez@grafana.com>
When a new page is created, it needs to add the init scripts that the
browser context has had applied to it.

Instead of retrieving the list of scripts to apply in the page, we're
sending the current page to the browser context, and it will loop over
the scripts it has in evaluateOnNewDocumentSources.
Bindings are a CDP runtime feature, where we can bind the current
frame with a method on the window object in the browser. When the
method is called with a payload, the listener in frame_session.go will
currently read from the event and logs the output.
By refactoring this one method, we need to follow the chain up so that
other methods that work with NewBrowserContext also return an error.
We need NewBrowserContext to return an error so that we can validate
the web vital init script ,which is currently being added to
evaluateOnNewDocumentSources without any validation (change to come in
the next commit).
@ankur22 ankur22 marked this pull request as draft March 27, 2023 14:43
@ankur22 ankur22 force-pushed the feat/690-web-vitals branch 4 times, most recently from c301a50 to f5d9782 Compare March 27, 2023 15:02
Now that we can return errors from NewBrowserContext, we can also
validate the web vital script. We might as well reuse the
AddInitScript to help us validate and add the web vital script to
evaluateOnNewDocumentSources and ready for use by the new pages.
The Web Vital init script glues together the existing change. We first
import the web vital lib and web vital init scripts into the website
when a new page is created. The init script uses the web vital lib
script which has been predownloaded, and the binding setup earlier to
send the web vital metric payload to xk6-browser. At the moment the
metric payload is logged (debug only).
This adds the six web vitals, as well as the three ratings for each.
When a web vital metric is send from the JS code to k6-browser via
the binding, it will need to extract the metric name and the rating.

The rating can be good, needs-improvement or poor.
This unmarshalls the web vital metric from the JS script; does some
error checking (ensuring that the metric is recognized and is
registered), and finally emits the metric.
We're now getting web vital metrics which are calculated by the JS
web vital script. It also contains first contentful paint FCP metric.
The metric coming from the JS lib also comes back with a rating. We
can't correlate that with the current CDP FCP. To keep web vital
metrics source consistent, we will remove the existing CDP FCP and opt
to work with the one coming from the JS lib.
@ankur22 ankur22 marked this pull request as ready for review March 27, 2023 15:31
Copy link
Member

@inancgumus inancgumus left a comment

Choose a reason for hiding this comment

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

Nice work 👏 It's great to see that you figured out the bindings and implemented WebVitals 🎉

I don't know how to test this, though. Could you help me doing that? It would be a good idea to provide a JS/Go test for this important feature.

Copy link
Member

Choose a reason for hiding this comment

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

The downside is that we will need to manually update this library, and users who do not update to latest k6 binary will be working with an outdated version of the library.

It's better to fixate on a single version rather than breaking our code when the original webvital js lib gets updated. So, to me, it's an upside :)

common/js/web_vital_init.js Outdated Show resolved Hide resolved
k6ext/metrics.go Show resolved Hide resolved
common/frame_session.go Outdated Show resolved Hide resolved
common/frame_session.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@ka3de ka3de left a comment

Choose a reason for hiding this comment

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

Great work @ankur22 ! 👏
Made some initial comments, I will test this better before giving the final approval.
As @inancgumus said, I think it would be good to work on a test to at least verify that the scripts are correctly included for every page, being called etc.

common/frame_session.go Outdated Show resolved Hide resolved
common/frame_session.go Outdated Show resolved Hide resolved
common/page.go Outdated Show resolved Hide resolved
ankur22 and others added 5 commits March 28, 2023 12:07
Co-authored-by: İnanç Gümüş <inanc.gumus@grafana.com>
Co-authored-by: Daniel Jiménez <daniel.jimenez@grafana.com>
This will hopefully help highlight that it is an internal k6-browser
function that was defined by us, and also not be used by anyone else.

Resolves: #836 (comment)
It was using POC code to import the module directly from the internet
instead of using the library that was downloaded and embedded in the
k6-browser binary and initialised when the browser context was
created.

Resolves: #836 (comment)
This test ensures that the embedded web vital scripts have been added
to the browser contexts internal evaluateOnNewDocumentSources slice.
It does not check whether this has then been applied to a page.
This allows us to read off the metric sample channel when it gets
populated with metrics during a test run. This will be useful to
assert that certain metrics have been emitted during a test.
This tests ensures that web vitals are measured and emitted to the
metric samples channel. We read off the channel and assert that
expected web vitals metrics have been emitted during the test run.
@ankur22
Copy link
Collaborator Author

ankur22 commented Mar 28, 2023

I've added a unit test and an integration test. WDYT @inancgumus @ka3de?

Copy link
Collaborator

@ka3de ka3de left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Member

@inancgumus inancgumus left a comment

Choose a reason for hiding this comment

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

👍

@ankur22 ankur22 merged commit c9ae92a into main Mar 29, 2023
16 checks passed
@ankur22 ankur22 deleted the feat/690-web-vitals branch March 29, 2023 12:51
ankur22 added a commit that referenced this pull request Mar 29, 2023
This will hopefully help highlight that it is an internal k6-browser
function that was defined by us, and also not be used by anyone else.

Resolves: #836 (comment)
ankur22 added a commit that referenced this pull request Mar 29, 2023
ankur22 added a commit that referenced this pull request Mar 29, 2023
It was using POC code to import the module directly from the internet
instead of using the library that was downloaded and embedded in the
k6-browser binary and initialised when the browser context was
created.

Resolves: #836 (comment)
ankur22 added a commit that referenced this pull request Apr 4, 2023
This is a regression. We had removed this in
#836, but it was
accidentally added back in
#838.
ankur22 added a commit that referenced this pull request Apr 5, 2023
This is a regression. We had removed this in
#836, but it was
accidentally added back in
#838.
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.

Add the Web Vital JS code with the existing injected script.
3 participants