fix(aci): add support for time window offsets to open period serializer (#102840)

Do this so that metric issue charts do not have their open period
markers offset by one time window. Edited the helper function that gets
the offset time to make it simpler, as we don't currently do anything
with threshold_period.

Author: mifu67

src/sentry/incidents/charts.py

@@ -136,6 +136,7 @@ def fetch_metric_issue_open_periods(
     open_period_identifier: int,
     time_period: Mapping[str, str],
     user: User | RpcUser | None = None,
+    time_window: int = 0,
 ) -> list[Any]:
     detector_id = open_period_identifier
     try:
@@ -157,6 +158,7 @@ def fetch_metric_issue_open_periods(
                 path=f"/organizations/{organization.slug}/open-periods/",
                 params={
                     "detectorId": detector_id,
+                    "bucketSize": time_window,
                     **time_period,
                 },
             )
@@ -260,6 +262,7 @@ def build_metric_alert_chart(
                 alert_context.action_identifier_id,
                 time_period,
                 user,
+                snuba_query.time_window,
             ),
         }
     else:

src/sentry/incidents/utils/process_update_helpers.py

@@ -219,9 +219,7 @@ def get_comparison_aggregation_value(
     return (aggregation_value / comparison_aggregate) * 100
 
 
-def calculate_event_date_from_update_date(
-    update_date: datetime, snuba_query: SnubaQuery, threshold_period: int = 1
-) -> datetime:
+def calculate_event_date_from_update_date(update_date: datetime, time_window: int) -> 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.
@@ -230,8 +228,4 @@ def calculate_event_date_from_update_date(
     # 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)))
+    return update_date - timedelta(seconds=time_window)

src/sentry/workflow_engine/endpoints/organization_open_periods.py

@@ -111,6 +111,8 @@ def get(self, request: Request, organization: Organization) -> Response:
 
         detector_id_param = request.GET.get("detectorId")
         group_id_param = request.GET.get("groupId")
+        # determines the time we need to subtract off of each timestamp before returning the data
+        bucket_size_param = request.GET.get("bucketSize", 0)
 
         if not detector_id_param and not group_id_param:
             raise ValidationError({"detail": "Must provide either detectorId or groupId"})
@@ -145,6 +147,6 @@ def get(self, request: Request, organization: Organization) -> Response:
             request=request,
             queryset=open_periods,
             paginator_cls=OffsetPaginator,
-            on_results=lambda x: serialize(x, request.user),
+            on_results=lambda x: serialize(x, request.user, time_window=int(bucket_size_param)),
             count_hits=True,
         )

src/sentry/workflow_engine/endpoints/serializers/group_open_period_serializer.py

@@ -4,6 +4,7 @@
 from typing import Any, TypedDict
 
 from sentry.api.serializers import Serializer, register, serialize
+from sentry.incidents.utils.process_update_helpers import calculate_event_date_from_update_date
 from sentry.models.groupopenperiod import GroupOpenPeriod, get_last_checked_for_open_period
 from sentry.models.groupopenperiodactivity import GroupOpenPeriodActivity, OpenPeriodActivityType
 from sentry.types.group import PriorityLevel
@@ -61,10 +62,15 @@ def get_attrs(self, item_list, user, **kwargs):
     def serialize(
         self, obj: GroupOpenPeriod, attrs: Mapping[str, Any], user, **kwargs
     ) -> GroupOpenPeriodResponse:
+        time_window = kwargs.get("time_window", 0)
         return GroupOpenPeriodResponse(
             id=str(obj.id),
-            start=obj.date_started,
-            end=obj.date_ended,
+            start=calculate_event_date_from_update_date(obj.date_started, time_window),
+            end=(
+                calculate_event_date_from_update_date(obj.date_ended, time_window)
+                if obj.date_ended is not None
+                else None
+            ),
             isOpen=obj.date_ended is None,
             lastChecked=get_last_checked_for_open_period(obj.group),
             activities=attrs.get("activities"),

src/sentry/workflow_engine/models/incident_groupopenperiod.py

@@ -149,7 +149,7 @@ def create_incident_for_open_period(cls, occurrence, alert_rule, group, open_per
             raise Exception("No source_id found in evidence_data for metric issue")
 
         calculated_start_date = calculate_event_date_from_update_date(
-            open_period.date_started, snuba_query
+            open_period.date_started, snuba_query.time_window
         )
 
         incident = create_incident(
@@ -336,7 +336,7 @@ def update_incident_based_on_open_period_status_change(
             )
             return
         calculated_date_closed = calculate_event_date_from_update_date(
-            open_period.date_ended, snuba_query
+            open_period.date_ended, snuba_query.time_window
         )
         update_incident_status(
             incident,

tests/sentry/incidents/test_charts.py

@@ -22,6 +22,7 @@
 from sentry.incidents.logic import CRITICAL_TRIGGER_LABEL
 from sentry.incidents.models.incident import Incident, IncidentActivityType, IncidentStatus
 from sentry.incidents.typings.metric_detector import AlertContext, OpenPeriodContext
+from sentry.incidents.utils.process_update_helpers import calculate_event_date_from_update_date
 from sentry.models.groupopenperiod import GroupOpenPeriod
 from sentry.models.groupopenperiodactivity import GroupOpenPeriodActivity, OpenPeriodActivityType
 from sentry.snuba.dataset import Dataset
@@ -30,6 +31,7 @@
 from sentry.testutils.helpers.features import with_feature
 from sentry.types.group import PriorityLevel
 from sentry.workflow_engine.models import DetectorGroup
+from tests.sentry.incidents.utils.test_metric_issue_base import BaseMetricIssueTest
 
 now = "2022-05-16T20:00:00"
 frozen_time = f"{now}Z"
@@ -133,7 +135,7 @@ def test_eap_alert(self, mock_client_get: MagicMock, mock_generate_chart: MagicM
         assert mock_client_get.call_args[1]["params"]["query"] == "span.op:pageload"
 
 
-class FetchOpenPeriodsTest(TestCase):
+class FetchOpenPeriodsTest(BaseMetricIssueTest):
     @freeze_time(frozen_time)
     @with_feature("organizations:incidents")
     def test_get_incidents_from_detector(self) -> None:
@@ -209,3 +211,42 @@ def test_use_open_period_serializer(self) -> None:
         assert activities[0]["id"] == str(opened_gopa.id)
         assert activities[0]["type"] == OpenPeriodActivityType(opened_gopa.type).to_str()
         assert activities[0]["value"] == PriorityLevel(group.priority).to_str()
+
+    @freeze_time(frozen_time)
+    @with_feature("organizations:incidents")
+    @with_feature("organizations:new-metric-issue-charts")
+    def test_use_open_period_serializer_with_offset(self) -> None:
+        group = self.create_group(type=MetricIssue.type_id, priority=PriorityLevel.HIGH)
+
+        # Link detector to group
+        DetectorGroup.objects.create(detector=self.detector, group=group)
+
+        group_open_period = GroupOpenPeriod.objects.get(group=group)
+
+        opened_gopa = GroupOpenPeriodActivity.objects.create(
+            date_added=group_open_period.date_added,
+            group_open_period=group_open_period,
+            type=OpenPeriodActivityType.OPENED,
+            value=group.priority,
+        )
+
+        time_period = incident_date_range(
+            60, group_open_period.date_started, group_open_period.date_ended
+        )
+
+        chart_data = fetch_metric_issue_open_periods(
+            self.organization,
+            self.detector.id,
+            time_period,
+            time_window=self.snuba_query.time_window,
+        )
+
+        assert chart_data[0]["id"] == str(group_open_period.id)
+        assert chart_data[0]["start"] == calculate_event_date_from_update_date(
+            group_open_period.date_started, self.snuba_query.time_window
+        )
+
+        activities = chart_data[0]["activities"]
+        assert activities[0]["id"] == str(opened_gopa.id)
+        assert activities[0]["type"] == OpenPeriodActivityType(opened_gopa.type).to_str()
+        assert activities[0]["value"] == PriorityLevel(group.priority).to_str()