From 4e24b2977c7b0334637ec38e5448c9bb068c39d1 Mon Sep 17 00:00:00 2001 From: Konrad Lalik Date: Thu, 20 Nov 2025 08:38:49 +0100 Subject: [PATCH] Alerting: Fix expression queries when coming from a panel (#114095) * Add getDefaultExpressions tests * Fix expression queries when building form values from a panel * make query params empty and add some better types * update snapshots * fix tests * Revert devenv changes * update shapshot --------- Co-authored-by: Gilles De Mey --- .../PanelAlertTabContent.test.tsx.snap | 8 +- ...eEditorGrafanaRecordingRules.test.tsx.snap | 8 +- .../SimplifiedRuleEditor.test.tsx.snap | 40 +--- .../RuleEditorGrafanaRules.test.tsx.snap | 8 +- .../alerting/unified/utils/rule-form.test.ts | 103 +++++++++ .../alerting/unified/utils/rule-form.ts | 210 +++++++++++------- .../PanelDataAlertingTab.test.tsx.snap | 8 +- 7 files changed, 246 insertions(+), 139 deletions(-) diff --git a/public/app/features/alerting/unified/__snapshots__/PanelAlertTabContent.test.tsx.snap b/public/app/features/alerting/unified/__snapshots__/PanelAlertTabContent.test.tsx.snap index 4c1b0bb8b65..e36613807ec 100644 --- a/public/app/features/alerting/unified/__snapshots__/PanelAlertTabContent.test.tsx.snap +++ b/public/app/features/alerting/unified/__snapshots__/PanelAlertTabContent.test.tsx.snap @@ -52,9 +52,7 @@ exports[`PanelAlertTabContent Will render alerts belonging to panel and a button "type": "and", }, "query": { - "params": [ - "B", - ], + "params": [], }, "reducer": { "params": [], @@ -90,9 +88,7 @@ exports[`PanelAlertTabContent Will render alerts belonging to panel and a button "type": "and", }, "query": { - "params": [ - "C", - ], + "params": [], }, "reducer": { "params": [], diff --git a/public/app/features/alerting/unified/__snapshots__/RuleEditorGrafanaRecordingRules.test.tsx.snap b/public/app/features/alerting/unified/__snapshots__/RuleEditorGrafanaRecordingRules.test.tsx.snap index fdd49bc80d7..7aa2e489209 100644 --- a/public/app/features/alerting/unified/__snapshots__/RuleEditorGrafanaRecordingRules.test.tsx.snap +++ b/public/app/features/alerting/unified/__snapshots__/RuleEditorGrafanaRecordingRules.test.tsx.snap @@ -78,9 +78,7 @@ exports[`RuleEditor grafana recording rules can create new grafana recording rul "type": "and", }, "query": { - "params": [ - "B", - ], + "params": [], }, "reducer": { "params": [], @@ -208,9 +206,7 @@ exports[`RuleEditor grafana recording rules can create new grafana recording rul "type": "and", }, "query": { - "params": [ - "B", - ], + "params": [], }, "reducer": { "params": [], diff --git a/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/simplifiedRouting/__snapshots__/SimplifiedRuleEditor.test.tsx.snap b/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/simplifiedRouting/__snapshots__/SimplifiedRuleEditor.test.tsx.snap index 9ffe68ad3b4..1ff6c2a0502 100644 --- a/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/simplifiedRouting/__snapshots__/SimplifiedRuleEditor.test.tsx.snap +++ b/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/simplifiedRouting/__snapshots__/SimplifiedRuleEditor.test.tsx.snap @@ -79,9 +79,7 @@ exports[`Can create a new grafana managed alert using simplified routing can cre "type": "and", }, "query": { - "params": [ - "B", - ], + "params": [], }, "reducer": { "params": [], @@ -117,9 +115,7 @@ exports[`Can create a new grafana managed alert using simplified routing can cre "type": "and", }, "query": { - "params": [ - "C", - ], + "params": [], }, "reducer": { "params": [], @@ -249,9 +245,7 @@ exports[`Can create a new grafana managed alert using simplified routing switch "type": "and", }, "query": { - "params": [ - "B", - ], + "params": [], }, "reducer": { "params": [], @@ -287,9 +281,7 @@ exports[`Can create a new grafana managed alert using simplified routing switch "type": "and", }, "query": { - "params": [ - "C", - ], + "params": [], }, "reducer": { "params": [], @@ -422,9 +414,7 @@ exports[`Can create a new grafana managed alert using simplified routing switch "type": "and", }, "query": { - "params": [ - "B", - ], + "params": [], }, "reducer": { "params": [], @@ -460,9 +450,7 @@ exports[`Can create a new grafana managed alert using simplified routing switch "type": "and", }, "query": { - "params": [ - "C", - ], + "params": [], }, "reducer": { "params": [], @@ -598,9 +586,7 @@ exports[`Can create a new grafana managed alert using simplified routing switch "type": "and", }, "query": { - "params": [ - "B", - ], + "params": [], }, "reducer": { "params": [], @@ -636,9 +622,7 @@ exports[`Can create a new grafana managed alert using simplified routing switch "type": "and", }, "query": { - "params": [ - "C", - ], + "params": [], }, "reducer": { "params": [], @@ -771,9 +755,7 @@ exports[`Can create a new grafana managed alert using simplified routing switch "type": "and", }, "query": { - "params": [ - "B", - ], + "params": [], }, "reducer": { "params": [], @@ -809,9 +791,7 @@ exports[`Can create a new grafana managed alert using simplified routing switch "type": "and", }, "query": { - "params": [ - "C", - ], + "params": [], }, "reducer": { "params": [], diff --git a/public/app/features/alerting/unified/rule-editor/__snapshots__/RuleEditorGrafanaRules.test.tsx.snap b/public/app/features/alerting/unified/rule-editor/__snapshots__/RuleEditorGrafanaRules.test.tsx.snap index 298f5e0542a..6dd5b9af077 100644 --- a/public/app/features/alerting/unified/rule-editor/__snapshots__/RuleEditorGrafanaRules.test.tsx.snap +++ b/public/app/features/alerting/unified/rule-editor/__snapshots__/RuleEditorGrafanaRules.test.tsx.snap @@ -81,9 +81,7 @@ exports[`RuleEditor grafana managed rules can create new grafana managed alert 1 "type": "and", }, "query": { - "params": [ - "B", - ], + "params": [], }, "reducer": { "params": [], @@ -119,9 +117,7 @@ exports[`RuleEditor grafana managed rules can create new grafana managed alert 1 "type": "and", }, "query": { - "params": [ - "C", - ], + "params": [], }, "reducer": { "params": [], 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 7137066229e..2e9b7a5730b 100644 --- a/public/app/features/alerting/unified/utils/rule-form.test.ts +++ b/public/app/features/alerting/unified/utils/rule-form.test.ts @@ -1,4 +1,5 @@ import { PromQuery } from '@grafana/prometheus'; +import { ExpressionDatasourceUID, ExpressionQueryType } from 'app/features/expressions/types'; import { RuleWithLocation } from 'app/types/unified-alerting'; import { AlertDataQuery, @@ -8,6 +9,7 @@ import { RulerAlertingRuleDTO, } from 'app/types/unified-alerting-dto'; +import { EvalFunction } from '../../state/alertDef'; import { mockDataSource, mockRuleWithLocation, mockRulerGrafanaRecordingRule } from '../mocks'; import { getDefaultFormValues } from '../rule-editor/formDefaults'; import { setupDataSources } from '../testSetup/datasources'; @@ -21,6 +23,7 @@ import { formValuesToRulerGrafanaRuleDTO, formValuesToRulerRuleDTO, getContactPointsFromDTO, + getDefaultExpressions, getInstantFromDataQuery, getNotificationSettingsForDTO, rulerRuleToFormValues, @@ -417,3 +420,103 @@ describe('getInstantFromDataQuery', () => { expect(result).toBe(false); }); }); + +describe('getDefaultExpressions', () => { + it('should create a reduce expression as the first query', () => { + const result = getDefaultExpressions('B', 'C'); + const reduceQuery = result[0]; + const model = reduceQuery.model; + + expect(reduceQuery.refId).toBe('B'); + expect(reduceQuery.datasourceUid).toBe(ExpressionDatasourceUID); + expect(reduceQuery.queryType).toBe(''); + expect(model.type).toBe(ExpressionQueryType.reduce); + expect(model.datasource?.uid).toBe(ExpressionDatasourceUID); + expect(model.reducer).toBe('last'); + }); + + it('should create reduce expression with proper conditions structure', () => { + const result = getDefaultExpressions('B', 'C'); + const reduceQuery = result[0]; + const model = reduceQuery.model; + + expect(model.conditions).toHaveLength(1); + expect(model.expression).toBe('A'); + expect(model.conditions?.[0]).toEqual({ + type: 'query', + evaluator: { + params: [], + type: EvalFunction.IsAbove, + }, + operator: { + type: 'and', + }, + query: { + params: [], + }, + reducer: { + params: [], + type: 'last', + }, + }); + }); + + it('should create a threshold expression as the second query', () => { + const result = getDefaultExpressions('B', 'C'); + const thresholdQuery = result[1]; + const model = thresholdQuery.model; + + expect(thresholdQuery.refId).toBe('C'); + expect(thresholdQuery.datasourceUid).toBe(ExpressionDatasourceUID); + expect(thresholdQuery.queryType).toBe(''); + expect(model.type).toBe(ExpressionQueryType.threshold); + expect(model.datasource?.uid).toBe(ExpressionDatasourceUID); + }); + + it('should create threshold expression with proper conditions structure', () => { + const result = getDefaultExpressions('B', 'C'); + const thresholdQuery = result[1]; + const model = thresholdQuery.model; + + expect(model.conditions).toHaveLength(1); + expect(model.conditions?.[0]).toEqual({ + type: 'query', + evaluator: { + params: [0], + type: EvalFunction.IsAbove, + }, + operator: { + type: 'and', + }, + query: { + params: [], + }, + reducer: { + params: [], + type: 'last', + }, + }); + }); + + it('should reference the reduce expression in the threshold expression', () => { + const result = getDefaultExpressions('B', 'C'); + const thresholdQuery = result[1]; + const model = thresholdQuery.model; + + expect(model.expression).toBe('B'); + }); + + it('should properly use different refIds throughout the structure', () => { + const result = getDefaultExpressions('X', 'Y'); + const reduceModel = result[0].model; + const thresholdModel = result[1].model; + + expect(result[0].refId).toBe('X'); + expect(reduceModel.refId).toBe('X'); + expect(reduceModel.conditions?.[0].query.params).toEqual([]); + + expect(result[1].refId).toBe('Y'); + expect(thresholdModel.refId).toBe('Y'); + expect(thresholdModel.expression).toBe('X'); + }); +}); diff --git a/public/app/features/alerting/unified/utils/rule-form.ts b/public/app/features/alerting/unified/utils/rule-form.ts index 899ed22575b..3c000d6cab7 100644 --- a/public/app/features/alerting/unified/utils/rule-form.ts +++ b/public/app/features/alerting/unified/utils/rule-form.ts @@ -536,85 +536,18 @@ export const getDefaultRecordingRulesQueries = ( }, ]; }; -const getDefaultExpressions = (...refIds: [string, string]): AlertQuery[] => { + +export const getDefaultExpressions = (...refIds: [string, string]) => { const refOne = refIds[0]; const refTwo = refIds[1]; - const reduceExpression: ExpressionQuery = { - refId: refIds[0], - type: ExpressionQueryType.reduce, - datasource: { - uid: ExpressionDatasourceUID, - type: ExpressionDatasourceRef.type, - }, - conditions: [ - { - type: 'query', - evaluator: { - params: [], - type: EvalFunction.IsAbove, - }, - operator: { - type: 'and', - }, - query: { - params: [refOne], - }, - reducer: { - params: [], - type: 'last', - }, - }, - ], - reducer: 'last', - expression: 'A', - }; + const reduceQuery = getDefaultReduceExpression({ inputRefId: 'A', reduceRefId: refOne }); + const thresholdQuery = getDefaultThresholdExpression({ inputRefId: refOne, thresholdRefId: refTwo }); - const thresholdExpression: ExpressionQuery = { - refId: refTwo, - type: ExpressionQueryType.threshold, - datasource: { - uid: ExpressionDatasourceUID, - type: ExpressionDatasourceRef.type, - }, - conditions: [ - { - type: 'query', - evaluator: { - params: [0], - type: EvalFunction.IsAbove, - }, - operator: { - type: 'and', - }, - query: { - params: [refTwo], - }, - reducer: { - params: [], - type: 'last', - }, - }, - ], - expression: refOne, - }; - - return [ - { - refId: refOne, - datasourceUid: ExpressionDatasourceUID, - queryType: '', - model: reduceExpression, - }, - { - refId: refTwo, - datasourceUid: ExpressionDatasourceUID, - queryType: '', - model: thresholdExpression, - }, - ]; + return [reduceQuery, thresholdQuery] as const; }; -const getDefaultExpressionsForRecording = (refOne: string): AlertQuery[] => { + +const getDefaultExpressionsForRecording = (refOne: string): Array> => { const reduceExpression: ExpressionQuery = { refId: refOne, type: ExpressionQueryType.reduce, @@ -633,7 +566,7 @@ const getDefaultExpressionsForRecording = (refOne: string): AlertQuery[] => { type: 'and', }, query: { - params: [refOne], + params: [], }, reducer: { params: [], @@ -655,6 +588,95 @@ const getDefaultExpressionsForRecording = (refOne: string): AlertQuery[] => { ]; }; +function getDefaultReduceExpression({ + inputRefId, + reduceRefId, +}: { + inputRefId: string; + reduceRefId: string; +}): AlertQuery { + const reduceExpression: ExpressionQuery = { + refId: reduceRefId, + type: ExpressionQueryType.reduce, + datasource: { + uid: ExpressionDatasourceUID, + type: ExpressionDatasourceRef.type, + }, + conditions: [ + { + type: 'query', + evaluator: { + params: [], + type: EvalFunction.IsAbove, + }, + operator: { + type: 'and', + }, + query: { + params: [], + }, + reducer: { + params: [], + type: 'last', + }, + }, + ], + reducer: 'last', + expression: inputRefId, + }; + + return { + refId: reduceRefId, + datasourceUid: ExpressionDatasourceUID, + queryType: '', + model: reduceExpression, + }; +} + +function getDefaultThresholdExpression({ + inputRefId, + thresholdRefId, +}: { + inputRefId: string; + thresholdRefId: string; +}): AlertQuery { + const thresholdExpression: ExpressionQuery = { + refId: thresholdRefId, + type: ExpressionQueryType.threshold, + datasource: { + uid: ExpressionDatasourceUID, + type: ExpressionDatasourceRef.type, + }, + conditions: [ + { + type: 'query', + evaluator: { + params: [0], + type: EvalFunction.IsAbove, + }, + operator: { + type: 'and', + }, + query: { + params: [], + }, + reducer: { + params: [], + type: 'last', + }, + }, + ], + expression: inputRefId, + }; + + return { + refId: thresholdRefId, + datasourceUid: ExpressionDatasourceUID, + queryType: '', + model: thresholdExpression, + }; +} + const dataQueriesToGrafanaQueries = async ( queries: DataQuery[], relativeTimeRange: RelativeTimeRange, @@ -736,14 +758,23 @@ export const panelToRuleFormValues = async ( return undefined; } + const lastQuery = queries.at(-1); + if (!lastQuery) { + return undefined; + } + if (!queries.find((query) => query.datasourceUid === ExpressionDatasourceUID)) { - const [reduceExpression, _thresholdExpression] = getDefaultExpressions(getNextRefId(queries), '-'); + const reduceExpression = getDefaultReduceExpression({ + inputRefId: lastQuery.refId, + reduceRefId: getNextRefId(queries), + }); queries.push(reduceExpression); - const [_reduceExpression, thresholdExpression] = getDefaultExpressions( - reduceExpression.refId, - getNextRefId(queries) - ); + const thresholdExpression = getDefaultThresholdExpression({ + inputRefId: reduceExpression.refId, + thresholdRefId: getNextRefId(queries), + }); + queries.push(thresholdExpression); } @@ -808,14 +839,23 @@ export const scenesPanelToRuleFormValues = async (vizPanel: VizPanel): Promise

query.datasourceUid === ExpressionDatasourceUID)) { - const [reduceExpression, _thresholdExpression] = getDefaultExpressions(getNextRefId(grafanaQueries), '-'); + const reduceExpression = getDefaultReduceExpression({ + inputRefId: lastQuery.refId, + reduceRefId: getNextRefId(grafanaQueries), + }); grafanaQueries.push(reduceExpression); - const [_reduceExpression, thresholdExpression] = getDefaultExpressions( - reduceExpression.refId, - getNextRefId(grafanaQueries) - ); + const thresholdExpression = getDefaultThresholdExpression({ + inputRefId: reduceExpression.refId, + thresholdRefId: getNextRefId(grafanaQueries), + }); + grafanaQueries.push(thresholdExpression); } diff --git a/public/app/features/dashboard-scene/panel-edit/PanelDataPane/__snapshots__/PanelDataAlertingTab.test.tsx.snap b/public/app/features/dashboard-scene/panel-edit/PanelDataPane/__snapshots__/PanelDataAlertingTab.test.tsx.snap index 8710a533fdd..858dc3812c1 100644 --- a/public/app/features/dashboard-scene/panel-edit/PanelDataPane/__snapshots__/PanelDataAlertingTab.test.tsx.snap +++ b/public/app/features/dashboard-scene/panel-edit/PanelDataPane/__snapshots__/PanelDataAlertingTab.test.tsx.snap @@ -47,9 +47,7 @@ exports[`PanelAlertTabContent Will render alerts belonging to panel and a button "type": "and", }, "query": { - "params": [ - "B", - ], + "params": [], }, "reducer": { "params": [], @@ -85,9 +83,7 @@ exports[`PanelAlertTabContent Will render alerts belonging to panel and a button "type": "and", }, "query": { - "params": [ - "C", - ], + "params": [], }, "reducer": { "params": [],