fix(grouping): Make system frames not contribute to app variant (#84382)

When we do grouping based on stacktrace, we actually produce two hashes, one based on the full stacktrace and one based on only the in-app frames. Right now, it's possible for non-app ("system") frames to be included in the app hash if they're subject to a `+group` stacktrace rule.

This fixes that by checking the results which come back from the rust enhancer (which is what actually does the application of the stacktrace rules) when we're calculating the app hash and overriding them if necessary, to weed out the rogue system frames. Doing this can change a stacktrace which rust thinks has contributing frames to one which no longer does (and therefore shouldn't contribute to the hash), so we also now check the full stacktrace in the app variant and override its `contributing` value in that case.

The effects of this change are easiest to see in the `test_variants/test_event_hash_variant` newstyle snapshots. (The metadata snapshots, because they don't show both app and system variants, do show the correct changes, but in a way which is less intuitive.)

- In both inputs[1],[2], the `handleRequest` frame from `router.js` is a system frame which has until now been counted as contributing to the app variant because of the `stack.function:handleRequest -app +group` rule.

- In `contributing_system_and_app_frames`, the app variant also has the in-app `playFetch` frame contributing, so even though the app hash changes when the `handleRequest` frame stops contributing, the app variant's stacktrace remains contributing. (Note that the hash change won't actually lead to new groups, since the old hash, which used to match the system hash, is still being produced by the system variant.)

  
(See PR description for a screenshot of the changes)


- In `contributing_system_frames`, the only contributing frame the app variant had was the `handleRequest` frame. With it now not contributing, the app variant has no contributing frames, so its stacktrace has switched from contributing to not contributing. (Again here, no new group will be formed because the system hash remains the same.)

  
(See PR description for a screenshot of the changes)


The other effect of this change is that it fixes what is currently broken behavior in the `has_too_many_contributing_frames` Seer utility function[3]. Right now, stacktraces like the one in the `contributing_system_frames` test input mistakenly count the app variant as the contributing one, and `contributing_system_frames` therefore looks to see how many in-app contributing frames there are. Frames like the `handleRequest` frame don't count in that tally, because they're not in-app, leading `has_too_many_contributing_frames` to mistakenly let through stacktraces it shouldn't.

With this fix, we can stop skipping the `has_too_many_contributing_frames` test demonstrating this[4]. This PR also adds dedicated tests in `test_enhancer.py`.

Note: Because the legacy config handles frames and stacktraces differently, this fix is limited to hash calculations using the newstyle configs.


[1] https://github.com/getsentry/sentry/blob/e4888e1eb290ab59dff98729f3992551bfe23f3e/tests/sentry/grouping/grouping_inputs/contributing_system_and_app_frames.json
[2] https://github.com/getsentry/sentry/blob/e4888e1eb290ab59dff98729f3992551bfe23f3e/tests/sentry/grouping/grouping_inputs/contributing_system_frames.json
[3] https://github.com/getsentry/sentry/blob/e4888e1eb290ab59dff98729f3992551bfe23f3e/src/sentry/seer/similarity/utils.py#L320-L379
[4] https://github.com/getsentry/sentry/blob/e4888e1eb290ab59dff98729f3992551bfe23f3e/tests/sentry/seer/similarity/test_utils.py#L1055-L1075

src/sentry/grouping/enhancer/__init__.py

@@ -176,6 +176,7 @@ def apply_category_and_updated_in_app_to_frames(
 
     def assemble_stacktrace_component(
         self,
+        variant_name: str,
         frame_components: list[FrameGroupingComponent],
         frames: list[dict[str, Any]],
         platform: str | None,
@@ -191,6 +192,10 @@ def assemble_stacktrace_component(
 
         rust_components = [RustComponent(contributes=c.contributes) for c in frame_components]
 
+        # Modify the rust components by applying +group/-group rules and getting hints for both
+        # those changes and the `in_app` changes applied by earlier in the ingestion process by
+        # `apply_category_and_updated_in_app_to_frames`. Also, get `hint` and `contributes` values
+        # for the overall stacktrace (returned in `rust_results`).
         rust_results = self.rust_enhancements.assemble_stacktrace_component(
             match_frames, make_rust_exception_data(exception_data), rust_components
         )
@@ -200,15 +205,61 @@ def assemble_stacktrace_component(
         # to Seer
         frame_counts: Counter[str] = Counter()
 
+        # Update frame components with results from rust
         for py_component, rust_component in zip(frame_components, rust_components):
-            py_component.update(contributes=rust_component.contributes, hint=rust_component.hint)
+            # TODO: Remove the first condition once we get rid of the legacy config
+            if (
+                not (self.bases and self.bases[0].startswith("legacy"))
+                and variant_name == "app"
+                and not py_component.in_app
+            ):
+                # System frames should never contribute in the app variant, so force
+                # `contribtues=False`, regardless of the rust results. Use the rust hint if it
+                # explains the `in_app` value (but not if it explains the `contributing` value,
+                # because we're ignoring that)
+                #
+                # TODO: Right now, if stacktrace rules have modified both the `in_app` and
+                # `contributes` values, then the hint you get back from the rust enhancers depends
+                # on the order in which those changes happened, which in turn depends on both the
+                # order of stacktrace rules and the order of the actions within a stacktrace rule.
+                # Ideally we'd get both hints back.
+                hint = (
+                    rust_component.hint
+                    if rust_component.hint and rust_component.hint.startswith("marked out of app")
+                    else py_component.hint
+                )
+                py_component.update(contributes=False, hint=hint)
+            else:
+                py_component.update(
+                    contributes=rust_component.contributes, hint=rust_component.hint
+                )
+
+            # Add this frame to our tally
             key = f"{"in_app" if py_component.in_app else "system"}_{"contributing" if py_component.contributes else "non_contributing"}_frames"
             frame_counts[key] += 1
 
+        # Because of the special case above, in which we ignore the rust-derived `contributes` value
+        # for certain frames, it's possible for the rust-derived `contributes` value for the overall
+        # stacktrace to be wrong, too (if in the process of ignoring rust we turn a stacktrace with
+        # at least one contributing frame into one without any). So we need to special-case here as
+        # well.
+        #
+        # TODO: Remove the first condition once we get rid of the legacy config
+        if (
+            not (self.bases and self.bases[0].startswith("legacy"))
+            and variant_name == "app"
+            and frame_counts["in_app_contributing_frames"] == 0
+        ):
+            stacktrace_contributes = False
+            stacktrace_hint = None
+        else:
+            stacktrace_contributes = rust_results.contributes
+            stacktrace_hint = rust_results.hint
+
         stacktrace_component = StacktraceGroupingComponent(
             values=frame_components,
-            hint=rust_results.hint,
-            contributes=rust_results.contributes,
+            hint=stacktrace_hint,
+            contributes=stacktrace_contributes,
             frame_counts=frame_counts,
         )
 

src/sentry/grouping/strategies/legacy.py

@@ -448,7 +448,7 @@ def stacktrace_legacy(
         prev_frame = frame
 
     stacktrace_component = context.config.enhancements.assemble_stacktrace_component(
-        frame_components, frames_for_filtering, event.platform
+        variant_name, frame_components, frames_for_filtering, event.platform
     )
     stacktrace_component.update(contributes=contributes, hint=hint)
     return {variant_name: stacktrace_component}

src/sentry/grouping/strategies/newstyle.py

@@ -464,6 +464,7 @@ def _single_stacktrace_variant(
         )
 
     stacktrace_component = context.config.enhancements.assemble_stacktrace_component(
+        variant_name,
         frame_components,
         frames_for_filtering,
         event.platform,

tests/sentry/grouping/snapshots/test_grouphash_metadata/test_metadata_from_variants/newstyle@2019_05_08/contributing_system_and_app_frames.pysnap

@@ -1,5 +1,5 @@
 ---
-created: '2025-01-30T21:41:17.184133+00:00'
+created: '2025-01-30T21:45:36.771348+00:00'
 creator: sentry
 source: tests/sentry/grouping/test_grouphash_metadata.py
 ---
@@ -24,13 +24,29 @@ metrics with tags: {
 ---
 contributing variants:
   app*
-    hash: "c5e4b4a9ad1803c4d4ca7feee5e430ae"
+    hash: "161ce02ecc5d6685a72e8e520ab726b3"
     contributing component: exception
     component:
       app*
         exception*
           stacktrace*
-            frame* (un-ignored by stack trace rule (function:handleRequest -app +group))
+            frame* (marked in-app by stack trace rule (function:playFetch +app +group))
+              filename*
+                "dogpark.js"
+              function*
+                "playFetch"
+              context-line*
+                "raise FailedToFetchError('Charlie didn't bring the ball back!');"
+          type*
+            "FailedToFetchError"
+  system*
+    hash: "c5e4b4a9ad1803c4d4ca7feee5e430ae"
+    contributing component: exception
+    component:
+      system*
+        exception*
+          stacktrace*
+            frame* (marked out of app by stack trace rule (function:handleRequest -app +group))
               filename*
                 "router.js"
               function*

tests/sentry/grouping/snapshots/test_grouphash_metadata/test_metadata_from_variants/newstyle@2019_05_08/contributing_system_frames.pysnap

@@ -1,13 +1,13 @@
 ---
-created: '2025-01-30T21:41:17.269522+00:00'
+created: '2025-01-30T21:45:36.965256+00:00'
 creator: sentry
 source: tests/sentry/grouping/test_grouphash_metadata.py
 ---
 hash_basis: stacktrace
 hashing_metadata: {
   "num_stacktraces": 1,
   "stacktrace_location": "exception",
-  "stacktrace_type": "in_app"
+  "stacktrace_type": "system"
 }
 ---
 metrics with tags: {
@@ -18,19 +18,19 @@ metrics with tags: {
   "grouping.grouphashmetadata.event_hashing_metadata.stacktrace": {
     "chained_exception": "False",
     "stacktrace_location": "exception",
-    "stacktrace_type": "in_app"
+    "stacktrace_type": "system"
   }
 }
 ---
 contributing variants:
-  app*
+  system*
     hash: "fe92cff6711f8a0a30cabb8b9245b1d6"
     contributing component: exception
     component:
-      app*
+      system*
         exception*
           stacktrace*
-            frame* (un-ignored by stack trace rule (function:handleRequest -app +group))
+            frame* (marked out of app by stack trace rule (function:handleRequest -app +group))
               filename*
                 "router.js"
               function*

tests/sentry/grouping/snapshots/test_grouphash_metadata/test_metadata_from_variants/newstyle@2019_10_29/contributing_system_and_app_frames.pysnap

@@ -1,5 +1,5 @@
 ---
-created: '2025-01-30T21:41:01.822552+00:00'
+created: '2025-01-30T21:46:13.738334+00:00'
 creator: sentry
 source: tests/sentry/grouping/test_grouphash_metadata.py
 ---
@@ -24,13 +24,29 @@ metrics with tags: {
 ---
 contributing variants:
   app*
-    hash: "c5e4b4a9ad1803c4d4ca7feee5e430ae"
+    hash: "161ce02ecc5d6685a72e8e520ab726b3"
     contributing component: exception
     component:
       app*
         exception*
           stacktrace*
-            frame* (un-ignored by stack trace rule (function:handleRequest -app +group))
+            frame* (marked in-app by stack trace rule (function:playFetch +app +group))
+              filename*
+                "dogpark.js"
+              function*
+                "playFetch"
+              context-line*
+                "raise FailedToFetchError('Charlie didn't bring the ball back!');"
+          type*
+            "FailedToFetchError"
+  system*
+    hash: "c5e4b4a9ad1803c4d4ca7feee5e430ae"
+    contributing component: exception
+    component:
+      system*
+        exception*
+          stacktrace*
+            frame* (marked out of app by stack trace rule (function:handleRequest -app +group))
               filename*
                 "router.js"
               function*

tests/sentry/grouping/snapshots/test_grouphash_metadata/test_metadata_from_variants/newstyle@2019_10_29/contributing_system_frames.pysnap

@@ -1,13 +1,13 @@
 ---
-created: '2025-01-30T21:41:01.889599+00:00'
+created: '2025-01-30T21:46:13.879780+00:00'
 creator: sentry
 source: tests/sentry/grouping/test_grouphash_metadata.py
 ---
 hash_basis: stacktrace
 hashing_metadata: {
   "num_stacktraces": 1,
   "stacktrace_location": "exception",
-  "stacktrace_type": "in_app"
+  "stacktrace_type": "system"
 }
 ---
 metrics with tags: {
@@ -18,19 +18,19 @@ metrics with tags: {
   "grouping.grouphashmetadata.event_hashing_metadata.stacktrace": {
     "chained_exception": "False",
     "stacktrace_location": "exception",
-    "stacktrace_type": "in_app"
+    "stacktrace_type": "system"
   }
 }
 ---
 contributing variants:
-  app*
+  system*
     hash: "fe92cff6711f8a0a30cabb8b9245b1d6"
     contributing component: exception
     component:
-      app*
+      system*
         exception*
           stacktrace*
-            frame* (un-ignored by stack trace rule (function:handleRequest -app +group))
+            frame* (marked out of app by stack trace rule (function:handleRequest -app +group))
               filename*
                 "router.js"
               function*

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

@@ -1,5 +1,5 @@
 ---
-created: '2025-01-30T21:40:46.962385+00:00'
+created: '2025-01-30T21:46:37.831164+00:00'
 creator: sentry
 source: tests/sentry/grouping/test_grouphash_metadata.py
 ---
@@ -24,13 +24,29 @@ metrics with tags: {
 ---
 contributing variants:
   app*
-    hash: "c5e4b4a9ad1803c4d4ca7feee5e430ae"
+    hash: "161ce02ecc5d6685a72e8e520ab726b3"
     contributing component: exception
     component:
       app*
         exception*
           stacktrace*
-            frame* (un-ignored by stack trace rule (function:handleRequest -app +group))
+            frame* (marked in-app by stack trace rule (function:playFetch +app +group))
+              filename*
+                "dogpark.js"
+              function*
+                "playFetch"
+              context-line*
+                "raise FailedToFetchError('Charlie didn't bring the ball back!');"
+          type*
+            "FailedToFetchError"
+  system*
+    hash: "c5e4b4a9ad1803c4d4ca7feee5e430ae"
+    contributing component: exception
+    component:
+      system*
+        exception*
+          stacktrace*
+            frame* (marked out of app by stack trace rule (function:handleRequest -app +group))
               filename*
                 "router.js"
               function*

tests/sentry/grouping/snapshots/test_grouphash_metadata/test_metadata_from_variants/newstyle@2023_01_11/contributing_system_frames.pysnap

@@ -1,13 +1,13 @@
 ---
-created: '2025-01-30T21:40:47.031377+00:00'
+created: '2025-01-30T21:46:37.985114+00:00'
 creator: sentry
 source: tests/sentry/grouping/test_grouphash_metadata.py
 ---
 hash_basis: stacktrace
 hashing_metadata: {
   "num_stacktraces": 1,
   "stacktrace_location": "exception",
-  "stacktrace_type": "in_app"
+  "stacktrace_type": "system"
 }
 ---
 metrics with tags: {
@@ -18,19 +18,19 @@ metrics with tags: {
   "grouping.grouphashmetadata.event_hashing_metadata.stacktrace": {
     "chained_exception": "False",
     "stacktrace_location": "exception",
-    "stacktrace_type": "in_app"
+    "stacktrace_type": "system"
   }
 }
 ---
 contributing variants:
-  app*
+  system*
     hash: "fe92cff6711f8a0a30cabb8b9245b1d6"
     contributing component: exception
     component:
-      app*
+      system*
         exception*
           stacktrace*
-            frame* (un-ignored by stack trace rule (function:handleRequest -app +group))
+            frame* (marked out of app by stack trace rule (function:handleRequest -app +group))
               filename*
                 "router.js"
               function*

tests/sentry/grouping/snapshots/test_variants/test_event_hash_variant/newstyle@2019_05_08/contributing_system_and_app_frames.pysnap

@@ -1,10 +1,10 @@
 ---
-created: '2025-01-30T21:38:17.979729+00:00'
+created: '2025-01-30T21:48:30.179389+00:00'
 creator: sentry
 source: tests/sentry/grouping/test_variants.py
 ---
 app:
-  hash: "c5e4b4a9ad1803c4d4ca7feee5e430ae"
+  hash: "161ce02ecc5d6685a72e8e520ab726b3"
   contributing component: exception
   component:
     app*
@@ -17,7 +17,7 @@ app:
               "runApp"
             context-line*
               "return server.serve(port);"
-          frame* (un-ignored by stack trace rule (function:handleRequest -app +group))
+          frame (non app frame)
             filename*
               "router.js"
             function*
@@ -44,11 +44,11 @@ app:
           "FailedToFetchError: Charlie didn't bring the ball back!"
 --------------------------------------------------------------------------
 system:
-  hash: null
-  contributing component: null
+  hash: "c5e4b4a9ad1803c4d4ca7feee5e430ae"
+  contributing component: exception
   component:
-    system (exception of app takes precedence)
-      exception (ignored because hash matches app variant)
+    system*
+      exception*
         stacktrace*
           frame (ignored by stack trace rule (function:runApp -app -group))
             filename*