Skip to content

feat(perf): Detect "Uncompressed Asset" performance issues #37633

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

Closed

Conversation

mjq-sentry
Copy link
Contributor

Large assets can affect First Contentful Paint (FCP) and overall page load time. One easy way to make text assets smaller is to apply HTTP compression to them. Mark transactions containing uncompressed script or CSS resource loads (over a configurable minimum size, currently 500 KiB) as having an Uncompressed Asset performance issue.

mjq added 2 commits August 9, 2022 17:30
Large assets can affect First Contentful Paint (FCP) and overall page load
time. One easy way to make text assets smaller is to apply HTTP compression to
them. Mark transactions containing uncompressed script or CSS resource loads
(over a configurable minimum size, currently 500 KiB) as having an Uncompressed
Asset performance issue.
@mjq-sentry mjq-sentry requested a review from k-fish August 9, 2022 21:34
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Aug 9, 2022
Copy link
Member

@k-fish k-fish left a comment

Choose a reason for hiding this comment

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

Looks good for a first pass, might be a bit too broad but it doesn't really cost anything to check.

As an aside, make sure to tag @wedamija on these PR's if the code owners doesn't already as well.

return

# Ignore assets that are already compressed.
if encoded_body_size != decoded_body_size:
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure they are always equal anyway, I'd maybe verify iirc message payload or body payload mean slightly different things, but I can't remember since it's been a while since I've fixed a compression issue. It's either decoded or transfer that can be larger because of headers, I believe.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good callout - this was true in my local testing but I'll double-check the SDK & Performance API docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Confirmed again that these are the same in practice locally, and it seems safe based on my reading of MDN (both encodedBodySize and decodedBodySize measure the size of the payload body specifically.)

Comment on lines +60 to +61
"size_threshold_bytes": 500 * 1024,
"allowed_span_ops": ["resource.css", "resource.script"],
Copy link
Member

Choose a reason for hiding this comment

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

I think it's possible we'll also want some timing threshold on top of this in practice since 500kb might be fine depending on your use case or user conditions.

@mjq-sentry
Copy link
Contributor Author

Closing in favour of detection for render-blocking assets in particular.

@mjq-sentry mjq-sentry closed this Aug 10, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Aug 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants