From 1bb4a7299bd755c641f2ca361ea43ba8935dae15 Mon Sep 17 00:00:00 2001 From: "grafana-delivery-bot[bot]" <132647405+grafana-delivery-bot[bot]@users.noreply.github.com> Date: Fri, 22 Sep 2023 01:02:51 +0200 Subject: [PATCH] [v10.1.x] Alerting: Add support for `keep_firing_for` field from external rulers (#75257) --- .../api/tooling/definitions/cortex-ruler.go | 13 ++-- .../alerting/unified/types/rule-form.ts | 2 + .../__snapshots__/rule-form.test.ts.snap | 70 +++++++++++++++++++ .../alerting/unified/utils/rule-form.test.ts | 55 ++++++++++++++- .../alerting/unified/utils/rule-form.ts | 26 ++++++- public/app/types/unified-alerting-dto.ts | 1 + 6 files changed, 157 insertions(+), 10 deletions(-) diff --git a/pkg/services/ngalert/api/tooling/definitions/cortex-ruler.go b/pkg/services/ngalert/api/tooling/definitions/cortex-ruler.go index f83e2218ea2..80852707af5 100644 --- a/pkg/services/ngalert/api/tooling/definitions/cortex-ruler.go +++ b/pkg/services/ngalert/api/tooling/definitions/cortex-ruler.go @@ -258,12 +258,13 @@ func (c *GettableRuleGroupConfig) validate() error { } type ApiRuleNode struct { - Record string `yaml:"record,omitempty" json:"record,omitempty"` - Alert string `yaml:"alert,omitempty" json:"alert,omitempty"` - Expr string `yaml:"expr" json:"expr"` - For *model.Duration `yaml:"for,omitempty" json:"for,omitempty"` - Labels map[string]string `yaml:"labels,omitempty" json:"labels,omitempty"` - Annotations map[string]string `yaml:"annotations,omitempty" json:"annotations,omitempty"` + Record string `yaml:"record,omitempty" json:"record,omitempty"` + Alert string `yaml:"alert,omitempty" json:"alert,omitempty"` + Expr string `yaml:"expr" json:"expr"` + For *model.Duration `yaml:"for,omitempty" json:"for,omitempty"` + KeepFiringFor *model.Duration `yaml:"keep_firing_for,omitempty" json:"keep_firing_for,omitempty"` + Labels map[string]string `yaml:"labels,omitempty" json:"labels,omitempty"` + Annotations map[string]string `yaml:"annotations,omitempty" json:"annotations,omitempty"` } type RuleType int diff --git a/public/app/features/alerting/unified/types/rule-form.ts b/public/app/features/alerting/unified/types/rule-form.ts index aa445b1793d..ca9aa17d0d0 100644 --- a/public/app/features/alerting/unified/types/rule-form.ts +++ b/public/app/features/alerting/unified/types/rule-form.ts @@ -32,5 +32,7 @@ export interface RuleFormValues { namespace: string; forTime: number; forTimeUnit: string; + keepFiringForTime?: number; + keepFiringForTimeUnit?: string; expression: string; } diff --git a/public/app/features/alerting/unified/utils/__snapshots__/rule-form.test.ts.snap b/public/app/features/alerting/unified/utils/__snapshots__/rule-form.test.ts.snap index bfd1da8464e..cde06160d02 100644 --- a/public/app/features/alerting/unified/utils/__snapshots__/rule-form.test.ts.snap +++ b/public/app/features/alerting/unified/utils/__snapshots__/rule-form.test.ts.snap @@ -59,3 +59,73 @@ exports[`formValuesToRulerGrafanaRuleDTO should not save both instant and range }, } `; + +exports[`formValuesToRulerGrafanaRuleDTO should not set keep_firing_for if values are undefined 1`] = ` +{ + "alert": "", + "annotations": { + "description": "", + "runbook_url": "", + "summary": "", + }, + "expr": "", + "for": "1m", + "keep_firing_for": undefined, + "labels": { + "": "", + }, +} +`; + +exports[`formValuesToRulerGrafanaRuleDTO should parse keep_firing_for 1`] = ` +{ + "annotations": [], + "expression": "B", + "forTime": 1, + "forTimeUnit": "m", + "keepFiringForTime": 1, + "keepFiringForTimeUnit": "m", + "labels": [ + { + "key": "", + "value": "", + }, + ], + "name": "A", +} +`; + +exports[`formValuesToRulerGrafanaRuleDTO should set keep_firing_for if values are populated 1`] = ` +{ + "alert": "", + "annotations": { + "description": "", + "runbook_url": "", + "summary": "", + }, + "expr": "", + "for": "1m", + "keep_firing_for": "1m", + "labels": { + "": "", + }, +} +`; + +exports[`formValuesToRulerGrafanaRuleDTO should set keepFiringForTime and keepFiringForTimeUnit to undefined if keep_firing_for not set 1`] = ` +{ + "annotations": [], + "expression": "B", + "forTime": 1, + "forTimeUnit": "m", + "keepFiringForTime": undefined, + "keepFiringForTimeUnit": undefined, + "labels": [ + { + "key": "", + "value": "", + }, + ], + "name": "A", +} +`; diff --git a/public/app/features/alerting/unified/utils/rule-form.test.ts b/public/app/features/alerting/unified/utils/rule-form.test.ts index adeae48e409..5ac93c82d7e 100644 --- a/public/app/features/alerting/unified/utils/rule-form.test.ts +++ b/public/app/features/alerting/unified/utils/rule-form.test.ts @@ -1,8 +1,14 @@ import { PromQuery } from 'app/plugins/datasource/prometheus/types'; +import { RulerAlertingRuleDTO } from 'app/types/unified-alerting-dto'; -import { RuleFormValues } from '../types/rule-form'; +import { RuleFormType, RuleFormValues } from '../types/rule-form'; -import { formValuesToRulerGrafanaRuleDTO, getDefaultFormValues } from './rule-form'; +import { + alertingRulerRuleToRuleForm, + formValuesToRulerGrafanaRuleDTO, + formValuesToRulerRuleDTO, + getDefaultFormValues, +} from './rule-form'; describe('formValuesToRulerGrafanaRuleDTO', () => { it('should correctly convert rule form values', () => { @@ -33,4 +39,49 @@ describe('formValuesToRulerGrafanaRuleDTO', () => { expect(formValuesToRulerGrafanaRuleDTO(values)).toMatchSnapshot(); }); + + it('should set keep_firing_for if values are populated', () => { + const formValues: RuleFormValues = { + ...getDefaultFormValues(), + type: RuleFormType.cloudAlerting, + condition: 'A', + keepFiringForTime: 1, + keepFiringForTimeUnit: 'm', + }; + + expect(formValuesToRulerRuleDTO(formValues)).toMatchSnapshot(); + }); + + it('should not set keep_firing_for if values are undefined', () => { + const formValues: RuleFormValues = { + ...getDefaultFormValues(), + type: RuleFormType.cloudAlerting, + condition: 'A', + }; + + expect(formValuesToRulerRuleDTO(formValues)).toMatchSnapshot(); + }); + + it('should parse keep_firing_for', () => { + const rule: RulerAlertingRuleDTO = { + alert: 'A', + expr: 'B', + for: '1m', + keep_firing_for: '1m', + labels: {}, + }; + + expect(alertingRulerRuleToRuleForm(rule)).toMatchSnapshot(); + }); + + it('should set keepFiringForTime and keepFiringForTimeUnit to undefined if keep_firing_for not set', () => { + const rule: RulerAlertingRuleDTO = { + alert: 'A', + expr: 'B', + for: '1m', + labels: {}, + }; + + expect(alertingRulerRuleToRuleForm(rule)).toMatchSnapshot(); + }); }); diff --git a/public/app/features/alerting/unified/utils/rule-form.ts b/public/app/features/alerting/unified/utils/rule-form.ts index f1301654ac8..46b058238b9 100644 --- a/public/app/features/alerting/unified/utils/rule-form.ts +++ b/public/app/features/alerting/unified/utils/rule-form.ts @@ -75,11 +75,17 @@ export const getDefaultFormValues = (): RuleFormValues => { }; export function formValuesToRulerRuleDTO(values: RuleFormValues): RulerRuleDTO { - const { name, expression, forTime, forTimeUnit, type } = values; + const { name, expression, forTime, forTimeUnit, keepFiringForTime, keepFiringForTimeUnit, type } = values; if (type === RuleFormType.cloudAlerting) { + let keepFiringFor: string | undefined; + if (keepFiringForTime && keepFiringForTimeUnit) { + keepFiringFor = `${keepFiringForTime}${keepFiringForTimeUnit}`; + } + return { alert: name, for: `${forTime}${forTimeUnit}`, + keep_firing_for: keepFiringFor, annotations: arrayToRecord(values.annotations || []), labels: arrayToRecord(values.labels || []), expr: expression, @@ -220,18 +226,34 @@ export function rulerRuleToFormValues(ruleWithLocation: RuleWithLocation): RuleF export function alertingRulerRuleToRuleForm( rule: RulerAlertingRuleDTO -): Pick { +): Pick< + RuleFormValues, + | 'name' + | 'forTime' + | 'forTimeUnit' + | 'keepFiringForTime' + | 'keepFiringForTimeUnit' + | 'expression' + | 'annotations' + | 'labels' +> { const defaultFormValues = getDefaultFormValues(); const [forTime, forTimeUnit] = rule.for ? parseInterval(rule.for) : [defaultFormValues.forTime, defaultFormValues.forTimeUnit]; + const [keepFiringForTime, keepFiringForTimeUnit] = rule.keep_firing_for + ? parseInterval(rule.keep_firing_for) + : [defaultFormValues.keepFiringForTime, defaultFormValues.keepFiringForTimeUnit]; + return { name: rule.alert, expression: rule.expr, forTime, forTimeUnit, + keepFiringForTime, + keepFiringForTimeUnit, annotations: listifyLabelsOrAnnotations(rule.annotations, false), labels: listifyLabelsOrAnnotations(rule.labels, true), }; diff --git a/public/app/types/unified-alerting-dto.ts b/public/app/types/unified-alerting-dto.ts index b3af0806f23..00dbfe4fa46 100644 --- a/public/app/types/unified-alerting-dto.ts +++ b/public/app/types/unified-alerting-dto.ts @@ -173,6 +173,7 @@ export interface RulerRecordingRuleDTO extends RulerRuleBaseDTO { export interface RulerAlertingRuleDTO extends RulerRuleBaseDTO { alert: string; for?: string; + keep_firing_for?: string; annotations?: Annotations; }