fix(grouping): Ensure custom titles use the correct frame (#103425)

This fixes a bug caused by #103268, which moved server-side fingerprinting before the call to `normalize_stacktraces_for_grouping`[1]. While it's 99% true that they're unrelated processes (and therefore can happen in any order), the one exception to that is the handling of custom fingerprints which also include title information. In cases where the custom title includes any of the frame variables (`function`, `module`, `package`, or `abs_path`), the frame that gets used is the top in-app frame in the stacktrace. But in-app rules are applied as part of `normalize_stacktraces_for_grouping`, so having it not run until after the custom title is set is obviously a problem.

To fix this, the handling of such titles (in other words, the filling-in of the variables and adding of the result to the event) has been moved to live alongside the filling-in-of-the-variables which we do for the fingerprint itself, which is after in-app rules have been applied.  A snapshot test illustrating the fix has also been added, showing that it's not the top frame but the top in-app frame which is used for the title.

(Why didn't I just switch the order of the two operations back, you ask? Because switching the order was the first step towards absorbing the normalization into variant calculation call which comes immediately after it[2], so server-side fingerprinting needed to get out of its original spot between them.)


[1] https://github.com/getsentry/sentry/blob/6812bc023ceb6695bf6b0096852fc184feb82509/src/sentry/grouping/ingest/hashing.py#L58-L66
[2] https://github.com/getsentry/sentry/blob/6812bc023ceb6695bf6b0096852fc184feb82509/src/sentry/grouping/ingest/hashing.py#L68-L69

Author: lobsterkatie

src/sentry/grouping/api.py

@@ -38,6 +38,7 @@
 from sentry.models.grouphash import GroupHash
 from sentry.utils.cache import cache
 from sentry.utils.hashlib import md5_text
+from sentry.utils.safe import get_path
 
 if TYPE_CHECKING:
     from sentry.grouping.fingerprinting import FingerprintingConfig
@@ -282,16 +283,9 @@ def apply_server_side_fingerprinting(
 
     fingerprint_match = fingerprinting_config.get_fingerprint_values_for_event(event)
     if fingerprint_match is not None:
+        # TODO: We don't need to return attributes as part of the fingerprint match anymore
         matched_rule, new_fingerprint, attributes = fingerprint_match
-
-        # A custom title attribute is stored in the event to override the
-        # default title.
-        if "title" in attributes:
-            event["title"] = expand_title_template(attributes["title"], event)
         event["fingerprint"] = new_fingerprint
-
-        # Persist the rule that matched with the fingerprint in the event
-        # dictionary for later debugging.
         fingerprint_info["matched_rule"] = matched_rule.to_json()
 
     if fingerprint_info:
@@ -405,6 +399,9 @@ def get_grouping_variants_for_event(
         else resolve_fingerprint_values(raw_fingerprint, event.data)
     )
 
+    # Check if the fingerprint includes a custom title, and if so, set the event's title accordingly.
+    _apply_custom_title_if_needed(fingerprint_info, event)
+
     # Run all of the event-data-based grouping strategies. Any which apply will create grouping
     # components, which will then be grouped into variants by variant type (system, app, default).
     context = GroupingContext(config or _load_default_grouping_config(), event)
@@ -458,6 +455,18 @@ def get_grouping_variants_for_event(
     return final_variants
 
 
+def _apply_custom_title_if_needed(fingerprint_info: FingerprintInfo, event: Event) -> None:
+    """
+    If the given event has a custom fingerprint which includes a title template, apply the custom
+    title to the event.
+    """
+    custom_title_template = get_path(fingerprint_info, "matched_rule", "attributes", "title")
+
+    if custom_title_template:
+        resolved_title = expand_title_template(custom_title_template, event.data)
+        event.data["title"] = resolved_title
+
+
 def get_contributing_variant_and_component(
     variants: dict[str, BaseVariant],
 ) -> tuple[BaseVariant, ContributingComponent | None]:

tests/sentry/grouping/__init__.py

@@ -26,6 +26,7 @@
     GROUPING_CONFIG_CLASSES,
     register_grouping_config,
 )
+from sentry.grouping.utils import expand_title_template
 from sentry.grouping.variants import BaseVariant
 from sentry.models.project import Project
 from sentry.services import eventstore
@@ -35,6 +36,7 @@
 from sentry.testutils.helpers.eventprocessing import save_new_event
 from sentry.testutils.pytest.fixtures import InstaSnapshotter, django_db_all
 from sentry.utils import json
+from sentry.utils.safe import get_path
 
 GROUPING_TESTS_DIR = path.dirname(__file__)
 GROUPING_INPUTS_DIR = path.join(GROUPING_TESTS_DIR, "grouping_inputs")
@@ -82,11 +84,22 @@ def _manually_save_event(
         mgr.normalize()
         data = mgr.get_data()
 
-        # Normalize the stacktrace for grouping.  This normally happens in `EventManager.save`.
+        # Before creating the event, manually run the parts of `EventManager.save` which are
+        # necessary for grouping.
+
         normalize_stacktraces_for_grouping(data, load_grouping_config(grouping_config))
 
         data.setdefault("fingerprint", ["{{ default }}"])
         apply_server_side_fingerprinting(data, fingerprinting_config)
+        fingerprint_info = data.get("_fingerprint_info", {})
+        custom_title_template = get_path(fingerprint_info, "matched_rule", "attributes", "title")
+
+        # Technically handling custom titles happens during grouping, not before it, but we're not
+        # running grouping until later, and the title needs to be set before we get metadata below.
+        if custom_title_template:
+            resolved_title = expand_title_template(custom_title_template, data)
+            data["title"] = resolved_title
+
         event_type = get_event_type(data)
         event_metadata = event_type.get_metadata(data)
         data.update(materialize_metadata(data, event_type, event_metadata))
@@ -311,8 +324,18 @@ def create_event(self) -> tuple[FingerprintingConfig, Event]:
         mgr.normalize()
         data = mgr.get_data()
 
+        # Before creating the event, manually run the parts of `EventManager.save` which are
+        # necessary for fingerprinting.
+
         data.setdefault("fingerprint", ["{{ default }}"])
         apply_server_side_fingerprinting(data, config)
+        fingerprint_info = data.get("_fingerprint_info", {})
+        custom_title_template = get_path(fingerprint_info, "matched_rule", "attributes", "title")
+
+        if custom_title_template:
+            resolved_title = expand_title_template(custom_title_template, data)
+            data["title"] = resolved_title
+
         event_type = get_event_type(data)
         event_metadata = event_type.get_metadata(data)
         data.update(materialize_metadata(data, event_type, event_metadata))

tests/sentry/grouping/fingerprint_inputs/fingerprint-title-uses-top-in-app-frame.json

@@ -0,0 +1,38 @@
+{
+  "_fingerprinting_rules": [
+    {
+      "matchers": [["module", "do_dog_stuff"]],
+      "fingerprint": ["{{ function }}"],
+      "attributes": {
+        "title": "Dogs are great ({{ function }})"
+      }
+    }
+  ],
+  "exception": {
+    "values": [
+      {
+        "stacktrace": {
+          "frames": [
+            {
+              "function": "fetch",
+              "module": "do_dog_stuff",
+              "in_app": true
+            },
+            {
+              "function": "throw_ball",
+              "module": "do_dog_stuff",
+              "in_app": true
+            },
+            {
+              "function": "compute_ball_arc",
+              "module": "physics",
+              "in_app": false
+            }
+          ]
+        },
+        "type": "MathError",
+        "value": "Missing necessary formulas"
+      }
+    ]
+  }
+}

tests/sentry/grouping/snapshots/test_fingerprinting/test_event_hash_variant/fingerprint_title_uses_top_in_app_frame.pysnap

@@ -0,0 +1,44 @@
+---
+source: tests/sentry/grouping/test_fingerprinting.py::test_event_hash_variant
+---
+config:
+  rules:
+  - attributes:
+      title: Dogs are great ({{ function }})
+    fingerprint:
+    - '{{ function }}'
+    matchers:
+    - - module
+      - do_dog_stuff
+    text: module:"do_dog_stuff" -> "{{ function }}" title="Dogs are great ({{ function
+      }})"
+  version: 1
+fingerprint:
+- '{{ function }}'
+title: Dogs are great (throw_ball)
+variants:
+  app:
+    component:
+      contributes: false
+      hint: ignored because custom server fingerprint takes precedence
+    contributes: false
+    hint: ignored because custom server fingerprint takes precedence
+    key: app_exception_stacktrace
+    type: component
+  custom_fingerprint:
+    contributes: true
+    hint: null
+    key: custom_fingerprint
+    matched_rule: module:"do_dog_stuff" -> "{{ function }}" title="Dogs are great
+      ({{ function }})"
+    type: custom_fingerprint
+    values:
+    - throw_ball
+  system:
+    component:
+      contributes: false
+      hint: ignored because custom server fingerprint takes precedence
+    contributes: false
+    hint: ignored because custom server fingerprint takes precedence
+    key: system_exception_stacktrace
+    type: component