Alerting: Fix evaluation interval validation (#56115)

Co-authored-by: Gilles De Mey <gilles.de.mey@gmail.com>

public/app/features/alerting/unified/components/rule-editor/GrafanaEvaluationBehavior.tsx

@@ -2,16 +2,12 @@ import { css } from '@emotion/css';
 import React, { FC, useState } from 'react';
 import { RegisterOptions, useFormContext } from 'react-hook-form';
 
-import { durationToMilliseconds, GrafanaTheme2, parseDuration } from '@grafana/data';
+import { GrafanaTheme2 } from '@grafana/data';
 import { Field, InlineLabel, Input, InputControl, useStyles2 } from '@grafana/ui';
 
 import { RuleFormValues } from '../../types/rule-form';
 import { checkEvaluationIntervalGlobalLimit } from '../../utils/config';
-import {
-  durationValidationPattern,
-  parseDurationToMilliseconds,
-  positiveDurationValidationPattern,
-} from '../../utils/time';
+import { parsePrometheusDuration } from '../../utils/time';
 import { CollapseToggle } from '../CollapseToggle';
 import { EvaluationIntervalLimitExceeded } from '../InvalidIntervalWarning';
 
@@ -20,43 +16,62 @@ import { RuleEditorSection } from './RuleEditorSection';
 
 const MIN_TIME_RANGE_STEP_S = 10; // 10 seconds
 
-const forValidationOptions = (evaluateEvery: string): RegisterOptions => ({
+export const forValidationOptions = (evaluateEvery: string): RegisterOptions => ({
   required: {
     value: true,
     message: 'Required.',
   },
-  pattern: durationValidationPattern,
-  validate: (value) => {
-    const millisFor = parseDurationToMilliseconds(value);
-    const millisEvery = parseDurationToMilliseconds(evaluateEvery);
-
-    // 0 is a special value meaning for equals evaluation interval
-    if (millisFor === 0) {
+  validate: (value: string) => {
+    // parsePrometheusDuration does not allow 0 but does allow 0s
+    if (value === '0') {
       return true;
     }
 
-    return millisFor >= millisEvery ? true : 'For must be greater than or equal to evaluate every.';
+    try {
+      const millisFor = parsePrometheusDuration(value);
+
+      // 0 is a special value meaning for equals evaluation interval
+      if (millisFor === 0) {
+        return true;
+      }
+
+      try {
+        const millisEvery = parsePrometheusDuration(evaluateEvery);
+        return millisFor >= millisEvery
+          ? true
+          : 'For duration must be greater than or equal to the evaluation interval.';
+      } catch (err) {
+        // if we fail to parse "every", assume validation is successful, or the error messages
+        // will overlap in the UI
+        return true;
+      }
+    } catch (error) {
+      return error instanceof Error ? error.message : 'Failed to parse duration';
+    }
   },
 });
 
-const evaluateEveryValidationOptions: RegisterOptions = {
+export const evaluateEveryValidationOptions: RegisterOptions = {
   required: {
     value: true,
     message: 'Required.',
   },
-  pattern: positiveDurationValidationPattern,
   validate: (value: string) => {
-    const duration = parseDuration(value);
-    if (Object.keys(duration).length) {
-      const diff = durationToMilliseconds(duration);
-      if (diff < MIN_TIME_RANGE_STEP_S * 1000) {
+    try {
+      const duration = parsePrometheusDuration(value);
+
+      if (duration < MIN_TIME_RANGE_STEP_S * 1000) {
         return `Cannot be less than ${MIN_TIME_RANGE_STEP_S} seconds.`;
       }
-      if (diff % (MIN_TIME_RANGE_STEP_S * 1000) !== 0) {
+
+      if (duration % (MIN_TIME_RANGE_STEP_S * 1000) !== 0) {
         return `Must be a multiple of ${MIN_TIME_RANGE_STEP_S} seconds.`;
       }
+
+      return true;
+    } catch (error) {
+      return error instanceof Error ? error.message : 'Failed to parse duration';
     }
-    return true;
   },
 };
 

public/app/features/alerting/unified/components/rules/EditRuleGroupModal.tsx

@@ -1,7 +1,6 @@
 import { css } from '@emotion/css';
 import React, { useEffect, useMemo } from 'react';
 
-import { isValidGoDuration } from '@grafana/data';
 import { Modal, Button, Form, Field, Input, useStyles2 } from '@grafana/ui';
 import { useCleanup } from 'app/core/hooks/useCleanup';
 import { useDispatch } from 'app/types';
@@ -12,8 +11,8 @@ import { updateLotexNamespaceAndGroupAction } from '../../state/actions';
 import { checkEvaluationIntervalGlobalLimit } from '../../utils/config';
 import { getRulesSourceName } from '../../utils/datasource';
 import { initialAsyncRequestState } from '../../utils/redux';
-import { durationValidationPattern } from '../../utils/time';
 import { EvaluationIntervalLimitExceeded } from '../InvalidIntervalWarning';
+import { evaluateEveryValidationOptions } from '../rule-editor/GrafanaEvaluationBehavior';
 
 interface ModalProps {
   namespace: CombinedRuleNamespace;
@@ -100,22 +99,7 @@ export function EditCloudGroupModal(props: ModalProps): React.ReactElement {
               <Input
                 id="groupInterval"
                 placeholder="1m"
-                {...register('groupInterval', {
-                  pattern: durationValidationPattern,
-                  validate: (input) => {
-                    const validDuration = isValidGoDuration(input);
-                    if (!validDuration) {
-                      return 'Invalid duration. Valid example: 1m (Available units: h, m, s)';
-                    }
-
-                    const limitExceeded = !checkEvaluationIntervalGlobalLimit(input).exceedsLimit;
-                    if (limitExceeded) {
-                      return true;
-                    }
-
-                    return false;
-                  },
-                })}
+                {...register('groupInterval', evaluateEveryValidationOptions)}
               />
             </Field>
             {checkEvaluationIntervalGlobalLimit(watch('groupInterval')).exceedsLimit && (

public/app/features/alerting/unified/components/rules/RuleConfigStatus.tsx

@@ -1,5 +1,5 @@
 import { css } from '@emotion/css';
-import React from 'react';
+import React, { useMemo } from 'react';
 
 import { GrafanaTheme2 } from '@grafana/data/src';
 import { config } from '@grafana/runtime/src';
@@ -15,7 +15,10 @@ interface RuleConfigStatusProps {
 export function RuleConfigStatus({ rule }: RuleConfigStatusProps) {
   const styles = useStyles2(getStyles);
 
-  const { exceedsLimit } = checkEvaluationIntervalGlobalLimit(rule.group.interval);
+  const { exceedsLimit } = useMemo(
+    () => checkEvaluationIntervalGlobalLimit(rule.group.interval),
+    [rule.group.interval]
+  );
 
   if (!exceedsLimit) {
     return null;

public/app/features/alerting/unified/utils/config.test.ts

@@ -0,0 +1,53 @@
+import { config } from '@grafana/runtime';
+
+import { checkEvaluationIntervalGlobalLimit } from './config';
+
+describe('checkEvaluationIntervalGlobalLimit', () => {
+  it('should NOT exceed limit if evaluate every is not valid duration', () => {
+    config.unifiedAlerting.minInterval = '2m30s';
+
+    const { globalLimit, exceedsLimit } = checkEvaluationIntervalGlobalLimit('123notvalidduration');
+
+    expect(globalLimit).toBe(150 * 1000);
+    expect(exceedsLimit).toBe(false);
+  });
+
+  it('should NOT exceed limit if config minInterval is not valid duration', () => {
+    config.unifiedAlerting.minInterval = '1A8IU3A';
+
+    const { globalLimit, exceedsLimit } = checkEvaluationIntervalGlobalLimit('1m30s');
+
+    expect(globalLimit).toBe(0);
+    expect(exceedsLimit).toBe(false);
+  });
+
+  it.each([
+    ['2m30s', '1m30s'],
+    ['30s', '10s'],
+    ['1d2h', '2h'],
+    ['1y', '90d'],
+  ])(
+    'should exceed limit if config minInterval (%s) is greater than evaluate every (%s)',
+    (minInterval, evaluateEvery) => {
+      config.unifiedAlerting.minInterval = minInterval;
+
+      const { globalLimit, exceedsLimit } = checkEvaluationIntervalGlobalLimit(evaluateEvery);
+
+      expect(globalLimit).toBeGreaterThan(0);
+      expect(exceedsLimit).toBe(true);
+    }
+  );
+
+  it.each([
+    ['1m30s', '2m30s'],
+    ['30s', '1d'],
+    ['1m10s', '1h30m15s'],
+  ])('should NOT exceed limit if config minInterval is lesser than evaluate every', (minInterval, evaluateEvery) => {
+    config.unifiedAlerting.minInterval = minInterval;
+
+    const { globalLimit, exceedsLimit } = checkEvaluationIntervalGlobalLimit(evaluateEvery);
+
+    expect(globalLimit).toBeGreaterThan(0);
+    expect(exceedsLimit).toBe(false);
+  });
+});

public/app/features/alerting/unified/utils/config.ts

@@ -1,21 +1,26 @@
-import { DataSourceInstanceSettings, DataSourceJsonData, isValidGoDuration, rangeUtil } from '@grafana/data';
+import { DataSourceInstanceSettings, DataSourceJsonData } from '@grafana/data';
 import { config } from '@grafana/runtime';
 
+import { isValidPrometheusDuration, parsePrometheusDuration } from './time';
+
 export function getAllDataSources(): Array<DataSourceInstanceSettings<DataSourceJsonData>> {
   return Object.values(config.datasources);
 }
 
 export function checkEvaluationIntervalGlobalLimit(alertGroupEvaluateEvery?: string) {
-  if (!alertGroupEvaluateEvery || !isValidGoDuration(alertGroupEvaluateEvery)) {
+  // config.unifiedAlerting.minInterval should be Prometheus-compatible duration
+  // However, Go's gtime library has issues with parsing y,w,d
+  if (!isValidPrometheusDuration(config.unifiedAlerting.minInterval)) {
     return { globalLimit: 0, exceedsLimit: false };
   }
 
-  if (!isValidGoDuration(config.unifiedAlerting.minInterval)) {
-    return { globalLimit: 0, exceedsLimit: false };
+  const evaluateEveryGlobalLimitMs = parsePrometheusDuration(config.unifiedAlerting.minInterval);
+
+  if (!alertGroupEvaluateEvery || !isValidPrometheusDuration(alertGroupEvaluateEvery)) {
+    return { globalLimit: evaluateEveryGlobalLimitMs, exceedsLimit: false };
   }
 
-  const evaluateEveryMs = rangeUtil.intervalToMs(alertGroupEvaluateEvery);
-  const evaluateEveryGlobalLimitMs = rangeUtil.intervalToMs(config.unifiedAlerting.minInterval);
+  const evaluateEveryMs = parsePrometheusDuration(alertGroupEvaluateEvery);
 
   const exceedsLimit = evaluateEveryGlobalLimitMs > evaluateEveryMs && evaluateEveryMs > 0;
 

public/app/features/alerting/unified/utils/time.test.ts

@@ -0,0 +1,15 @@
+import { isValidPrometheusDuration } from './time';
+
+describe('isValidPrometheusDuration', () => {
+  const validDurations = ['20h30m10s45ms', '1m30s', '20s4h', '90s', '10s', '20h20h', '2d4h20m'];
+
+  it.each(validDurations)('%s should be valid', (duration) => {
+    expect(isValidPrometheusDuration(duration)).toBe(true);
+  });
+
+  const invalidDurations = ['20h 30m 10s 45ms', '10Y', 'sample text', 'm'];
+
+  it.each(invalidDurations)('%s should NOT be valid', (duration) => {
+    expect(isValidPrometheusDuration(duration)).toBe(false);
+  });
+});

public/app/features/alerting/unified/utils/time.ts

@@ -3,6 +3,13 @@ import { describeInterval } from '@grafana/data/src/datetime/rangeutil';
 
 import { TimeOptions } from '../types/time';
 
+/**
+ * ⚠️
+ * Some of these functions might be confusing, but there is a significant difference between "Golang duration",
+ * supported by the time.ParseDuration() function and "prometheus duration" which is similar but does not support anything
+ * smaller than seconds and adds the following supported units: "d, w, y"
+ */
+
 export function parseInterval(value: string): [number, string] {
   const match = value.match(/(\d+)(\w+)/);
   if (match) {
@@ -21,22 +28,74 @@ export const timeOptions = Object.entries(TimeOptions).map(([key, value]) => ({
   value: value,
 }));
 
-// 1h, 10m and such
-export const positiveDurationValidationPattern = {
-  value: new RegExp(`^\\d+(${Object.values(TimeOptions).join('|')})$`),
-  message: `Must be of format "(number)(unit)" , for example "1m". Available units: ${Object.values(TimeOptions).join(
-    ', '
-  )}`,
+export function parseDurationToMilliseconds(duration: string) {
+  return durationToMilliseconds(parseDuration(duration));
+}
+
+export function isValidPrometheusDuration(duration: string): boolean {
+  try {
+    parsePrometheusDuration(duration);
+    return true;
+  } catch (err) {
+    return false;
+  }
+}
+
+const PROMETHEUS_SUFFIX_MULTIPLIER: Record<string, number> = {
+  ms: 1,
+  s: 1000,
+  m: 60 * 1000,
+  h: 60 * 60 * 1000,
+  d: 24 * 60 * 60 * 1000,
+  w: 7 * 24 * 60 * 60 * 1000,
+  y: 365 * 24 * 60 * 60 * 1000,
 };
 
-// 1h, 10m or 0 (without units)
-export const durationValidationPattern = {
-  value: new RegExp(`^\\d+(${Object.values(TimeOptions).join('|')})|0$`),
-  message: `Must be of format "(number)(unit)", for example "1m", or just "0". Available units: ${Object.values(
+const DURATION_REGEXP = new RegExp(/^(?:(?<value>\d+)(?<type>ms|s|m|h|d|w|y))|0$/);
+const INVALID_FORMAT = new Error(
+  `Must be of format "(number)(unit)", for example "1m", or just "0". Available units: ${Object.values(
     TimeOptions
-  ).join(', ')}`,
-};
+  ).join(', ')}`
+);
 
-export function parseDurationToMilliseconds(duration: string) {
-  return durationToMilliseconds(parseDuration(duration));
+/**
+ * According to https://prometheus.io/docs/alerting/latest/configuration/#configuration-file
+ * see <duration>
+ *
+ * @returns Duration in milliseconds
+ */
+export function parsePrometheusDuration(duration: string): number {
+  let input = duration;
+  let parts: Array<[number, string]> = [];
+
+  function matchDuration(part: string) {
+    const match = DURATION_REGEXP.exec(part);
+    const hasValueAndType = match?.groups?.value && match?.groups?.type;
+
+    if (!match || !hasValueAndType) {
+      throw INVALID_FORMAT;
+    }
+
+    if (match && match.groups?.value && match.groups?.type) {
+      input = input.replace(match[0], '');
+      parts.push([Number(match.groups.value), match.groups.type]);
+    }
+
+    if (input) {
+      matchDuration(input);
+    }
+  }
+
+  matchDuration(duration);
+
+  if (!parts.length) {
+    throw INVALID_FORMAT;
+  }
+
+  const totalDuration = parts.reduce((acc, [value, type]) => {
+    const duration = value * PROMETHEUS_SUFFIX_MULTIPLIER[type];
+    return acc + duration;
+  }, 0);
+
+  return totalDuration;
 }