Remove most long window metric alert rule options (#8241)

Author: jonathanawesome

packages/migrations/src/clickhouse-actions/019-metric-alert-target-daily-rollup.ts

@@ -2,9 +2,9 @@ import type { Action } from '../clickhouse';
 
 // Daily by-target rollup for the metric-alert evaluator. Mirrors the
 // minutely/hourly rollups in 018 but bucketed per day, so long-window rules
-// (>= 7 days) read ~60 daily buckets for a 30-day query instead of ~1,440
-// hourly ones. The 90-day TTL covers the 60-day (2x window) scan of a 30-day
-// rule.
+// (the 7-day max) read ~14 daily buckets (current + previous window) instead of
+// ~336 hourly ones. The 90-day TTL comfortably covers the 14-day (2x window)
+// scan of a 7-day rule.
 const tableColumns = `
   target LowCardinality(String) CODEC(ZSTD(1)),
   timestamp DateTime('UTC') CODEC(DoubleDelta, LZ4),

packages/services/api/src/modules/alerts/providers/metric-alert-rules-manager.ts

@@ -337,7 +337,7 @@ export class MetricAlertRulesManager {
       timeWindowMinutes > METRIC_ALERT_RULE_TIME_WINDOW_MAX_MINUTES
     ) {
       throw new MetricAlertRuleValidationError(
-        `Time window must be a whole number of minutes between ${METRIC_ALERT_RULE_TIME_WINDOW_MIN_MINUTES} and ${METRIC_ALERT_RULE_TIME_WINDOW_MAX_MINUTES} (30 days).`,
+        `Time window must be a whole number of minutes between ${METRIC_ALERT_RULE_TIME_WINDOW_MIN_MINUTES} and ${METRIC_ALERT_RULE_TIME_WINDOW_MAX_MINUTES} (7 days).`,
       );
     }
     // Windows at/above the daily-rollup threshold read whole-day buckets, so they

packages/services/api/src/modules/commerce/constants.ts

@@ -42,18 +42,19 @@ export const METRIC_ALERT_RULES_PER_TARGET_LIMIT = 10;
  * Min = 1: the evaluator cron runs every minute, so a sub-1-minute window
  * never gets a fresh evaluation and the rule's behaviour is undefined.
  *
- * Max = 30 days: rules with longer windows query ClickHouse's
- * `operations_hourly` rollup, which is bounded by the retention policy. 30
- * days is the practical ceiling beyond which queries start scanning data
- * that may already be aged out.
+ * Max = 7 days: the product ceiling for alert windows. At the max the window
+ * equals METRIC_ALERT_RULE_DAILY_ROLLUP_THRESHOLD_MINUTES, so it reads
+ * ClickHouse's whole-day rollup and must be a whole number of days (7d
+ * qualifies). Longer windows added little alerting value and pushed queries
+ * toward the edge of the retention policy.
  *
  * The UI form's `Select` exposes a smaller subset of this range for UX
  * reasons; these constants are the absolute bounds the API enforces against
  * any caller (form, seed scripts, customer integrations).
  */
 export const MINUTES_PER_DAY = 24 * 60; // 1440
 export const METRIC_ALERT_RULE_TIME_WINDOW_MIN_MINUTES = 1;
-export const METRIC_ALERT_RULE_TIME_WINDOW_MAX_MINUTES = 30 * MINUTES_PER_DAY; // 43200
+export const METRIC_ALERT_RULE_TIME_WINDOW_MAX_MINUTES = 7 * MINUTES_PER_DAY; // 10080
 
 // Windows >= this read the daily ClickHouse rollup (whole-day buckets), so they
 // must be a whole number of days. Mirrors DAILY_THRESHOLD_MINUTES in the workflows

packages/services/workflows/src/lib/metric-alert-evaluator.spec.ts

@@ -76,8 +76,8 @@ describe('groupRulesByQuery', () => {
 
   test('at >= 7d a TRAFFIC rule and a latency rule split into hourly + daily groups', () => {
     const groups = groupRulesByQuery([
-      makeRule({ id: 'a', type: 'TRAFFIC', timeWindowMinutes: 43200 }),
-      makeRule({ id: 'b', type: 'LATENCY', metric: 'P95', timeWindowMinutes: 43200 }),
+      makeRule({ id: 'a', type: 'TRAFFIC', timeWindowMinutes: 10080 }),
+      makeRule({ id: 'b', type: 'LATENCY', metric: 'P95', timeWindowMinutes: 10080 }),
     ]);
     expect(groups.size).toBe(2);
     const tiers = [...groups.values()]
@@ -104,7 +104,6 @@ describe('evaluationIntervalMinutes', () => {
     expect(evaluationIntervalMinutes(1440)).toBe(15);
     expect(evaluationIntervalMinutes(1441)).toBe(30);
     expect(evaluationIntervalMinutes(10080)).toBe(30);
-    expect(evaluationIntervalMinutes(43200)).toBe(30);
   });
 });
 
@@ -125,7 +124,6 @@ describe('resolutionFor', () => {
     expect(resolutionFor(DAILY_THRESHOLD_MINUTES, true)).toBe('daily');
     // allowDailyRollup=false (a TRAFFIC group) never reaches daily.
     expect(resolutionFor(DAILY_THRESHOLD_MINUTES, false)).toBe('hourly');
-    expect(resolutionFor(43200, false)).toBe('hourly');
   });
 });
 
@@ -136,13 +134,13 @@ describe('isRuleDue', () => {
   const ago = (minutes: number) => new Date(evalTime.getTime() - minutes * 60_000).toISOString();
 
   test('a never-evaluated rule is always due', () => {
-    expect(isRuleDue(makeRule({ lastEvaluatedAt: null, timeWindowMinutes: 43200 }), evalTime)).toBe(
+    expect(isRuleDue(makeRule({ lastEvaluatedAt: null, timeWindowMinutes: 10080 }), evalTime)).toBe(
       true,
     );
   });
 
-  test('30-day rule: due once its 30-min interval has elapsed', () => {
-    const base = { timeWindowMinutes: 43200, state: 'NORMAL' as const };
+  test('7-day rule: due once its 30-min interval has elapsed', () => {
+    const base = { timeWindowMinutes: 10080, state: 'NORMAL' as const };
     expect(isRuleDue(makeRule({ ...base, lastEvaluatedAt: ago(0) }), evalTime)).toBe(false);
     expect(isRuleDue(makeRule({ ...base, lastEvaluatedAt: ago(29) }), evalTime)).toBe(false);
     expect(isRuleDue(makeRule({ ...base, lastEvaluatedAt: ago(30) }), evalTime)).toBe(true);
@@ -153,7 +151,7 @@ describe('isRuleDue', () => {
     const almost = new Date(evalTime.getTime() - (30 * 60_000 - 5_000)).toISOString();
     expect(
       isRuleDue(
-        makeRule({ timeWindowMinutes: 43200, state: 'NORMAL', lastEvaluatedAt: almost }),
+        makeRule({ timeWindowMinutes: 10080, state: 'NORMAL', lastEvaluatedAt: almost }),
         evalTime,
       ),
     ).toBe(false);
@@ -168,13 +166,13 @@ describe('isRuleDue', () => {
   test('PENDING/RECOVERING keep full 1-min resolution regardless of window', () => {
     for (const state of ['PENDING', 'RECOVERING'] as const) {
       expect(
-        isRuleDue(makeRule({ timeWindowMinutes: 43200, state, lastEvaluatedAt: ago(1) }), evalTime),
+        isRuleDue(makeRule({ timeWindowMinutes: 10080, state, lastEvaluatedAt: ago(1) }), evalTime),
       ).toBe(true);
     }
-    // The same 30-day rule in a steady state, 1 min after eval, is NOT due.
+    // The same 7-day rule in a steady state, 1 min after eval, is NOT due.
     for (const state of ['NORMAL', 'FIRING'] as const) {
       expect(
-        isRuleDue(makeRule({ timeWindowMinutes: 43200, state, lastEvaluatedAt: ago(1) }), evalTime),
+        isRuleDue(makeRule({ timeWindowMinutes: 10080, state, lastEvaluatedAt: ago(1) }), evalTime),
       ).toBe(false);
     }
   });
@@ -337,7 +335,7 @@ describe('queryClickHouseWindows', () => {
       { clientFilters: [{ name: 'web', versions: null }] },
       makeLogger().logger,
     );
-    await queryClickHouseWindows(clickhouse, target, 43200, conds, evalTime);
+    await queryClickHouseWindows(clickhouse, target, DAILY_THRESHOLD_MINUTES, conds, evalTime);
     const { sql } = calls[0];
     expect(sql).toContain('FROM operations_daily');
     expect(sql).not.toContain('_by_target');

packages/services/workflows/src/lib/metric-alert-evaluator.ts

@@ -252,7 +252,7 @@ export function evaluationIntervalMinutes(timeWindowMinutes: number): number {
   if (timeWindowMinutes <= 60) return 1; // ≤ 1h window: every tick (unchanged)
   if (timeWindowMinutes <= 360) return 5; // ≤ 6h window: every 5 min
   if (timeWindowMinutes <= 1440) return 15; // ≤ 24h window: every 15 min
-  return 30; // > 24h (7d, 30d): every 30 min
+  return 30; // > 24h (up to the 7d max): every 30 min
 }
 
 // PENDING/RECOVERING rules stay at 1-min resolution so the confirmationMinutes

packages/web/app/src/components/target/alerts/alert-form.tsx

@@ -224,8 +224,6 @@ const RANGE_OPTIONS = [
   { value: '360', label: '6h' },
   { value: '1440', label: '1d' },
   { value: '10080', label: '7d' },
-  { value: '20160', label: '14d' },
-  { value: '43200', label: '30d' },
 ] as const;
 
 // Condition labels depend on threshold type. For a fixed value the metric is
@@ -583,7 +581,7 @@ export function AlertForm(props: AlertFormProps) {
 
   const previewWindowMinutes = Math.min(
     (parseInt(watchedValues.timeWindowMinutes, 10) || 10_080) * 2,
-    43_200,
+    20_160,
   );
   const { period, resolution } = useMemo(() => {
     const now = new Date();

packages/web/app/src/components/target/alerts/alert-metric-chart.tsx

@@ -81,10 +81,12 @@ const SEVERITY_COLOR_KEY: Record<string, 'critical' | 'warning' | 'info'> = {
   INFO: 'info',
 };
 
-// The preview fetch span is capped at 30 days (see `previewWindowMinutes` in
-// alert-form.tsx), so for windows longer than 15 days the "previous" window
-// falls outside the fetched data and can't be drawn or compared.
-const PREVIEW_SPAN_CAP_MINUTES = 43_200;
+// The preview fetch span is capped at 14 days (see `previewWindowMinutes` in
+// alert-form.tsx), so for windows longer than 7 days the "previous" window
+// falls outside the fetched data and can't be drawn or compared. With the rule
+// window itself capped at 7d, this always holds, but the guard stays as a
+// backstop against a larger `timeWindowMinutes` reaching the chart.
+const PREVIEW_SPAN_CAP_MINUTES = 20_160;
 
 const MS_PER_MINUTE = 60_000;