fix(trace): Handle missing groups (#109942)

- Groups being missing shouldn't break the entire trace view. Adding
None to the result type so we can handle errors this way and then
continue rendering the trace as best we can

Author: wmak

src/sentry/snuba/trace.py

@@ -18,7 +18,10 @@
 )
 from sentry_protos.snuba.v1.request_common_pb2 import RequestMeta, TraceItemType
 from sentry_protos.snuba.v1.trace_item_attribute_pb2 import AttributeKey, AttributeValue
-from sentry_protos.snuba.v1.trace_item_filter_pb2 import ComparisonFilter, TraceItemFilter
+from sentry_protos.snuba.v1.trace_item_filter_pb2 import (
+    ComparisonFilter,
+    TraceItemFilter,
+)
 from snuba_sdk import Column as SnubaColumn
 from snuba_sdk import Function
 
@@ -111,7 +114,9 @@ class TraceOccurrenceEvent(TypedDict):
     issue_data: TraceIssueOccurrenceData
 
 
-def _serialize_rpc_issue(event: dict[str, Any], group_cache: dict[int, Group]) -> SerializedIssue:
+def _serialize_rpc_issue(
+    event: dict[str, Any], group_cache: dict[int, Group]
+) -> SerializedIssue | None:
     def _qualify_short_id(project: str, short_id: int | None) -> str | None:
         """Logic for qualified_short_id is copied from property on the Group model
         to prevent an N+1 query from accessing project.slug everytime"""
@@ -127,7 +132,11 @@ def _qualify_short_id(project: str, short_id: int | None) -> str | None:
         if issue_id in group_cache:
             issue = group_cache[issue_id]
         else:
-            issue = Group.objects.get(id=issue_id, project__id=occurrence.project_id)
+            try:
+                issue = Group.objects.get(id=issue_id, project__id=occurrence.project_id)
+            except Group.DoesNotExist as e:
+                logger.error(e)
+                return None
             group_cache[issue_id] = issue
         return SerializedIssue(
             event_id=occurrence.event_id,
@@ -154,7 +163,11 @@ def _qualify_short_id(project: str, short_id: int | None) -> str | None:
         if issue_id in group_cache:
             issue = group_cache[issue_id]
         else:
-            issue = Group.objects.get(id=issue_id, project__id=event["project.id"])
+            try:
+                issue = Group.objects.get(id=issue_id, project__id=event["project.id"])
+            except Group.DoesNotExist as e:
+                logger.error(e)
+                return None
             group_cache[issue_id] = issue
 
         return SerializedIssue(
@@ -179,7 +192,7 @@ def _serialize_rpc_event(
     event: dict[str, Any],
     group_cache: dict[int, Group],
     additional_attributes: list[str] | None = None,
-) -> SerializedEvent | SerializedIssue | SerializedUptimeCheck:
+) -> SerializedEvent | SerializedIssue | SerializedUptimeCheck | None:
     if event.get("event_type") not in ("span", "uptime_check"):
         return _serialize_rpc_issue(event, group_cache)
 
@@ -189,11 +202,25 @@ def _serialize_rpc_event(
         if attribute in event
     }
     children = [
-        _serialize_rpc_event(child, group_cache, additional_attributes)
-        for child in event["children"]
+        child
+        for child in [
+            _serialize_rpc_event(child, group_cache, additional_attributes)
+            for child in event["children"]
+        ]
+        if child is not None
+    ]
+    errors = [
+        error
+        for error in [_serialize_rpc_issue(error, group_cache) for error in event["errors"]]
+        if error is not None
+    ]
+    occurrences = [
+        occurrence
+        for occurrence in [
+            _serialize_rpc_issue(error, group_cache) for error in event["occurrences"]
+        ]
+        if occurrence is not None
     ]
-    errors = [_serialize_rpc_issue(error, group_cache) for error in event["errors"]]
-    occurrences = [_serialize_rpc_issue(error, group_cache) for error in event["occurrences"]]
 
     if event.get("event_type") == "uptime_check":
         return SerializedUptimeCheck(
@@ -389,7 +416,9 @@ def _uptime_results_query(
     )
 
 
-def _run_uptime_results_query(uptime_query: TraceItemTableRequest) -> list[TraceItemTableResponse]:
+def _run_uptime_results_query(
+    uptime_query: TraceItemTableRequest,
+) -> list[TraceItemTableResponse]:
     return table_rpc([uptime_query])
 
 
@@ -651,4 +680,10 @@ def query_trace_data(
             result.extend(errors)
     group_cache: dict[int, Group] = {}
     with sentry_sdk.start_span(op="serializing_data"):
-        return [_serialize_rpc_event(root, group_cache, additional_attributes) for root in result]
+        return [
+            event
+            for event in [
+                _serialize_rpc_event(root, group_cache, additional_attributes) for root in result
+            ]
+            if event is not None
+        ]

tests/snuba/api/endpoints/test_organization_trace.py

@@ -11,6 +11,7 @@
 from sentry.conf.types.uptime import UptimeRegionConfig
 from sentry.issues.ingest import save_issue_occurrence
 from sentry.issues.issue_occurrence import IssueOccurrence
+from sentry.models.group import Group
 from sentry.search.events.types import SnubaParams
 from sentry.testutils.cases import UptimeResultEAPTestCase
 from sentry.testutils.helpers.datetime import before_now
@@ -23,7 +24,9 @@
 
 logger = logging.getLogger(__name__)
 
-from sentry_protos.snuba.v1.trace_item_attribute_pb2 import AttributeValue as ProtoAttributeValue
+from sentry_protos.snuba.v1.trace_item_attribute_pb2 import (
+    AttributeValue as ProtoAttributeValue,
+)
 
 from sentry.snuba.trace import _serialize_columnar_uptime_item
 from sentry.testutils.cases import TestCase
@@ -350,6 +353,32 @@ def test_with_errors_data(self) -> None:
         assert error_event["issue_id"] == error.group_id
         assert error_event["start_timestamp"] == error_data["timestamp"]
 
+    def test_with_missing_group(self) -> None:
+        self.load_trace()
+        _, start = self.get_start_end_from_day_ago(123)
+        error_data = load_data(
+            "javascript",
+            timestamp=start,
+        )
+        error_data["contexts"]["trace"] = {
+            "type": "trace",
+            "trace_id": self.trace_id,
+            "span_id": self.root_event.data["contexts"]["trace"]["span_id"],
+        }
+        error_data["tags"] = [["transaction", "/transaction/gen1-0"]]
+        self.store_event(error_data, project_id=self.gen1_project.id)
+
+        with self.feature(self.FEATURES):
+            with mock.patch("sentry.snuba.trace.Group.objects.get", side_effect=Group.DoesNotExist):
+                response = self.client_get(
+                    data={"timestamp": self.day_ago},
+                )
+        assert response.status_code == 200, response.content
+        data = response.data
+        assert len(data) == 1
+        self.assert_trace_data(data[0])
+        assert len(data[0]["errors"]) == 0
+
     def test_with_errors_data_with_overlapping_span_id(self) -> None:
         self.load_trace()
         _, start = self.get_start_end_from_day_ago(123)