Alerting: Add relativeTimeRange from dataSource when using Resample expresions (#56652)

* Fix: Add relativeTimeRange from dataSource when using Resample expression

* Add test for relativeTimeRange when updating resample expression

* update time range for expressions when data sources are updated

* Get data source recursively from expression up to the last query that this tree is referencing to

* Remove unnecessary produce from immer, and check if there is a cycle in the queries structure
This commit is contained in:
Sonia Aguilar
2022-10-17 12:29:05 +02:00
committed by GitHub
parent 21792fdf37
commit 3f26ffde94
4 changed files with 221 additions and 9 deletions

View File

@@ -29,6 +29,7 @@ import {
updateExpression,
updateExpressionRefId,
updateExpressionType,
updateExpressionTimeRange,
} from './reducer';
interface Props {
@@ -118,7 +119,7 @@ export const QueryAndExpressionsStep: FC<Props> = ({ 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;

View File

@@ -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 = {

View File

@@ -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<ExpressionQuery>('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, 'model' | 'datasourceUid' | 'relativeTimeRange'>
): AlertQuery[] => {
const refId = getNextRefIdChar(queries);
const query: AlertQuery = {
...queryToAdd,
refId,

View File

@@ -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;
}
}
};