From 7ccdea6f6778eb29ccd92b0bf88f953ab3e90355 Mon Sep 17 00:00:00 2001 From: Gilles De Mey Date: Thu, 30 Nov 2023 16:13:05 +0100 Subject: [PATCH] Alerting: Fixes combination of multiple predicates for rule search (#78910) --- .../unified/hooks/useFilteredRules.test.ts | 34 +++++++++ .../unified/hooks/useFilteredRules.ts | 74 +++++++++++++------ 2 files changed, 85 insertions(+), 23 deletions(-) diff --git a/public/app/features/alerting/unified/hooks/useFilteredRules.test.ts b/public/app/features/alerting/unified/hooks/useFilteredRules.test.ts index d0e7a36f9df..4c249840916 100644 --- a/public/app/features/alerting/unified/hooks/useFilteredRules.test.ts +++ b/public/app/features/alerting/unified/hooks/useFilteredRules.test.ts @@ -166,6 +166,40 @@ describe('filterRules', function () { expect(filtered[0].groups[0].rules[0].name).toBe('Memory too low'); }); + it('should be able to combine multiple predicates with AND', () => { + const rules = [ + mockCombinedRule({ + name: 'Memory too low', + labels: { team: 'operations', region: 'EMEA' }, + promRule: mockPromAlertingRule({ + health: RuleHealth.Ok, + }), + }), + mockCombinedRule({ + name: 'Memory too low', + labels: { team: 'operations', region: 'NASA' }, + promRule: mockPromAlertingRule({ + health: RuleHealth.Ok, + }), + }), + ]; + + const ns = mockCombinedRuleNamespace({ + groups: [mockCombinedRuleGroup('Resources usage group', rules)], + }); + + const filtered = filterRules( + [ns], + getFilter({ + ruleHealth: RuleHealth.Ok, + labels: ['team=operations', 'region=EMEA'], + }) + ); + + expect(filtered[0]?.groups[0]?.rules).toHaveLength(1); + expect(filtered[0]?.groups[0]?.rules[0]?.name).toBe('Memory too low'); + }); + // Typos there are deliberate to test the fuzzy search it.each(['nasa', 'alrt rul', 'nasa ruls'])('should filter out rules by namespace = "%s"', (namespaceFilter) => { const cpuRule = mockCombinedRule({ name: 'High CPU usage' }); diff --git a/public/app/features/alerting/unified/hooks/useFilteredRules.ts b/public/app/features/alerting/unified/hooks/useFilteredRules.ts index 2d98221faf9..2b8ef7722d5 100644 --- a/public/app/features/alerting/unified/hooks/useFilteredRules.ts +++ b/public/app/features/alerting/unified/hooks/useFilteredRules.ts @@ -1,6 +1,6 @@ import uFuzzy from '@leeoniya/ufuzzy'; import { produce } from 'immer'; -import { compact, isEmpty } from 'lodash'; +import { chain, compact, isEmpty } from 'lodash'; import { useCallback, useEffect, useMemo } from 'react'; import { getDataSourceSrv } from '@grafana/runtime'; @@ -124,6 +124,7 @@ export const filterRules = ( } const namespaceFilter = filterState.namespace; + if (namespaceFilter) { const namespaceHaystack = filteredNamespaces.map((ns) => ns.name); @@ -185,45 +186,72 @@ const reduceGroups = (filterState: RulesFilter) => { } filteredRules = filteredRules.filter((rule) => { - if (filterState.ruleType && filterState.ruleType !== rule.promRule?.type) { - return false; + const promRuleDefition = rule.promRule; + + // this will track what properties we're checking predicates for + // all predicates must be "true" to include the rule in the result set + // (this will result in an AND operation for our matchers) + const matchesFilterFor = chain(filterState) + // ⚠️ keep this list of properties we filter for here up-to-date ⚠️ + // We are ignoring predicates we've matched before we get here (like "freeFormWords") + .pick(['ruleType', 'dataSourceNames', 'ruleHealth', 'labels', 'ruleState']) + .omitBy(isEmpty) + .mapValues(() => false) + .value(); + + if ('ruleType' in matchesFilterFor && filterState.ruleType === promRuleDefition?.type) { + matchesFilterFor.ruleType = true; } - const doesNotQueryDs = isGrafanaRulerRule(rule.rulerRule) && !isQueryingDataSource(rule.rulerRule, filterState); - if (filterState.dataSourceNames?.length && doesNotQueryDs) { - return false; + if ('dataSourceNames' in matchesFilterFor) { + const doesNotQueryDs = isGrafanaRulerRule(rule.rulerRule) && isQueryingDataSource(rule.rulerRule, filterState); + + if (doesNotQueryDs) { + matchesFilterFor.dataSourceNames = true; + } } - if (filterState.ruleHealth && rule.promRule) { - const ruleHealth = getRuleHealth(rule.promRule.health); - return filterState.ruleHealth === ruleHealth; + if ('ruleHealth' in filterState && promRuleDefition) { + const ruleHealth = getRuleHealth(promRuleDefition.health); + const match = filterState.ruleHealth === ruleHealth; + + if (match) { + matchesFilterFor.ruleHealth = true; + } } // Query strings can match alert name, label keys, and label values - if (filterState.labels.length > 0) { - // const matchers = parseMatchers(filters.queryString); + if ('labels' in matchesFilterFor) { const matchers = compact(filterState.labels.map(looseParseMatcher)); + // check if the label we query for exists in _either_ the rule definition or in any of its alerts const doRuleLabelsMatchQuery = matchers.length > 0 && labelsMatchMatchers(rule.labels, matchers); const doAlertsContainMatchingLabels = matchers.length > 0 && - rule.promRule && - rule.promRule.type === PromRuleType.Alerting && - rule.promRule.alerts && - rule.promRule.alerts.some((alert) => labelsMatchMatchers(alert.labels, matchers)); + promRuleDefition && + promRuleDefition.type === PromRuleType.Alerting && + promRuleDefition.alerts && + promRuleDefition.alerts.some((alert) => labelsMatchMatchers(alert.labels, matchers)); - if (!(doRuleLabelsMatchQuery || doAlertsContainMatchingLabels)) { - return false; + if (doRuleLabelsMatchQuery || doAlertsContainMatchingLabels) { + matchesFilterFor.labels = true; } } - if ( - filterState.ruleState && - !(rule.promRule && isAlertingRule(rule.promRule) && rule.promRule.state === filterState.ruleState) - ) { - return false; + + if ('ruleState' in matchesFilterFor) { + const promRule = rule.promRule; + const hasPromRuleDefinition = promRule && isAlertingRule(promRule); + + const ruleStateMatches = hasPromRuleDefinition && promRule.state === filterState.ruleState; + + if (ruleStateMatches) { + matchesFilterFor.ruleState = true; + } } - return true; + + return Object.values(matchesFilterFor).every((match) => match === true); }); + // Add rules to the group that match the rule list filters if (filteredRules.length) { groupAcc.push({