Browse Source

fix(issue-platform): Fix organization event details endpoint to return occurrence (#44906)

This goes through a slightly different path than our oldest/latest event
endpoints, and so the occurrence isn't returned. Moved fetching of the
occurrence to inside `GroupEvent` so that we'll always have the
occurrence available when we have a `GroupEvent`.
Dan Fuller 2 years ago
parent
commit
3b015bf4cc

+ 3 - 0
src/sentry/api/endpoints/organization_event_details.py

@@ -35,6 +35,9 @@ class OrganizationEventDetailsEndpoint(OrganizationEventsEndpointBase):
         if event is None:
             return Response({"detail": "Event not found"}, status=404)
 
+        if event.group:
+            event = event.for_group(event.group)
+
         data = serialize(event)
         data["projectSlug"] = project_slug
 

+ 13 - 2
src/sentry/eventstore/models.py

@@ -1,6 +1,7 @@
 from __future__ import annotations
 
 import abc
+import logging
 import string
 from copy import deepcopy
 from datetime import datetime
@@ -28,6 +29,8 @@ from sentry.utils.canonical import CanonicalKeyView
 from sentry.utils.safe import get_path, trim
 from sentry.utils.strings import truncatechars
 
+logger = logging.getLogger(__name__)
+
 # Keys in the event payload we do not want to send to the event stream / snuba.
 EVENTSTREAM_PRUNED_KEYS = ("debug_meta", "_meta")
 
@@ -735,7 +738,15 @@ class GroupEvent(BaseEvent):
         return group_event
 
     @property
-    def occurrence(self) -> IssueOccurrence:
+    def occurrence(self) -> Optional[IssueOccurrence]:
+        if not self._occurrence and self.occurrence_id:
+            self._occurrence = IssueOccurrence.fetch(self.occurrence_id, self.project_id)
+            if self._occurrence is None:
+                logger.error(
+                    "Failed to fetch occurrence for event",
+                    extra={"group_id": self.group_id, "occurrence_id": self.occurrence_id},
+                )
+
         return self._occurrence
 
     @occurrence.setter
@@ -744,7 +755,7 @@ class GroupEvent(BaseEvent):
 
     @property
     def occurrence_id(self) -> Optional[str]:
-        if self.occurrence:
+        if self._occurrence:
             return self.occurrence.id
 
         column = self._get_column_name(Columns.OCCURRENCE_ID)

+ 3 - 1
src/sentry/eventstore/snuba/backend.py

@@ -220,7 +220,7 @@ class SnubaEventStorage(EventStorage):
             try:
                 result = snuba.raw_query(
                     dataset=dataset,
-                    selected_columns=["group_id"],
+                    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]},
@@ -251,6 +251,8 @@ class SnubaEventStorage(EventStorage):
                 return None
 
             event.group_id = result["data"][0]["group_id"]
+            # Inject the snuba data here to make sure any snuba columns are available
+            event._snuba_data = result["data"][0]
 
         return event
 

+ 1 - 11
src/sentry/models/group.py

@@ -32,7 +32,6 @@ from sentry.db.models import (
 )
 from sentry.eventstore.models import GroupEvent
 from sentry.issues.grouptype import ErrorGroupType, GroupCategory, get_group_type_by_type_id
-from sentry.issues.issue_occurrence import IssueOccurrence
 from sentry.issues.query import apply_performance_conditions
 from sentry.models.grouphistory import record_group_history_from_activity_type
 from sentry.snuba.dataset import Dataset
@@ -217,16 +216,7 @@ def get_oldest_or_latest_event_for_environments(
     )
 
     if events:
-        group_event = events[0].for_group(group)
-        occurrence_id = group_event.occurrence_id
-        if occurrence_id:
-            group_event.occurrence = IssueOccurrence.fetch(occurrence_id, group.project_id)
-            if group_event.occurrence is None:
-                logger.error(
-                    "Failed to fetch occurrence for event",
-                    extra={"group_id": group.id, "occurrence_id": occurrence_id},
-                )
-        return group_event
+        return events[0].for_group(group)
 
     return None
 

+ 37 - 0
tests/sentry/eventstore/test_models.py

@@ -1,4 +1,5 @@
 import pickle
+from unittest import mock
 
 import pytest
 
@@ -6,12 +7,15 @@ from sentry import eventstore, nodestore
 from sentry.db.models.fields.node import NodeData, NodeIntegrityFailure
 from sentry.eventstore.models import Event, GroupEvent
 from sentry.grouping.enhancer import Enhancements
+from sentry.issues.issue_occurrence import IssueOccurrence
+from sentry.issues.occurrence_consumer import process_event_and_issue_occurrence
 from sentry.models import Environment
 from sentry.snuba.dataset import Dataset
 from sentry.testutils import TestCase
 from sentry.testutils.helpers.datetime import before_now, iso_format
 from sentry.testutils.silo import region_silo_test
 from sentry.utils import snuba
+from tests.sentry.issues.test_utils import OccurrenceTestMixin
 
 
 @region_silo_test
@@ -555,6 +559,39 @@ class GroupEventFromEventTest(TestCase):
             group_event.project
 
 
+@region_silo_test
+class GroupEventOccurrenceTest(TestCase, OccurrenceTestMixin):
+    def test(self):
+        occurrence_data = self.build_occurrence_data(project_id=self.project.id)
+        occurrence, group_info = process_event_and_issue_occurrence(
+            occurrence_data,
+            event_data={
+                "event_id": occurrence_data["event_id"],
+                "project_id": occurrence_data["project_id"],
+                "level": "info",
+            },
+        )
+
+        event = Event(
+            occurrence_data["project_id"],
+            occurrence_data["event_id"],
+            group_info.group.id,
+            data={},
+            snuba_data={"occurrence_id": occurrence.id},
+        )
+        with mock.patch.object(IssueOccurrence, "fetch", wraps=IssueOccurrence.fetch) as fetch_mock:
+            group_event = event.for_group(event.group)
+            assert group_event.occurrence == occurrence
+            assert fetch_mock.call_count == 1
+            # Access the property again, call count shouldn't increase since we're cached
+            group_event.occurrence
+            assert fetch_mock.call_count == 1
+            # Call count should increase if we do it a second time
+            group_event.occurrence = None
+            assert group_event.occurrence == occurrence
+            assert fetch_mock.call_count == 2
+
+
 @pytest.mark.django_db
 def test_renormalization(monkeypatch, factories, task_runner, default_project):
     from sentry_relay.processing import StoreNormalizer

+ 32 - 1
tests/snuba/api/endpoints/test_organization_event_details.py

@@ -3,11 +3,13 @@ from datetime import timedelta
 import pytest
 from django.urls import NoReverseMatch, reverse
 
+from sentry.issues.occurrence_consumer import process_event_and_issue_occurrence
 from sentry.models import Group
 from sentry.testutils import APITestCase, SnubaTestCase
 from sentry.testutils.helpers.datetime import before_now, iso_format
 from sentry.testutils.silo import region_silo_test
 from sentry.utils.samples import load_data
+from tests.sentry.issues.test_utils import OccurrenceTestMixin
 
 
 def format_project_event(project_slug, event_id):
@@ -15,7 +17,7 @@ def format_project_event(project_slug, event_id):
 
 
 @region_silo_test
-class OrganizationEventDetailsEndpointTest(APITestCase, SnubaTestCase):
+class OrganizationEventDetailsEndpointTest(APITestCase, SnubaTestCase, OccurrenceTestMixin):
     def setUp(self):
         super().setUp()
         min_ago = iso_format(before_now(minutes=1))
@@ -264,3 +266,32 @@ class OrganizationEventDetailsEndpointTest(APITestCase, SnubaTestCase):
             )
 
         assert response.status_code == 404, response.content
+
+    def test_generic_event(self):
+        occurrence_data = self.build_occurrence_data(project_id=self.project.id)
+        occurrence, group_info = process_event_and_issue_occurrence(
+            occurrence_data,
+            event_data={
+                "event_id": occurrence_data["event_id"],
+                "project_id": occurrence_data["project_id"],
+                "level": "info",
+            },
+        )
+
+        url = reverse(
+            "sentry-api-0-organization-event-details",
+            kwargs={
+                "organization_slug": self.project.organization.slug,
+                "project_slug": self.project.slug,
+                "event_id": occurrence_data["event_id"],
+            },
+        )
+
+        with self.feature("organizations:discover-basic"):
+            response = self.client.get(url, format="json")
+
+        assert response.status_code == 200, response.content
+        assert response.data["id"] == occurrence_data["event_id"]
+        assert response.data["projectSlug"] == self.project.slug
+        assert response.data["occurrence"] is not None
+        assert response.data["occurrence"]["id"] == occurrence.id