Skip to content

fix(Page.getMetrics): do not report a few metrics #1001

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

Jump to bottom
Merged
merged 1 commit into from
Oct 10, 2017
Merged

fix(Page.getMetrics): do not report a few metrics #1001

merged 1 commit into from
Oct 10, 2017

Conversation

@aslushnikov
Copy link
Contributor

@aslushnikov aslushnikov commented Oct 10, 2017

A few of the reported metrics in the Page.getMetrics are already
reported with lifecycle events.

This patch excludes these metrics for now.

@aslushnikov
fix(Page.getMetrics): do not report a few metrics
4d1beeb 
A few of the reported metrics in the Page.getMetrics are already
reported with lifecycle events.

This patch excludes these metrics for now.
@aslushnikov aslushnikov requested a review from a1ph October 10, 2017 22:19
@aslushnikov aslushnikov merged commit 464b6a9 into puppeteer:master Oct 10, 2017
ithinkihaveacat pushed a commit to ithinkihaveacat/puppeteer that referenced this pull request Oct 31, 2017
@aslushnikov @ithinkihaveacat
fix(Page.getMetrics): do not report a few metrics (puppeteer#1001)
9a1bae1 
A few of the reported metrics in the Page.getMetrics are already
reported with lifecycle events.

This patch excludes these metrics for now.
@Everettss
Copy link

Everettss commented Nov 9, 2017 •
edited
Loading

Oh god. Why did you @aslushnikov remove this metrics?
Can you point how to now get FirstMeaningfulPaint DomContentLoaded NavigationStart times without:

await page._client.send('Performance.enable');
const metrics = await page._client.send('Performance.getMetrics');

?
It is very confusing that page.getMetrics() doesn't report basic metrics (!), and we are forced to search them in some "lifecycle events" (whatever they are).

@aslushnikov
Copy link
Contributor Author

aslushnikov commented Nov 11, 2017

Oh god. Why did you @aslushnikov remove this metrics?
Can you point how to now get FirstMeaningfulPaint DomContentLoaded NavigationStart times

@Everettss you can get DOMContentLoaded and NavigationStart from window.performance DOM api. But, we'll bring them back!

There is too much movement in the First*Paint metrics, so we're hesitant exposing them at this point. What are your usecases for them?

@inikulin
Copy link

inikulin commented Nov 19, 2017

@aslushnikov

There is too much movement in the First*Paint metrics, so we're hesitant exposing them at this point. What are your usecases for them?

Like... measuring performance?

@aslushnikov aslushnikov deleted the avoid-few-metrics branch January 25, 2018 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@a1ph a1ph a1ph approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@aslushnikov @Everettss @inikulin @a1ph