diff --git a/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/QueryAndExpressionsStep.tsx b/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/QueryAndExpressionsStep.tsx index 3e99c6f88fd..50981d11b02 100644 --- a/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/QueryAndExpressionsStep.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/QueryAndExpressionsStep.tsx @@ -29,6 +29,7 @@ import { updateExpression, updateExpressionRefId, updateExpressionType, + updateExpressionTimeRange, } from './reducer'; interface Props { @@ -118,7 +119,7 @@ export const QueryAndExpressionsStep: FC = ({ editingExistingRule }) => { const onChangeQueries = useCallback( (updatedQueries: AlertQuery[]) => { dispatch(setDataQueries(updatedQueries)); - + dispatch(updateExpressionTimeRange()); // check if we need to rewire expressions updatedQueries.forEach((query, index) => { const oldRefId = queries[index].refId; diff --git a/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/reducer.test.tsx b/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/reducer.test.tsx index 835e8d24d41..923c1ca34f2 100644 --- a/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/reducer.test.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/reducer.test.tsx @@ -1,4 +1,4 @@ -import { getDefaultRelativeTimeRange } from '@grafana/data'; +import { getDefaultRelativeTimeRange, RelativeTimeRange } from '@grafana/data'; import { getDataSourceSrv } from '@grafana/runtime/src/services/__mocks__/dataSourceSrv'; import { dataSource as expressionDatasource, @@ -19,6 +19,7 @@ import { setDataQueries, updateExpression, updateExpressionRefId, + updateExpressionTimeRange, updateExpressionType, } from './reducer'; @@ -146,6 +147,153 @@ describe('Query and expressions reducer', () => { const newState = queriesAndExpressionsReducer(initialState, updateExpression(newExpression)); expect(newState).toMatchSnapshot(); }); + it('should use time range from data source when updating an expression', () => { + const expressionQuery: AlertQuery = { + refId: 'B', + queryType: 'expression', + datasourceUid: '-100', + relativeTimeRange: { from: 900, to: 1000 }, + model: { + queryType: 'query', + datasource: '__expr__', + refId: 'B', + expression: 'C', + type: ExpressionQueryType.classic, + window: '10s', + } as ExpressionQuery, + }; + + const expressionQuery2: AlertQuery = { + refId: 'C', + queryType: 'expression', + datasourceUid: '-100', + relativeTimeRange: { from: 1, to: 3 }, + model: { + queryType: 'query', + datasource: '__expr__', + refId: 'C', + expression: 'A', + type: ExpressionQueryType.classic, + window: '10s', + } as ExpressionQuery, + }; + + const queryA: AlertQuery = { + refId: 'A', + relativeTimeRange: { from: 900, to: 1000 }, + datasourceUid: 'dsuid', + model: { refId: 'A' }, + queryType: 'query', + }; + const newExpression: ExpressionQuery = { + ...expressionQuery2.model, + type: ExpressionQueryType.resample, + }; + + const initialState: QueriesAndExpressionsState = { + queries: [queryA, expressionQuery, expressionQuery2], + }; + + const newState = queriesAndExpressionsReducer(initialState, updateExpression(newExpression)); + expect(newState).toStrictEqual({ + queries: [ + { + refId: 'A', + relativeTimeRange: { from: 900, to: 1000 }, + datasourceUid: 'dsuid', + model: { refId: 'A' }, + queryType: 'query', + }, + { + datasourceUid: '-100', + relativeTimeRange: { from: 900, to: 1000 }, + model: { + datasource: '__expr__', + expression: 'C', + queryType: 'query', + refId: 'B', + type: 'classic_conditions', + window: '10s', + }, + queryType: 'expression', + refId: 'B', + }, + { + datasourceUid: '-100', + relativeTimeRange: { from: 900, to: 1000 }, + model: { + datasource: '__expr__', + expression: 'A', + queryType: 'query', + refId: 'C', + type: 'resample', + window: '10s', + }, + queryType: 'expression', + refId: 'C', + }, + ], + }); + }); + + it('Should update time range for all expressions that have this data source when dispatching updateExpressionTimeRange', () => { + const expressionQuery: AlertQuery = { + refId: 'B', + queryType: 'expression', + datasourceUid: '-100', + model: { + queryType: 'query', + datasource: '__expr__', + refId: 'B', + expression: 'A', + type: ExpressionQueryType.classic, + window: '10s', + } as ExpressionQuery, + }; + const customTimeRange: RelativeTimeRange = { from: 900, to: 1000 }; + + const queryA: AlertQuery = { + refId: 'A', + relativeTimeRange: customTimeRange, + datasourceUid: 'dsuid', + model: { refId: 'A' }, + queryType: 'query', + }; + + const initialState: QueriesAndExpressionsState = { + queries: [queryA, expressionQuery], + }; + + const newState = queriesAndExpressionsReducer(initialState, updateExpressionTimeRange()); + expect(newState).toStrictEqual({ + queries: [ + { + refId: 'A', + relativeTimeRange: customTimeRange, + datasourceUid: 'dsuid', + model: { refId: 'A' }, + queryType: 'query', + }, + { + datasourceUid: '-100', + model: { + datasource: '__expr__', + expression: 'A', + queryType: 'query', + refId: 'B', + type: ExpressionQueryType.classic, + window: '10s', + }, + queryType: 'expression', + refId: 'B', + relativeTimeRange: { + from: 900, + to: 1000, + }, + }, + ], + }); + }); it('should update an expression refId and rewire expressions', () => { const initialState: QueriesAndExpressionsState = { diff --git a/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/reducer.ts b/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/reducer.ts index d5533ffdf7b..c4bdd8a2e77 100644 --- a/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/reducer.ts +++ b/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/reducer.ts @@ -2,6 +2,7 @@ import { createAction, createReducer } from '@reduxjs/toolkit'; import { DataQuery, RelativeTimeRange, getDefaultRelativeTimeRange } from '@grafana/data'; import { getNextRefIdChar } from 'app/core/utils/query'; +import { findDataSourceFromExpressionRecursive } from 'app/features/alerting/utils/dataSourceFromExpression'; import { dataSource as expressionDatasource, ExpressionDatasourceUID, @@ -18,6 +19,15 @@ export interface QueriesAndExpressionsState { queries: AlertQuery[]; } +const findDataSourceFromExpression = ( + queries: AlertQuery[], + expression: string | undefined +): AlertQuery | null | undefined => { + const firstReference = queries.find((alertQuery) => alertQuery.refId === expression); + const dataSource = firstReference && findDataSourceFromExpressionRecursive(queries, firstReference); + return dataSource; +}; + const initialState: QueriesAndExpressionsState = { queries: [], }; @@ -32,6 +42,7 @@ export const updateExpression = createAction('updateExpression' export const updateExpressionRefId = createAction<{ oldRefId: string; newRefId: string }>('updateExpressionRefId'); export const rewireExpressions = createAction<{ oldRefId: string; newRefId: string }>('rewireExpressions'); export const updateExpressionType = createAction<{ refId: string; type: ExpressionQueryType }>('updateExpressionType'); +export const updateExpressionTimeRange = createAction('updateExpressionTimeRange'); export const queriesAndExpressionsReducer = createReducer(initialState, (builder) => { // data queries actions @@ -78,14 +89,33 @@ export const queriesAndExpressionsReducer = createReducer(initialState, (builder }) .addCase(updateExpression, (state, { payload }) => { state.queries = state.queries.map((query) => { - return query.refId === payload.refId - ? { - ...query, - model: payload, - } - : query; + const dataSourceAlertQuery = findDataSourceFromExpression(state.queries, payload.expression); + + const relativeTimeRange = dataSourceAlertQuery + ? dataSourceAlertQuery.relativeTimeRange + : getDefaultRelativeTimeRange(); + + if (query.refId === payload.refId) { + query.model = payload; + if (payload.type === ExpressionQueryType.resample) { + query.relativeTimeRange = relativeTimeRange; + } + } + return query; }); }) + .addCase(updateExpressionTimeRange, (state) => { + const newState = state.queries.map((query) => { + // It's an expression , let's update the relativeTimeRange with its dataSource relativeTimeRange + if (query.datasourceUid === ExpressionDatasourceUID) { + const dataSource = findDataSourceFromExpression(state.queries, query.model.expression); + const relativeTimeRange = dataSource ? dataSource.relativeTimeRange : getDefaultRelativeTimeRange(); + query.relativeTimeRange = relativeTimeRange; + } + return query; + }); + state.queries = newState; + }) .addCase(updateExpressionRefId, (state, { payload }) => { const { newRefId, oldRefId } = payload; @@ -138,7 +168,6 @@ const addQuery = ( queryToAdd: Pick ): AlertQuery[] => { const refId = getNextRefIdChar(queries); - const query: AlertQuery = { ...queryToAdd, refId, diff --git a/public/app/features/alerting/utils/dataSourceFromExpression.ts b/public/app/features/alerting/utils/dataSourceFromExpression.ts new file mode 100644 index 00000000000..9cb150ac2dd --- /dev/null +++ b/public/app/features/alerting/utils/dataSourceFromExpression.ts @@ -0,0 +1,34 @@ +import { ExpressionDatasourceUID } from 'app/features/expressions/ExpressionDatasource'; +import { AlertQuery } from 'app/types/unified-alerting-dto'; + +export const hasCyclicalReferences = (queries: AlertQuery[]) => { + try { + JSON.stringify(queries); + return false; + } catch (e) { + return true; + } +}; + +export const findDataSourceFromExpressionRecursive = ( + queries: AlertQuery[], + alertQuery: AlertQuery +): AlertQuery | null | undefined => { + //Check if this is not cyclical structre + if (hasCyclicalReferences(queries)) { + return null; + } + // We have the data source in this dataQuery + if (alertQuery.datasourceUid !== ExpressionDatasourceUID) { + return alertQuery; + } + // alertQuery it's an expression, we have to traverse all the tree up to the data source + else { + const alertQueryReferenced = queries.find((alertQuery_) => alertQuery_.refId === alertQuery.model.expression); + if (alertQueryReferenced) { + return findDataSourceFromExpressionRecursive(queries, alertQueryReferenced); + } else { + return null; + } + } +};