Skip to content

Commit 8e05121

Browse files
authored
ref(grouping): Label stacktrace rule hints with rule origin (#103266)
In grouping info, we tell people what rule has been used to mark a frame in- or out-of-app, or to ignore or unignore it. However, we give no indication if the rule is ours or one of theirs. (This is useful information when trying to debug grouping.) This PR fixes that, by keeping a set of the string representations of any custom rules the user has created, comparing that set to the rules mentioned in grouping info hints, and modifying those hints accordingly. Thus `marked in-app by stacktrace rule (...)` becomes either `marked in-app by custom stacktrace rule (...)` (if the rule is in the set) or `marked in-app by built-in stacktrace rule (...)` (if it's not).
1 parent ddb060f commit 8e05121

File tree

282 files changed

+5001
-4895
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

282 files changed

+5001
-4895
lines changed

src/sentry/grouping/enhancer/__init__.py

Lines changed: 54 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import base64
44
import logging
55
import os
6+
import re
67
import zlib
78
from collections.abc import Sequence
89
from dataclasses import dataclass
@@ -43,6 +44,8 @@
4344
# base64 strings never contain '#')
4445
BASE64_ENHANCEMENTS_DELIMITER = b"#"
4546

47+
HINT_STACKTRACE_RULE_REGEX = re.compile(r"stack trace rule \((.+)\)$")
48+
4649
VALID_PROFILING_MATCHER_PREFIXES = (
4750
"stack.abs_path",
4851
"path", # stack.abs_path alias
@@ -61,6 +64,7 @@ class EnhancementsConfigData:
6164
# Note: This is not an actual config, just a container to make it easier to pass data between
6265
# functions while loading a config.
6366
rules: list[EnhancementRule]
67+
rule_strings: list[str]
6468
rust_enhancements: RustEnhancements
6569
version: int | None = None
6670
bases: list[str] | None = None
@@ -170,12 +174,26 @@ def _can_use_hint(
170174
return True
171175

172176

177+
def _add_rule_source_to_hint(hint: str | None, custom_rules: set[str]) -> str | None:
178+
"""Add 'custom' or 'built-in' to the rule description in the given hint (if any)."""
179+
if not hint:
180+
return None
181+
182+
def _add_type_to_rule(rule_regex_match: re.Match) -> str:
183+
rule_str = rule_regex_match.group(1)
184+
rule_type = "custom" if rule_str in custom_rules else "built-in"
185+
return f"{rule_type} stack trace rule ({rule_str})"
186+
187+
return HINT_STACKTRACE_RULE_REGEX.sub(_add_type_to_rule, hint)
188+
189+
173190
def _get_hint_for_frame(
174191
variant_name: str,
175192
frame: dict[str, Any],
176193
frame_component: FrameGroupingComponent,
177194
rust_frame: RustFrame,
178195
desired_hint_type: Literal["in-app", "contributes"],
196+
custom_rules: set[str],
179197
) -> str | None:
180198
"""
181199
Determine a hint to use for the frame, handling special-casing and precedence.
@@ -210,7 +228,10 @@ def _get_hint_for_frame(
210228
if variant_name == "app" and rust_hint_type == "in-app" and rust_in_app == client_in_app:
211229
can_use_rust_hint = False
212230

213-
return rust_hint if can_use_rust_hint else incoming_hint if can_use_incoming_hint else None
231+
raw_hint = rust_hint if can_use_rust_hint else incoming_hint if can_use_incoming_hint else None
232+
233+
# Add 'custom' or 'built-in' to any stacktrace rule description as appropriate
234+
return _add_rule_source_to_hint(raw_hint, custom_rules)
214235

215236

216237
def _split_rules(
@@ -243,15 +264,22 @@ def _split_rules(
243264
if rule is not None # mypy appeasment
244265
]
245266

246-
classifier_rules_text = "\n".join(rule.text for rule in classifier_rules)
247-
contributes_rules_text = "\n".join(rule.text for rule in contributes_rules)
267+
classifier_rule_strings = [rule.text for rule in classifier_rules]
268+
contributes_rule_strings = [rule.text for rule in contributes_rules]
269+
270+
classifier_rules_text = "\n".join(classifier_rule_strings)
271+
contributes_rules_text = "\n".join(contributes_rule_strings)
248272

249273
classifier_rust_enhancements = _get_rust_enhancements("config_string", classifier_rules_text)
250274
contributes_rust_enhancements = _get_rust_enhancements("config_string", contributes_rules_text)
251275

252276
return (
253-
EnhancementsConfigData(classifier_rules, classifier_rust_enhancements),
254-
EnhancementsConfigData(contributes_rules, contributes_rust_enhancements),
277+
EnhancementsConfigData(
278+
classifier_rules, classifier_rule_strings, classifier_rust_enhancements
279+
),
280+
EnhancementsConfigData(
281+
contributes_rules, contributes_rule_strings, contributes_rust_enhancements
282+
),
255283
)
256284

257285

@@ -363,6 +391,12 @@ def __init__(
363391
self.bases, contributes_config.rust_enhancements, type="contributes"
364392
)
365393

394+
# We store the rule strings individually in a set so it's quick to test if a given rule
395+
# mentioned in a hint is custom or built-in
396+
self.custom_rule_strings = set(
397+
classifier_config.rule_strings + contributes_config.rule_strings
398+
)
399+
366400
def apply_category_and_updated_in_app_to_frames(
367401
self,
368402
frames: Sequence[dict[str, Any]],
@@ -458,13 +492,23 @@ def assemble_stacktrace_component(
458492

459493
in_app_hint = (
460494
_get_hint_for_frame(
461-
variant_name, frame, frame_component, in_app_rust_frame, "in-app"
495+
variant_name,
496+
frame,
497+
frame_component,
498+
in_app_rust_frame,
499+
"in-app",
500+
self.custom_rule_strings,
462501
)
463502
if variant_name == "app"
464503
else None # In-app hints don't apply to the system stacktrace
465504
)
466505
contributes_hint = _get_hint_for_frame(
467-
variant_name, frame, frame_component, contributes_rust_frame, "contributes"
506+
variant_name,
507+
frame,
508+
frame_component,
509+
contributes_rust_frame,
510+
"contributes",
511+
self.custom_rule_strings,
468512
)
469513
hint = _combine_hints(variant_name, frame_component, in_app_hint, contributes_hint)
470514

@@ -524,7 +568,9 @@ def _get_config_from_base64_bytes(cls, bytes_str: bytes) -> EnhancementsConfigDa
524568
except (LookupError, AttributeError, TypeError, ValueError) as e:
525569
raise ValueError("invalid stack trace rule config: %s" % e)
526570

527-
return EnhancementsConfigData(rules, rust_enhancements, version, bases)
571+
return EnhancementsConfigData(
572+
rules, [rule.text for rule in rules], rust_enhancements, version, bases
573+
)
528574

529575
@classmethod
530576
def from_base64_string(

tests/sentry/grouping/enhancements/test_hints.py

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,14 @@ def test_get_hint_for_frame(
308308
rust_frame = DummyRustFrame(hint=rust_hint)
309309

310310
assert (
311-
_get_hint_for_frame(variant_name, frame, frame_component, rust_frame, desired_hint_type) # type: ignore[arg-type]
311+
_get_hint_for_frame(
312+
variant_name,
313+
frame,
314+
frame_component,
315+
rust_frame, # type: ignore[arg-type] # rust frame mock fails typecheck
316+
desired_hint_type,
317+
set(),
318+
)
312319
== expected_result
313320
)
314321

@@ -343,3 +350,39 @@ def test_combining_hints(
343350
_combine_hints(variant_name, frame_component, in_app_hint, contributes_hint)
344351
== expected_result
345352
)
353+
354+
355+
def test_adds_rule_source_to_stacktrace_rule_hints() -> None:
356+
frame = {"in_app": True}
357+
frame_component = FrameGroupingComponent(in_app=True, values=[])
358+
custom_rules = {"function:roll_over +app"}
359+
360+
built_in_rule_rust_frame = DummyRustFrame(
361+
hint="marked in-app by stack trace rule (function:shake +app)"
362+
)
363+
custom_rule_rust_frame = DummyRustFrame(
364+
hint="marked in-app by stack trace rule (function:roll_over +app)"
365+
)
366+
367+
assert (
368+
_get_hint_for_frame(
369+
"app",
370+
frame,
371+
frame_component,
372+
built_in_rule_rust_frame, # type: ignore[arg-type] # rust frame mock fails typecheck
373+
"in-app",
374+
custom_rules,
375+
)
376+
== "marked in-app by built-in stack trace rule (function:shake +app)"
377+
)
378+
assert (
379+
_get_hint_for_frame(
380+
"app",
381+
frame,
382+
frame_component,
383+
custom_rule_rust_frame, # type: ignore[arg-type] # rust frame mock fails typecheck
384+
"in-app",
385+
custom_rules,
386+
)
387+
== "marked in-app by custom stack trace rule (function:roll_over +app)"
388+
)

tests/sentry/grouping/snapshots/grouphash_metadata/test_metadata_from_variants/newstyle@2023_01_11/block_invoke.pysnap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,6 @@ contributing variants:
3232
frame* (marked in-app by the client)
3333
function*
3434
"__46+[FudgeGlobalHandler setupGlobalHandlersIfNeeded]_block_invoke_2"
35-
frame* (marked in-app by stack trace rule (family:native package:**/Containers/Bundle/Application/** +app))
35+
frame* (marked in-app by built-in stack trace rule (family:native package:**/Containers/Bundle/Application/** +app))
3636
function*
3737
"__99+[Something else]_block_invoke_2"

tests/sentry/grouping/snapshots/grouphash_metadata/test_metadata_from_variants/newstyle@2023_01_11/cocoa_dispatch_client_callout.pysnap

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,16 +29,16 @@ contributing variants:
2929
app*
3030
threads*
3131
stacktrace*
32-
frame* (marked in-app by stack trace rule (family:native package:**/Containers/Bundle/Application/** +app))
32+
frame* (marked in-app by built-in stack trace rule (family:native package:**/Containers/Bundle/Application/** +app))
3333
function*
3434
"unicorn"
3535
frame* (marked in-app by the client)
3636
function*
3737
"__46+[FudgeGlobalHandler setupGlobalHandlersIfNeeded]_block_invoke_2"
38-
frame* (marked in-app by stack trace rule (family:native package:**/Containers/Bundle/Application/** +app))
38+
frame* (marked in-app by built-in stack trace rule (family:native package:**/Containers/Bundle/Application/** +app))
3939
function*
4040
"FudgeLogTaggedError"
41-
frame* (marked in-app by stack trace rule (family:native package:**/Containers/Bundle/Application/** +app))
41+
frame* (marked in-app by built-in stack trace rule (family:native package:**/Containers/Bundle/Application/** +app))
4242
function*
4343
"SentrySetupInteractor.setupSentry"
4444
system*

tests/sentry/grouping/snapshots/grouphash_metadata/test_metadata_from_variants/newstyle@2023_01_11/contributing_system_and_app_frames.pysnap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ contributing variants:
2929
app*
3030
exception*
3131
stacktrace*
32-
frame* (marked in-app by stack trace rule (function:playFetch +app))
32+
frame* (marked in-app by custom stack trace rule (function:playFetch +app))
3333
filename*
3434
"dogpark.js"
3535
function*

tests/sentry/grouping/snapshots/grouphash_metadata/test_metadata_from_variants/newstyle@2023_01_11/stacktrace_rust.pysnap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ contributing variants:
2929
app*
3030
exception*
3131
stacktrace*
32-
frame* (marked in-app by stack trace rule (family:native function:log_demo::* +app))
32+
frame* (marked in-app by custom stack trace rule (family:native function:log_demo::* +app))
3333
function*
3434
"log_demo::main"
3535
type*

tests/sentry/grouping/snapshots/grouphash_metadata/test_metadata_from_variants/newstyle@2023_01_11/stacktrace_rust2.pysnap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ contributing variants:
2929
app*
3030
exception*
3131
stacktrace*
32-
frame* (marked in-app by stack trace rule (family:native function:log_demo::* +app))
32+
frame* (marked in-app by custom stack trace rule (family:native function:log_demo::* +app))
3333
function*
3434
"log_demo::main"
3535
type*

tests/sentry/grouping/snapshots/grouphash_metadata/test_metadata_from_variants/newstyle@2023_01_11/unreal_assert_mac.pysnap

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -29,79 +29,79 @@ contributing variants:
2929
app*
3030
exception*
3131
stacktrace*
32-
frame* (marked in-app by stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
32+
frame* (marked in-app by built-in stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
3333
function*
3434
"-[FCocoaGameThread main]"
35-
frame* (marked in-app by stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
35+
frame* (marked in-app by built-in stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
3636
function*
3737
"-[UEAppDelegate runGameThread:]"
38-
frame* (marked in-app by stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
38+
frame* (marked in-app by built-in stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
3939
function*
4040
"GuardedMain"
41-
frame* (marked in-app by stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
41+
frame* (marked in-app by built-in stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
4242
function*
4343
"FEngineLoop::Tick"
44-
frame* (marked in-app by stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
44+
frame* (marked in-app by built-in stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
4545
function*
4646
"FSlateApplication::Tick"
47-
frame* (marked in-app by stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
47+
frame* (marked in-app by built-in stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
4848
function*
4949
"FSlateApplication::TickPlatform"
50-
frame* (marked in-app by stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
50+
frame* (marked in-app by built-in stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
5151
function*
5252
"FMacApplication::ProcessDeferredEvents"
53-
frame* (marked in-app by stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
53+
frame* (marked in-app by built-in stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
5454
function*
5555
"FMacApplication::ProcessEvent"
56-
frame* (marked in-app by stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
56+
frame* (marked in-app by built-in stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
5757
function*
5858
"FMacApplication::ProcessMouseUpEvent"
59-
frame* (marked in-app by stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
59+
frame* (marked in-app by built-in stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
6060
function*
6161
"FSlateApplication::OnMouseUp"
62-
frame* (marked in-app by stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
62+
frame* (marked in-app by built-in stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
6363
function*
6464
"FSlateApplication::ProcessMouseButtonUpEvent"
65-
frame* (marked in-app by stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
65+
frame* (marked in-app by built-in stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
6666
function*
6767
"FSlateApplication::RoutePointerUpEvent"
68-
frame* (marked in-app by stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
68+
frame* (marked in-app by built-in stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
6969
function*
7070
"SButton::OnMouseButtonUp"
71-
frame* (marked in-app by stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
71+
frame* (marked in-app by built-in stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
7272
function*
7373
"SButton::ExecuteOnClick"
74-
frame* (marked in-app by stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
74+
frame* (marked in-app by built-in stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
7575
function*
7676
"UButton::SlateHandleClicked"
77-
frame* (marked in-app by stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
77+
frame* (marked in-app by built-in stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
7878
function*
7979
"TMulticastScriptDelegate<T>::ProcessMulticastDelegate<T>"
80-
frame* (marked in-app by stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
80+
frame* (marked in-app by built-in stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
8181
function*
8282
"UObject::ProcessEvent"
83-
frame* (marked in-app by stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
83+
frame* (marked in-app by built-in stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
8484
function*
8585
"UFunction::Invoke"
86-
frame* (marked in-app by stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
86+
frame* (marked in-app by built-in stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
8787
function*
8888
"ProcessLocalScriptFunction"
89-
frame* (marked in-app by stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
89+
frame* (marked in-app by built-in stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
9090
function*
9191
"ProcessLocalFunction"
92-
frame* (marked in-app by stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
92+
frame* (marked in-app by built-in stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
9393
function*
9494
"ProcessScriptFunction<T>"
95-
frame* (marked in-app by stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
95+
frame* (marked in-app by built-in stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
9696
function*
9797
"ProcessLocalScriptFunction"
98-
frame* (marked in-app by stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
98+
frame* (marked in-app by built-in stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
9999
function*
100100
"UObject::execCallMathFunction"
101-
frame* (marked in-app by stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
101+
frame* (marked in-app by built-in stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
102102
function*
103103
"USentryPlaygroundUtils::execTerminate"
104-
frame* (marked in-app by stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
104+
frame* (marked in-app by built-in stack trace rule (family:native function:FDebug::CheckVerifyFailedImpl* v+app -app ^-app))
105105
function*
106106
"USentryPlaygroundUtils::Terminate"
107107
type*

0 commit comments

Comments
 (0)