ref(grouping): Always use default config when loading unknown config (#96610)

Currently, we have inconsistent behavior when loading a grouping config with an id we don't recognize. If the config id comes from the project, and we've therefore gotten it by using `ProjectGroupingConfigLoader._get_config_id`, it's guaranteed to be one we've heard of, because if it's not, `_get_config_id` will instead return the id of the default config[1]. If instead the config id comes from an event's `grouping_config` entry (which we grab with no validation[2]), trying to load the grouping config will raise a `GroupingConfigNotFound` error[3].

To make the behavior consistent, this switches from raising the error in the latter case to just returning the default config. Since this was the only place we were raising `GroupingConfigNotFound`, this also lets us remove all of our handling of it, as well as the error class itself.


[1] https://github.com/getsentry/sentry/blob/65fa0a9af7d186bc5142cf85e6fdb9a59e08a406/src/sentry/grouping/api.py#L147-L148
[2] https://github.com/getsentry/sentry/blob/65fa0a9af7d186bc5142cf85e6fdb9a59e08a406/src/sentry/grouping/api.py#L190
[3] https://github.com/getsentry/sentry/blob/65fa0a9af7d186bc5142cf85e6fdb9a59e08a406/src/sentry/grouping/api.py#L229-L230

Author: lobsterkatie

src/sentry/grouping/api.py

@@ -68,10 +68,6 @@ class GroupHashInfo:
 NULL_GROUPHASH_INFO = GroupHashInfo(NULL_GROUPING_CONFIG, {}, [], [], None)
 
 
-class GroupingConfigNotFound(LookupError):
-    pass
-
-
 class GroupingConfig(TypedDict):
     id: str
     enhancements: str
@@ -220,14 +216,18 @@ def get_default_grouping_config_dict(config_id: str | None = None) -> GroupingCo
 
 
 def load_grouping_config(config_dict: GroupingConfig | None = None) -> StrategyConfiguration:
-    """Loads the given grouping config."""
+    """
+    Load the given grouping config, or the default config if none is provided or if the given
+    config is not recognized.
+    """
     if config_dict is None:
         config_dict = get_default_grouping_config_dict()
     elif "id" not in config_dict:
         raise ValueError("Malformed configuration dictionary")
     config_id = config_dict["id"]
     if config_id not in CONFIGURATIONS:
-        raise GroupingConfigNotFound(config_id)
+        config_dict = get_default_grouping_config_dict()
+        config_id = config_dict["id"]
     return CONFIGURATIONS[config_id](enhancements=config_dict["enhancements"])
 
 

src/sentry/grouping/grouping_info.py

@@ -1,9 +1,7 @@
 import logging
 from typing import Any
 
-from sentry.api.exceptions import ResourceDoesNotExist
 from sentry.eventstore.models import Event, GroupEvent
-from sentry.grouping.api import GroupingConfigNotFound
 from sentry.grouping.variants import BaseVariant, PerformanceProblemVariant
 from sentry.models.project import Project
 from sentry.performance_issues.performance_detection import EventPerformanceProblem
@@ -20,29 +18,23 @@ def get_grouping_info(
     # produced hashes that would normally also appear in the event.
     hashes = event.get_hashes()
 
-    try:
-        if event.get_event_type() == "transaction":
-            # Transactions events are grouped using performance detection. They
-            # are not subject to grouping configs, and the only relevant
-            # grouping variant is `PerformanceProblemVariant`.
-
-            problems = EventPerformanceProblem.fetch_multi([(event, h) for h in hashes])
-
-            # Create a variant for every problem associated with the event
-            # TODO: Generate more unique keys, in case this event has more than
-            # one problem of a given type
-            variants: dict[str, BaseVariant] = {
-                problem.problem.type.slug: PerformanceProblemVariant(problem)
-                for problem in problems
-                if problem
-            }
-        else:
-            variants = event.get_grouping_variants(
-                force_config=config_name, normalize_stacktraces=True
-            )
-
-    except GroupingConfigNotFound:
-        raise ResourceDoesNotExist(detail="Unknown grouping config")
+    if event.get_event_type() == "transaction":
+        # Transactions events are grouped using performance detection. They
+        # are not subject to grouping configs, and the only relevant
+        # grouping variant is `PerformanceProblemVariant`.
+
+        problems = EventPerformanceProblem.fetch_multi([(event, h) for h in hashes])
+
+        # Create a variant for every problem associated with the event
+        # TODO: Generate more unique keys, in case this event has more than
+        # one problem of a given type
+        variants: dict[str, BaseVariant] = {
+            problem.problem.type.slug: PerformanceProblemVariant(problem)
+            for problem in problems
+            if problem
+        }
+    else:
+        variants = event.get_grouping_variants(force_config=config_name, normalize_stacktraces=True)
 
     grouping_info = get_grouping_info_from_variants(variants)
 

src/sentry/tasks/embeddings_grouping/backfill_seer_grouping_records_for_project.py

@@ -5,7 +5,6 @@
 
 from sentry import options
 from sentry.api.exceptions import ResourceDoesNotExist
-from sentry.grouping.api import GroupingConfigNotFound
 from sentry.grouping.enhancer.exceptions import InvalidEnhancerConfig
 from sentry.models.project import Project
 from sentry.seer.similarity.utils import (
@@ -35,7 +34,7 @@
 from sentry.utils import metrics
 
 SEER_ACCEPTABLE_FAILURE_REASONS = ["Gateway Timeout", "Service Unavailable"]
-EVENT_INFO_EXCEPTIONS = (GroupingConfigNotFound, ResourceDoesNotExist, InvalidEnhancerConfig)
+EVENT_INFO_EXCEPTIONS = (ResourceDoesNotExist, InvalidEnhancerConfig)
 
 logger = logging.getLogger(__name__)
 

tests/sentry/api/endpoints/test_event_grouping_info.py

@@ -4,7 +4,7 @@
 import pytest
 from django.urls import reverse
 
-from sentry.api.exceptions import ResourceDoesNotExist
+from sentry.conf.server import DEFAULT_GROUPING_CONFIG
 from sentry.grouping.grouping_info import get_grouping_info
 from sentry.testutils.cases import APITestCase, PerformanceIssueTestCase
 from sentry.testutils.skips import requires_snuba
@@ -113,12 +113,20 @@ def test_no_event(self) -> None:
         assert response.status_text == "Not Found"
 
     def test_get_grouping_info_unkown_grouping_config(self) -> None:
+        """Show we use the default config when the config we're given is unrecognized"""
+
         data = load_data(platform="javascript")
         event = self.store_event(data=data, project_id=self.project.id)
 
-        with pytest.raises(ResourceDoesNotExist):
+        with mock.patch(
+            "sentry.grouping.api.get_grouping_variants_for_event"
+        ) as mock_get_grouping_variants:
             get_grouping_info("fake-config", self.project, event)
 
+            mock_get_grouping_variants.assert_called_once()
+            assert mock_get_grouping_variants.call_args.args[0] == event
+            assert mock_get_grouping_variants.call_args.args[1].id == DEFAULT_GROUPING_CONFIG
+
     @mock.patch("sentry.grouping.grouping_info.logger")
     @mock.patch("sentry.grouping.grouping_info.metrics")
     def test_get_grouping_info_hash_mismatch(self, mock_metrics, mock_logger):

tests/sentry/tasks/test_backfill_seer_grouping_records.py

@@ -19,7 +19,6 @@
 from sentry.conf.server import SEER_SIMILARITY_MODEL_VERSION
 from sentry.db.models.manager.base_query_set import BaseQuerySet
 from sentry.eventstore.models import Event
-from sentry.grouping.api import GroupingConfigNotFound
 from sentry.grouping.enhancer.exceptions import InvalidEnhancerConfig
 from sentry.models.group import Group, GroupStatus
 from sentry.models.grouphash import GroupHash
@@ -1150,7 +1149,7 @@ def test_empty_nodestore(
     def test_nodestore_grouping_config_not_found(
         self, mock_call_next_backfill, mock_lookup_group_data_stacktrace_bulk
     ):
-        exceptions = (GroupingConfigNotFound(), ResourceDoesNotExist(), InvalidEnhancerConfig())
+        exceptions = (ResourceDoesNotExist(), InvalidEnhancerConfig())
 
         for exception in exceptions:
             mock_lookup_group_data_stacktrace_bulk.side_effect = exception