search

HTTPArchive / custom-metrics

Custom metrics to use with WebPageTest agents
Apache License 2.0
19 stars 22 forks source link

Add custom metric to measure generated content (size and percent) #108

Closed kevinfarrugia closed 7 months ago

kevinfarrugia commented 7 months ago

Adds support for generated-content custom metric that includes percentage and size in KB. This is helpful to recognize web pages that rely on JavaScript for generating most of the content, such as CSR apps.

As an example, see 01_test custom metric in the following runs:

{"percent":"8.55","sizeInKB":"2.42"}    
{"percent":"85.40","sizeInKB":"19.92"}

Test websites:

tunetheweb commented 7 months ago

While I agree with your suggestions @rviscomi I wonder if it's worth the divergence from the Catchpoint ones (which btw will be what will is generated from a manual run on webpagetest.httparchive.org)?

One of the reasons we're adding these is that it was a confusing that they half available in our instance (in manual runs, but not the crawl).

Maybe we should go with it as is for now and try to suggest upstream changes to theirs and, if that is accepted, then merge that change in?

pmeenan commented 7 months ago

We are running a fork so I could just delete the catchpoint version of the metrics. All of the server custom metrics should be disabled anyway to be consistent with the crawl anyway.

On Thu, Jan 18, 2024 at 12:40 PM Barry Pollard @.***> wrote:

While I agree with your suggestions @rviscomi https://github.com/rviscomi I wonder if it's worth the divergence from the Catchpoint ones (which btw will be what will is generated from a manual run on webpagetest.httparchive.org)?

One of the reasons we're adding these is that it was a confusing that they half available in our instance (in manual runs, but not the crawl).

Maybe we should go with it as is for now and try to suggest upstream changes to theirs and, if that is accepted, then merge that change in?

— Reply to this email directly, view it on GitHub https://github.com/HTTPArchive/custom-metrics/pull/108#issuecomment-1898933214, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADMOBIJY4U6CFEVONQMUQLYPFNAVAVCNFSM6AAAAABCAQ6FKGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOJYHEZTGMRRGQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>

tunetheweb commented 7 months ago

That SGTM. It will still be inconsistent with webpagetest.org but I can live with that.

@kevinfarrugia do you want to add to this PR?

kevinfarrugia commented 7 months ago

Yes, it makes sense to me. There are already some small changes, so I think it's fine to not have the same implementation.

I'll just double check everything and push an update.

kevinfarrugia commented 7 months ago

Heads up that one of the tests in the GitHub action did not run.

kevinfarrugia commented 7 months ago

I took a look at the test results for https://almanac.httparchive.org/en/2022/ and it reports negative values. This is correct. The initial HTML response is larger than the one after JavaScript executes as JavaScript is used to update or remove elements, for example those containing the CSS classes js-hide or js-enable.

kevinfarrugia commented 7 months ago

@pmeenan If I understood correctly, removing the catchpoint version of the metric should be done from the webpagetest repo.

If that is correct, is there anything else needed to include in this PR?

pmeenan commented 7 months ago

I removed all of the server-side custom metrics from our fork (just now) so it will accurately reflect what will be measured in the crawl.

rviscomi commented 7 months ago

@kevinfarrugia is it expected that the results for pesuar.com are {}?

In the top comment you noted that you got {"percent":"85.40","sizeInKB":"19.92"}

kevinfarrugia commented 7 months ago

No, that is not expected. The results should be {"percent":"85.40","sizeInKB":"19.92"} but the test run is failing. This issue occurs even on other URLs with or without the custom metric.

If I add the custom metric to webpagetest.org it works as expected.

rviscomi commented 7 months ago

Weird ok. I can get the custom metric to succeed in a manual test:

{"percent":"0.8502","sizeInKB":"19.32"}
rviscomi commented 7 months ago

@pmeenan are you concerned about the test failure or do you think we can safely merge this? (the test consistently failed on this page a few times)

pmeenan commented 7 months ago

Likely safe to merge (worst case should be that the metric is missing). That said, it might be worth some error handling for the case where the response_body (or even request 0) doesn't exist. I THINK that if the custom metric throws it should just be empty but that's the only part that doesn't look safe.

github-actions[bot] commented 7 months ago
Custom metrics for https://almanac.httparchive.org/en/2022/ WPT test run results: http://webpagetest.httparchive.org/results.php?test=240126_6D_5
Custom metrics for https://imkev.dev/ WPT test run results: http://webpagetest.httparchive.org/results.php?test=240126_NY_6 Changed custom metrics values: ```json { "_generated-content": { "percent": "0.0855", "sizeInKB": "2.42" } } ```
Custom metrics for https://pesuar.com/ WPT test run results: http://webpagetest.httparchive.org/results.php?test=240126_4F_7 Changed custom metrics values: ```json {} ```