From bef9ff3b1a248103db8700a9dbbd663c3d3cd1ef Mon Sep 17 00:00:00 2001 From: Sonia Aguilar <33540275+soniaAguilarPeiron@users.noreply.github.com> Date: Tue, 22 Oct 2024 10:22:42 +0200 Subject: [PATCH] Alerting: Remove threshold as default expression for grafana-managed recording rules (#94949) * remove threshold as default expression for grafana-managed recording rule * fix * remove eslint disable line --- .../components/expressions/Expression.tsx | 68 ++++++++++++------- .../alert-rule-form/AlertRuleForm.tsx | 6 +- .../alerting/unified/utils/rule-form.ts | 49 +++++++++++-- 3 files changed, 94 insertions(+), 29 deletions(-) diff --git a/public/app/features/alerting/unified/components/expressions/Expression.tsx b/public/app/features/alerting/unified/components/expressions/Expression.tsx index 2a81ae90707..56db8a1bc98 100644 --- a/public/app/features/alerting/unified/components/expressions/Expression.tsx +++ b/public/app/features/alerting/unified/components/expressions/Expression.tsx @@ -20,6 +20,8 @@ import { import { AlertQuery, PromAlertingRuleState } from 'app/types/unified-alerting-dto'; import { usePagination } from '../../hooks/usePagination'; +import { RuleFormValues } from '../../types/rule-form'; +import { isGrafanaRecordingRuleByType } from '../../utils/rules'; import { PopupCard } from '../HoverCard'; import { Spacer } from '../Spacer'; import { AlertStateTag } from '../rules/AlertStateTag'; @@ -58,7 +60,9 @@ export const Expression: FC = ({ const queryType = query?.type; - const { setError, clearErrors } = useFormContext(); + const { setError, clearErrors, watch } = useFormContext(); + const type = watch('type'); + const isGrafanaRecordingRule = type ? isGrafanaRecordingRuleByType(type) : false; const onQueriesValidationError = useCallback( (errorMsg: string | undefined) => { @@ -158,20 +162,25 @@ export const Expression: FC = ({ {hasResults && ( <> - - -
- - - - - -
+ + {!isGrafanaRecordingRule && ( +
+ + + + + +
+ )} )} @@ -182,9 +191,10 @@ export const Expression: FC = ({ interface ExpressionResultProps { series: DataFrame[]; isAlertCondition?: boolean; + isRecordingRule?: boolean; } export const PAGE_SIZE = 20; -export const ExpressionResult: FC = ({ series, isAlertCondition }) => { +export const ExpressionResult: FC = ({ series, isAlertCondition, isRecordingRule = false }) => { const { pageItems, previousPage, nextPage, numberOfPages, pageStart, pageEnd } = usePagination(series, 1, PAGE_SIZE); const styles = useStyles2(getStyles); @@ -212,7 +222,13 @@ export const ExpressionResult: FC = ({ series, isAlertCon !isTimeSeriesResults && pageItems.map((frame, index) => ( // There's no way to uniquely identify a frame that doesn't cause render bugs :/ (Gilles) - + ))} {emptyResults &&
No data
} {shouldShowPagination && ( @@ -353,6 +369,7 @@ const Header: FC = ({ interface FrameProps extends Pick { frame: DataFrame; index: number; + isRecordingRule?: boolean; } const OpeningBracket = () => {'{'}; @@ -361,7 +378,7 @@ const ClosingBracket = () => {'}'}; const Quote = () => "; const Equals = () => {'='}; -const FrameRow: FC = ({ frame, index, isAlertCondition }) => { +function FrameRow({ frame, index, isAlertCondition, isRecordingRule }: FrameProps) { const styles = useStyles2(getStyles); const name = getSeriesName(frame) || 'Series ' + index; @@ -374,6 +391,7 @@ const FrameRow: FC = ({ frame, index, isAlertCondition }) => { const showNormal = isAlertCondition && value === 0; const title = `${hasLabels ? '' : name}${hasLabels ? `{${formatLabels(labelsRecord)}}` : ''}`; + const shouldRenderSumary = !isRecordingRule; return (
@@ -401,14 +419,18 @@ const FrameRow: FC = ({ frame, index, isAlertCondition }) => {
{value}
- {showFiring && } - {showNormal && } + {shouldRenderSumary && ( + <> + {showFiring && } + {showNormal && } + + )} ); -}; - -const TimeseriesRow: FC = ({ frame, index }) => { +} +interface TimeseriesRowProps extends Omit {} +const TimeseriesRow: FC = ({ frame, index }) => { const styles = useStyles2(getStyles); const valueField = frame.fields[1]; // field 0 is "time", field 1 is "value" diff --git a/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/AlertRuleForm.tsx b/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/AlertRuleForm.tsx index 4d12179b1fc..b10805b707c 100644 --- a/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/AlertRuleForm.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/AlertRuleForm.tsx @@ -53,6 +53,7 @@ import { } from '../../../utils/rule-form'; import * as ruleId from '../../../utils/rule-id'; import { fromRulerRule, fromRulerRuleAndRuleGroupIdentifier, stringifyIdentifier } from '../../../utils/rule-id'; +import { isGrafanaRecordingRuleByType } from '../../../utils/rules'; import { createRelativeUrl } from '../../../utils/url'; import { GrafanaRuleExporter } from '../../export/GrafanaRuleExporter'; import { AlertRuleNameAndMetric } from '../AlertRuleNameInput'; @@ -102,12 +103,13 @@ export const AlertRuleForm = ({ existing, prefill }: Props) => { if (queryParams.has('defaults')) { return formValuesFromQueryParams(queryParams.get('defaults') ?? '', ruleType); } + const defaultRuleType = ruleType || RuleFormType.grafana; return { ...getDefaultFormValues(), condition: 'C', - queries: getDefaultQueries(), - type: ruleType || RuleFormType.grafana, + queries: getDefaultQueries(isGrafanaRecordingRuleByType(defaultRuleType)), + type: defaultRuleType, evaluateEvery: evaluateEvery, }; }, [existing, prefill, queryParams, evaluateEvery, ruleType]); diff --git a/public/app/features/alerting/unified/utils/rule-form.ts b/public/app/features/alerting/unified/utils/rule-form.ts index 231a3c51acf..8fb355423c5 100644 --- a/public/app/features/alerting/unified/utils/rule-form.ts +++ b/public/app/features/alerting/unified/utils/rule-form.ts @@ -497,14 +497,16 @@ export function recordingRulerRuleToRuleForm( }; } -export const getDefaultQueries = (): AlertQuery[] => { +export const getDefaultQueries = (isRecordingRule = false): AlertQuery[] => { const dataSource = getDefaultOrFirstCompatibleDataSource(); if (!dataSource) { - return [...getDefaultExpressions('A', 'B')]; + const expressions = isRecordingRule ? getDefaultExpressionsForRecording('A') : getDefaultExpressions('A', 'B'); + return [...expressions]; } const relativeTimeRange = getDefaultRelativeTimeRange(); + const expressions = isRecordingRule ? getDefaultExpressionsForRecording('B') : getDefaultExpressions('B', 'C'); return [ { refId: 'A', @@ -515,7 +517,7 @@ export const getDefaultQueries = (): AlertQuery[] => { refId: 'A', }, }, - ...getDefaultExpressions('B', 'C'), + ...expressions, ]; }; @@ -536,7 +538,6 @@ export const getDefaultRecordingRulesQueries = ( }, ]; }; - const getDefaultExpressions = (...refIds: [string, string]): AlertQuery[] => { const refOne = refIds[0]; const refTwo = refIds[1]; @@ -615,6 +616,46 @@ const getDefaultExpressions = (...refIds: [string, string]): AlertQuery[] => { }, ]; }; +const getDefaultExpressionsForRecording = (refOne: string): AlertQuery[] => { + const reduceExpression: ExpressionQuery = { + refId: refOne, + 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', + }; + + return [ + { + refId: refOne, + datasourceUid: ExpressionDatasourceUID, + queryType: '', + model: reduceExpression, + }, + ]; +}; const dataQueriesToGrafanaQueries = async ( queries: DataQuery[],