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(perf-issues): respect span render blocking status in detector #44981

Merged
merged 1 commit into from
Feb 22, 2023

Conversation

mjq-sentry
Copy link
Contributor

As of browser SDK 7.38.0, we now log the render-blocking status reported by the browser in resource spans' data hash. (Background info: MDN). If a value is present and that span is non-blocking, ignore our heuristics and treat the resource as non-blocking in the Large Render Blocking Asset detector.

This PR also updates each test case to start from a valid event and then minimally mutate it into an invalid one, because I was losing confidence that each test was really testing what it should.

As of browser SDK 7.38.0, we now log the render-blocking status reported by the
browser in resource spans' `data` hash. If a value is present and that value is
`non-blocking`, ignore our heuristics and treat the value as non-blocking.

This commit also updates each test case to start from a valid event and then
minimally mutate it into an invalid one, because I was losing confidence that
each test was really testing what it should.
@mjq-sentry mjq-sentry requested a review from a team as a code owner February 22, 2023 20:33
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Feb 22, 2023
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.

👍 checking for non-blocking to exclude it vs. render blocking to include.

@@ -485,13 +485,17 @@ def visit_span(self, span: Span):
self.fcp = None

def _is_blocking_render(self, span):
data = span.get("data", None)
render_blocking_status = data and data.get("resource.render_blocking_status")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
render_blocking_status = data and data.get("resource.render_blocking_status")
render_blocking_status = data and data.get("resource.render_blocking_status", None)

protect against missing field? or nah

@mjq-sentry mjq-sentry merged commit dd56cd0 into master Feb 22, 2023
@mjq-sentry mjq-sentry deleted the mjq/rba-data-prop branch February 22, 2023 21:45
@github-actions github-actions bot locked and limited conversation to collaborators Mar 10, 2023
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.

None yet

3 participants