ref(aci milestone 3): rework incident/open period dual writes (#98396)

For the purposes of <new system, old models>, we only need to create an
IncidentGroupOpenPeriod relationship if an org is single processing
metric issues through the workflow engine. Otherwise, the incidents and
open periods are created independently.

If an org is single processing via the workflow engine, then we need to
create and update incidents based on open period creation/group status
updates, as we will no longer be creating incidents through the
subscription processor.
- Create: when saving a metric issue occurrence, create the
incident/IGOP relationship if they don't already exist for the current
open period
- Update: update the incident when a metric issue group's priority
changes
- Close: close the incident when a metric issue group's status changes
to RESOLVED

Author: mifu67

src/sentry/incidents/logic.py

@@ -101,7 +101,6 @@
 from sentry.utils.not_set import NOT_SET, NotSet
 from sentry.utils.snuba import is_measurement
 from sentry.workflow_engine.endpoints.validators.utils import toggle_detector
-from sentry.workflow_engine.models import IncidentGroupOpenPeriod
 from sentry.workflow_engine.models.detector import Detector
 
 # We can return an incident as "windowed" which returns a range of points around the start of the incident
@@ -182,14 +181,6 @@ def create_incident(
             incident_type=incident_type.value,
         )
 
-        # If this is a metric alert incident, check for pending group relationships
-        if (
-            alert_rule
-            and incident_type == IncidentType.ALERT_TRIGGERED
-            and features.has("organizations:incident-group-open-period-write", organization)
-        ):
-            IncidentGroupOpenPeriod.create_pending_relationships_for_incident(incident, alert_rule)
-
     return incident
 
 

src/sentry/incidents/subscription_processor.py

@@ -41,6 +41,7 @@
 )
 from sentry.incidents.tasks import handle_trigger_action
 from sentry.incidents.utils.process_update_helpers import (
+    calculate_event_date_from_update_date,
     get_comparison_aggregation_value,
     get_crash_rate_alert_metrics_aggregation_value_helper,
 )
@@ -631,25 +632,6 @@ def process_update(self, subscription_update: QuerySubscriptionUpdate) -> None:
             # before the next one then we might alert twice.
             self.update_alert_rule_stats()
 
-    def calculate_event_date_from_update_date(self, update_date: datetime) -> datetime:
-        """
-        Calculates the date that an event actually happened based on the date that we
-        received the update. This takes into account time window and threshold period.
-        :return:
-        """
-        # Subscriptions label buckets by the end of the bucket, whereas discover
-        # labels them by the front. This causes us an off-by-one error with event dates,
-        # so to prevent this we subtract a bucket off of the date.
-        update_date -= timedelta(seconds=self.alert_rule.snuba_query.time_window)
-        # We want to also subtract `frequency * (threshold_period - 1)` from the date.
-        # This allows us to show the actual start of the event, rather than the date
-        # of the last update that we received.
-        return update_date - timedelta(
-            seconds=(
-                self.alert_rule.snuba_query.resolution * (self.alert_rule.threshold_period - 1)
-            )
-        )
-
     def trigger_alert_threshold(
         self, trigger: AlertRuleTrigger, metric_value: float
     ) -> IncidentTrigger | None:
@@ -695,7 +677,9 @@ def trigger_alert_threshold(
 
             # Only create a new incident if we don't already have an active incident for the AlertRule
             if not self.active_incident:
-                detected_at = self.calculate_event_date_from_update_date(self.last_update)
+                detected_at = calculate_event_date_from_update_date(
+                    self.last_update, self.alert_rule.snuba_query, self.alert_rule.threshold_period
+                )
                 self.active_incident = create_incident(
                     organization=self.alert_rule.organization,
                     incident_type=IncidentType.ALERT_TRIGGERED,
@@ -767,7 +751,11 @@ def trigger_resolve_threshold(
                     self.active_incident,
                     IncidentStatus.CLOSED,
                     status_method=IncidentStatusMethod.RULE_TRIGGERED,
-                    date_closed=self.calculate_event_date_from_update_date(self.last_update),
+                    date_closed=calculate_event_date_from_update_date(
+                        self.last_update,
+                        self.alert_rule.snuba_query,
+                        self.alert_rule.threshold_period,
+                    ),
                 )
                 self.active_incident = None
                 self.incident_trigger_map.clear()

src/sentry/incidents/utils/process_update_helpers.py

@@ -217,3 +217,21 @@ def get_comparison_aggregation_value(
         return None
 
     return (aggregation_value / comparison_aggregate) * 100
+
+
+def calculate_event_date_from_update_date(
+    update_date: datetime, snuba_query: SnubaQuery, threshold_period: int = 1
+) -> datetime:
+    """
+    Calculates the date that an event actually happened based on the date that we
+    received the update. This takes into account time window and threshold period.
+    :return:
+    """
+    # Subscriptions label buckets by the end of the bucket, whereas discover
+    # labels them by the front. This causes us an off-by-one error with event dates,
+    # so to prevent this we subtract a bucket off of the date.
+    update_date -= timedelta(seconds=snuba_query.time_window)
+    # We want to also subtract `frequency * (threshold_period - 1)` from the date.
+    # This allows us to show the actual start of the event, rather than the date
+    # of the last update that we received.
+    return update_date - timedelta(seconds=(snuba_query.resolution * (threshold_period - 1)))

src/sentry/issues/ingest.py

@@ -74,7 +74,7 @@ def save_issue_occurrence(
 
         # Create IncidentGroupOpenPeriod relationship for metric issues
         if occurrence.type == MetricIssue and features.has(
-            "organizations:incident-group-open-period-write", event.organization
+            "organizations:workflow-engine-single-process-metric-issues", event.organization
         ):
             open_period = get_latest_open_period(group_info.group)
             if open_period:

src/sentry/issues/priority.py

@@ -43,6 +43,10 @@ def update_priority(
     """
     Update the priority of a group and record the change in the activity and group history.
     """
+    from sentry.incidents.grouptype import MetricIssue
+    from sentry.workflow_engine.models.incident_groupopenperiod import (
+        update_incident_activity_based_on_group_activity,
+    )
 
     if priority is None or group.priority == priority:
         return
@@ -58,6 +62,11 @@ def update_priority(
             "reason": reason,
         },
     )
+    # TODO (aci cleanup): if the group corresponds to a metric issue, then update its incident activity
+    # we will remove this once we've fully deprecated the Incident model
+    if group.type == MetricIssue.type_id:
+        update_incident_activity_based_on_group_activity(group, priority)
+
     record_group_history(group, status=PRIORITY_TO_GROUP_HISTORY_STATUS[priority], actor=actor)
 
     issue_update_priority.send_robust(

src/sentry/models/group.py

@@ -446,8 +446,12 @@ def update_group_status(
         from_substatus: int | None = None,
     ) -> None:
         """For each groups, update status to `status` and create an Activity."""
+        from sentry.incidents.grouptype import MetricIssue
         from sentry.models.activity import Activity
         from sentry.models.groupopenperiod import update_group_open_period
+        from sentry.workflow_engine.models.incident_groupopenperiod import (
+            update_incident_based_on_open_period_status_change,
+        )
 
         modified_groups_list = []
         selected_groups = Group.objects.filter(id__in=[g.id for g in groups]).exclude(
@@ -518,6 +522,10 @@ def update_group_status(
                     new_status=GroupStatus.UNRESOLVED,
                 )
 
+            # TODO (aci cleanup): remove this once we've deprecated the incident model
+            if group.type == MetricIssue.type_id:
+                update_incident_based_on_open_period_status_change(group, status)
+
     def from_share_id(self, share_id: str) -> Group:
         if not share_id or len(share_id) != 32:
             raise Group.DoesNotExist