From 171cd60480459cbd7893c49e0d5839bb4dc4e50c Mon Sep 17 00:00:00 2001 From: Sonia Aguilar <33540275+soniaAguilarPeiron@users.noreply.github.com> Date: Thu, 15 Dec 2022 08:28:47 +0100 Subject: [PATCH] Alerting: Enable editing evaluation interval in alert form when creating a new group (#60083) * Enable editing evaluation interval in alert form when creating a new group * Disable group when no folder selected and focus on group when clicking add new * Improve group selector showing interval as a description for each option * Fix evaluate every input and label aligment * Fix columns width in EditRuleGroupModal rules table * Reduce amount of space in the evaluation behaviour section --- public/api-merged.json | 54 +++++++ .../unified/RuleEditorGrafanaRules.test.tsx | 2 +- .../components/rule-editor/AlertRuleForm.tsx | 4 +- .../components/rule-editor/FolderAndGroup.tsx | 37 +++-- .../rule-editor/GrafanaEvaluationBehavior.tsx | 137 +++++++++++++----- .../components/rule-editor/SelectWIthAdd.tsx | 11 +- .../components/rules/EditRuleGroupModal.tsx | 69 +++++---- .../alerting/unified/types/rule-form.ts | 1 + .../alerting/unified/utils/rule-form.ts | 3 + 9 files changed, 232 insertions(+), 86 deletions(-) diff --git a/public/api-merged.json b/public/api-merged.json index 469438c375e..482a1d14b0a 100644 --- a/public/api-merged.json +++ b/public/api-merged.json @@ -11297,6 +11297,60 @@ } } }, + "BacktestConfig": { + "type": "object", + "properties": { + "annotations": { + "type": "object", + "additionalProperties": { + "type": "string" + } + }, + "condition": { + "type": "string" + }, + "data": { + "type": "array", + "items": { + "$ref": "#/definitions/AlertQuery" + } + }, + "for": { + "$ref": "#/definitions/Duration" + }, + "from": { + "type": "string", + "format": "date-time" + }, + "interval": { + "$ref": "#/definitions/Duration" + }, + "labels": { + "type": "object", + "additionalProperties": { + "type": "string" + } + }, + "no_data_state": { + "type": "string", + "enum": [ + "Alerting", + "NoData", + "OK" + ] + }, + "title": { + "type": "string" + }, + "to": { + "type": "string", + "format": "date-time" + } + } + }, + "BacktestResult": { + "$ref": "#/definitions/Frame" + }, "BasicAuth": { "type": "object", "title": "BasicAuth contains basic HTTP authentication credentials.", diff --git a/public/app/features/alerting/unified/RuleEditorGrafanaRules.test.tsx b/public/app/features/alerting/unified/RuleEditorGrafanaRules.test.tsx index 83022dd54f8..0f3e8a1e23e 100644 --- a/public/app/features/alerting/unified/RuleEditorGrafanaRules.test.tsx +++ b/public/app/features/alerting/unified/RuleEditorGrafanaRules.test.tsx @@ -128,7 +128,7 @@ describe('RuleEditor grafana managed rules', () => { await clickSelectOption(folderInput, 'Folder A'); const groupInput = await ui.inputs.group.find(); await userEvent.click(byRole('combobox').get(groupInput)); - await clickSelectOption(groupInput, 'group1 (1m)'); + await clickSelectOption(groupInput, 'group1'); await userEvent.type(ui.inputs.annotationValue(1).get(), 'some description'); // TODO remove skipPointerEventsCheck once https://github.com/jsdom/jsdom/issues/3232 is fixed diff --git a/public/app/features/alerting/unified/components/rule-editor/AlertRuleForm.tsx b/public/app/features/alerting/unified/components/rule-editor/AlertRuleForm.tsx index 26e1dfec4bc..e250922e515 100644 --- a/public/app/features/alerting/unified/components/rule-editor/AlertRuleForm.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/AlertRuleForm.tsx @@ -1,5 +1,5 @@ import { css } from '@emotion/css'; -import React, { FC, useMemo, useState } from 'react'; +import React, { FC, useEffect, useMemo, useState } from 'react'; import { DeepMap, FieldError, FormProvider, useForm, useFormContext, UseFormWatch } from 'react-hook-form'; import { Link } from 'react-router-dom'; @@ -180,6 +180,8 @@ export const AlertRuleForm: FC = ({ existing, prefill }) => { }); } }; + const evaluateEveryInForm = watch('evaluateEvery'); + useEffect(() => setEvaluateEvery(evaluateEveryInForm), [evaluateEveryInForm]); return ( diff --git a/public/app/features/alerting/unified/components/rule-editor/FolderAndGroup.tsx b/public/app/features/alerting/unified/components/rule-editor/FolderAndGroup.tsx index ba471fe0534..adb5128b38e 100644 --- a/public/app/features/alerting/unified/components/rule-editor/FolderAndGroup.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/FolderAndGroup.tsx @@ -40,8 +40,16 @@ const useGetGroups = (groupfoldersForGrafana: RulerRulesConfigDTO | null | undef return groupOptions; }; -function mapGroupsToOptions(groups: string[]): Array> { - return groups.map((group) => ({ label: group, value: group })); +function mapGroupsToOptions( + groupsForFolder: RulerRulesConfigDTO | null | undefined, + groups: string[], + folderTitle: string +): Array> { + return groups.map((group) => ({ + label: group, + value: group, + description: `${getIntervalForGroup(groupsForFolder, group, folderTitle)}`, + })); } interface FolderAndGroupProps { initialFolder: RuleForm | null; @@ -52,11 +60,14 @@ export const useGetGroupOptionsFromFolder = (folderTitle: string) => { const groupfoldersForGrafana = rulerRuleRequests[GRAFANA_RULES_SOURCE_NAME]; - const groupOptions: Array> = mapGroupsToOptions( - useGetGroups(groupfoldersForGrafana?.result, folderTitle) - ); const groupsForFolder = groupfoldersForGrafana?.result; - return { groupOptions, groupsForFolder, loading: groupfoldersForGrafana?.loading }; + + const groupOptions: Array> = mapGroupsToOptions( + groupsForFolder, + useGetGroups(groupfoldersForGrafana?.result, folderTitle), + folderTitle + ); + return { groupOptions, loading: groupfoldersForGrafana?.loading }; }; const useRuleFolderFilter = (existingRuleForm: RuleForm | null) => { @@ -93,6 +104,7 @@ export function FolderAndGroup({ initialFolder }: FolderAndGroupProps) { formState: { errors }, watch, control, + setValue, } = useFormContext(); const styles = useStyles2(getStyles); @@ -105,7 +117,7 @@ export function FolderAndGroup({ initialFolder }: FolderAndGroupProps) { const [selectedGroup, setSelectedGroup] = useState(group); const initialRender = useRef(true); - const { groupOptions, groupsForFolder, loading } = useGetGroupOptionsFromFolder(folder?.title ?? ''); + const { groupOptions, loading } = useGetGroupOptionsFromFolder(folder?.title ?? ''); useEffect(() => setSelectedGroup(group), [group, setSelectedGroup]); @@ -120,6 +132,10 @@ export function FolderAndGroup({ initialFolder }: FolderAndGroupProps) { initialRender.current = false; }, [group, folder?.title]); + useEffect(() => { + setValue('group', selectedGroup); + }, [selectedGroup, setValue]); + const groupIsInGroupOptions = useCallback( (group_: string) => { return groupOptions.includes((groupInList: SelectableValue) => groupInList.label === group_); @@ -189,16 +205,15 @@ export function FolderAndGroup({ initialFolder }: FolderAndGroupProps) { ) : ( ) => - `${option.label} (${getIntervalForGroup(groupsForFolder, option.label ?? '', folder?.title ?? '')})` - } + getOptionLabel={(option: SelectableValue) => `${option.label}`} value={selectedGroup} custom={isAddingGroup} onCustomChange={(custom: boolean) => setIsAddingGroup(custom)} - placeholder="Evaluation group name" + placeholder={isAddingGroup ? 'New evaluation group name' : 'Evaluation group name'} onChange={(value: string) => { field.onChange(value); setSelectedGroup(value); diff --git a/public/app/features/alerting/unified/components/rule-editor/GrafanaEvaluationBehavior.tsx b/public/app/features/alerting/unified/components/rule-editor/GrafanaEvaluationBehavior.tsx index 2e93419ae59..710e584b09e 100644 --- a/public/app/features/alerting/unified/components/rule-editor/GrafanaEvaluationBehavior.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/GrafanaEvaluationBehavior.tsx @@ -4,7 +4,7 @@ import { RegisterOptions, useFormContext } from 'react-hook-form'; import { GrafanaTheme2, SelectableValue } from '@grafana/data'; import { Stack } from '@grafana/experimental'; -import { Button, Card, Field, InlineLabel, Input, InputControl, useStyles2 } from '@grafana/ui'; +import { Button, Field, InlineLabel, Input, InputControl, useStyles2 } from '@grafana/ui'; import { RulerRuleDTO, RulerRuleGroupDTO, RulerRulesConfigDTO } from 'app/types/unified-alerting-dto'; import { logInfo, LogMessages } from '../../Analytics'; @@ -13,7 +13,7 @@ import { RuleForm, RuleFormValues } from '../../types/rule-form'; import { GRAFANA_RULES_SOURCE_NAME } from '../../utils/datasource'; import { parsePrometheusDuration } from '../../utils/time'; import { CollapseToggle } from '../CollapseToggle'; -import { EditCloudGroupModal } from '../rules/EditRuleGroupModal'; +import { EditCloudGroupModal, evaluateEveryValidationOptions } from '../rules/EditRuleGroupModal'; import { MINUTE } from './AlertRuleForm'; import { FolderAndGroup, useGetGroupOptionsFromFolder } from './FolderAndGroup'; @@ -79,6 +79,49 @@ const useIsNewGroup = (folder: string, group: string) => { return !groupIsInGroupOptions(group); }; +export const EvaluateEveryNewGroup = ({ rules }: { rules: RulerRulesConfigDTO | null | undefined }) => { + const { + watch, + register, + formState: { errors }, + } = useFormContext(); + const styles = useStyles2(getStyles); + const evaluateEveryId = 'eval-every-input'; + return ( + +
+ + + Evaluate every + + + + + +
+ + ); +}; + function FolderGroupAndEvaluationInterval({ initialFolder, evaluateEvery, @@ -89,7 +132,7 @@ function FolderGroupAndEvaluationInterval({ setEvaluateEvery: (value: string) => void; }) { const styles = useStyles2(getStyles); - const { watch } = useFormContext(); + const { watch, setValue } = useFormContext(); const [isEditingGroup, setIsEditingGroup] = useState(false); const group = watch('group'); @@ -101,10 +144,15 @@ function FolderGroupAndEvaluationInterval({ const isNewGroup = useIsNewGroup(folder?.title ?? '', group); useEffect(() => { - group && - folder && - setEvaluateEvery(getIntervalForGroup(groupfoldersForGrafana?.result, group, folder?.title ?? '')); - }, [group, folder, groupfoldersForGrafana?.result, setEvaluateEvery]); + if (!isNewGroup) { + group && + folder && + setEvaluateEvery(getIntervalForGroup(groupfoldersForGrafana?.result, group, folder?.title ?? '')); + } else { + setEvaluateEvery(MINUTE); + setValue('evaluateEvery', MINUTE); + } + }, [group, folder, groupfoldersForGrafana?.result, setEvaluateEvery, isNewGroup, setValue]); const closeEditGroupModal = (saved = false) => { if (!saved) { @@ -130,43 +178,42 @@ function FolderGroupAndEvaluationInterval({ /> )} {folder && group && ( - - Evaluation behavior - - -
- {`Alert rules in the `} {group} group are evaluated every{' '} - {evaluateEvery}. -
- -
- {!isNewGroup && ( -
- {`Evaluation group interval applies to every rule within a group. It overwrites intervals defined for existing alert rules.`} -
+
+ +
+ {isNewGroup && group ? ( + + ) : ( + +
+ {`Alert rules in the `} {group} group are evaluated every{' '} + {evaluateEvery}. +
+ {!isNewGroup && ( +
+ {`Evaluation group interval applies to every rule within a group. It overwrites intervals defined for existing alert rules.`} +
+ )} + )} -
- - - +
- {isNewGroup && ( -
- {`To edit the evaluation group interval, save the alert rule.`} + {!isNewGroup && ( +
+
)} - - - + +
)}
); @@ -280,8 +327,11 @@ const getStyles = (theme: GrafanaTheme2) => ({ align-self: left; margin-right: ${theme.spacing(1)}; `, - cardContainer: css` + evaluationContainer: css` + background-color: ${theme.colors.background.secondary}; + padding: ${theme.spacing(2)}; max-width: ${theme.breakpoints.values.sm}px; + font-size: ${theme.typography.size.sm}; `, intervalChangedLabel: css` margin-bottom: ${theme.spacing(1)}; @@ -297,4 +347,11 @@ const getStyles = (theme: GrafanaTheme2) => ({ bold: css` font-weight: bold; `, + alignInterval: css` + margin-top: ${theme.spacing(1)}; + margin-left: -${theme.spacing(1)}; + `, + marginTop: css` + margin-top: ${theme.spacing(1)}; + `, }); diff --git a/public/app/features/alerting/unified/components/rule-editor/SelectWIthAdd.tsx b/public/app/features/alerting/unified/components/rule-editor/SelectWIthAdd.tsx index 8a05c83cc0a..9cb999d6903 100644 --- a/public/app/features/alerting/unified/components/rule-editor/SelectWIthAdd.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/SelectWIthAdd.tsx @@ -1,4 +1,4 @@ -import React, { FC, useEffect, useMemo, useState } from 'react'; +import React, { FC, useEffect, useMemo, useRef, useState } from 'react'; import { SelectableValue } from '@grafana/data'; import { Input, Select } from '@grafana/ui'; @@ -43,6 +43,14 @@ export const SelectWithAdd: FC = ({ [options, addLabel] ); + const inputRef = useRef(null); + + useEffect(() => { + if (inputRef.current && isCustom) { + inputRef.current.focus(); + } + }, [isCustom]); + if (isCustom) { return ( = ({ placeholder={placeholder} className={className} disabled={disabled} + ref={inputRef} onChange={(e) => onChange(e.currentTarget.value)} /> ); diff --git a/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.tsx b/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.tsx index 466ba2a0431..c6ef58599cb 100644 --- a/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.tsx +++ b/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.tsx @@ -151,7 +151,7 @@ export const RulesForGroupTable = ({ renderCell: ({ data: { alertName } }) => { return <>{alertName}; }, - size: 0.6, + size: '330px', }, { id: 'for', @@ -174,7 +174,7 @@ export const RulesForGroupTable = ({ return <>{numberEvaluations}; } }, - size: 0.2, + size: 0.6, }, ]; }, [currentInterval]); @@ -208,6 +208,37 @@ interface FormValues { groupInterval: string; } +export const evaluateEveryValidationOptions = ( + rules: RulerRulesConfigDTO | null | undefined, + groupName: string, + nameSpaceName: string +): 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(rules, groupName, nameSpaceName, 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'; + } + }, +}); + export function EditCloudGroupModal(props: ModalProps): React.ReactElement { const { nameSpaceAndGroup: { namespace, group }, @@ -285,35 +316,6 @@ export function EditCloudGroupModal(props: ModalProps): React.ReactElement { const rulerRuleRequests = useUnifiedAlertingSelector((state) => state.rulerRules); const groupfoldersForSource = rulerRuleRequests[sourceName]; - 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, groupName, nameSpaceName, 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 ( diff --git a/public/app/features/alerting/unified/types/rule-form.ts b/public/app/features/alerting/unified/types/rule-form.ts index b87fc0c2e5c..3a097ab778b 100644 --- a/public/app/features/alerting/unified/types/rule-form.ts +++ b/public/app/features/alerting/unified/types/rule-form.ts @@ -27,6 +27,7 @@ export interface RuleFormValues { noDataState: GrafanaAlertStateDecision; execErrState: GrafanaAlertStateDecision; folder: RuleForm | null; + evaluateEvery: string; evaluateFor: string; // cortex / loki rules diff --git a/public/app/features/alerting/unified/utils/rule-form.ts b/public/app/features/alerting/unified/utils/rule-form.ts index 1d62bf960cf..a55810ab18b 100644 --- a/public/app/features/alerting/unified/utils/rule-form.ts +++ b/public/app/features/alerting/unified/utils/rule-form.ts @@ -29,6 +29,7 @@ import { } from 'app/types/unified-alerting-dto'; import { EvalFunction } from '../../state/alertDef'; +import { MINUTE } from '../components/rule-editor/AlertRuleForm'; import { RuleFormType, RuleFormValues } from '../types/rule-form'; import { getRulesAccess } from './access-control'; @@ -60,6 +61,7 @@ export const getDefaultFormValues = (): RuleFormValues => { noDataState: GrafanaAlertStateDecision.NoData, execErrState: GrafanaAlertStateDecision.Error, evaluateFor: '5m', + evaluateEvery: MINUTE, // cortex / loki namespace: '', @@ -124,6 +126,7 @@ export function rulerRuleToFormValues(ruleWithLocation: RuleWithLocation): RuleF name: ga.title, type: RuleFormType.grafana, group: group.name, + evaluateEvery: group.interval || defaultFormValues.evaluateEvery, evaluateFor: rule.for || '0', noDataState: ga.no_data_state, execErrState: ga.exec_err_state,