Skip to content

feat(perf-issues): respect span render blocking status in detector #44981

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Feb 22, 2023
Merged

feat(perf-issues): respect span render blocking status in detector #44981

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/sentry/utils/performance_issues/performance_detection.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

@k-fish k-fish Feb 22, 2023

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

if render_blocking_status == "non-blocking":
return False

span_end_timestamp = timedelta(seconds=span.get("timestamp", 0))
fcp_timestamp = self.transaction_start + self.fcp
if span_end_timestamp >= fcp_timestamp:
return False

minimum_size_bytes = self.settings.get("minimum_size_bytes")
data = span.get("data", None)
encoded_body_size = data and data.get("Encoded Body Size", 0) or 0
if encoded_body_size < minimum_size_bytes or encoded_body_size > self.MAX_SIZE_BYTES:
return False
Expand Down
162 changes: 52 additions & 110 deletions tests/sentry/utils/performance_issues/test_render_blocking_asset_detector.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ def _valid_render_blocking_asset_event(url: str) -> Event:
"Transfer Size": 1200000,
"Encoded Body Size": 1200000,
"Decoded Body Size": 2000000,
"resource.render_blocking_status": "blocking",
},
),
],
Expand Down Expand Up @@ -81,135 +82,76 @@ def test_detects_render_blocking_asset(self):
)
]

def test_not_detect_render_block_asset(self):
event = {
"event_id": "a" * 16,
"project": PROJECT_ID,
"measurements": {
"fcp": {
"value": 2500.0,
"unit": "millisecond",
}
},
"spans": [
modify_span_start(
create_span("resource.script", duration=1000.0),
2000.0,
),
],
}
def test_does_not_detect_if_resource_overlaps_fcp(self):
event = _valid_render_blocking_asset_event("https://example.com/a.js")

for span in event["spans"]:
if span["op"] == "resource.script":
modify_span_start(span, 2000.0)

assert self.find_problems(event) == []

def test_does_not_detect_with_no_fcp(self):
event = {
"event_id": "a" * 16,
"project": PROJECT_ID,
"measurements": {
"fcp": {
"value": None,
"unit": "millisecond",
}
},
"spans": [
create_span("resource.script", duration=1000.0),
],
}

event = _valid_render_blocking_asset_event("https://example.com/a.js")
event["measurements"]["fcp"]["value"] = None
assert self.find_problems(event) == []

def test_does_not_detect_with_no_measurements(self):
event = {
"event_id": "a" * 16,
"project": PROJECT_ID,
"measurements": None,
"spans": [
create_span("resource.script", duration=1000.0),
],
}

event = _valid_render_blocking_asset_event("https://example.com/a.js")
event["measurements"] = None
assert self.find_problems(event) == []

def test_does_not_detect_with_short_render_blocking_asset(self):
event = {
"event_id": "a" * 16,
"project": PROJECT_ID,
"measurements": {
"fcp": {
"value": 2500.0,
"unit": "millisecond",
}
},
"spans": [
create_span("resource.script", duration=200.0),
],
}

event = _valid_render_blocking_asset_event("https://example.com/a.js")
for span in event["spans"]:
if span["op"] == "resource.script":
span["timestamp"] = 0.1
assert self.find_problems(event) == []

def test_does_not_detect_if_too_small(self):
event = {
"event_id": "a" * 16,
"project": PROJECT_ID,
"measurements": {
"fcp": {
"value": 2500.0,
"unit": "millisecond",
}
},
"spans": [
create_span(
"resource.script",
duration=1000.0,
data={
"Transfer Size": 900000,
"Encoded Body Size": 900000,
"Decoded Body Size": 1700000,
},
),
],
}
event = _valid_render_blocking_asset_event("https://example.com/a.js")
for span in event["spans"]:
if span["op"] == "resource.script":
span["data"]["Encoded Body Size"] = 900000
assert self.find_problems(event) == []

def test_does_not_detect_if_missing_size(self):
event = {
"event_id": "a" * 16,
"project": PROJECT_ID,
"measurements": {
"fcp": {
"value": 2500.0,
"unit": "millisecond",
}
},
"spans": [
create_span("resource.script", duration=1000.0),
],
}
event = _valid_render_blocking_asset_event("https://example.com/a.js")
for span in event["spans"]:
if span["op"] == "resource.script":
del span["data"]
assert self.find_problems(event) == []

def test_does_not_detect_if_too_large(self):
event = {
"event_id": "a" * 16,
"project": PROJECT_ID,
"measurements": {
"fcp": {
"value": 2500.0,
"unit": "millisecond",
}
},
"spans": [
create_span(
"resource.script",
duration=1000.0,
data={
# A resource span with these stats was really logged.
"Transfer Size": 299,
"Encoded Body Size": 18446744073709552000,
"Decoded Body Size": 0,
},
),
],
}
event = _valid_render_blocking_asset_event("https://example.com/a.js")
for span in event["spans"]:
if span["op"] == "resource.script":
# This is a real value we saw in production.
span["data"]["Encoded Body Size"] = 18446744073709552000
assert self.find_problems(event) == []

def test_detects_if_render_blocking_status_is_missing(self):
event = _valid_render_blocking_asset_event("https://example.com/a.js")
for span in event["spans"]:
del span["data"]["resource.render_blocking_status"]

assert self.find_problems(event) == [
PerformanceProblem(
fingerprint="1-1004-ba43281143a88ba902029356cb543dd0bff8f41c",
op="resource.script",
desc="https://example.com/a.js",
type=PerformanceRenderBlockingAssetSpanGroupType,
parent_span_ids=[],
cause_span_ids=[],
offender_span_ids=["bbbbbbbbbbbbbbbb"],
)
]

def test_does_not_detect_if_render_blocking_status_is_non_blocking(self):
event = _valid_render_blocking_asset_event("https://example.com/a.js")
for span in event["spans"]:
span["data"]["resource.render_blocking_status"] = "non-blocking"

assert self.find_problems(event) == []


Expand Down