-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
core: collect node stack traces #14420
base: main
Are you sure you want to change the base?
Conversation
* @return {Promise<*>} | ||
*/ | ||
async _evaluateInContext(expression, contextId) { | ||
async _evaluateInContext(expression, contextId, returnByValue = true) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't love the changes I made here. Any suggestions?
I need to not return by value in the gatherer to get access to the backend node ids (result.objectId
is not returned when returning Runtime.evaluate
by value).
Hmmm... FR api tests show that |
Can you include a sketch of this? It's not clear that all stacks of js-created-nodes should be gathered vs selectively gathering them (e.g. for some of the TraceElements, maybe), for instance. edit: it sounds like you do want it on all elements in the report, but it would still be helpful to have a sketch of what it would look like and what specifically developers could do with that information. Would it be that helpful for most elements in the report? FPS has taken a lot of upkeep (tests, bug fixes, refactoring overhead), and ideally the tradeoff of increasing that upkeep would come from clear new functionality. |
This collects the node stack traces from the protocol, and associates them with our lhIds. Some implementation details are similar to full-page-screenshot–namely, how we must look at both of our execution contexts, and how the data is wired to the details renderer.
This PR stops short at making this information visible in the report, deferring that for later. Currently it just adds some data value to the dom on
lh-node
elements:The artifact is stored as a mapping from
lhId
to the raw protocol response.The audit is stored as a compressed representation of stack traces, de-duping frames/stacks to reduce the size. For cnn.com, this reduced the audit from being 28% of the LHR to just 2%.