Skip to content

Commit dd56cd0

Browse files
mjqmjq
authored
feat(perf-issues): respect span render blocking status in detector (#44981)
As of browser SDK 7.38.0, we now [log the render-blocking status](getsentry/sentry-javascript#7127) reported by the browser in resource spans' `data` hash. (Background info: [MDN](https://developer.mozilla.org/en-US/docs/Web/API/PerformanceResourceTiming/renderBlockingStatus)). 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 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.
1 parent e0a0009 commit dd56cd0

File tree

2 files changed

+57
-111
lines changed

2 files changed

+57
-111
lines changed

‎src/sentry/utils/performance_issues/performance_detection.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -485,13 +485,17 @@ def visit_span(self, span: Span):
485485
self.fcp = None
486486

487487
def _is_blocking_render(self, span):
488+
data = span.get("data", None)
489+
render_blocking_status = data and data.get("resource.render_blocking_status")
490+
if render_blocking_status == "non-blocking":
491+
return False
492+
488493
span_end_timestamp = timedelta(seconds=span.get("timestamp", 0))
489494
fcp_timestamp = self.transaction_start + self.fcp
490495
if span_end_timestamp >= fcp_timestamp:
491496
return False
492497

493498
minimum_size_bytes = self.settings.get("minimum_size_bytes")
494-
data = span.get("data", None)
495499
encoded_body_size = data and data.get("Encoded Body Size", 0) or 0
496500
if encoded_body_size < minimum_size_bytes or encoded_body_size > self.MAX_SIZE_BYTES:
497501
return False

‎tests/sentry/utils/performance_issues/test_render_blocking_asset_detector.py

Lines changed: 52 additions & 110 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ def _valid_render_blocking_asset_event(url: str) -> Event:
3838
"Transfer Size": 1200000,
3939
"Encoded Body Size": 1200000,
4040
"Decoded Body Size": 2000000,
41+
"resource.render_blocking_status": "blocking",
4142
},
4243
),
4344
],
@@ -81,135 +82,76 @@ def test_detects_render_blocking_asset(self):
8182
)
8283
]
8384

84-
def test_not_detect_render_block_asset(self):
85-
event = {
86-
"event_id": "a" * 16,
87-
"project": PROJECT_ID,
88-
"measurements": {
89-
"fcp": {
90-
"value": 2500.0,
91-
"unit": "millisecond",
92-
}
93-
},
94-
"spans": [
95-
modify_span_start(
96-
create_span("resource.script", duration=1000.0),
97-
2000.0,
98-
),
99-
],
100-
}
85+
def test_does_not_detect_if_resource_overlaps_fcp(self):
86+
event = _valid_render_blocking_asset_event("https://example.com/a.js")
87+
88+
for span in event["spans"]:
89+
if span["op"] == "resource.script":
90+
modify_span_start(span, 2000.0)
10191

10292
assert self.find_problems(event) == []
10393

10494
def test_does_not_detect_with_no_fcp(self):
105-
event = {
106-
"event_id": "a" * 16,
107-
"project": PROJECT_ID,
108-
"measurements": {
109-
"fcp": {
110-
"value": None,
111-
"unit": "millisecond",
112-
}
113-
},
114-
"spans": [
115-
create_span("resource.script", duration=1000.0),
116-
],
117-
}
118-
95+
event = _valid_render_blocking_asset_event("https://example.com/a.js")
96+
event["measurements"]["fcp"]["value"] = None
11997
assert self.find_problems(event) == []
12098

12199
def test_does_not_detect_with_no_measurements(self):
122-
event = {
123-
"event_id": "a" * 16,
124-
"project": PROJECT_ID,
125-
"measurements": None,
126-
"spans": [
127-
create_span("resource.script", duration=1000.0),
128-
],
129-
}
130-
100+
event = _valid_render_blocking_asset_event("https://example.com/a.js")
101+
event["measurements"] = None
131102
assert self.find_problems(event) == []
132103

133104
def test_does_not_detect_with_short_render_blocking_asset(self):
134-
event = {
135-
"event_id": "a" * 16,
136-
"project": PROJECT_ID,
137-
"measurements": {
138-
"fcp": {
139-
"value": 2500.0,
140-
"unit": "millisecond",
141-
}
142-
},
143-
"spans": [
144-
create_span("resource.script", duration=200.0),
145-
],
146-
}
147-
105+
event = _valid_render_blocking_asset_event("https://example.com/a.js")
106+
for span in event["spans"]:
107+
if span["op"] == "resource.script":
108+
span["timestamp"] = 0.1
148109
assert self.find_problems(event) == []
149110

150111
def test_does_not_detect_if_too_small(self):
151-
event = {
152-
"event_id": "a" * 16,
153-
"project": PROJECT_ID,
154-
"measurements": {
155-
"fcp": {
156-
"value": 2500.0,
157-
"unit": "millisecond",
158-
}
159-
},
160-
"spans": [
161-
create_span(
162-
"resource.script",
163-
duration=1000.0,
164-
data={
165-
"Transfer Size": 900000,
166-
"Encoded Body Size": 900000,
167-
"Decoded Body Size": 1700000,
168-
},
169-
),
170-
],
171-
}
112+
event = _valid_render_blocking_asset_event("https://example.com/a.js")
113+
for span in event["spans"]:
114+
if span["op"] == "resource.script":
115+
span["data"]["Encoded Body Size"] = 900000
172116
assert self.find_problems(event) == []
173117

174118
def test_does_not_detect_if_missing_size(self):
175-
event = {
176-
"event_id": "a" * 16,
177-
"project": PROJECT_ID,
178-
"measurements": {
179-
"fcp": {
180-
"value": 2500.0,
181-
"unit": "millisecond",
182-
}
183-
},
184-
"spans": [
185-
create_span("resource.script", duration=1000.0),
186-
],
187-
}
119+
event = _valid_render_blocking_asset_event("https://example.com/a.js")
120+
for span in event["spans"]:
121+
if span["op"] == "resource.script":
122+
del span["data"]
188123
assert self.find_problems(event) == []
189124

190125
def test_does_not_detect_if_too_large(self):
191-
event = {
192-
"event_id": "a" * 16,
193-
"project": PROJECT_ID,
194-
"measurements": {
195-
"fcp": {
196-
"value": 2500.0,
197-
"unit": "millisecond",
198-
}
199-
},
200-
"spans": [
201-
create_span(
202-
"resource.script",
203-
duration=1000.0,
204-
data={
205-
# A resource span with these stats was really logged.
206-
"Transfer Size": 299,
207-
"Encoded Body Size": 18446744073709552000,
208-
"Decoded Body Size": 0,
209-
},
210-
),
211-
],
212-
}
126+
event = _valid_render_blocking_asset_event("https://example.com/a.js")
127+
for span in event["spans"]:
128+
if span["op"] == "resource.script":
129+
# This is a real value we saw in production.
130+
span["data"]["Encoded Body Size"] = 18446744073709552000
131+
assert self.find_problems(event) == []
132+
133+
def test_detects_if_render_blocking_status_is_missing(self):
134+
event = _valid_render_blocking_asset_event("https://example.com/a.js")
135+
for span in event["spans"]:
136+
del span["data"]["resource.render_blocking_status"]
137+
138+
assert self.find_problems(event) == [
139+
PerformanceProblem(
140+
fingerprint="1-1004-ba43281143a88ba902029356cb543dd0bff8f41c",
141+
op="resource.script",
142+
desc="https://example.com/a.js",
143+
type=PerformanceRenderBlockingAssetSpanGroupType,
144+
parent_span_ids=[],
145+
cause_span_ids=[],
146+
offender_span_ids=["bbbbbbbbbbbbbbbb"],
147+
)
148+
]
149+
150+
def test_does_not_detect_if_render_blocking_status_is_non_blocking(self):
151+
event = _valid_render_blocking_asset_event("https://example.com/a.js")
152+
for span in event["spans"]:
153+
span["data"]["resource.render_blocking_status"] = "non-blocking"
154+
213155
assert self.find_problems(event) == []
214156

215157

0 commit comments

Comments
 (0)