From ddb2fc1ae65ca20cc6fe048265811ac75cadac17 Mon Sep 17 00:00:00 2001 From: Domas Date: Fri, 14 May 2021 13:41:13 +0300 Subject: [PATCH] Alerting: fix maching loki prom rules to rule rules, rename "Inactive" to "Normal" (#34111) --- .../alerting/unified/RuleList.test.tsx | 12 +++--- .../components/rules/AlertStateTag.tsx | 5 ++- ...teSection.tsx => RuleListStateSection.tsx} | 4 +- .../components/rules/RuleListStateView.tsx | 2 +- .../unified/components/rules/RulesFilter.tsx | 6 ++- .../unified/components/rules/RulesTable.tsx | 12 +++++- .../hooks/useCombinedRuleNamespaces.ts | 37 ++++++++++++++----- .../features/alerting/unified/utils/rules.ts | 10 +++++ 8 files changed, 66 insertions(+), 22 deletions(-) rename public/app/features/alerting/unified/components/rules/{RuleListSateSection.tsx => RuleListStateSection.tsx} (91%) diff --git a/public/app/features/alerting/unified/RuleList.test.tsx b/public/app/features/alerting/unified/RuleList.test.tsx index 5c4f50f78d9..78fa54a6d32 100644 --- a/public/app/features/alerting/unified/RuleList.test.tsx +++ b/public/app/features/alerting/unified/RuleList.test.tsx @@ -244,16 +244,16 @@ describe('RuleList', () => { let ruleRows = table.querySelectorAll(':scope > tbody > tr'); expect(ruleRows).toHaveLength(4); - expect(ruleRows[0]).toHaveTextContent('n/a'); + expect(ruleRows[0]).toHaveTextContent('Recording rule'); expect(ruleRows[0]).toHaveTextContent('recordingrule'); - expect(ruleRows[1]).toHaveTextContent('firing'); + expect(ruleRows[1]).toHaveTextContent('Firing'); expect(ruleRows[1]).toHaveTextContent('alertingrule'); - expect(ruleRows[2]).toHaveTextContent('pending'); + expect(ruleRows[2]).toHaveTextContent('Pending'); expect(ruleRows[2]).toHaveTextContent('p-rule'); - expect(ruleRows[3]).toHaveTextContent('inactive'); + expect(ruleRows[3]).toHaveTextContent('Normal'); expect(ruleRows[3]).toHaveTextContent('i-rule'); expect(byText('Labels').query()).not.toBeInTheDocument(); @@ -277,8 +277,8 @@ describe('RuleList', () => { let instanceRows = instancesTable?.querySelectorAll(':scope > tbody > tr'); expect(instanceRows).toHaveLength(2); - expect(instanceRows![0]).toHaveTextContent('firingfoo=barseverity=warning2021-03-18 13:47:05'); - expect(instanceRows![1]).toHaveTextContent('firingfoo=bazseverity=error2021-03-18 13:47:05'); + expect(instanceRows![0]).toHaveTextContent('Firingfoo=barseverity=warning2021-03-18 13:47:05'); + expect(instanceRows![1]).toHaveTextContent('Firingfoo=bazseverity=error2021-03-18 13:47:05'); // expand details of an instance userEvent.click(ui.alertCollapseToggle.get(instanceRows![0])); diff --git a/public/app/features/alerting/unified/components/rules/AlertStateTag.tsx b/public/app/features/alerting/unified/components/rules/AlertStateTag.tsx index 5b465b19188..216ae1b92fa 100644 --- a/public/app/features/alerting/unified/components/rules/AlertStateTag.tsx +++ b/public/app/features/alerting/unified/components/rules/AlertStateTag.tsx @@ -1,5 +1,6 @@ import { GrafanaAlertState, PromAlertingRuleState } from 'app/types/unified-alerting-dto'; import React, { FC } from 'react'; +import { alertStateToReadable } from '../../utils/rules'; import { State, StateTag } from '../StateTag'; const alertStateToState: Record = { @@ -17,4 +18,6 @@ interface Props { state: PromAlertingRuleState | GrafanaAlertState; } -export const AlertStateTag: FC = ({ state }) => {state}; +export const AlertStateTag: FC = ({ state }) => ( + {alertStateToReadable(state)} +); diff --git a/public/app/features/alerting/unified/components/rules/RuleListSateSection.tsx b/public/app/features/alerting/unified/components/rules/RuleListStateSection.tsx similarity index 91% rename from public/app/features/alerting/unified/components/rules/RuleListSateSection.tsx rename to public/app/features/alerting/unified/components/rules/RuleListStateSection.tsx index ea5799d6e87..e2a63dc27f8 100644 --- a/public/app/features/alerting/unified/components/rules/RuleListSateSection.tsx +++ b/public/app/features/alerting/unified/components/rules/RuleListStateSection.tsx @@ -3,8 +3,8 @@ import { GrafanaTheme } from '@grafana/data'; import { useStyles } from '@grafana/ui'; import { CombinedRule } from 'app/types/unified-alerting'; import { PromAlertingRuleState } from 'app/types/unified-alerting-dto'; -import { capitalize } from 'lodash'; import React, { FC, useState } from 'react'; +import { alertStateToReadable } from '../../utils/rules'; import { CollapseToggle } from '../CollapseToggle'; import { RulesTable } from './RulesTable'; @@ -26,7 +26,7 @@ export const RuleListStateSection: FC = ({ rules, state, defaultCollapsed isCollapsed={collapsed} onToggle={() => setCollapsed(!collapsed)} /> - {capitalize(state)} ({rules.length}) + {alertStateToReadable(state)} ({rules.length}) {!collapsed && } diff --git a/public/app/features/alerting/unified/components/rules/RuleListStateView.tsx b/public/app/features/alerting/unified/components/rules/RuleListStateView.tsx index 9a682aae4ad..a512c57b7f9 100644 --- a/public/app/features/alerting/unified/components/rules/RuleListStateView.tsx +++ b/public/app/features/alerting/unified/components/rules/RuleListStateView.tsx @@ -4,7 +4,7 @@ import { PromAlertingRuleState } from 'app/types/unified-alerting-dto'; import React, { FC, useMemo } from 'react'; import { getFiltersFromUrlParams } from '../../utils/misc'; import { isAlertingRule } from '../../utils/rules'; -import { RuleListStateSection } from './RuleListSateSection'; +import { RuleListStateSection } from './RuleListStateSection'; interface Props { namespaces: CombinedRuleNamespace[]; diff --git a/public/app/features/alerting/unified/components/rules/RulesFilter.tsx b/public/app/features/alerting/unified/components/rules/RulesFilter.tsx index fac44ccc10f..2c8939b217c 100644 --- a/public/app/features/alerting/unified/components/rules/RulesFilter.tsx +++ b/public/app/features/alerting/unified/components/rules/RulesFilter.tsx @@ -8,6 +8,7 @@ import { PromAlertingRuleState } from 'app/types/unified-alerting-dto'; import { useQueryParams } from 'app/core/hooks/useQueryParams'; import { getFiltersFromUrlParams } from '../../utils/misc'; import { DataSourcePicker } from '@grafana/runtime'; +import { alertStateToReadable } from '../../utils/rules'; const RulesFilter = () => { const [queryParams, setQueryParams] = useQueryParams(); @@ -19,7 +20,10 @@ const RulesFilter = () => { const { dataSource, alertState, queryString } = getFiltersFromUrlParams(queryParams); const styles = useStyles(getStyles); - const stateOptions = Object.entries(PromAlertingRuleState).map(([key, value]) => ({ label: key, value })); + const stateOptions = Object.entries(PromAlertingRuleState).map(([key, value]) => ({ + label: alertStateToReadable(value), + value, + })); const handleDataSourceChange = (dataSourceValue: DataSourceInstanceSettings) => { setQueryParams({ dataSource: dataSourceValue.name }); diff --git a/public/app/features/alerting/unified/components/rules/RulesTable.tsx b/public/app/features/alerting/unified/components/rules/RulesTable.tsx index 8f89c103fec..be39b592859 100644 --- a/public/app/features/alerting/unified/components/rules/RulesTable.tsx +++ b/public/app/features/alerting/unified/components/rules/RulesTable.tsx @@ -1,7 +1,7 @@ import { GrafanaTheme2 } from '@grafana/data'; import { ConfirmModal, useStyles2 } from '@grafana/ui'; import React, { FC, Fragment, useState } from 'react'; -import { getRuleIdentifier, isAlertingRule, stringifyRuleIdentifier } from '../../utils/rules'; +import { getRuleIdentifier, isAlertingRule, isRecordingRule, stringifyRuleIdentifier } from '../../utils/rules'; import { CollapseToggle } from '../CollapseToggle'; import { css, cx } from '@emotion/css'; import { RuleDetails } from './RuleDetails'; @@ -126,7 +126,15 @@ export const RulesTable: FC = ({ data-testid="rule-collapse-toggle" /> - {promRule && isAlertingRule(promRule) ? : 'n/a'} + + {promRule && isAlertingRule(promRule) ? ( + + ) : promRule && isRecordingRule(promRule) ? ( + 'Recording rule' + ) : ( + 'n/a' + )} + {rule.name} {showGroupColumn && ( {isCloudRulesSource(rulesSource) ? `${namespace.name} > ${group.name}` : namespace.name} diff --git a/public/app/features/alerting/unified/hooks/useCombinedRuleNamespaces.ts b/public/app/features/alerting/unified/hooks/useCombinedRuleNamespaces.ts index d5819d15032..7d965b9ff77 100644 --- a/public/app/features/alerting/unified/hooks/useCombinedRuleNamespaces.ts +++ b/public/app/features/alerting/unified/hooks/useCombinedRuleNamespaces.ts @@ -164,23 +164,42 @@ function rulerRuleToCombinedRule( }; } +// find existing rule in group that matches the given prom rule function getExistingRuleInGroup( rule: Rule, group: CombinedRuleGroup, rulesSource: RulesSource ): CombinedRule | undefined { - return isGrafanaRulesSource(rulesSource) - ? group!.rules.find((existingRule) => existingRule.name === rule.name) // assume grafana groups have only the one rule. check name anyway because paranoid - : group!.rules.find((existingRule) => { - return !existingRule.promRule && isCombinedRuleEqualToPromRule(existingRule, rule); - }); + if (isGrafanaRulesSource(rulesSource)) { + // assume grafana groups have only the one rule. check name anyway because paranoid + return group!.rules.find((existingRule) => existingRule.name === rule.name); + } + return ( + // try finding a rule that matches name, labels, annotations and query + group!.rules.find( + (existingRule) => !existingRule.promRule && isCombinedRuleEqualToPromRule(existingRule, rule, true) + ) ?? + // if that fails, try finding a rule that only matches name, labels and annotations. + // loki & prom can sometimes modify the query so it doesnt match, eg `2 > 1` becomes `1` + group!.rules.find( + (existingRule) => !existingRule.promRule && isCombinedRuleEqualToPromRule(existingRule, rule, false) + ) + ); } -function isCombinedRuleEqualToPromRule(combinedRule: CombinedRule, rule: Rule): boolean { +function isCombinedRuleEqualToPromRule(combinedRule: CombinedRule, rule: Rule, checkQuery = true): boolean { if (combinedRule.name === rule.name) { return ( - JSON.stringify([hashQuery(combinedRule.query), combinedRule.labels, combinedRule.annotations]) === - JSON.stringify([hashQuery(rule.query), rule.labels || {}, isAlertingRule(rule) ? rule.annotations || {} : {}]) + JSON.stringify([ + checkQuery ? hashQuery(combinedRule.query) : '', + combinedRule.labels, + combinedRule.annotations, + ]) === + JSON.stringify([ + checkQuery ? hashQuery(rule.query) : '', + rule.labels || {}, + isAlertingRule(rule) ? rule.annotations || {} : {}, + ]) ); } return false; @@ -194,6 +213,6 @@ function hashQuery(query: string) { } // whitespace could be added or removed query = query.replace(/\s|\n/g, ''); - // labels matchers can be reordered, so sort the enitre string, esentially comparing just hte character counts + // labels matchers can be reordered, so sort the enitre string, esentially comparing just the character counts return query.split('').sort().join(''); } diff --git a/public/app/features/alerting/unified/utils/rules.ts b/public/app/features/alerting/unified/utils/rules.ts index 45c5e66069f..1eae1f31a09 100644 --- a/public/app/features/alerting/unified/utils/rules.ts +++ b/public/app/features/alerting/unified/utils/rules.ts @@ -1,6 +1,8 @@ import { Annotations, + GrafanaAlertState, Labels, + PromAlertingRuleState, PromRuleType, RulerAlertingRuleDTO, RulerGrafanaRuleDTO, @@ -20,6 +22,7 @@ import { import { AsyncRequestState } from './redux'; import { RULER_NOT_SUPPORTED_MSG } from './constants'; import { hash } from './misc'; +import { capitalize } from 'lodash'; export function isAlertingRule(rule: Rule): rule is AlertingRule { return rule.type === PromRuleType.Alerting; @@ -132,3 +135,10 @@ export function ruleWithLocationToRuleIdentifier(ruleWithLocation: RuleWithLocat ruleWithLocation.rule ); } + +export function alertStateToReadable(state: PromAlertingRuleState | GrafanaAlertState): string { + if (state === PromAlertingRuleState.Inactive) { + return 'Normal'; + } + return capitalize(state); +}