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

New: Added Chrome Traces to cdp connector #420

Closed
wants to merge 1 commit into from

Conversation

axemclion
Copy link

Chrome Traces has much more information about performance and
rules can use the new events - tracing::data and tracing::end
for trace information.

This information can be used for new rules like firstPaint,
total time taken to parseHTML, long frames, rendering, etc.

@jsf-clabot
Copy link

jsf-clabot commented Aug 11, 2017

CLA assistant check
All committers have signed the CLA.

@alrra
Copy link
Contributor

alrra commented Aug 11, 2017

@axemclion Thanks for the PR!

Can you sign the CLA? Thanks!


Cc: @molant @sarvaje

@panarasi
Copy link

Sorry, I think I sent the PR using a wrong account. I work for Microsoft.

@molant
Copy link
Member

molant commented Aug 11, 2017

@panarasi you still need to sign the CLA. The code now belongs to the JS Foundation 😊

@sarvaje
Copy link
Contributor

sarvaje commented Aug 11, 2017

@panarasi the build is failing because of some eslint errors

Chrome Traces has much more information about performance and
rules can use the new events - tracing::data and tracing::end
for trace information.

This information can be used for new rules like firstPaint,
total time taken to parseHTML, long frames, rendering, etc.
@axemclion
Copy link
Author

@sarvaje Fixed eslint issues.

@axemclion
Copy link
Author

axemclion commented Aug 11, 2017

In line 551, I am not using reject since the whole block is wrapped inside a setTimeout anyway. Is that ok ?


});
debug(`Ending Collecting Chrome traces`);
Tracing.end();
Copy link
Contributor

Choose a reason for hiding this comment

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

Can Tracing.end throw an exception? if so, you can wrap it in a try/catch and call to reject in the catch

Copy link
Author

Choose a reason for hiding this comment

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

I think Tracing.end would throw just as Page.navigate or other chrome commands would. This just sends the command over to chrome over the protocol, so IMHO, it should be fine.

@molant
Copy link
Member

molant commented Mar 2, 2018

Closing this as there's more work that needs to happen to enable it in all connectors but definitely something we want to add in the future.

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.

6 participants