Skip to content

Commit cdca991

Browse files
authored
feat(perf): Add detection for render-blocking asset performance issues (#37826)
* feat(perf): Add detection for render-blocking asset performance issues Tag transactions that have slow asset load spans before a slow FCP as having render-blocking assets. The thresholds are configurable, but currently we're looking for transactions with an FCP between 2s and 10s, where an asset load takes up at least 25% of that time. The thresholds will be tuned before we start generating actual Performance Issues from this data - tagging the transactions will let us see what we're detecting it and validate/tune it before it becomes visible to users. This detector's use of event properties is a little awkward given the current `PerformanceDetector` interface, but I thought it would be better to get the rest of our planned detectors in before we refactor too much. Fixes PERF-1677
1 parent 8ec8409 commit cdca991

File tree

2 files changed

+146
-6
lines changed

2 files changed

+146
-6
lines changed

src/sentry/utils/performance_issues/performance_detection.py

Lines changed: 78 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ class DetectorType(Enum):
2424
DUPLICATE_SPANS = "duplicates"
2525
SEQUENTIAL_SLOW_SPANS = "sequential"
2626
LONG_TASK_SPANS = "long_task"
27+
RENDER_BLOCKING_ASSET_SPAN = "render_blocking_assets"
2728

2829

2930
# Facade in front of performance detection to limit impact of detection on our events ingestion
@@ -85,6 +86,12 @@ def get_default_detection_settings():
8586
"allowed_span_ops": ["ui.long-task", "ui.sentry.long-task"],
8687
}
8788
],
89+
DetectorType.RENDER_BLOCKING_ASSET_SPAN: {
90+
"fcp_minimum_threshold": 2000.0, # ms
91+
"fcp_maximum_threshold": 10000.0, # ms
92+
"fcp_ratio_threshold": 0.25,
93+
"allowed_span_ops": ["resource.link", "resource.script"],
94+
},
8895
}
8996

9097

@@ -94,11 +101,14 @@ def _detect_performance_issue(data: Event, sdk_span: Any):
94101

95102
detection_settings = get_default_detection_settings()
96103
detectors = {
97-
DetectorType.DUPLICATE_SPANS: DuplicateSpanDetector(detection_settings),
98-
DetectorType.DUPLICATE_SPANS_HASH: DuplicateSpanHashDetector(detection_settings),
99-
DetectorType.SLOW_SPAN: SlowSpanDetector(detection_settings),
100-
DetectorType.SEQUENTIAL_SLOW_SPANS: SequentialSlowSpanDetector(detection_settings),
101-
DetectorType.LONG_TASK_SPANS: LongTaskSpanDetector(detection_settings),
104+
DetectorType.DUPLICATE_SPANS: DuplicateSpanDetector(detection_settings, data),
105+
DetectorType.DUPLICATE_SPANS_HASH: DuplicateSpanHashDetector(detection_settings, data),
106+
DetectorType.SLOW_SPAN: SlowSpanDetector(detection_settings, data),
107+
DetectorType.SEQUENTIAL_SLOW_SPANS: SequentialSlowSpanDetector(detection_settings, data),
108+
DetectorType.LONG_TASK_SPANS: LongTaskSpanDetector(detection_settings, data),
109+
DetectorType.RENDER_BLOCKING_ASSET_SPAN: RenderBlockingAssetSpanDetector(
110+
detection_settings, data
111+
),
102112
}
103113

104114
for span in spans:
@@ -143,8 +153,9 @@ class PerformanceDetector(ABC):
143153
Classes of this type have their visit functions called as the event is walked once and will store a performance issue if one is detected.
144154
"""
145155

146-
def __init__(self, settings: Dict[str, Any]):
156+
def __init__(self, settings: Dict[str, Any], event: Event):
147157
self.settings = settings[self.settings_key]
158+
self._event = event
148159
self.init()
149160

150161
@abstractmethod
@@ -170,6 +181,9 @@ def settings_for_span(self, span: Span):
170181
return op, span_id, op_prefix, span_duration, setting
171182
return None
172183

184+
def event(self) -> Event:
185+
return self._event
186+
173187
@property
174188
@abstractmethod
175189
def settings_key(self) -> DetectorType:
@@ -411,6 +425,64 @@ def visit_span(self, span: Span):
411425
)
412426

413427

428+
class RenderBlockingAssetSpanDetector(PerformanceDetector):
429+
__slots__ = ("stored_issues", "fcp", "transaction_start")
430+
431+
settings_key = DetectorType.RENDER_BLOCKING_ASSET_SPAN
432+
433+
def init(self):
434+
self.stored_issues = {}
435+
self.transaction_start = timedelta(seconds=self.event().get("transaction_start", 0))
436+
self.fcp = None
437+
438+
# Only concern ourselves with transactions where the FCP is within the
439+
# range we care about.
440+
fcp_hash = self.event().get("measurements", {}).get("fcp", {})
441+
if "value" in fcp_hash and ("unit" not in fcp_hash or fcp_hash["unit"] == "millisecond"):
442+
fcp = timedelta(milliseconds=fcp_hash.get("value"))
443+
fcp_minimum_threshold = timedelta(
444+
milliseconds=self.settings.get("fcp_minimum_threshold")
445+
)
446+
fcp_maximum_threshold = timedelta(
447+
milliseconds=self.settings.get("fcp_maximum_threshold")
448+
)
449+
if fcp >= fcp_minimum_threshold and fcp < fcp_maximum_threshold:
450+
self.fcp = fcp
451+
452+
def visit_span(self, span: Span):
453+
if not self.fcp:
454+
return
455+
456+
op = span.get("op", None)
457+
allowed_span_ops = self.settings.get("allowed_span_ops")
458+
if op not in allowed_span_ops:
459+
return False
460+
461+
if self._is_blocking_render(span):
462+
span_id = span.get("span_id", None)
463+
fingerprint = fingerprint_span(span)
464+
if span_id and fingerprint:
465+
self.stored_issues[fingerprint] = PerformanceSpanIssue(span_id, op, [span_id])
466+
467+
# If we visit a span that starts after FCP, then we know we've already
468+
# seen all possible render-blocking resource spans.
469+
span_start_timestamp = timedelta(seconds=span.get("start_timestamp", 0))
470+
fcp_timestamp = self.transaction_start + self.fcp
471+
if span_start_timestamp >= fcp_timestamp:
472+
# Early return for all future span visits.
473+
self.fcp = None
474+
475+
def _is_blocking_render(self, span):
476+
span_end_timestamp = timedelta(seconds=span.get("timestamp", 0))
477+
fcp_timestamp = self.transaction_start + self.fcp
478+
if span_end_timestamp >= fcp_timestamp:
479+
return False
480+
481+
span_duration = get_span_duration(span)
482+
fcp_ratio_threshold = self.settings.get("fcp_ratio_threshold")
483+
return span_duration / self.fcp > fcp_ratio_threshold
484+
485+
414486
# Reports metrics and creates spans for detection
415487
def report_metrics_for_detectors(
416488
event_id: Optional[str], detectors: Dict[str, PerformanceDetector], sdk_span: Any

tests/sentry/utils/performance_issues/test_performance_detection.py

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,3 +283,71 @@ def test_calls_detect_long_task(self):
283283
),
284284
]
285285
)
286+
287+
def test_calls_detect_render_blocking_asset(self):
288+
render_blocking_asset_event = {
289+
"event_id": "a" * 16,
290+
"measurements": {
291+
"fcp": {
292+
"value": 2500.0,
293+
"unit": "millisecond",
294+
}
295+
},
296+
"spans": [
297+
create_span("resource.script", duration=1000.0),
298+
],
299+
}
300+
non_render_blocking_asset_event = {
301+
"event_id": "a" * 16,
302+
"measurements": {
303+
"fcp": {
304+
"value": 2500.0,
305+
"unit": "millisecond",
306+
}
307+
},
308+
"spans": [
309+
modify_span_start(
310+
create_span("resource.script", duration=1000.0),
311+
2000.0,
312+
),
313+
],
314+
}
315+
short_render_blocking_asset_event = {
316+
"event_id": "a" * 16,
317+
"measurements": {
318+
"fcp": {
319+
"value": 2500.0,
320+
"unit": "millisecond",
321+
}
322+
},
323+
"spans": [
324+
create_span("resource.script", duration=200.0),
325+
],
326+
}
327+
328+
sdk_span_mock = Mock()
329+
330+
_detect_performance_issue(non_render_blocking_asset_event, sdk_span_mock)
331+
assert sdk_span_mock.containing_transaction.set_tag.call_count == 0
332+
333+
_detect_performance_issue(short_render_blocking_asset_event, sdk_span_mock)
334+
assert sdk_span_mock.containing_transaction.set_tag.call_count == 0
335+
336+
_detect_performance_issue(render_blocking_asset_event, sdk_span_mock)
337+
assert sdk_span_mock.containing_transaction.set_tag.call_count == 3
338+
sdk_span_mock.containing_transaction.set_tag.assert_has_calls(
339+
[
340+
call(
341+
"_pi_all_issue_count",
342+
1,
343+
),
344+
call(
345+
"_pi_transaction",
346+
"aaaaaaaaaaaaaaaa",
347+
),
348+
call(
349+
"_pi_render_blocking_assets",
350+
"bbbbbbbbbbbbbbbb",
351+
),
352+
]
353+
)

0 commit comments

Comments
 (0)