fix(issue-platform): Fix get_event_by_id to correctly return occurrence information for transactions (#49059)

When cleaning up some flags I noticed that we're not properly handling
transactions with performance issues in `get_event_by_id`. We'd convert
them to a `GroupEvent`, but not fetch the occurrence info from snuba, so
the occurrence would be unavailable.

Author: wedamija

src/sentry/api/endpoints/project_event_details.py

@@ -9,7 +9,6 @@
 from sentry.api.bases.project import ProjectEndpoint
 from sentry.api.serializers import IssueEventSerializer, serialize
 from sentry.eventstore.models import Event
-from sentry.issues.query import apply_performance_conditions
 from sentry.models.project import Project
 
 
@@ -21,22 +20,14 @@ def wrap_event_response(request_user: Any, event: Event, project: Project, envir
     prev_event_id = None
 
     if event.group_id:
-        if event.get_event_type() == "transaction":
-            conditions = apply_performance_conditions([], event.group)
-            _filter = eventstore.Filter(
-                conditions=conditions,
-                project_ids=[event.project_id],
-            )
-        else:
-            conditions = [["event.type", "!=", "transaction"]]
-            _filter = eventstore.Filter(
-                conditions=conditions,
-                project_ids=[event.project_id],
-                group_ids=[event.group_id],
-            )
-
+        conditions = []
         if environments:
             conditions.append(["environment", "IN", environments])
+        _filter = eventstore.Filter(
+            conditions=conditions,
+            project_ids=[event.project_id],
+            group_ids=[event.group_id],
+        )
 
         prev_ids, next_ids = eventstore.get_adjacent_event_ids(event, filter=_filter)
 

src/sentry/eventstore/snuba/backend.py

@@ -214,15 +214,12 @@ def get_event_by_id(
         if len(event.data) == 0:
             return None
 
-        if group_id is not None and event.get_event_type() != "generic":
-            # Set passed group_id if not a transaction
-            if event.get_event_type() == "transaction" and not skip_transaction_groupevent:
-                logger.warning("eventstore.passed-group-id-for-transaction")
-                return event.for_group(Group.objects.get(id=group_id))
-            else:
-                event.group_id = group_id
-
-        elif event.get_event_type() != "transaction":
+        if group_id is not None and (
+            event.get_event_type() == "error"
+            or (event.get_event_type() == "transaction" and skip_transaction_groupevent)
+        ):
+            event.group_id = group_id
+        elif event.get_event_type() != "transaction" or group_id:
             # Load group_id from Snuba if not a transaction
             raw_query_kwargs = {}
             if event.datetime > timezone.now() - timedelta(hours=1):
@@ -233,16 +230,21 @@ def get_event_by_id(
                     ["timestamp", ">", datetime.fromtimestamp(random.randint(0, 1000000000))]
                 ]
             dataset = (
-                Dataset.IssuePlatform if event.get_event_type() == "generic" else Dataset.Events
+                Dataset.IssuePlatform
+                if event.get_event_type() in ("transaction", "generic")
+                else Dataset.Events
             )
             try:
                 tenant_ids = tenant_ids or {"organization_id": event.project.organization_id}
+                filter_keys = {"project_id": [project_id], "event_id": [event_id]}
+                if group_id:
+                    filter_keys["group_id"] = [group_id]
                 result = snuba.raw_query(
                     dataset=dataset,
                     selected_columns=self.__get_columns(dataset),
                     start=event.datetime,
                     end=event.datetime + timedelta(seconds=1),
-                    filter_keys={"project_id": [project_id], "event_id": [event_id]},
+                    filter_keys=filter_keys,
                     limit=1,
                     referrer="eventstore.get_event_by_id_nodestore",
                     tenant_ids=tenant_ids,
@@ -274,13 +276,18 @@ def get_event_by_id(
             # Inject the snuba data here to make sure any snuba columns are available
             event._snuba_data = result["data"][0]
 
+        # Set passed group_id if not a transaction
+        if event.get_event_type() == "transaction" and not skip_transaction_groupevent and group_id:
+            logger.warning("eventstore.passed-group-id-for-transaction")
+            return event.for_group(Group.objects.get(id=group_id))
+
         return event
 
     def _get_dataset_for_event(self, event):
-        if event.get_event_type() == "transaction":
-            return snuba.Dataset.Transactions
-        elif event.get_event_type() == "generic":
+        if getattr(event, "occurrence", None) or event.get_event_type() == "generic":
             return snuba.Dataset.IssuePlatform
+        elif event.get_event_type() == "transaction":
+            return snuba.Dataset.Transactions
         else:
             return snuba.Dataset.Discover
 

src/sentry/testutils/cases.py

@@ -83,6 +83,7 @@
 from sentry.auth.superuser import ORG_ID as SU_ORG_ID
 from sentry.auth.superuser import Superuser
 from sentry.event_manager import EventManager
+from sentry.eventstore.models import Event
 from sentry.eventstream.snuba import SnubaEventStream
 from sentry.issues.grouptype import NoiseConfig, PerformanceNPlusOneGroupType
 from sentry.issues.ingest import send_issue_occurrence_to_eventstream
@@ -106,6 +107,7 @@
     IdentityStatus,
     NotificationSetting,
     Organization,
+    Project,
     ProjectOption,
     Release,
     ReleaseCommit,
@@ -136,6 +138,7 @@
 from sentry.utils import json
 from sentry.utils.auth import SsoSession
 from sentry.utils.json import dumps_htmlsafe
+from sentry.utils.performance_issues.performance_detection import detect_performance_problems
 from sentry.utils.pytest.selenium import Browser
 from sentry.utils.retries import TimedRetryPolicy
 from sentry.utils.samples import load_data
@@ -486,32 +489,55 @@ class TransactionTestCase(BaseTestCase, TransactionTestCase):
 
 class PerformanceIssueTestCase(BaseTestCase):
     def create_performance_issue(
-        self, tags=None, contexts=None, fingerprint="group1", transaction=None
+        self,
+        tags=None,
+        contexts=None,
+        fingerprint=None,
+        transaction=None,
+        event_data=None,
+        issue_type=None,
+        noise_limit=0,
+        project_id=None,
+        detector_option="performance.issues.n_plus_one_db.problem-creation",
     ):
-        event_data = load_data(
-            "transaction-n-plus-one",
-            timestamp=before_now(minutes=10),
-            fingerprint=[f"{PerformanceNPlusOneGroupType.type_id}-{fingerprint}"],
-        )
+        if issue_type is None:
+            issue_type = PerformanceNPlusOneGroupType
+        if event_data is None:
+            event_data = load_data(
+                "transaction-n-plus-one",
+                timestamp=before_now(minutes=10),
+            )
         if tags is not None:
             event_data["tags"] = tags
         if contexts is not None:
             event_data["contexts"] = contexts
         if transaction:
             event_data["transaction"] = transaction
+        if project_id is None:
+            project_id = self.project.id
 
         perf_event_manager = EventManager(event_data)
         perf_event_manager.normalize()
 
+        def detect_performance_problems_interceptor(data: Event, project: Project):
+            perf_problems = detect_performance_problems(data, project)
+            if fingerprint:
+                for perf_problem in perf_problems:
+                    perf_problem.fingerprint = fingerprint
+            return perf_problems
+
         with mock.patch(
             "sentry.issues.ingest.send_issue_occurrence_to_eventstream",
             side_effect=send_issue_occurrence_to_eventstream,
-        ) as mock_eventstream, mock.patch.object(
-            PerformanceNPlusOneGroupType, "noise_config", new=NoiseConfig(0, timedelta(minutes=1))
+        ) as mock_eventstream, mock.patch(
+            "sentry.event_manager.detect_performance_problems",
+            side_effect=detect_performance_problems_interceptor,
+        ), mock.patch.object(
+            issue_type, "noise_config", new=NoiseConfig(noise_limit, timedelta(minutes=1))
         ), override_options(
             {
                 "performance.issues.all.problem-detection": 1.0,
-                "performance.issues.n_plus_one_db.problem-creation": 1.0,
+                detector_option: 1.0,
                 "performance.issues.send_to_issues_platform": True,
                 "performance.issues.create_issues_through_platform": True,
             }
@@ -520,10 +546,11 @@ def create_performance_issue(
                 "projects:performance-suspect-spans-ingestion",
             ]
         ):
-            event = perf_event_manager.save(self.project.id)
-            group_event = event.for_group(mock_eventstream.call_args[0][2].group)
-            group_event.occurrence = mock_eventstream.call_args[0][1]
-            return group_event
+            event = perf_event_manager.save(project_id)
+            if mock_eventstream.call_args:
+                event = event.for_group(mock_eventstream.call_args[0][2].group)
+                event.occurrence = mock_eventstream.call_args[0][1]
+            return event
 
 
 class APITestCase(BaseTestCase, BaseAPITestCase):

src/sentry/utils/samples.py

@@ -114,6 +114,7 @@ def load_data(
     spans=None,
     trace_context=None,
     fingerprint=None,
+    event_id=None,
 ):
     # NOTE: Before editing this data, make sure you understand the context
     # in which its being used. It is NOT only used for local development and
@@ -256,6 +257,9 @@ def load_data(
 
             data["fingerprint"] = fingerprint
 
+    if event_id is not None:
+        data["event_id"] = event_id
+
     data["platform"] = platform
     # XXX: Message is a legacy alias for logentry. Do not overwrite if set.
     if "message" not in data:

tests/sentry/eventstore/snuba/test_backend.py

@@ -3,20 +3,17 @@
 from sentry.eventstore.base import Filter
 from sentry.eventstore.models import Event
 from sentry.eventstore.snuba.backend import SnubaEventStorage
-from sentry.issues.grouptype import (
-    PerformanceRenderBlockingAssetSpanGroupType,
-    PerformanceSlowDBQueryGroupType,
-)
-from sentry.issues.query import apply_performance_conditions
+from sentry.issues.grouptype import PerformanceNPlusOneGroupType
 from sentry.testutils import SnubaTestCase, TestCase
+from sentry.testutils.cases import PerformanceIssueTestCase
 from sentry.testutils.helpers.datetime import before_now, iso_format
 from sentry.testutils.silo import region_silo_test
 from sentry.utils import snuba
 from sentry.utils.samples import load_data
 
 
 @region_silo_test(stable=True)
-class SnubaEventStorageTest(TestCase, SnubaTestCase):
+class SnubaEventStorageTest(TestCase, SnubaTestCase, PerformanceIssueTestCase):
     def setUp(self):
         super().setUp()
         self.min_ago = iso_format(before_now(minutes=1))
@@ -66,23 +63,27 @@ def setUp(self):
         self.transaction_event = self.store_event(data=event_data, project_id=self.project1.id)
 
         event_data_2 = load_data(
-            platform="transaction",
-            fingerprint=[f"{PerformanceRenderBlockingAssetSpanGroupType.type_id}-group3"],
+            platform="transaction-n-plus-one",
+            fingerprint=[f"{PerformanceNPlusOneGroupType.type_id}-group3"],
         )
         event_data_2["timestamp"] = iso_format(before_now(seconds=30))
         event_data_2["start_timestamp"] = iso_format(before_now(seconds=31))
         event_data_2["event_id"] = "e" * 32
 
-        self.transaction_event_2 = self.store_event(data=event_data_2, project_id=self.project2.id)
+        self.transaction_event_2 = self.create_performance_issue(
+            event_data=event_data_2, project_id=self.project2.id
+        )
 
         event_data_3 = load_data(
-            "transaction", fingerprint=[f"{PerformanceSlowDBQueryGroupType.type_id}-group3"]
+            "transaction-n-plus-one", fingerprint=[f"{PerformanceNPlusOneGroupType.type_id}-group3"]
         )
         event_data_3["timestamp"] = iso_format(before_now(seconds=30))
         event_data_3["start_timestamp"] = iso_format(before_now(seconds=31))
         event_data_3["event_id"] = "f" * 32
 
-        self.transaction_event_3 = self.store_event(data=event_data_3, project_id=self.project2.id)
+        self.transaction_event_3 = self.create_performance_issue(
+            event_data=event_data_3, project_id=self.project2.id
+        )
 
         """
         event_data_4 = load_data("transaction")
@@ -182,23 +183,23 @@ def test_get_event_by_id_cached(self):
         with mock.patch("sentry.eventstore.snuba.backend.Event") as mock_event:
             dummy_event = Event(
                 project_id=self.project2.id,
-                event_id="f" * 32,
-                data={"something": "hi", "timestamp": self.min_ago},
+                event_id="1" * 32,
+                data={"something": "hi", "timestamp": self.min_ago, "type": "error"},
             )
             mock_event.return_value = dummy_event
-            event = self.eventstore.get_event_by_id(self.project2.id, "f" * 32)
+            event = self.eventstore.get_event_by_id(self.project2.id, "1" * 32)
             # Result of query should be None
             assert event is None
 
         # Now we store the event properly, so it will exist in Snuba.
         self.store_event(
-            data={"event_id": "f" * 32, "timestamp": self.min_ago},
+            data={"event_id": "1" * 32, "timestamp": self.min_ago, "type": "error"},
             project_id=self.project2.id,
         )
 
         # Make sure that the negative cache isn't causing the event to not show up
-        event = self.eventstore.get_event_by_id(self.project2.id, "f" * 32)
-        assert event.event_id == "f" * 32
+        event = self.eventstore.get_event_by_id(self.project2.id, "1" * 32)
+        assert event.event_id == "1" * 32
         assert event.project_id == self.project2.id
         assert event.group_id == event.group.id
 
@@ -299,19 +300,23 @@ def test_adjacent_event_ids_same_timestamp(self):
         assert next_event is None
 
     def test_transaction_get_next_prev_event_id(self):
-        group = self.transaction_event_2.groups[0]
+        group = self.transaction_event_2.group
         _filter = Filter(
             project_ids=[self.project2.id],
-            conditions=apply_performance_conditions([], group),
+            group_ids=[group.id],
+        )
+        event = self.eventstore.get_event_by_id(
+            self.project2.id, self.transaction_event_3.event_id, group_id=group.id
         )
-        event = self.eventstore.get_event_by_id(self.project2.id, "f" * 32)
         prev_event, next_event = self.eventstore.get_adjacent_event_ids(event, filter=_filter)
 
-        assert prev_event == (str(self.project2.id), "e" * 32)
+        assert prev_event == (str(self.project2.id), self.transaction_event_2.event_id)
         assert next_event is None
 
-        event = self.eventstore.get_event_by_id(self.project2.id, "e" * 32)
+        event = self.eventstore.get_event_by_id(
+            self.project2.id, self.transaction_event_2.event_id, group_id=group.id
+        )
         prev_event, next_event = self.eventstore.get_adjacent_event_ids(event, filter=_filter)
 
         assert prev_event is None
-        assert next_event == (str(self.project2.id), "f" * 32)
+        assert next_event == (str(self.project2.id), self.transaction_event_3.event_id)