Switch to using detector_id for the lookups

Author: snigdhas

src/sentry/models/groupopenperiod.py

@@ -243,7 +243,7 @@ def get_open_periods_for_group(
     return open_periods
 
 
-def create_open_period(group: Group, start_time: datetime) -> None:
+def create_open_period(group: Group, start_time: datetime) -> GroupOpenPeriod:
     if not features.has("organizations:issue-open-periods", group.project.organization):
         return
 
@@ -254,7 +254,7 @@ def create_open_period(group: Group, start_time: datetime) -> None:
 
     # There are some historical cases where we log multiple regressions for the same group,
     # but we only want to create a new open period for the first regression
-    GroupOpenPeriod.objects.create(
+    return GroupOpenPeriod.objects.create(
         group=group,
         project=group.project,
         date_started=start_time,

src/sentry/workflow_engine/models/incident_groupopenperiod.py

@@ -10,8 +10,10 @@
     FlexibleForeignKey,
     region_silo_model,
 )
+from sentry.incidents.models.alert_rule import AlertRule
 from sentry.incidents.models.incident import Incident
 from sentry.models.groupopenperiod import GroupOpenPeriod
+from sentry.workflow_engine.models.alertrule_detector import AlertRuleDetector
 
 logger = logging.getLogger(__name__)
 
@@ -51,21 +53,14 @@ def create_from_occurrence(self, occurrence, group, open_period):
             open_period: The GroupOpenPeriod for the group
         """
         try:
-            # Extract alert_id from evidence_data
-            alert_id = occurrence.evidence_data.get("alert_id")
-            if not alert_id:
-                logger.warning(
-                    "No alert_id found in evidence_data for metric issue",
-                    extra={
-                        "group_id": group.id,
-                        "occurrence_id": occurrence.id,
-                    },
-                )
-                return None
+            # Extract alert_id from evidence_data using the detector_id
+            detector_id = occurrence.evidence_data.get("detector_id")
+            if detector_id:
+                alert_id = AlertRuleDetector.objects.get(detector_id=detector_id).alert_rule_id
+            else:
+                raise Exception("No detector_id found in evidence_data for metric issue")
 
             # Try to find the active incident for this alert rule and project
-            from sentry.incidents.models.alert_rule import AlertRule
-
             try:
                 alert_rule = AlertRule.objects.get(id=alert_id)
                 incident = Incident.objects.get_active_incident(
@@ -88,7 +83,7 @@ def create_from_occurrence(self, occurrence, group, open_period):
             else:
                 # Incident doesn't exist yet, create a placeholder relationship
                 # that will be updated when the incident is created
-                return self.create_placeholder_relationship(alert_id, open_period, group.project)
+                return self.create_placeholder_relationship(detector_id, open_period, group.project)
 
         except Exception as e:
             logger.exception(
@@ -133,20 +128,20 @@ def create_relationship(self, incident, open_period):
             return None
 
     @classmethod
-    def create_placeholder_relationship(self, alert_id, open_period, project):
+    def create_placeholder_relationship(self, detector_id, open_period, project):
         """
         Creates a placeholder relationship when the incident doesn't exist yet.
         This will be updated when the incident is created.
 
         Args:
-            alert_id: The alert rule ID
+            detector_id: The detector ID
             open_period: The GroupOpenPeriod to link
             project: The project for the group
         """
         try:
             # Store the alert_id in the open_period data for later lookup
             data = open_period.data or {}
-            data["pending_incident_alert_id"] = alert_id
+            data["pending_incident_detector_id"] = detector_id
             open_period.update(data=data)
 
             return None
@@ -155,7 +150,7 @@ def create_placeholder_relationship(self, alert_id, open_period, project):
             logger.exception(
                 "Failed to create placeholder IncidentGroupOpenPeriod relationship",
                 extra={
-                    "alert_id": alert_id,
+                    "detector_id": detector_id,
                     "open_period_id": open_period.id,
                     "error": str(e),
                 },
@@ -174,9 +169,10 @@ def create_pending_relationships_for_incident(self, incident, alert_rule):
             alert_rule: The AlertRule that triggered the incident
         """
         try:
-            # Find all open periods that have a pending incident alert_id for this alert rule
+            # Find all open periods that have a pending incident detector_id for this alert rule
+            detector_id = AlertRuleDetector.objects.get(alert_rule_id=alert_rule.id).detector_id
             pending_open_periods = GroupOpenPeriod.objects.filter(
-                data__pending_incident_alert_id=alert_rule.id,
+                data__pending_incident_detector_id=detector_id,
                 group__project__in=incident.projects.all(),
             )
 
@@ -186,7 +182,7 @@ def create_pending_relationships_for_incident(self, incident, alert_rule):
                 if relationship:
                     # Remove the pending flag from the open_period data
                     data = open_period.data or {}
-                    data.pop("pending_incident_alert_id", None)
+                    data.pop("pending_incident_detector_id", None)
                     open_period.update(data=data)
 
         except Exception as e:

tests/sentry/issues/test_ingest_incident_integration.py

@@ -24,6 +24,14 @@ def setUp(self) -> None:
             projects=[self.project],
             name="Test Alert Rule",
         )
+        self.detector = self.create_detector(
+            project=self.project,
+            name="Test Detector",
+        )
+        self.alert_rule_detector = self.create_alert_rule_detector(
+            alert_rule_id=self.alert_rule.id,
+            detector=self.detector,
+        )
 
     def save_issue_occurrence(
         self, group_type: int = MetricIssue.type_id
@@ -40,7 +48,7 @@ def save_issue_occurrence(
             "issue_title": "Test Issue",
             "subtitle": "Test Subtitle",
             "resource_id": None,
-            "evidence_data": {"alert_id": self.alert_rule.id},
+            "evidence_data": {"detector_id": self.detector.id},
             "evidence_display": [
                 {"name": "Test Evidence", "value": "Test Value", "important": True}
             ],
@@ -84,7 +92,7 @@ def test_save_issue_occurrence_creates_placeholder_when_incident_doesnt_exist(se
         assert group is not None
 
         open_period = GroupOpenPeriod.objects.get(group=group)
-        assert open_period.data["pending_incident_alert_id"] == self.alert_rule.id
+        assert open_period.data["pending_incident_detector_id"] == self.detector.id
 
         assert not IncidentGroupOpenPeriod.objects.filter(group_open_period=open_period).exists()
 

tests/sentry/workflow_engine/models/test_incident_groupopenperiod.py

@@ -1,12 +1,15 @@
+import uuid
 from typing import Any
 from unittest.mock import patch
 
 from django.utils import timezone
 
 from sentry.incidents.grouptype import MetricIssue
+from sentry.incidents.models.incident import IncidentStatus
 from sentry.issues.ingest import save_issue_occurrence
 from sentry.issues.issue_occurrence import IssueOccurrenceData
-from sentry.models.groupopenperiod import GroupOpenPeriod
+from sentry.models.group import GroupStatus
+from sentry.models.groupopenperiod import GroupOpenPeriod, create_open_period
 from sentry.testutils.cases import TestCase
 from sentry.testutils.helpers.features import with_feature
 from sentry.workflow_engine.models import IncidentGroupOpenPeriod
@@ -22,6 +25,14 @@ def setUp(self) -> None:
             projects=[self.project],
             name="Test Alert Rule",
         )
+        self.detector = self.create_detector(
+            project=self.project,
+            name="Test Detector",
+        )
+        self.alert_rule_detector = self.create_alert_rule_detector(
+            alert_rule_id=self.alert_rule.id,
+            detector=self.detector,
+        )
         self.group = self.create_group(project=self.project)
         self.group.type = MetricIssue.type_id
         self.group.save()
@@ -32,14 +43,14 @@ def save_issue_occurrence(self, include_alert_id: bool = True) -> tuple[Any, Gro
         )
 
         occurrence_data: IssueOccurrenceData = {
-            "id": "1",
+            "id": str(uuid.uuid4()),
             "project_id": self.project.id,
             "event_id": event.event_id,
             "fingerprint": ["test-fingerprint"],
             "issue_title": "Test Issue",
             "subtitle": "Test Subtitle",
             "resource_id": None,
-            "evidence_data": {"alert_id": self.alert_rule.id} if include_alert_id else {},
+            "evidence_data": {"detector_id": self.detector.id} if include_alert_id else {},
             "evidence_display": [
                 {"name": "Test Evidence", "value": "Test Value", "important": True}
             ],
@@ -55,7 +66,9 @@ def save_issue_occurrence(self, include_alert_id: bool = True) -> tuple[Any, Gro
         assert group_info is not None
         assert group_info.group.type == MetricIssue.type_id
 
-        open_period = GroupOpenPeriod.objects.get(group=group_info.group)
+        open_period = (
+            GroupOpenPeriod.objects.filter(group=group_info.group).order_by("-date_started").first()
+        )
         assert open_period is not None
         assert open_period.date_ended is None
         return occurrence, open_period
@@ -88,7 +101,7 @@ def test_create_from_occurrence_without_incident(self) -> None:
 
         assert result is None
         open_period.refresh_from_db()
-        assert open_period.data["pending_incident_alert_id"] == self.alert_rule.id
+        assert open_period.data["pending_incident_detector_id"] == self.detector.id
 
     @with_feature("organizations:issue-open-periods")
     def test_create_from_occurrence_no_alert_id(self) -> None:
@@ -125,9 +138,9 @@ def test_create_relationship_new(self) -> None:
         assert result.group_open_period == open_period
 
     @with_feature("organizations:issue-open-periods")
-    def test_create_relationship_existing(self) -> None:
-        """Test updating an existing relationship"""
-        occurrence, open_period = self.save_issue_occurrence()
+    def test_create_second_relationship(self) -> None:
+        """Test creating a second relationship for a new incident"""
+        _, open_period = self.save_issue_occurrence()
 
         incident1 = self.create_incident(
             organization=self.organization,
@@ -139,42 +152,49 @@ def test_create_relationship_existing(self) -> None:
         # Create initial relationship
         relationship = IncidentGroupOpenPeriod.create_relationship(incident1, open_period)
         assert relationship.incident_id == incident1.id
+        assert relationship.group_open_period == open_period
 
-        # Create new incident and update relationship
+        incident1.status = IncidentStatus.CLOSED.value
+        incident1.save()
+
+        open_period.group.update(status=GroupStatus.RESOLVED)
+        open_period.update(date_ended=timezone.now())
+        open_period_2 = create_open_period(open_period.group, timezone.now())
+
+        # Create new incident and new relationship
         incident2 = self.create_incident(
             organization=self.organization,
             title="Test Incident 2",
             date_started=timezone.now(),
             alert_rule=self.alert_rule,
         )
 
-        result = IncidentGroupOpenPeriod.create_relationship(incident2, open_period)
-
+        result = IncidentGroupOpenPeriod.create_relationship(incident2, open_period_2)
         assert result is not None
         assert result.incident_id == incident2.id
         assert result.incident_identifier == incident2.identifier
-        assert result.group_open_period == open_period
+        assert result.group_open_period == open_period_2
 
     @with_feature("organizations:issue-open-periods")
     def test_create_placeholder_relationship(self) -> None:
         """Test creating a placeholder relationship"""
         occurrence, open_period = self.save_issue_occurrence()
 
         result = IncidentGroupOpenPeriod.create_placeholder_relationship(
-            self.alert_rule.id, open_period, self.project
+            self.detector.id, open_period, self.project
         )
 
         assert result is None
         open_period.refresh_from_db()
-        assert open_period.data["pending_incident_alert_id"] == self.alert_rule.id
+        assert open_period.data["pending_incident_detector_id"] == self.detector.id
 
     @with_feature("organizations:issue-open-periods")
     def test_create_pending_relationships_for_incident(self) -> None:
         """Test creating relationships for pending open periods"""
         occurrence, open_period = self.save_issue_occurrence()
 
         # Create a placeholder relationship
-        open_period.data = {"pending_incident_alert_id": self.alert_rule.id}
+        open_period.data = {"pending_incident_detector_id": self.detector.id}
         open_period.save()
 
         incident = self.create_incident(
@@ -193,7 +213,7 @@ def test_create_pending_relationships_for_incident(self) -> None:
 
         # Check that placeholder was cleaned up
         open_period.refresh_from_db()
-        assert "pending_incident_alert_id" not in open_period.data
+        assert "pending_incident_detector_id" not in open_period.data
 
     @with_feature("organizations:issue-open-periods")
     def test_create_pending_relationships_for_incident_no_pending(self) -> None: