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

core: add priority to network-requests debug audit #14340

Merged
merged 1 commit into from
Aug 31, 2022
Merged

Conversation

brendankenny
Copy link
Member

this is one last major missing piece for analyzing loads from LHRs for preload opportunities. Priority can sometimes be inferred, and critical-request-chains has some of this information, but in practice inference isn't reliable and the crc data format is a nightmare for joining in SQL environments.

@brendankenny brendankenny requested a review from a team as a code owner August 31, 2022 21:56
@brendankenny brendankenny requested review from connorjclark and removed request for a team August 31, 2022 21:56
@@ -73,6 +73,7 @@ class NetworkRequests extends Audit {
statusCode: record.statusCode,
mimeType: record.mimeType,
resourceType: record.resourceType,
priority: record.priority,
Copy link
Collaborator

Choose a reason for hiding this comment

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

FYI, this is the "final" priority of the resource. Is this what you need?

this.priority = data.newPriority;

I think it'd be good to name this finalPriority to be clear.

Copy link
Member Author

Choose a reason for hiding this comment

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

FYI, this is the "final" priority of the resource. Is this what you need?

Yes, that's all that really matters in this case (to really reconstruct the whole load you'll need something like the full devtoolsLog anyways, not just a summary like this).

finalPriority seems OK but if it's compelling enough to change we should switch it on the real network-request and this can be updated to match, rather than going activist in the debug audit :)

Copy link
Member

Choose a reason for hiding this comment

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

iirc, devtools sdk uses initialPriority but doesn't prefix stuff w/final

(yes im very aware that i heavily advocated for final with our lhr URL names. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants