From 0507b3564f8681af0ddf5fbcd8dbd70d6e17fe38 Mon Sep 17 00:00:00 2001 From: Konrad Lalik Date: Fri, 22 Jul 2022 12:52:56 +0200 Subject: [PATCH] Alerting: Fix alert panel instance-based rules filtering (#52583) --- .betterer.results | 6 ------ .../app/features/alerting/state/alertDef.ts | 9 ++++++++- .../panel/alertlist/AlertInstances.tsx | 2 ++ .../panel/alertlist/UnifiedAlertList.tsx | 19 ++++++++++++++++-- .../unified-alerting/GroupedView.tsx | 20 +++++++++++++++---- .../unified-alerting/UngroupedView.tsx | 4 ++-- 6 files changed, 45 insertions(+), 15 deletions(-) diff --git a/.betterer.results b/.betterer.results index d84e88a2d19..bbbb097736c 100644 --- a/.betterer.results +++ b/.betterer.results @@ -8645,12 +8645,6 @@ exports[`better eslint`] = { [0, 0, 0, "Unexpected any. Specify a different type.", "1"], [0, 0, 0, "Unexpected any. Specify a different type.", "2"] ], - "public/app/plugins/panel/alertlist/UnifiedAlertList.tsx:5381": [ - [0, 0, 0, "Do not use any type assertions.", "0"] - ], - "public/app/plugins/panel/alertlist/unified-alerting/UngroupedView.tsx:5381": [ - [0, 0, 0, "Do not use any type assertions.", "0"] - ], "public/app/plugins/panel/annolist/AnnoListPanel.test.tsx:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"], [0, 0, 0, "Unexpected any. Specify a different type.", "1"], diff --git a/public/app/features/alerting/state/alertDef.ts b/public/app/features/alerting/state/alertDef.ts index 41f0d3e9660..69004b28310 100644 --- a/public/app/features/alerting/state/alertDef.ts +++ b/public/app/features/alerting/state/alertDef.ts @@ -1,5 +1,6 @@ import { isArray, reduce } from 'lodash'; +import { IconName } from '@grafana/ui'; import { QueryPartDef, QueryPart } from 'app/features/alerting/state/query_part'; const alertQueryDef = new QueryPartDef({ @@ -87,7 +88,13 @@ function normalizeAlertState(state: string) { return state.toLowerCase().replace(/_/g, '').split(' ')[0]; } -function getStateDisplayModel(state: string) { +interface AlertStateDisplayModel { + text: string; + iconClass: IconName; + stateClass: string; +} + +function getStateDisplayModel(state: string): AlertStateDisplayModel { const normalizedState = normalizeAlertState(state); switch (normalizedState) { diff --git a/public/app/plugins/panel/alertlist/AlertInstances.tsx b/public/app/plugins/panel/alertlist/AlertInstances.tsx index b3638af561a..e5f8680040d 100644 --- a/public/app/plugins/panel/alertlist/AlertInstances.tsx +++ b/public/app/plugins/panel/alertlist/AlertInstances.tsx @@ -29,6 +29,8 @@ export const AlertInstances: FC = ({ alerts, options }) => { setDisplayInstances((display) => !display); }, []); + // TODO Filtering instances here has some implications + // If a rule has 0 instances after filtering there is no way not to show that rule const filteredAlerts = useMemo( (): Alert[] => filterAlerts(options, sortAlerts(options.sortOrder, alerts)) ?? [], [alerts, options] diff --git a/public/app/plugins/panel/alertlist/UnifiedAlertList.tsx b/public/app/plugins/panel/alertlist/UnifiedAlertList.tsx index 53ae44a13a9..5f8d9f49c33 100644 --- a/public/app/plugins/panel/alertlist/UnifiedAlertList.tsx +++ b/public/app/plugins/panel/alertlist/UnifiedAlertList.tsx @@ -25,6 +25,7 @@ import { PromAlertingRuleState } from 'app/types/unified-alerting-dto'; import { GroupMode, SortOrder, UnifiedAlertListOptions } from './types'; import GroupedModeView from './unified-alerting/GroupedView'; import UngroupedModeView from './unified-alerting/UngroupedView'; +import { filterAlerts } from './util'; export function UnifiedAlertList(props: PanelProps) { const dispatch = useDispatch(); @@ -143,14 +144,15 @@ function filterRules(props: PanelProps, rules: PromRule const replacedLabelFilter = replaceVariables(options.alertInstanceLabelFilter); const matchers = parseMatchers(replacedLabelFilter); // Reduce rules and instances to only those that match - filteredRules = filteredRules.reduce((rules, rule) => { + filteredRules = filteredRules.reduce((rules, rule) => { const filteredAlerts = (rule.rule.alerts ?? []).filter(({ labels }) => labelsMatchMatchers(labels, matchers)); if (filteredAlerts.length) { rules.push({ ...rule, rule: { ...rule.rule, alerts: filteredAlerts } }); } return rules; - }, [] as PromRuleWithLocation[]); + }, []); } + if (options.folder) { filteredRules = filteredRules.filter((rule) => { return rule.namespaceName === options.folder.title; @@ -166,6 +168,19 @@ function filterRules(props: PanelProps, rules: PromRule ); } + // Remove rules having 0 instances + // AlertInstances filters instances and we need to prevent situation + // when we display a rule with 0 instances + filteredRules = filteredRules.reduce((rules, rule) => { + const filteredAlerts = filterAlerts(options, rule.rule.alerts ?? []); + if (filteredAlerts.length) { + // We intentionally don't set alerts to filteredAlerts + // because later we couldn't display that some alerts are hidden (ref AlertInstances filtering) + rules.push(rule); + } + return rules; + }, []); + return filteredRules; } diff --git a/public/app/plugins/panel/alertlist/unified-alerting/GroupedView.tsx b/public/app/plugins/panel/alertlist/unified-alerting/GroupedView.tsx index 2f3106fd3d0..ab96cb715b0 100644 --- a/public/app/plugins/panel/alertlist/unified-alerting/GroupedView.tsx +++ b/public/app/plugins/panel/alertlist/unified-alerting/GroupedView.tsx @@ -2,11 +2,12 @@ import React, { FC, useMemo } from 'react'; import { useStyles2 } from '@grafana/ui'; import { AlertLabel } from 'app/features/alerting/unified/components/AlertLabel'; -import { PromRuleWithLocation } from 'app/types/unified-alerting'; +import { Alert, PromRuleWithLocation } from 'app/types/unified-alerting'; import { AlertInstances } from '../AlertInstances'; import { getStyles } from '../UnifiedAlertList'; import { GroupedRules, UnifiedAlertListOptions } from '../types'; +import { filterAlerts } from '../util'; type GroupedModeProps = { rules: PromRuleWithLocation[]; @@ -19,7 +20,7 @@ const GroupedModeView: FC = ({ rules, options }) => { const groupBy = options.groupBy; const groupedRules = useMemo(() => { - const groupedRules = new Map(); + const groupedRules = new Map(); const hasInstancesWithMatchingLabels = (rule: PromRuleWithLocation) => groupBy ? alertHasEveryLabel(rule, groupBy) : true; @@ -33,8 +34,19 @@ const GroupedModeView: FC = ({ rules, options }) => { }); }); - return groupedRules; - }, [groupBy, rules]); + // Remove groups having no instances + // This is different from filtering Rules without instances that we do in UnifiedAlertList + const filteredGroupedRules = Array.from(groupedRules.entries()).reduce((acc, [groupKey, groupAlerts]) => { + const filteredAlerts = filterAlerts(options, groupAlerts); + if (filteredAlerts.length > 0) { + acc.set(groupKey, filteredAlerts); + } + + return acc; + }, new Map()); + + return filteredGroupedRules; + }, [groupBy, rules, options]); return ( <> diff --git a/public/app/plugins/panel/alertlist/unified-alerting/UngroupedView.tsx b/public/app/plugins/panel/alertlist/unified-alerting/UngroupedView.tsx index 08d8a328b03..2aa297d1656 100644 --- a/public/app/plugins/panel/alertlist/unified-alerting/UngroupedView.tsx +++ b/public/app/plugins/panel/alertlist/unified-alerting/UngroupedView.tsx @@ -2,7 +2,7 @@ import { css } from '@emotion/css'; import React, { FC } from 'react'; import { GrafanaTheme2, intervalToAbbreviatedDurationString } from '@grafana/data'; -import { Icon, IconName, useStyles2 } from '@grafana/ui'; +import { Icon, useStyles2 } from '@grafana/ui'; import alertDef from 'app/features/alerting/state/alertDef'; import { alertStateToReadable, alertStateToState, getFirstActiveAt } from 'app/features/alerting/unified/utils/rules'; import { PromRuleWithLocation } from 'app/types/unified-alerting'; @@ -33,7 +33,7 @@ const UngroupedModeView: FC = ({ rules, options }) => {