Alerting: Improving group modal with validation on evaluation interval (#57830)

* Show rules list for the group with the For duration, and add validation for keeping all rules in the same group with a valid For

* Sort rules by For duration

* Add number evaluations column in alert list

* Add Error badge in column #evaluations in case of invalid For

* Add test for getNumberEvaluationsToStartAlerting method

* Move re-usable new InfoIcon component into a separate file in unified components folder

* Add edge case for getNumberEvaluationsToStartAlerting method, and change some namings

public/app/features/alerting/unified/RuleList.test.tsx

@@ -4,11 +4,12 @@ import userEvent from '@testing-library/user-event';
 import React from 'react';
 import { Provider } from 'react-redux';
 import { Router } from 'react-router-dom';
-import { byLabelText, byRole, byTestId, byText } from 'testing-library-selector';
+import { byRole, byTestId, byText } from 'testing-library-selector';
 
 import { locationService, setDataSourceSrv, logInfo } from '@grafana/runtime';
 import { contextSrv } from 'app/core/services/context_srv';
 import * as ruleActionButtons from 'app/features/alerting/unified/components/rules/RuleActionsButtons';
+import * as actions from 'app/features/alerting/unified/state/actions';
 import { configureStore } from 'app/store/configureStore';
 import { AccessControlAction } from 'app/types';
 import { PromAlertingRuleState, PromApplication } from 'app/types/unified-alerting-dto';
@@ -57,9 +58,11 @@ jest.mock('@grafana/runtime', () => {
 });
 
 jest.spyOn(config, 'getAllDataSources');
+jest.spyOn(actions, 'rulesInSameGroupHaveInvalidFor').mockReturnValue([]);
 
 const mocks = {
   getAllDataSourcesMock: jest.mocked(config.getAllDataSources),
+  rulesInSameGroupHaveInvalidForMock: jest.mocked(actions.rulesInSameGroupHaveInvalidFor),
 
   api: {
     discoverFeatures: jest.mocked(discoverFeatures),
@@ -121,9 +124,11 @@ const ui = {
   newRuleButton: byRole('link', { name: 'New alert rule' }),
 
   editGroupModal: {
-    namespaceInput: byLabelText('Namespace'),
-    ruleGroupInput: byLabelText('Rule group'),
-    intervalInput: byLabelText('Rule group evaluation interval'),
+    namespaceInput: byRole('textbox', { hidden: true, name: /namespace/i }),
+    ruleGroupInput: byRole('textbox', { name: 'Evaluation group', exact: true }),
+    intervalInput: byRole('textbox', {
+      name: /Rule group evaluation interval Evaluation interval should be smaller or equal to 'For' values for existing rules in this group./i,
+    }),
     saveButton: byRole('button', { name: /Save changes/ }),
   },
 };
@@ -131,6 +136,7 @@ const ui = {
 describe('RuleList', () => {
   beforeEach(() => {
     contextSrv.isEditor = true;
+    mocks.rulesInSameGroupHaveInvalidForMock.mockReturnValue([]);
   });
 
   afterEach(() => {
@@ -553,9 +559,12 @@ describe('RuleList', () => {
 
         // open edit dialog
         await userEvent.click(ui.editCloudGroupIcon.get(groups[0]));
-
-        expect(ui.editGroupModal.namespaceInput.get()).toHaveValue('namespace1');
-        expect(ui.editGroupModal.ruleGroupInput.get()).toHaveValue('group1');
+        await expect(screen.getByRole('textbox', { hidden: true, name: /namespace/i })).toHaveDisplayValue(
+          'namespace1'
+        );
+        await expect(screen.getByRole('textbox', { name: 'Evaluation group', exact: true })).toHaveDisplayValue(
+          'group1'
+        );
         await fn();
       });
     }
@@ -603,9 +612,14 @@ describe('RuleList', () => {
 
     testCase('rename just the lotex group', async () => {
       // make changes to form
-      await userEvent.clear(ui.editGroupModal.ruleGroupInput.get());
-      await userEvent.type(ui.editGroupModal.ruleGroupInput.get(), 'super group');
-      await userEvent.type(ui.editGroupModal.intervalInput.get(), '5m');
+      await userEvent.clear(screen.getByRole('textbox', { name: 'Evaluation group', exact: true }));
+      await userEvent.type(screen.getByRole('textbox', { name: 'Evaluation group', exact: true }), 'super group');
+      await userEvent.type(
+        screen.getByRole('textbox', {
+          name: /rule group evaluation interval evaluation interval should be smaller or equal to 'for' values for existing rules in this group\./i,
+        }),
+        '5m'
+      );
 
       // submit, check that appropriate calls were made
       await userEvent.click(ui.editGroupModal.saveButton.get());

public/app/features/alerting/unified/components/InfoIcon.tsx

@@ -0,0 +1,11 @@
+import React from 'react';
+
+import { Icon, Tooltip } from '@grafana/ui';
+
+export function InfoIcon({ text }: { text: string }) {
+  return (
+    <Tooltip placement="top" content={<div>{text}</div>}>
+      <Icon name="info-circle" size="xs" />
+    </Tooltip>
+  );
+}

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

@@ -14,7 +14,7 @@ import { EvaluationIntervalLimitExceeded } from '../InvalidIntervalWarning';
 import { GrafanaAlertStatePicker } from './GrafanaAlertStatePicker';
 import { RuleEditorSection } from './RuleEditorSection';
 
-const MIN_TIME_RANGE_STEP_S = 10; // 10 seconds
+export const MIN_TIME_RANGE_STEP_S = 10; // 10 seconds
 
 export const forValidationOptions = (evaluateEvery: string): RegisterOptions => ({
   required: {

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

@@ -1,18 +1,190 @@
 import { css } from '@emotion/css';
 import React, { useEffect, useMemo } from 'react';
+import { FormProvider, RegisterOptions, useForm, useFormContext } from 'react-hook-form';
 
-import { Modal, Button, Form, Field, Input, useStyles2 } from '@grafana/ui';
+import { GrafanaTheme2 } from '@grafana/data';
+import { Stack } from '@grafana/experimental';
+import { Modal, Button, Field, Input, useStyles2, Label, Badge } from '@grafana/ui';
+import { useAppNotification } from 'app/core/copy/appNotification';
 import { useCleanup } from 'app/core/hooks/useCleanup';
 import { useDispatch } from 'app/types';
 import { CombinedRuleGroup, CombinedRuleNamespace } from 'app/types/unified-alerting';
+import { RulerRulesConfigDTO, RulerRuleGroupDTO, RulerRuleDTO } from 'app/types/unified-alerting-dto';
 
 import { useUnifiedAlertingSelector } from '../../hooks/useUnifiedAlertingSelector';
-import { updateLotexNamespaceAndGroupAction } from '../../state/actions';
+import { rulesInSameGroupHaveInvalidFor, updateLotexNamespaceAndGroupAction } from '../../state/actions';
 import { checkEvaluationIntervalGlobalLimit } from '../../utils/config';
 import { getRulesSourceName } from '../../utils/datasource';
 import { initialAsyncRequestState } from '../../utils/redux';
+import { isAlertingRulerRule, isGrafanaRulerRule } from '../../utils/rules';
+import { parsePrometheusDuration } from '../../utils/time';
+import { DynamicTable, DynamicTableColumnProps, DynamicTableItemProps } from '../DynamicTable';
+import { InfoIcon } from '../InfoIcon';
 import { EvaluationIntervalLimitExceeded } from '../InvalidIntervalWarning';
-import { evaluateEveryValidationOptions } from '../rule-editor/GrafanaEvaluationBehavior';
+import { MIN_TIME_RANGE_STEP_S } from '../rule-editor/GrafanaEvaluationBehavior';
+
+const MINUTE = '1m';
+interface AlertInfo {
+  alertName: string;
+  forDuration: string;
+  evaluationsToFire: number;
+}
+function ForError({ message }: { message: string }) {
+  return <Badge color="orange" icon="exclamation-triangle" text={'Error'} tooltip={message} />;
+}
+
+export const getNumberEvaluationsToStartAlerting = (forDuration: string, currentEvaluation: string) => {
+  const evalNumberMs = safeParseDurationstr(currentEvaluation);
+  const forNumber = safeParseDurationstr(forDuration);
+  if (forNumber === 0 && evalNumberMs !== 0) {
+    return 1;
+  }
+  if (evalNumberMs === 0) {
+    return 0;
+  } else {
+    const evaluationsBeforeCeil = forNumber / evalNumberMs;
+    return evaluationsBeforeCeil < 1 ? 0 : Math.ceil(forNumber / evalNumberMs) + 1;
+  }
+};
+
+export const getAlertInfo = (alert: RulerRuleDTO, currentEvaluation: string): AlertInfo => {
+  const emptyAlert: AlertInfo = {
+    alertName: '',
+    forDuration: '0s',
+    evaluationsToFire: 0,
+  };
+  if (isGrafanaRulerRule(alert)) {
+    return {
+      alertName: alert.grafana_alert.title,
+      forDuration: alert.for,
+      evaluationsToFire: getNumberEvaluationsToStartAlerting(alert.for, currentEvaluation),
+    };
+  }
+  if (isAlertingRulerRule(alert)) {
+    return {
+      alertName: alert.alert,
+      forDuration: alert.for ?? '1m',
+      evaluationsToFire: getNumberEvaluationsToStartAlerting(alert.for ?? '1m', currentEvaluation),
+    };
+  }
+  return emptyAlert;
+};
+export const isValidEvaluation = (evaluation: string) => {
+  try {
+    const duration = parsePrometheusDuration(evaluation);
+
+    if (duration < MIN_TIME_RANGE_STEP_S * 1000) {
+      return false;
+    }
+
+    if (duration % (MIN_TIME_RANGE_STEP_S * 1000) !== 0) {
+      return false;
+    }
+
+    return true;
+  } catch (error) {
+    return false;
+  }
+};
+
+export const getGroupFromRuler = (
+  rulerRules: RulerRulesConfigDTO | null | undefined,
+  groupName: string,
+  folderName: string
+) => {
+  const folderObj: Array<RulerRuleGroupDTO<RulerRuleDTO>> = rulerRules ? rulerRules[folderName] : [];
+  return folderObj?.find((rulerRuleGroup) => rulerRuleGroup.name === groupName);
+};
+
+export const getIntervalForGroup = (
+  rulerRules: RulerRulesConfigDTO | null | undefined,
+  groupName: string,
+  folderName: string
+) => {
+  const group = getGroupFromRuler(rulerRules, groupName, folderName);
+  const interval = group?.interval ?? MINUTE;
+  return interval;
+};
+
+export const safeParseDurationstr = (duration: string): number => {
+  try {
+    return parsePrometheusDuration(duration);
+  } catch (e) {
+    return 0;
+  }
+};
+
+type AlertsWithForTableColumnProps = DynamicTableColumnProps<AlertInfo>;
+type AlertsWithForTableProps = DynamicTableItemProps<AlertInfo>;
+
+export const RulesForGroupTable = ({
+  rulerRules,
+  groupName,
+  folderName,
+}: {
+  rulerRules: RulerRulesConfigDTO | null | undefined;
+  groupName: string;
+  folderName: string;
+}) => {
+  const styles = useStyles2(getStyles);
+  const group = getGroupFromRuler(rulerRules, groupName, folderName);
+  const rules: RulerRuleDTO[] = group?.rules ?? [];
+
+  const { watch } = useFormContext<FormValues>();
+  const currentInterval = watch('groupInterval');
+
+  const rows: AlertsWithForTableProps[] = rules
+    .slice()
+    .map((rule: RulerRuleDTO, index) => ({
+      id: index,
+      data: getAlertInfo(rule, currentInterval),
+    }))
+    .sort(
+      (alert1, alert2) => safeParseDurationstr(alert1.data.forDuration) - safeParseDurationstr(alert2.data.forDuration)
+    );
+
+  const columns: AlertsWithForTableColumnProps[] = useMemo(() => {
+    return [
+      {
+        id: 'alertName',
+        label: 'Alert',
+        renderCell: ({ data: { alertName } }) => {
+          return <>{alertName}</>;
+        },
+        size: 0.6,
+      },
+      {
+        id: 'for',
+        label: 'For',
+        renderCell: ({ data: { forDuration } }) => {
+          return <>{forDuration}</>;
+        },
+        size: 0.2,
+      },
+      {
+        id: 'numberEvaluations',
+        label: '#Evaluations',
+        renderCell: ({ data: { evaluationsToFire: numberEvaluations } }) => {
+          if (!isValidEvaluation(currentInterval)) {
+            return <ForError message={'Invalid evaluation interval format'} />;
+          }
+          if (numberEvaluations === 0) {
+            return <ForError message="Invalid 'For' value: it should be greater or equal to evaluation interval." />;
+          } else {
+            return <>{numberEvaluations}</>;
+          }
+        },
+        size: 0.2,
+      },
+    ];
+  }, [currentInterval]);
+
+  return (
+    <div className={styles.tableWrapper}>
+      <DynamicTable items={rows} cols={columns} />
+    </div>
+  );
+};
 
 interface ModalProps {
   namespace: CombinedRuleNamespace;
@@ -32,6 +204,7 @@ export function EditCloudGroupModal(props: ModalProps): React.ReactElement {
   const dispatch = useDispatch();
   const { loading, error, dispatched } =
     useUnifiedAlertingSelector((state) => state.updateLotexNamespaceAndGroup) ?? initialAsyncRequestState;
+  const notifyApp = useAppNotification();
 
   const defaultValues = useMemo(
     (): FormValues => ({
@@ -64,18 +237,77 @@ export function EditCloudGroupModal(props: ModalProps): React.ReactElement {
     );
   };
 
+  const formAPI = useForm<FormValues>({
+    mode: 'onBlur',
+    defaultValues,
+    shouldFocusError: true,
+  });
+  const {
+    handleSubmit,
+    register,
+    watch,
+    formState: { isDirty, errors },
+  } = formAPI;
+
+  const onInvalid = () => {
+    notifyApp.error('There are errors in the form. Correct the errors and retry.');
+  };
+
+  const rulerRuleRequests = useUnifiedAlertingSelector((state) => state.rulerRules);
+  const groupfoldersForSource = rulerRuleRequests[getRulesSourceName(namespace.rulesSource)];
+
+  const evaluateEveryValidationOptions: RegisterOptions = {
+    required: {
+      value: true,
+      message: 'Required.',
+    },
+    validate: (value: string) => {
+      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 (duration % (MIN_TIME_RANGE_STEP_S * 1000) !== 0) {
+          return `Must be a multiple of ${MIN_TIME_RANGE_STEP_S} seconds.`;
+        }
+        if (
+          rulesInSameGroupHaveInvalidFor(groupfoldersForSource.result, group.name, namespace.name, value).length === 0
+        ) {
+          return true;
+        } else {
+          return `Invalid evaluation interval. Evaluation interval should be smaller or equal to 'For' values for existing rules in this group.`;
+        }
+      } catch (error) {
+        return error instanceof Error ? error.message : 'Failed to parse duration';
+      }
+    },
+  };
+
   return (
     <Modal
       className={styles.modal}
       isOpen={true}
-      title="Edit namespace or rule group"
+      title="Edit namespace or evaluation group"
       onDismiss={onClose}
       onClickBackdrop={onClose}
     >
-      <Form defaultValues={defaultValues} onSubmit={onSubmit} key={JSON.stringify(defaultValues)}>
-        {({ register, errors, formState: { isDirty }, watch }) => (
+      <FormProvider {...formAPI}>
+        <form onSubmit={(e) => e.preventDefault()} key={JSON.stringify(defaultValues)}>
           <>
-            <Field label="Namespace" invalid={!!errors.namespaceName} error={errors.namespaceName?.message}>
+            <Field
+              label={
+                <Label htmlFor="namespaceName">
+                  <Stack gap={0.5}>
+                    NameSpace
+                    <InfoIcon text={'Name space can be updated'} />
+                  </Stack>
+                </Label>
+              }
+              invalid={!!errors.namespaceName}
+              error={errors.namespaceName?.message}
+            >
               <Input
                 id="namespaceName"
                 {...register('namespaceName', {
@@ -83,16 +315,37 @@ export function EditCloudGroupModal(props: ModalProps): React.ReactElement {
                 })}
               />
             </Field>
-            <Field label="Rule group" invalid={!!errors.groupName} error={errors.groupName?.message}>
+            <Field
+              label={
+                <Label htmlFor="groupName">
+                  <Stack gap={0.5}>
+                    Evaluation group
+                    <InfoIcon text={'Group name can be updated'} />
+                  </Stack>
+                </Label>
+              }
+              invalid={!!errors.groupName}
+              error={errors.groupName?.message}
+            >
               <Input
                 id="groupName"
                 {...register('groupName', {
-                  required: 'Rule group name is required.',
+                  required: 'Evaluation group name is required.',
                 })}
               />
             </Field>
             <Field
-              label="Rule group evaluation interval"
+              label={
+                <Label
+                  htmlFor="groupInterval"
+                  description="Evaluation interval should be smaller or equal to 'For' values for existing rules in this group."
+                >
+                  <Stack gap={0.5}>
+                    Rule group evaluation interval
+                    <InfoIcon text={'How frequently to evaluate rules.'} />
+                  </Stack>
+                </Label>
+              }
               invalid={!!errors.groupInterval}
               error={errors.groupInterval?.message}
             >
@@ -102,9 +355,23 @@ export function EditCloudGroupModal(props: ModalProps): React.ReactElement {
                 {...register('groupInterval', evaluateEveryValidationOptions)}
               />
             </Field>
+
             {checkEvaluationIntervalGlobalLimit(watch('groupInterval')).exceedsLimit && (
               <EvaluationIntervalLimitExceeded />
             )}
+            {rulerRuleRequests && (
+              <>
+                <div>List of rules that belong to this group</div>
+                <div className={styles.evalRequiredLabel}>
+                  #Evaluations column represents the number of evaluations needed before alert starts firing.
+                </div>
+                <RulesForGroupTable
+                  rulerRules={groupfoldersForSource?.result}
+                  groupName={group.name}
+                  folderName={namespace.name}
+                />
+              </>
+            )}
 
             <Modal.ButtonRow>
               <Button
@@ -116,19 +383,38 @@ export function EditCloudGroupModal(props: ModalProps): React.ReactElement {
               >
                 Close
               </Button>
-              <Button type="submit" disabled={!isDirty || loading}>
+              <Button
+                type="button"
+                disabled={!isDirty || loading}
+                onClick={handleSubmit((values) => onSubmit(values), onInvalid)}
+              >
                 {loading ? 'Saving...' : 'Save changes'}
               </Button>
             </Modal.ButtonRow>
           </>
-        )}
-      </Form>
+        </form>
+      </FormProvider>
     </Modal>
   );
 }
 
-const getStyles = () => ({
+const getStyles = (theme: GrafanaTheme2) => ({
   modal: css`
     max-width: 560px;
   `,
+  formInput: css`
+    width: 275px;
+    & + & {
+      margin-left: ${theme.spacing(3)};
+    }
+  `,
+  tableWrapper: css`
+    margin-top: ${theme.spacing(2)};
+    margin-bottom: ${theme.spacing(2)};
+    height: 225px;
+    overflow: auto;
+  `,
+  evalRequiredLabel: css`
+    font-size: ${theme.typography.bodySmall.fontSize};
+  `,
 });

public/app/features/alerting/unified/components/rules/getNumberEvaluationsToStartAlerting.test.ts

@@ -0,0 +1,17 @@
+import { getNumberEvaluationsToStartAlerting } from './EditRuleGroupModal';
+describe('getNumberEvaluationsToStartAlerting method', () => {
+  it('should return 0 in case of invalid data', () => {
+    expect(getNumberEvaluationsToStartAlerting('sd', 'ksdh')).toBe(0);
+    expect(getNumberEvaluationsToStartAlerting('0s', '1dfa0m')).toBe(0);
+  });
+  it('should return 1 in case of zero For and valid interval', () => {
+    expect(getNumberEvaluationsToStartAlerting('0s', '10m')).toBe(1);
+  });
+  it('should return correct number in case of valid data', () => {
+    expect(getNumberEvaluationsToStartAlerting('1m', '10m')).toBe(0);
+    expect(getNumberEvaluationsToStartAlerting('10m', '10m')).toBe(2);
+    expect(getNumberEvaluationsToStartAlerting('18m', '10m')).toBe(3);
+    expect(getNumberEvaluationsToStartAlerting('1h41m', '10m')).toBe(12);
+    expect(getNumberEvaluationsToStartAlerting('101m', '10m')).toBe(12);
+  });
+});

public/app/features/alerting/unified/state/actions.ts

@@ -1,4 +1,4 @@
-import { createAsyncThunk } from '@reduxjs/toolkit';
+import { createAsyncThunk, AsyncThunk } from '@reduxjs/toolkit';
 import { isEmpty } from 'lodash';
 
 import { locationService } from '@grafana/runtime';
@@ -60,6 +60,7 @@ import {
   FetchRulerRulesFilter,
   setRulerRuleGroup,
 } from '../api/ruler';
+import { getAlertInfo, safeParseDurationstr, getGroupFromRuler } from '../components/rules/EditRuleGroupModal';
 import { RuleFormType, RuleFormValues } from '../types/rule-form';
 import { addDefaultsToAlertmanagerConfig, removeMuteTimingFromRoute } from '../utils/alertmanager';
 import {
@@ -752,8 +753,28 @@ interface UpdateNamespaceAndGroupOptions {
   groupInterval?: string;
 }
 
+export const rulesInSameGroupHaveInvalidFor = (
+  rulerRules: RulerRulesConfigDTO | null | undefined,
+  groupName: string,
+  folderName: string,
+  everyDuration: string
+) => {
+  const group = getGroupFromRuler(rulerRules, groupName, folderName);
+
+  const rulesSameGroup: RulerRuleDTO[] = group?.rules ?? [];
+
+  return rulesSameGroup.filter((rule: RulerRuleDTO) => {
+    const { forDuration } = getAlertInfo(rule, everyDuration);
+    return safeParseDurationstr(forDuration) < safeParseDurationstr(everyDuration);
+  });
+};
+
 // allows renaming namespace, renaming group and changing group interval, all in one go
-export const updateLotexNamespaceAndGroupAction = createAsyncThunk(
+export const updateLotexNamespaceAndGroupAction: AsyncThunk<
+  void,
+  UpdateNamespaceAndGroupOptions,
+  { state: StoreState }
+> = createAsyncThunk<void, UpdateNamespaceAndGroupOptions, { state: StoreState }>(
   'unifiedalerting/updateLotexNamespaceAndGroup',
   async (options: UpdateNamespaceAndGroupOptions, thunkAPI): Promise<void> => {
     return withAppEvents(
@@ -790,8 +811,29 @@ export const updateLotexNamespaceAndGroupAction = createAsyncThunk(
           ) {
             throw new Error('Nothing changed.');
           }
-
+          // validation for new groupInterval
+          if (groupInterval !== existingGroup.interval) {
+            const storeState = thunkAPI.getState();
+            const groupfoldersForSource = storeState?.unifiedAlerting.rulerRules[rulesSourceName];
+            const notValidRules = rulesInSameGroupHaveInvalidFor(
+              groupfoldersForSource?.result,
+              groupName,
+              namespaceName,
+              groupInterval ?? '1m'
+            );
+            if (notValidRules.length > 0) {
+              throw new Error(
+                `These alerts belonging to this group will have an invalid 'For' value: ${notValidRules
+                  .map((rule) => {
+                    const { alertName } = getAlertInfo(rule, groupInterval ?? '');
+                    return alertName;
+                  })
+                  .join(',')}`
+              );
+            }
+          }
           // if renaming namespace - make new copies of all groups, then delete old namespace
+
           if (newNamespaceName !== namespaceName) {
             for (const group of rulesResult[namespaceName]) {
               await setRulerRuleGroup(