Skip to content

Commit 311c342

Browse files
authored
ref(grouping): Split legacy stacktrace logic into new function (#91696)
In order to disentangle a bit of the conditional logic in `assemble_stacktrace_component`, this the splits it into two functions: the main `assemble_stacktrace_component` and a new `assemble_stacktrace_component_legacy` function. This lets us remove the `if legacy config` logic from the main `assemble_stacktrace_component` and remove the split enhancements logic from `assemble_stacktrace_component_legacy`, simplifying both. This also means that when we remove the legacy config, nothing about the main `assemble_stacktrace_component` will have to change.
1 parent 58bb4d4 commit 311c342

File tree

2 files changed

+69
-28
lines changed

2 files changed

+69
-28
lines changed

src/sentry/grouping/enhancer/__init__.py

Lines changed: 68 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -525,6 +525,71 @@ def apply_category_and_updated_in_app_to_frames(
525525
),
526526
)
527527

528+
def assemble_stacktrace_component_legacy(
529+
self,
530+
variant_name: str,
531+
frame_components: list[FrameGroupingComponent],
532+
frames: list[dict[str, Any]],
533+
platform: str | None,
534+
exception_data: dict[str, Any] | None = None,
535+
) -> StacktraceGroupingComponent:
536+
"""
537+
This assembles a `stacktrace` grouping component out of the given
538+
`frame` components and source frames.
539+
540+
This also handles cases where the entire stacktrace should be discarded.
541+
"""
542+
543+
match_frames: list[Any] = [create_match_frame(frame, platform) for frame in frames]
544+
rust_frames = [RustFrame(contributes=c.contributes) for c in frame_components]
545+
rust_exception_data = make_rust_exception_data(exception_data)
546+
547+
# Modify the rust frames by applying +group/-group rules and getting hints for both those
548+
# changes and the `in_app` changes applied by earlier in the ingestion process by
549+
# `apply_category_and_updated_in_app_to_frames`. Also, get `hint` and `contributes` values
550+
# for the overall stacktrace (returned in `rust_results`).
551+
rust_stacktrace_results = self.rust_enhancements.assemble_stacktrace_component(
552+
match_frames, rust_exception_data, rust_frames
553+
)
554+
555+
# Tally the number of each type of frame in the stacktrace. Later on, this will allow us to
556+
# both collect metrics and use the information in decisions about whether to send the event
557+
# to Seer
558+
frame_counts: Counter[str] = Counter()
559+
560+
# Update frame components with results from rust
561+
for frame, frame_component, rust_frame in zip(frames, frame_components, rust_frames):
562+
rust_contributes = bool(rust_frame.contributes) # bool-ing this for mypy's sake
563+
rust_hint = rust_frame.hint
564+
rust_hint_type = (
565+
None
566+
if rust_hint is None
567+
else "in-app" if rust_hint.startswith("marked") else "contributes"
568+
)
569+
570+
hint = get_hint_for_frame(variant_name, frame, frame_component, rust_frame)
571+
572+
if not (variant_name == "system" and rust_hint_type == "in-app"):
573+
hint = rust_hint
574+
575+
frame_component.update(contributes=rust_contributes, hint=hint)
576+
577+
# Add this frame to our tally
578+
key = f"{"in_app" if frame_component.in_app else "system"}_{"contributing" if frame_component.contributes else "non_contributing"}_frames"
579+
frame_counts[key] += 1
580+
581+
stacktrace_contributes = rust_stacktrace_results.contributes
582+
stacktrace_hint = rust_stacktrace_results.hint
583+
584+
stacktrace_component = StacktraceGroupingComponent(
585+
values=frame_components,
586+
hint=stacktrace_hint,
587+
contributes=stacktrace_contributes,
588+
frame_counts=frame_counts,
589+
)
590+
591+
return stacktrace_component
592+
528593
def assemble_stacktrace_component(
529594
self,
530595
variant_name: str,
@@ -611,21 +676,12 @@ def assemble_stacktrace_component(
611676
in_app_rust_frames,
612677
contributes_rust_frames,
613678
):
614-
frame_type = "in-app" if frame_component.in_app else "system"
615-
rust_contributes = bool(rust_frame.contributes) # bool-ing this for mypy's sake
616-
rust_hint = rust_frame.hint
617-
rust_hint_type = (
618-
None
619-
if rust_hint is None
620-
else "in-app" if rust_hint.startswith("marked") else "contributes"
621-
)
622-
623679
# System frames should never contribute in the app variant, so if that's what we have,
624680
# force `contribtues=False`, regardless of the rust results
625-
if variant_name == "app" and frame_type == "system":
681+
if variant_name == "app" and not frame_component.in_app:
626682
contributes = False
627683
else:
628-
contributes = rust_contributes
684+
contributes = rust_frame.contributes
629685

630686
hint = get_hint_for_frame(variant_name, frame, frame_component, rust_frame)
631687
if self.run_split_enhancements:
@@ -640,15 +696,6 @@ def assemble_stacktrace_component(
640696
variant_name, frame, frame_component, contributes_rust_frame, "contributes"
641697
)
642698

643-
# TODO: Remove this workaround once we remove the legacy config. It's done this way (as
644-
# a second pass at setting the values that undoes what the first pass did, rather than
645-
# being incorporated into the first pass) so that we won't have to change any of the
646-
# main logic when we remove it.
647-
if self.bases and self.bases[0].startswith("legacy"):
648-
contributes = rust_contributes
649-
if not (variant_name == "system" and rust_hint_type == "in-app"):
650-
hint = rust_hint
651-
652699
frame_component.update(contributes=contributes, hint=hint)
653700

654701
# Add this frame to our tally
@@ -690,13 +737,7 @@ def assemble_stacktrace_component(
690737
# stacktrace to be wrong, too (if in the process of ignoring rust we turn a stacktrace with
691738
# at least one contributing frame into one without any). So we need to special-case here as
692739
# well.
693-
#
694-
# TODO: Remove the first condition once we get rid of the legacy config
695-
if (
696-
not (self.bases and self.bases[0].startswith("legacy"))
697-
and variant_name == "app"
698-
and frame_counts["in_app_contributing_frames"] == 0
699-
):
740+
if variant_name == "app" and frame_counts["in_app_contributing_frames"] == 0:
700741
stacktrace_contributes = False
701742
stacktrace_hint = None
702743
else:

src/sentry/grouping/strategies/legacy.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -452,7 +452,7 @@ def stacktrace_legacy(
452452
frames_for_filtering.append(frame.get_raw_data())
453453
prev_frame = frame
454454

455-
stacktrace_component = context.config.enhancements.assemble_stacktrace_component(
455+
stacktrace_component = context.config.enhancements.assemble_stacktrace_component_legacy(
456456
variant_name, frame_components, frames_for_filtering, event.platform
457457
)
458458
stacktrace_component.update(contributes=contributes, hint=hint)

0 commit comments

Comments
 (0)