Navigation Menu

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(Page): Support Page.getMetrics and metrics event. #939

Merged
merged 6 commits into from Oct 10, 2017

Conversation

a1ph
Copy link
Contributor

@a1ph a1ph commented Oct 2, 2017

Provides access to the current page performance metrics.
Allows to push page metrics from the page JavaScript with console.timeStamp()

Fixes #309

Provides access to the current page performance metrics.
Allows to push page metrics from the page JavaScript with console.timeStamp()
@aslushnikov
Copy link
Contributor

Thanks Alexei!

Let's list the metric names explicitly so that it's easier to consume.

test/test.js Outdated
checkMetrics(metrics.metrics);
}));
function checkMetrics(metrics) {
expect(metrics['Timestamp']).toBeGreaterThan(0);
Copy link
Contributor

Choose a reason for hiding this comment

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

can we also check that there are no other fields beyond these? this would allow us to update docs when something new is added

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are other fields coming over the protocol. But we don't want to document/expose those in puppeteer.

lib/Page.js Outdated
_buildMetricsObject(metrics) {
const result = {};
for (const metric of metrics || [])
result[metric.name] = metric.value;
Copy link
Contributor

Choose a reason for hiding this comment

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

since we don't want to expose certain numbers, can we white-list properties that we copy (instead of copying everything)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@aslushnikov aslushnikov merged commit b82d319 into puppeteer:master Oct 10, 2017
ithinkihaveacat pushed a commit to ithinkihaveacat/puppeteer that referenced this pull request Oct 31, 2017
Provides access to the current page performance metrics.
Allows to push page metrics from the page JavaScript with console.timeStamp()

Fixes puppeteer#309
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.

None yet

2 participants