feat(aci): Add marker for previous interval to metric detector chart

static/app/views/detectors/components/details/metric/chart.tsx

@@ -143,11 +143,13 @@ function MetricDetectorChart({
 
   const openPeriodMarkerResult = useIncidentMarkers({
     incidents: incidentPeriods,
+    includePreviousIntervalMarker: true,
     seriesName: t('Open Periods'),
     seriesId: '__incident_marker__',
     yAxisIndex: 1, // Use index 1 to avoid conflict with main chart axis
     seriesTooltip: incidentSeriesTooltip,
     markLineTooltip: incidentMarklineTooltip,
+    intervalMs: snubaQuery.timeWindow * 1000,
     onClick: context => {
       const startMs = context.period.start;
       const endMs = context.period.end ?? Date.now();

static/app/views/detectors/components/forms/metric/metricDetectorChart.tsx

@@ -210,6 +210,7 @@ export function MetricDetectorChart({
     yAxisIndex: 1, // Use index 1 to avoid conflict with main chart axis
     seriesTooltip: anomalySeriesTooltip,
     markLineTooltip: anomalyMarklineTooltip,
+    intervalMs: interval * 1000,
   });
 
   // Calculate y-axis bounds to ensure all thresholds are visible

static/app/views/detectors/hooks/useIncidentMarkers.spec.tsx

@@ -0,0 +1,71 @@
+import {renderHookWithProviders} from 'sentry-test/reactTestingLibrary';
+
+import {
+  useIncidentMarkers,
+  type IncidentPeriod,
+} from 'sentry/views/detectors/hooks/useIncidentMarkers';
+
+describe('useIncidentMarkers', () => {
+  const incident: IncidentPeriod = {
+    id: 'incident-1',
+    name: 'Test Incident',
+    color: '#ff0000',
+    type: 'test',
+    start: 10_000,
+    end: 10_000 + 60_000, // ms
+  };
+  const intervalMs = 60_000; // 1 minute
+
+  describe('includePreviousIntervalMarker', () => {
+    it('omits trigger-interval marker when includePreviousIntervalMarker is false', () => {
+      const {result} = renderHookWithProviders(() =>
+        useIncidentMarkers({
+          incidents: [incident],
+          seriesName: 'Incidents',
+          intervalMs,
+          includePreviousIntervalMarker: false,
+        })
+      );
+
+      // Just a single open period marker
+      const series = result.current.incidentMarkerSeries;
+      expect(series).not.toBeNull();
+      expect(series!.data).toHaveLength(1);
+    });
+
+    it('adds trigger-interval marker when includePreviousIntervalMarker is true', () => {
+      const {result} = renderHookWithProviders(() =>
+        useIncidentMarkers({
+          incidents: [incident],
+          seriesName: 'Incidents',
+          intervalMs,
+          includePreviousIntervalMarker: true,
+        })
+      );
+
+      const series = result.current.incidentMarkerSeries;
+      const data = series!.data as IncidentPeriod[];
+      expect(data).toHaveLength(2);
+
+      const triggerInterval = data[0];
+      const openPeriod = data[1];
+
+      expect(triggerInterval).toEqual(
+        expect.objectContaining({
+          color: incident.color,
+          // Should start 1 interval before the incident and end at the start of the incident
+          start: incident.start - intervalMs,
+          end: incident.start,
+        })
+      );
+
+      expect(openPeriod).toEqual(
+        expect.objectContaining({
+          color: incident.color,
+          start: incident.start,
+          end: incident.end,
+        })
+      );
+    });
+  });
+});

static/app/views/detectors/hooks/useIncidentMarkers.tsx

@@ -66,14 +66,72 @@ export interface IncidentPeriod {
 
 interface IncidentMarkerSeriesProps {
   incidentPeriods: IncidentPeriod[];
+  intervalMs: number;
   markLineTooltip: UseIncidentMarkersProps['markLineTooltip'];
   seriesId: string;
   seriesName: string;
   seriesTooltip: UseIncidentMarkersProps['seriesTooltip'];
   theme: Theme;
   yAxisIndex: number;
+  includePreviousIntervalMarker?: boolean;
 }
 
+interface IncidentMarkerPeriod extends IncidentPeriod {
+  type: 'trigger-interval' | 'open-period';
+}
+
+function createTriggerPeriodMarkerData({
+  period,
+  intervalMs,
+}: {
+  intervalMs: number;
+  period: IncidentPeriod;
+}): IncidentMarkerPeriod {
+  return {
+    ...period,
+    start: period.start - intervalMs,
+    end: period.start,
+    type: 'trigger-interval',
+  };
+}
+
+function createOpenPeriodMarkerData({
+  period,
+}: {
+  period: IncidentPeriod;
+}): IncidentMarkerPeriod {
+  return {
+    ...period,
+    type: 'open-period',
+  };
+}
+
+const makeStripeBackgroundSvgNode = (color: string) => {
+  return {
+    key: 'stripe-background',
+    tag: 'svg',
+    attrs: {
+      width: '2',
+      height: `${INCIDENT_MARKER_HEIGHT}`,
+      viewBox: `0 0 2 ${INCIDENT_MARKER_HEIGHT}`,
+      shapeRendering: 'crispEdges',
+    },
+    children: [
+      {
+        key: 'stripe-background-line',
+        tag: 'rect',
+        attrs: {
+          x: '0',
+          y: '0',
+          width: '1',
+          height: `${INCIDENT_MARKER_HEIGHT}`,
+          fill: color,
+        },
+      },
+    ],
+  };
+};
+
 /**
  * Creates a custom series that renders incident highlights underneath the main chart
  */
@@ -85,19 +143,31 @@ function IncidentMarkerSeries({
   yAxisIndex,
   seriesTooltip,
   markLineTooltip,
+  intervalMs,
+  includePreviousIntervalMarker,
 }: IncidentMarkerSeriesProps): CustomSeriesOption | null {
   if (!incidentPeriods.length) {
     return null;
   }
 
+  // TODO: Handle case where trigger period may overlap previous open period
+  const markerData = incidentPeriods.flatMap(period => {
+    return includePreviousIntervalMarker
+      ? [
+          createTriggerPeriodMarkerData({period, intervalMs}),
+          createOpenPeriodMarkerData({period}),
+        ]
+      : [createOpenPeriodMarkerData({period})];
+  });
+
   /**
    * Renders incident highlight rectangles underneath the main chart
    */
   const renderIncidentHighlight: CustomSeriesRenderItem = (
     params: CustomSeriesRenderItemParams,
     api: CustomSeriesRenderItemAPI
   ): CustomSeriesRenderItemReturn => {
-    const dataItem = incidentPeriods[params.dataIndex];
+    const dataItem = markerData[params.dataIndex];
 
     if (!dataItem) {
       return {type: 'group', children: []};
@@ -133,61 +203,70 @@ function IncidentMarkerSeries({
       y: incidentStartY + renderMarkerPadding - 1,
       width: width - renderMarkerPadding,
       height: INCIDENT_MARKER_HEIGHT,
-      r: 2,
+      r: [0, 2, 2, 0],
     };
 
     return {
       type: 'rect',
       transition: ['shape'],
       shape,
       style: {
-        fill: dataItem.color,
-        opacity: 0.9,
+        fill:
+          dataItem.type === 'trigger-interval'
+            ? {
+                svgElement: makeStripeBackgroundSvgNode(dataItem.color),
+                svgWidth: 2,
+                svgHeight: INCIDENT_MARKER_HEIGHT,
+              }
+            : dataItem.color,
+        opacity: 1,
       },
     };
   };
 
   // Create mark lines for the start of each incident period
-  const markLineData: MarkLineComponentOption['data'] = incidentPeriods.map(period => {
-    const lineStyle: MarkLineComponentOption['lineStyle'] = {
-      color: period.color ?? theme.gray400,
-      type: 'solid',
-      width: 1,
-      opacity: 0.8,
-    };
+  const markLineData: MarkLineComponentOption['data'] = markerData
+    .filter(period => period.type === 'open-period')
+    .map(period => {
+      const lineStyle: MarkLineComponentOption['lineStyle'] = {
+        color: period.color ?? theme.gray400,
+        type: 'solid',
+        width: 1,
+        opacity: 0.8,
+      };
 
-    return {
-      xAxis: period.start,
-      lineStyle,
-      emphasis: {
-        lineStyle: {
-          ...lineStyle,
-          width: 2,
-          opacity: 1,
+      return {
+        xAxis: period.start,
+        lineStyle,
+        emphasis: {
+          lineStyle: {
+            ...lineStyle,
+            width: 2,
+            opacity: 1,
+          },
         },
-      },
-      label: {
-        show: false,
-      },
-      tooltip: {
-        trigger: 'item',
-        position: 'bottom',
-        formatter: markLineTooltip
-          ? () => {
-              return markLineTooltip({theme, period});
-            }
-          : undefined,
-      },
-    };
-  });
+        label: {
+          show: false,
+        },
+        tooltip: {
+          trigger: 'item',
+          position: 'bottom',
+          formatter: markLineTooltip
+            ? () => {
+                return markLineTooltip({theme, period});
+              }
+            : undefined,
+        },
+      };
+    });
 
   return {
     id: seriesId ?? INCIDENT_MARKER_SERIES_ID,
     name: seriesName ?? t('Incidents'),
     type: 'custom',
     yAxisIndex,
     renderItem: renderIncidentHighlight,
-    data: incidentPeriods,
+    data: markerData,
     color: theme.red300,
     animation: false,
     markLine: MarkLine({
@@ -212,7 +291,19 @@ function IncidentMarkerSeries({
 
 interface UseIncidentMarkersProps {
   incidents: IncidentPeriod[];
+  intervalMs: number;
   seriesName: string;
+  /**
+   * If true, adds a marker for the duration of the interval before the beginning
+   * of an open period. This is used to communicate to the user that the change was
+   * detected in the preceding interval, which can be a source of confusion in the
+   * case of large intervals (like 1 day).
+   *
+   * If we stored a historical list of evaluated values from the detector, we could
+   * plot that as a line series instead of using this marker, but that is not something
+   * that is supported at present.
+   */
+  includePreviousIntervalMarker?: boolean;
   /**
    * Provide a custom tooltip for the mark line items
    */
@@ -248,6 +339,8 @@ export function useIncidentMarkers({
   markLineTooltip,
   yAxisIndex = 0,
   onClick,
+  intervalMs,
+  includePreviousIntervalMarker,
 }: UseIncidentMarkersProps): UseIncidentMarkersResult {
   const theme = useTheme();
   const chartRef = useRef<ECharts | null>(null);
@@ -329,7 +422,7 @@ export function useIncidentMarkers({
           renderItem: () => null,
           markArea: {
             itemStyle: {
-              color: data.hoverColor,
+              color: data.hoverColor ?? data.color,
               opacity: 0.2,
             },
             data: [
@@ -427,6 +520,8 @@ export function useIncidentMarkers({
       seriesId,
       seriesTooltip,
       markLineTooltip,
+      intervalMs,
+      includePreviousIntervalMarker,
     });
   }, [
     incidentPeriods,
@@ -436,6 +531,8 @@ export function useIncidentMarkers({
     seriesId,
     seriesTooltip,
     markLineTooltip,
+    intervalMs,
+    includePreviousIntervalMarker,
   ]);
 
   return {