Skip to content

Commit 3f43ac8

Browse files
authored
feat(grouping): Add helper to combine hints (#91833)
When a frame is affected by both an in-app rule and a contributes rule, the split enhancements allow us to get both hints, rather than having one clobber the other (as is the case now). That does not mean, however, that both hints are always relevant. This PR adds a helper, `_combine_hints`, to appropriately use or ignore each hint, and combine them when necessary. It's not yet used, but will be when we switch to actually using the split enhancements, rather than just running them alongside the non-split ones. Note that `_combine_hints` needs the final `contributes` value to already be in place when it runs, so this also moves the setting of that value earlier in `assemble_stacktrace_component`.
1 parent 4fc6d16 commit 3f43ac8

File tree

2 files changed

+76
-2
lines changed

2 files changed

+76
-2
lines changed

src/sentry/grouping/enhancer/__init__.py

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,46 @@ def _split_rules(
257257
)
258258

259259

260+
def _combine_hints(
261+
variant_name: str,
262+
frame_component: FrameGroupingComponent,
263+
in_app_hint: str | None,
264+
contributes_hint: str | None,
265+
) -> str | None:
266+
"""
267+
Given possible in-app and contributes hints, determine the frame's final hint.
268+
"""
269+
frame_type = "in-app" if frame_component.in_app else "system"
270+
271+
# In-app hints never apply to the system stacktrace, so even if the contributes hint is `None`,
272+
# it's the one we want
273+
if variant_name == "system":
274+
return contributes_hint
275+
276+
# From here on out everything we're doing is for the app variant
277+
278+
# System frames never contribute to the app stacktrace, so if they've already been marked out of
279+
# app, we don't care whether or not they're ignored (or un-ignored), because they weren't going
280+
# to count anyway.
281+
if frame_type == "system":
282+
return in_app_hint
283+
284+
# If only one hint exists, return that one
285+
if in_app_hint and not contributes_hint:
286+
return in_app_hint
287+
if contributes_hint and not in_app_hint:
288+
return contributes_hint
289+
290+
# If neither hint exists, return None
291+
if not in_app_hint and not contributes_hint:
292+
return None
293+
294+
# Combine the hints in such as way that we get "marked in-app by xxx AND un-ignored by yyy" and
295+
# "marked in-app by xxx BUT ignored by yyy"
296+
conjunction = "and" if frame_component.contributes else "but"
297+
return f"{in_app_hint} {conjunction} {contributes_hint}"
298+
299+
260300
def is_valid_profiling_matcher(matchers: list[str]) -> bool:
261301
for matcher in matchers:
262302
if not matcher.startswith(VALID_PROFILING_MATCHER_PREFIXES):
@@ -683,6 +723,8 @@ def assemble_stacktrace_component(
683723
else:
684724
contributes = rust_frame.contributes
685725

726+
frame_component.update(contributes=contributes)
727+
686728
hint = get_hint_for_frame(variant_name, frame, frame_component, rust_frame)
687729
if self.run_split_enhancements:
688730
split_in_app_hint = (
@@ -696,7 +738,7 @@ def assemble_stacktrace_component(
696738
variant_name, frame, frame_component, contributes_rust_frame, "contributes"
697739
)
698740

699-
frame_component.update(contributes=contributes, hint=hint)
741+
frame_component.update(hint=hint)
700742

701743
# Add this frame to our tally
702744
key = f"{"in_app" if frame_component.in_app else "system"}_{"contributing" if frame_component.contributes else "non_contributing"}_frames"

tests/sentry/grouping/enhancements/test_hints.py

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import pytest
77

88
from sentry.grouping.component import FrameGroupingComponent
9-
from sentry.grouping.enhancer import get_hint_for_frame
9+
from sentry.grouping.enhancer import _combine_hints, get_hint_for_frame
1010

1111

1212
@dataclass
@@ -431,3 +431,35 @@ def test_get_hint_for_frame(
431431
get_hint_for_frame(variant_name, frame, frame_component, rust_frame, desired_hint_type) # type: ignore[arg-type]
432432
== expected_result
433433
)
434+
435+
436+
@pytest.mark.parametrize(
437+
["variant_name", "in_app", "contributes", "in_app_hint", "contributes_hint", "expected_result"],
438+
[
439+
("app", True, True, in_app_hint, None, in_app_hint),
440+
("app", True, True, in_app_hint, unignored_hint, f"{in_app_hint} and {unignored_hint}"),
441+
("app", True, False, in_app_hint, ignored_hint, f"{in_app_hint} but {ignored_hint}"),
442+
("app", False, True, out_of_app_hint, None, out_of_app_hint),
443+
("app", False, True, out_of_app_hint, unignored_hint, out_of_app_hint),
444+
("app", False, False, out_of_app_hint, ignored_hint, out_of_app_hint),
445+
("system", True, True, None, None, None),
446+
("system", True, True, None, unignored_hint, unignored_hint),
447+
("system", True, False, None, ignored_hint, ignored_hint),
448+
("system", False, True, None, None, None),
449+
("system", False, True, None, unignored_hint, unignored_hint),
450+
("system", False, False, None, ignored_hint, ignored_hint),
451+
],
452+
)
453+
def test_combining_hints(
454+
variant_name,
455+
in_app,
456+
contributes,
457+
in_app_hint,
458+
contributes_hint,
459+
expected_result,
460+
):
461+
frame_component = FrameGroupingComponent(in_app=in_app, contributes=contributes, values=[])
462+
assert (
463+
_combine_hints(variant_name, frame_component, in_app_hint, contributes_hint)
464+
== expected_result
465+
)

0 commit comments

Comments
 (0)