mirror of https://github.com/grafana/grafana.git
				
				
				
			Alerting: Add DAG errors to alert rule creation and view (#99423)
* catch error in query tab when running query throws an error
* add translations
* fix translations
* update query runner to omit nodes that failed to link
* remove unused function
* add DAG errors to AlertingQueryRunner
* bump CI
* fix test
* update test
* fix i18n
* revert code pieve
* Bring the piece of code back 😁
* bail from runner when no queries are to be executed
* add tests and translations
* refactor prepareQueries to omit broken refs and exclude descendant nodes
* update code comments
* fix omitting descendant nodes
* add all broken or missing nodes to panel errors
* go go drone
* remove unused function
* fix prettier and translations
* add export
---------
Co-authored-by: Gilles De Mey <gilles.de.mey@gmail.com>
			
			
This commit is contained in:
		
							parent
							
								
									29afe7d2cc
								
							
						
					
					
						commit
						67722de343
					
				|  | @ -1545,7 +1545,7 @@ exports[`better eslint`] = { | |||
|     "public/app/features/alerting/unified/GrafanaRuleQueryViewer.tsx:5381": [ | ||||
|       [0, 0, 0, "\'@grafana/data/src/datetime/rangeutil\' import is restricted from being used by a pattern. Import from the public export instead.", "0"], | ||||
|       [0, 0, 0, "No untranslated strings in text props. Wrap text with <Trans /> or use t()", "1"], | ||||
|       [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "2"], | ||||
|       [0, 0, 0, "No untranslated strings in text props in text props. Wrap text with <Trans /> or use t() or use t()", "2"], | ||||
|       [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "3"], | ||||
|       [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "4"], | ||||
|       [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "5"], | ||||
|  | @ -1558,7 +1558,8 @@ exports[`better eslint`] = { | |||
|       [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "12"], | ||||
|       [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "13"], | ||||
|       [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "14"], | ||||
|       [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "15"] | ||||
|       [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "15"], | ||||
|       [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "16"] | ||||
|     ], | ||||
|     "public/app/features/alerting/unified/NotificationPoliciesPage.tsx:5381": [ | ||||
|       [0, 0, 0, "No untranslated strings in text props. Wrap text with <Trans /> or use t()", "0"], | ||||
|  | @ -2339,6 +2340,9 @@ exports[`better eslint`] = { | |||
|       [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "10"], | ||||
|       [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "11"] | ||||
|     ], | ||||
|     "public/app/features/alerting/unified/components/rule-editor/dag.test.ts:5381": [ | ||||
|       [0, 0, 0, "\'@grafana/runtime/src/utils/DataSourceWithBackend\' import is restricted from being used by a pattern. Import from the public export instead.", "0"] | ||||
|     ], | ||||
|     "public/app/features/alerting/unified/components/rule-editor/labels/LabelsButtons.tsx:5381": [ | ||||
|       [0, 0, 0, "No untranslated strings in text props. Wrap text with <Trans /> or use t()", "0"], | ||||
|       [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "1"] | ||||
|  | @ -2463,9 +2467,18 @@ exports[`better eslint`] = { | |||
|       [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "3"], | ||||
|       [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "4"] | ||||
|     ], | ||||
|     "public/app/features/alerting/unified/components/rule-viewer/tabs/Query.tsx:5381": [ | ||||
|       [0, 0, 0, "No untranslated strings in text props. Wrap text with <Trans /> or use t()", "0"], | ||||
|       [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "1"] | ||||
|     "public/app/features/alerting/unified/components/rule-viewer/tabs/Details.tsx:5381": [ | ||||
|       [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "0"], | ||||
|       [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "1"], | ||||
|       [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "2"], | ||||
|       [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "3"], | ||||
|       [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "4"], | ||||
|       [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "5"], | ||||
|       [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "6"], | ||||
|       [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "7"], | ||||
|       [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "8"], | ||||
|       [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "9"], | ||||
|       [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "10"] | ||||
|     ], | ||||
|     "public/app/features/alerting/unified/components/rule-viewer/tabs/Routing.tsx:5381": [ | ||||
|       [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "0"] | ||||
|  | @ -2919,6 +2932,9 @@ exports[`better eslint`] = { | |||
|     "public/app/features/alerting/unified/rule-list/components/RuleListIcon.tsx:5381": [ | ||||
|       [0, 0, 0, "\'@grafana/ui/src/components/Text/Text\' import is restricted from being used by a pattern. Import from the public export instead.", "0"] | ||||
|     ], | ||||
|     "public/app/features/alerting/unified/state/AlertingQueryRunner.test.ts:5381": [ | ||||
|       [0, 0, 0, "\'@grafana/runtime/src/utils/DataSourceWithBackend\' import is restricted from being used by a pattern. Import from the public export instead.", "0"] | ||||
|     ], | ||||
|     "public/app/features/alerting/unified/state/actions.ts:5381": [ | ||||
|       [0, 0, 0, "\'@grafana/runtime/src/utils/logging\' import is restricted from being used by a pattern. Import from the public export instead.", "0"] | ||||
|     ], | ||||
|  |  | |||
|  | @ -1,7 +1,6 @@ | |||
| import { render, screen, waitFor } from 'test/test-utils'; | ||||
| 
 | ||||
| import { DataSourceRef } from '@grafana/schema'; | ||||
| import { AlertQuery } from 'app/types/unified-alerting-dto'; | ||||
| import { AlertDataQuery, AlertQuery } from 'app/types/unified-alerting-dto'; | ||||
| 
 | ||||
| import { GrafanaRuleQueryViewer } from './GrafanaRuleQueryViewer'; | ||||
| import { mockCombinedRule } from './mocks'; | ||||
|  | @ -10,71 +9,79 @@ describe('GrafanaRuleQueryViewer', () => { | |||
|   it('renders without crashing', async () => { | ||||
|     const rule = mockCombinedRule(); | ||||
| 
 | ||||
|     const getDataSourceQuery = (refId: string) => { | ||||
|       const query: AlertQuery = { | ||||
|         refId: refId, | ||||
|         datasourceUid: 'abc123', | ||||
|         queryType: '', | ||||
|         relativeTimeRange: { | ||||
|           from: 600, | ||||
|           to: 0, | ||||
|         }, | ||||
|         model: { | ||||
|           refId: 'A', | ||||
|         }, | ||||
|       }; | ||||
|       return query; | ||||
|     }; | ||||
|     const queries = [ | ||||
|       getDataSourceQuery('A'), | ||||
|       getDataSourceQuery('B'), | ||||
|       getDataSourceQuery('C'), | ||||
|       getDataSourceQuery('D'), | ||||
|       getDataSourceQuery('E'), | ||||
|     ]; | ||||
| 
 | ||||
|     const getExpression = (refId: string, dsRef: DataSourceRef) => { | ||||
|       const expr = { | ||||
|         refId: refId, | ||||
|         datasourceUid: '__expr__', | ||||
|         queryType: '', | ||||
|         model: { | ||||
|           refId: refId, | ||||
|           type: 'classic_conditions', | ||||
|           datasource: dsRef, | ||||
|           conditions: [ | ||||
|             { | ||||
|               type: 'query', | ||||
|               evaluator: { | ||||
|                 params: [3], | ||||
|                 type: 'gt', | ||||
|               }, | ||||
|               operator: { | ||||
|                 type: 'and', | ||||
|               }, | ||||
|               query: { | ||||
|                 params: ['A'], | ||||
|               }, | ||||
|               reducer: { | ||||
|                 params: [], | ||||
|                 type: 'last', | ||||
|               }, | ||||
|             }, | ||||
|           ], | ||||
|         }, | ||||
|       }; | ||||
|       return expr; | ||||
|     }; | ||||
| 
 | ||||
|     const expressions = [ | ||||
|       getExpression('A', { type: '' }), | ||||
|       getExpression('B', { type: '' }), | ||||
|       getExpression('C', { type: '' }), | ||||
|       getExpression('D', { type: '' }), | ||||
|     ]; | ||||
|     const expressions = [getExpression('F'), getExpression('G'), getExpression('H'), getExpression('I')]; | ||||
|     render(<GrafanaRuleQueryViewer queries={[...queries, ...expressions]} condition="A" rule={rule} />); | ||||
| 
 | ||||
|     await waitFor(() => expect(screen.getByTestId('queries-container')).toHaveStyle('flex-wrap: wrap')); | ||||
|     expect(screen.getByTestId('expressions-container')).toHaveStyle('flex-wrap: wrap'); | ||||
|   }); | ||||
| 
 | ||||
|   it('should catch cyclical references', async () => { | ||||
|     const rule = mockCombinedRule(); | ||||
| 
 | ||||
|     const queries = [ | ||||
|       getExpression('A'), // this always points to A
 | ||||
|     ]; | ||||
| 
 | ||||
|     jest.spyOn(console, 'error').mockImplementation((message) => { | ||||
|       expect(message).toMatch(/Failed to parse thresholds/i); | ||||
|     }); | ||||
|     render(<GrafanaRuleQueryViewer queries={queries} condition="A" rule={rule} />); | ||||
|   }); | ||||
| }); | ||||
| 
 | ||||
| function getDataSourceQuery(sourceRefId: string) { | ||||
|   const query: AlertQuery<AlertDataQuery> = { | ||||
|     refId: sourceRefId, | ||||
|     datasourceUid: 'abc123', | ||||
|     queryType: '', | ||||
|     relativeTimeRange: { | ||||
|       from: 600, | ||||
|       to: 0, | ||||
|     }, | ||||
|     model: { | ||||
|       refId: sourceRefId, | ||||
|     }, | ||||
|   }; | ||||
|   return query; | ||||
| } | ||||
| const queries = [ | ||||
|   getDataSourceQuery('A'), | ||||
|   getDataSourceQuery('B'), | ||||
|   getDataSourceQuery('C'), | ||||
|   getDataSourceQuery('D'), | ||||
|   getDataSourceQuery('E'), | ||||
| ]; | ||||
| 
 | ||||
| function getExpression(refId: string) { | ||||
|   const expr = { | ||||
|     refId: refId, | ||||
|     datasourceUid: '__expr__', | ||||
|     queryType: '', | ||||
|     model: { | ||||
|       refId: refId, | ||||
|       type: 'classic_conditions', | ||||
|       datasource: { type: '' }, | ||||
|       conditions: [ | ||||
|         { | ||||
|           type: 'query', | ||||
|           evaluator: { | ||||
|             params: [3], | ||||
|             type: 'gt', | ||||
|           }, | ||||
|           operator: { | ||||
|             type: 'and', | ||||
|           }, | ||||
|           query: { | ||||
|             params: ['A'], | ||||
|           }, | ||||
|           reducer: { | ||||
|             params: [], | ||||
|             type: 'last', | ||||
|           }, | ||||
|         }, | ||||
|       ], | ||||
|     }, | ||||
|   }; | ||||
|   return expr; | ||||
| } | ||||
|  |  | |||
|  | @ -1,12 +1,12 @@ | |||
| import { css, cx } from '@emotion/css'; | ||||
| import { keyBy, startCase } from 'lodash'; | ||||
| import { keyBy, startCase, uniqueId } from 'lodash'; | ||||
| import * as React from 'react'; | ||||
| 
 | ||||
| import { DataSourceInstanceSettings, DataSourceRef, GrafanaTheme2, PanelData, urlUtil } from '@grafana/data'; | ||||
| import { secondsToHms } from '@grafana/data/src/datetime/rangeutil'; | ||||
| import { config } from '@grafana/runtime'; | ||||
| import { Preview } from '@grafana/sql/src/components/visual-query-builder/Preview'; | ||||
| import { Badge, ErrorBoundaryAlert, LinkButton, Stack, Text, useStyles2 } from '@grafana/ui'; | ||||
| import { Alert, Badge, ErrorBoundaryAlert, LinkButton, Stack, Text, useStyles2 } from '@grafana/ui'; | ||||
| import { CombinedRule } from 'app/types/unified-alerting'; | ||||
| 
 | ||||
| import { AlertDataQuery, AlertQuery } from '../../../types/unified-alerting-dto'; | ||||
|  | @ -188,9 +188,6 @@ const getQueryPreviewStyles = (theme: GrafanaTheme2) => ({ | |||
|   contentBox: css({ | ||||
|     flex: '1 0 100%', | ||||
|   }), | ||||
|   visualization: css({ | ||||
|     padding: theme.spacing(1), | ||||
|   }), | ||||
|   dataSource: css({ | ||||
|     border: `1px solid ${theme.colors.border.weak}`, | ||||
|     borderRadius: theme.shape.radius.default, | ||||
|  | @ -208,6 +205,8 @@ interface ExpressionPreviewProps extends Pick<AlertQuery, 'refId'> { | |||
| } | ||||
| 
 | ||||
| function ExpressionPreview({ refId, model, evalData, isAlertCondition }: ExpressionPreviewProps) { | ||||
|   const styles = useStyles2(getQueryBoxStyles); | ||||
| 
 | ||||
|   function renderPreview() { | ||||
|     switch (model.type) { | ||||
|       case ExpressionQueryType.math: | ||||
|  | @ -243,7 +242,14 @@ function ExpressionPreview({ refId, model, evalData, isAlertCondition }: Express | |||
|       ]} | ||||
|       isAlertCondition={isAlertCondition} | ||||
|     > | ||||
|       {renderPreview()} | ||||
|       <div className={styles.previewWrapper}> | ||||
|         {evalData?.errors?.map((error) => ( | ||||
|           <Alert key={uniqueId()} title="Expression failed" severity="error" bottomSpacing={1}> | ||||
|             {error.message} | ||||
|           </Alert> | ||||
|         ))} | ||||
|         {renderPreview()} | ||||
|       </div> | ||||
|       <Spacer /> | ||||
|       {evalData && <ExpressionResult series={evalData.series} isAlertCondition={isAlertCondition} />} | ||||
|     </QueryBox> | ||||
|  | @ -310,6 +316,9 @@ const getQueryBoxStyles = (theme: GrafanaTheme2) => ({ | |||
|     border: `1px solid ${theme.colors.border.weak}`, | ||||
|     borderRadius: theme.shape.radius.default, | ||||
|   }), | ||||
|   previewWrapper: css({ | ||||
|     padding: theme.spacing(1), | ||||
|   }), | ||||
| }); | ||||
| 
 | ||||
| function ClassicConditionViewer({ model }: { model: ExpressionQuery }) { | ||||
|  | @ -345,7 +354,6 @@ function ClassicConditionViewer({ model }: { model: ExpressionQuery }) { | |||
| 
 | ||||
| const getClassicConditionViewerStyles = (theme: GrafanaTheme2) => ({ | ||||
|   container: css({ | ||||
|     padding: theme.spacing(1), | ||||
|     display: 'grid', | ||||
|     gridTemplateColumns: 'repeat(6, max-content)', | ||||
|     gap: theme.spacing(0, 1), | ||||
|  | @ -378,7 +386,6 @@ function ReduceConditionViewer({ model }: { model: ExpressionQuery }) { | |||
| 
 | ||||
| const getReduceConditionViewerStyles = (theme: GrafanaTheme2) => ({ | ||||
|   container: css({ | ||||
|     padding: theme.spacing(1), | ||||
|     display: 'grid', | ||||
|     gap: theme.spacing(0.5), | ||||
|     gridTemplateRows: '1fr 1fr', | ||||
|  | @ -417,7 +424,6 @@ function ResampleExpressionViewer({ model }: { model: ExpressionQuery }) { | |||
| 
 | ||||
| const getResampleExpressionViewerStyles = (theme: GrafanaTheme2) => ({ | ||||
|   container: css({ | ||||
|     padding: theme.spacing(1), | ||||
|     display: 'grid', | ||||
|     gap: theme.spacing(0.5), | ||||
|     gridTemplateColumns: 'repeat(4, 1fr)', | ||||
|  | @ -486,7 +492,6 @@ const getExpressionViewerStyles = (theme: GrafanaTheme2) => { | |||
|       maxWidth: '100%', | ||||
|     }), | ||||
|     container: css({ | ||||
|       padding: theme.spacing(1), | ||||
|       display: 'flex', | ||||
|       gap: theme.spacing(0.5), | ||||
|     }), | ||||
|  |  | |||
|  | @ -0,0 +1,11 @@ | |||
| // Jest Snapshot v1, https://goo.gl/fbAQLP | ||||
| 
 | ||||
| exports[`working with dag should throw on references to self 1`] = ` | ||||
| [ | ||||
|   { | ||||
|     "error": [Error: cannot link A to A since it would create a cycle], | ||||
|     "source": "A", | ||||
|     "target": "A", | ||||
|   }, | ||||
| ] | ||||
| `; | ||||
|  | @ -1,11 +1,16 @@ | |||
| import { ExpressionDatasourceRef } from '@grafana/runtime/src/utils/DataSourceWithBackend'; | ||||
| import { Graph } from 'app/core/utils/dag'; | ||||
| import { EvalFunction } from 'app/features/alerting/state/alertDef'; | ||||
| import { ExpressionQuery, ExpressionQueryType } from 'app/features/expressions/types'; | ||||
| import { AlertQuery } from 'app/types/unified-alerting-dto'; | ||||
| 
 | ||||
| import { | ||||
|   DAGError, | ||||
|   _getDescendants, | ||||
|   _getOriginsOfRefId, | ||||
|   createDagFromQueries, | ||||
|   fingerprintGraph, | ||||
|   getTargets, | ||||
|   parseRefsFromMathExpression, | ||||
| } from './dag'; | ||||
| 
 | ||||
|  | @ -93,6 +98,31 @@ describe('working with dag', () => { | |||
|       dag.getNode('A'); | ||||
|     }).not.toThrow(); | ||||
|   }); | ||||
| 
 | ||||
|   it('should throw on references to self', () => { | ||||
|     const queries: Array<AlertQuery<ExpressionQuery>> = [ | ||||
|       { | ||||
|         refId: 'A', | ||||
|         model: { refId: 'A', expression: '$A', datasource: ExpressionDatasourceRef, type: ExpressionQueryType.math }, | ||||
|         queryType: '', | ||||
|         datasourceUid: '__expr__', | ||||
|       }, | ||||
|     ]; | ||||
| 
 | ||||
|     expect(() => createDagFromQueries(queries)).toThrowError(/failed to create DAG from queries/i); | ||||
| 
 | ||||
|     // now assert we get the correct error diagnostics
 | ||||
|     try { | ||||
|       createDagFromQueries(queries); | ||||
|     } catch (error) { | ||||
|       if (!(error instanceof Error)) { | ||||
|         throw error; | ||||
|       } | ||||
| 
 | ||||
|       expect(error instanceof DAGError).toBe(true); | ||||
|       expect(error!.cause).toMatchSnapshot(); | ||||
|     } | ||||
|   }); | ||||
| }); | ||||
| 
 | ||||
| describe('getOriginsOfRefId', () => { | ||||
|  | @ -157,3 +187,60 @@ describe('fingerprints', () => { | |||
|     expect(fingerprintGraph(graph)).toMatchInlineSnapshot(`"A:B: B:C:A, D C::B D:B:"`); | ||||
|   }); | ||||
| }); | ||||
| 
 | ||||
| describe('getTargets', () => { | ||||
|   it('should correct get targets from Math expression', () => { | ||||
|     const expression: ExpressionQuery = { | ||||
|       refId: 'C', | ||||
|       type: ExpressionQueryType.math, | ||||
|       datasource: ExpressionDatasourceRef, | ||||
|       expression: '$A + $B', | ||||
|     }; | ||||
| 
 | ||||
|     expect(getTargets(expression)).toEqual(['A', 'B']); | ||||
|   }); | ||||
| 
 | ||||
|   it('should be able to find the targets of a classic condition', () => { | ||||
|     const expression: ExpressionQuery = { | ||||
|       refId: 'C', | ||||
|       type: ExpressionQueryType.classic, | ||||
|       datasource: ExpressionDatasourceRef, | ||||
|       expression: '', | ||||
|       conditions: [ | ||||
|         { | ||||
|           evaluator: { | ||||
|             params: [0, 0], | ||||
|             type: EvalFunction.IsAbove, | ||||
|           }, | ||||
|           operator: { type: 'and' }, | ||||
|           query: { params: ['A'] }, | ||||
|           reducer: { params: [], type: 'avg' }, | ||||
|           type: 'query', | ||||
|         }, | ||||
|         { | ||||
|           evaluator: { | ||||
|             params: [0, 0], | ||||
|             type: EvalFunction.IsAbove, | ||||
|           }, | ||||
|           operator: { type: 'and' }, | ||||
|           query: { params: ['B'] }, | ||||
|           reducer: { params: [], type: 'avg' }, | ||||
|           type: 'query', | ||||
|         }, | ||||
|       ], | ||||
|     }; | ||||
| 
 | ||||
|     expect(getTargets(expression)).toEqual(['A', 'B']); | ||||
|   }); | ||||
| 
 | ||||
|   it('should work for any other expression type', () => { | ||||
|     const expression: ExpressionQuery = { | ||||
|       refId: 'C', | ||||
|       type: ExpressionQueryType.reduce, | ||||
|       datasource: ExpressionDatasourceRef, | ||||
|       expression: 'A', | ||||
|     }; | ||||
| 
 | ||||
|     expect(getTargets(expression)).toEqual(['A']); | ||||
|   }); | ||||
| }); | ||||
|  |  | |||
|  | @ -1,4 +1,4 @@ | |||
| import { compact, memoize, uniq } from 'lodash'; | ||||
| import { compact, memoize, reject, uniq } from 'lodash'; | ||||
| 
 | ||||
| import { Edge, Graph, Node } from 'app/core/utils/dag'; | ||||
| import { isExpressionQuery } from 'app/features/expressions/guards'; | ||||
|  | @ -12,6 +12,9 @@ import { AlertQuery } from 'app/types/unified-alerting-dto'; | |||
| export function createDagFromQueries(queries: AlertQuery[]): Graph { | ||||
|   const graph = new Graph(); | ||||
| 
 | ||||
|   // collect link errors in here so we can throw a single error with all nodes that failed to link
 | ||||
|   const linkErrors: LinkError[] = []; | ||||
| 
 | ||||
|   const nodes = queries.map((query) => query.refId); | ||||
|   graph.createNodes(nodes); | ||||
| 
 | ||||
|  | @ -25,18 +28,66 @@ export function createDagFromQueries(queries: AlertQuery[]): Graph { | |||
|     const targets = getTargets(query.model); | ||||
| 
 | ||||
|     targets.forEach((target) => { | ||||
|       const isSelf = source === target; | ||||
| 
 | ||||
|       if (source && target && !isSelf) { | ||||
|         graph.link(target, source); | ||||
|       if (source && target) { | ||||
|         try { | ||||
|           graph.link(target, source); | ||||
|         } catch (error) { | ||||
|           linkErrors.push({ source, target, error }); | ||||
|         } | ||||
|       } | ||||
|     }); | ||||
|   }); | ||||
| 
 | ||||
|   if (linkErrors.length > 0) { | ||||
|     throw new DAGError('failed to create DAG from queries', { cause: linkErrors }); | ||||
|   } | ||||
| 
 | ||||
|   return graph; | ||||
| } | ||||
| 
 | ||||
| function getTargets(model: ExpressionQuery) { | ||||
| /** | ||||
|  * This function attempts to create a "clean" DAG where only the nodes that successfully link are left | ||||
|  * ⚠️ This is a recursive function and very expensive for larger DAGs or large amount of queries | ||||
|  */ | ||||
| export function createDAGFromQueriesSafe( | ||||
|   queries: AlertQuery[], | ||||
|   collectedLinkErrors: LinkError[] = [] | ||||
| ): [Graph, LinkError[]] { | ||||
|   try { | ||||
|     return [createDagFromQueries(queries), collectedLinkErrors]; | ||||
|   } catch (error) { | ||||
|     if (error instanceof DAGError) { | ||||
|       const linkErrors = error.cause; | ||||
|       collectedLinkErrors.push(...linkErrors); | ||||
| 
 | ||||
|       const updatedQueries = reject(queries, (query) => | ||||
|         linkErrors.some((linkError) => linkError.source === query.refId) | ||||
|       ); | ||||
| 
 | ||||
|       return createDAGFromQueriesSafe(updatedQueries, collectedLinkErrors); | ||||
|     } | ||||
|   } | ||||
| 
 | ||||
|   return [new Graph(), collectedLinkErrors]; | ||||
| } | ||||
| 
 | ||||
| export interface LinkError { | ||||
|   source: string; | ||||
|   target: string; | ||||
|   error: unknown; | ||||
| } | ||||
| 
 | ||||
| /** DAGError subclass, this is just a regular error but with LinkError[] as the cause */ | ||||
| export class DAGError extends Error { | ||||
|   constructor(message: string, options: { cause: LinkError[] }) { | ||||
|     super(message, options); | ||||
|     this.cause = options?.cause ?? []; | ||||
|   } | ||||
| 
 | ||||
|   cause: LinkError[]; | ||||
| } | ||||
| 
 | ||||
| export function getTargets(model: ExpressionQuery) { | ||||
|   const isMathExpression = model.type === ExpressionQueryType.math; | ||||
|   const isClassicCondition = model.type === ExpressionQueryType.classic; | ||||
| 
 | ||||
|  |  | |||
|  | @ -2,6 +2,7 @@ import { useCallback, useEffect, useMemo } from 'react'; | |||
| 
 | ||||
| import { config } from '@grafana/runtime'; | ||||
| import { Alert, Stack } from '@grafana/ui'; | ||||
| import { Trans, t } from 'app/core/internationalization'; | ||||
| import { CombinedRule } from 'app/types/unified-alerting'; | ||||
| 
 | ||||
| import { GrafanaRuleQueryViewer, QueryPreview } from '../../../GrafanaRuleQueryViewer'; | ||||
|  | @ -39,47 +40,47 @@ const QueryResults = ({ rule }: Props) => { | |||
| 
 | ||||
|   const isFederatedRule = isFederatedRuleGroup(rule.group); | ||||
| 
 | ||||
|   if (isPreviewLoading) { | ||||
|     return <Trans i18nKey="alerting.common.loading">Loading...</Trans>; | ||||
|   } | ||||
| 
 | ||||
|   return ( | ||||
|     <> | ||||
|       {isPreviewLoading ? ( | ||||
|         'Loading...' | ||||
|       ) : ( | ||||
|         <> | ||||
|           {isGrafanaRulerRule(rule.rulerRule) && !isFederatedRule && ( | ||||
|             <GrafanaRuleQueryViewer | ||||
|               rule={rule} | ||||
|               condition={rule.rulerRule.grafana_alert.condition} | ||||
|               queries={queries} | ||||
|               evalDataByQuery={queryPreviewData} | ||||
|             /> | ||||
|           )} | ||||
|       {isGrafanaRulerRule(rule.rulerRule) && !isFederatedRule && ( | ||||
|         <GrafanaRuleQueryViewer | ||||
|           rule={rule} | ||||
|           condition={rule.rulerRule.grafana_alert.condition} | ||||
|           queries={queries} | ||||
|           evalDataByQuery={queryPreviewData} | ||||
|         /> | ||||
|       )} | ||||
| 
 | ||||
|           {!isGrafanaRulerRule(rule.rulerRule) && | ||||
|             !isFederatedRule && | ||||
|             queryPreviewData && | ||||
|             Object.keys(queryPreviewData).length > 0 && ( | ||||
|               <Stack direction="column" gap={1}> | ||||
|                 {queries.map((query) => { | ||||
|                   return ( | ||||
|                     <QueryPreview | ||||
|                       key={query.refId} | ||||
|                       rule={rule} | ||||
|                       refId={query.refId} | ||||
|                       model={query.model} | ||||
|                       dataSource={Object.values(config.datasources).find((ds) => ds.uid === query.datasourceUid)} | ||||
|                       queryData={queryPreviewData[query.refId]} | ||||
|                       relativeTimeRange={query.relativeTimeRange} | ||||
|                     /> | ||||
|                   ); | ||||
|                 })} | ||||
|               </Stack> | ||||
|             )} | ||||
|           {!isFederatedRule && !allDataSourcesAvailable && ( | ||||
|             <Alert title="Query not available" severity="warning"> | ||||
|               Cannot display the query preview. Some of the data sources used in the queries are not available. | ||||
|             </Alert> | ||||
|           )} | ||||
|         </> | ||||
|       {!isGrafanaRulerRule(rule.rulerRule) && | ||||
|         !isFederatedRule && | ||||
|         queryPreviewData && | ||||
|         Object.keys(queryPreviewData).length > 0 && ( | ||||
|           <Stack direction="column" gap={1}> | ||||
|             {queries.map((query) => { | ||||
|               return ( | ||||
|                 <QueryPreview | ||||
|                   key={query.refId} | ||||
|                   rule={rule} | ||||
|                   refId={query.refId} | ||||
|                   model={query.model} | ||||
|                   dataSource={Object.values(config.datasources).find((ds) => ds.uid === query.datasourceUid)} | ||||
|                   queryData={queryPreviewData[query.refId]} | ||||
|                   relativeTimeRange={query.relativeTimeRange} | ||||
|                 /> | ||||
|               ); | ||||
|             })} | ||||
|           </Stack> | ||||
|         )} | ||||
|       {!isFederatedRule && !allDataSourcesAvailable && ( | ||||
|         <Alert title={t('alerting.rule-view.query.datasources-na.title', 'Query not available')} severity="warning"> | ||||
|           <Trans i18nKey="alerting.rule-view.query.datasources-na.description"> | ||||
|             Cannot display the query preview. Some of the data sources used in the queries are not available. | ||||
|           </Trans> | ||||
|         </Alert> | ||||
|       )} | ||||
|     </> | ||||
|   ); | ||||
|  |  | |||
|  | @ -14,6 +14,7 @@ import { | |||
|   rangeUtil, | ||||
| } from '@grafana/data'; | ||||
| import { DataSourceSrv, DataSourceWithBackend, FetchResponse } from '@grafana/runtime'; | ||||
| import { ExpressionDatasourceRef } from '@grafana/runtime/src/utils/DataSourceWithBackend'; | ||||
| import { DataQuery } from '@grafana/schema'; | ||||
| import { BackendSrv } from 'app/core/services/backend_srv'; | ||||
| import { | ||||
|  | @ -22,6 +23,7 @@ import { | |||
| } from 'app/features/alerting/unified/components/settings/__mocks__/server'; | ||||
| import { setupMswServer } from 'app/features/alerting/unified/mockApi'; | ||||
| import { setupDataSources } from 'app/features/alerting/unified/testSetup/datasources'; | ||||
| import { ExpressionQuery, ExpressionQueryType } from 'app/features/expressions/types'; | ||||
| import { AlertDataQuery, AlertQuery } from 'app/types/unified-alerting-dto'; | ||||
| 
 | ||||
| import { AlertingQueryResponse, AlertingQueryRunner } from './AlertingQueryRunner'; | ||||
|  | @ -239,8 +241,8 @@ describe('AlertingQueryRunner', () => { | |||
|         }), | ||||
|         createQuery('B', { | ||||
|           model: { | ||||
|             expression: 'A', // depends on A
 | ||||
|             refId: 'B', | ||||
|             hide: false, | ||||
|           }, | ||||
|         }), | ||||
|         createQuery('C', { | ||||
|  | @ -309,6 +311,66 @@ const expectDataFrameWithValues = ({ time, values }: { time: number[]; values: n | |||
|   }; | ||||
| }; | ||||
| 
 | ||||
| describe('prepareQueries', () => { | ||||
|   it('should skip node that fail to link', async () => { | ||||
|     const queries = [ | ||||
|       createQuery('A', { | ||||
|         model: { | ||||
|           refId: 'A', | ||||
|           hide: true, // this node will be omitted
 | ||||
|         }, | ||||
|       }), | ||||
|       createQuery('B', { | ||||
|         model: { | ||||
|           refId: 'B', | ||||
|           hide: false, // this node will _not_ be omitted
 | ||||
|         }, | ||||
|       }), | ||||
|       createExpression('C', { | ||||
|         model: { | ||||
|           refId: 'C', | ||||
|           type: ExpressionQueryType.math, | ||||
|           expression: '$A', // this node will be omitted because it is a descendant of A (omitted)
 | ||||
|         }, | ||||
|       }), | ||||
|       createExpression('D', { | ||||
|         model: { | ||||
|           refId: 'D', | ||||
|           type: ExpressionQueryType.math, | ||||
|           expression: '$ZZZ', // this node will be omitted, ref does not exist
 | ||||
|         }, | ||||
|       }), | ||||
|       createExpression('E', { | ||||
|         model: { | ||||
|           refId: 'E', | ||||
|           type: ExpressionQueryType.math, | ||||
|           expression: '$B', // this node will be omitted, ref does not exist
 | ||||
|         }, | ||||
|       }), | ||||
|       createExpression('F', { | ||||
|         model: { | ||||
|           refId: 'F', | ||||
|           type: ExpressionQueryType.math, | ||||
|           expression: '$D', // this node will be omitted, because D is broken too
 | ||||
|         }, | ||||
|       }), | ||||
|     ]; | ||||
| 
 | ||||
|     const runner = new AlertingQueryRunner( | ||||
|       mockBackendSrv({ | ||||
|         fetch: () => of(), | ||||
|       }), | ||||
|       mockDataSourceSrv({ filterQuery: (model: AlertDataQuery) => model.hide !== true }) | ||||
|     ); | ||||
| 
 | ||||
|     const queriesToRun = await runner.prepareQueries(queries); | ||||
| 
 | ||||
|     expect(queriesToRun).toHaveLength(2); | ||||
|     expect(queriesToRun[0]).toStrictEqual(queries[1]); | ||||
|     expect(queriesToRun[1]).toStrictEqual(queries[4]); | ||||
|   }); | ||||
| }); | ||||
| 
 | ||||
| const createDataFrameJSON = (values: number[]): DataFrameJSON => { | ||||
|   const startTime = 1620051602238; | ||||
|   const timeValues = values.map((_, index) => startTime + (index + 1) * 10000); | ||||
|  | @ -326,7 +388,7 @@ const createDataFrameJSON = (values: number[]): DataFrameJSON => { | |||
|   }; | ||||
| }; | ||||
| 
 | ||||
| const createQuery = (refId: string, options?: Partial<AlertQuery>): AlertQuery => { | ||||
| const createQuery = (refId: string, options?: Partial<AlertQuery<DataQuery>>): AlertQuery<DataQuery> => { | ||||
|   return defaultsDeep(options, { | ||||
|     refId, | ||||
|     queryType: '', | ||||
|  | @ -335,3 +397,16 @@ const createQuery = (refId: string, options?: Partial<AlertQuery>): AlertQuery = | |||
|     relativeTimeRange: getDefaultRelativeTimeRange(), | ||||
|   }); | ||||
| }; | ||||
| 
 | ||||
| const createExpression = ( | ||||
|   refId: string, | ||||
|   options?: Partial<AlertQuery<ExpressionQuery>> | ||||
| ): AlertQuery<ExpressionQuery> => { | ||||
|   return defaultsDeep(options, { | ||||
|     refId, | ||||
|     queryType: '', | ||||
|     datasourceUid: EXTERNAL_VANILLA_ALERTMANAGER_UID, | ||||
|     model: { refId, datasource: ExpressionDatasourceRef }, | ||||
|     relativeTimeRange: getDefaultRelativeTimeRange(), | ||||
|   }); | ||||
| }; | ||||
|  |  | |||
|  | @ -15,13 +15,14 @@ import { | |||
|   withLoadingIndicator, | ||||
| } from '@grafana/data'; | ||||
| import { DataSourceWithBackend, FetchResponse, getDataSourceSrv, toDataQueryError } from '@grafana/runtime'; | ||||
| import { t } from 'app/core/internationalization'; | ||||
| import { BackendSrv, getBackendSrv } from 'app/core/services/backend_srv'; | ||||
| import { isExpressionQuery } from 'app/features/expressions/guards'; | ||||
| import { cancelNetworkRequestsOnUnsubscribe } from 'app/features/query/state/processing/canceler'; | ||||
| import { setStructureRevision } from 'app/features/query/state/processing/revision'; | ||||
| import { AlertQuery } from 'app/types/unified-alerting-dto'; | ||||
| 
 | ||||
| import { createDagFromQueries, getDescendants } from '../components/rule-editor/dag'; | ||||
| import { LinkError, createDAGFromQueriesSafe, getDescendants } from '../components/rule-editor/dag'; | ||||
| import { getTimeRangeForExpression } from '../utils/timeRange'; | ||||
| 
 | ||||
| export interface AlertingQueryResult { | ||||
|  | @ -53,11 +54,17 @@ export class AlertingQueryRunner { | |||
|   async run(queries: AlertQuery[], condition: string) { | ||||
|     const queriesToRun = await this.prepareQueries(queries); | ||||
| 
 | ||||
|     // if we don't have any queries to run we just bail
 | ||||
|     if (queriesToRun.length === 0) { | ||||
|       return; | ||||
|     } | ||||
| 
 | ||||
|     this.subscription = runRequest(this.backendSrv, queriesToRun, condition).subscribe({ | ||||
|     // if the condition isn't part of the queries to run, try to run the alert rule without it.
 | ||||
|     // It indicates that the "condition" node points to a non-existent node. We still want to be able to evaluate the other nodes.
 | ||||
|     const isConditionAvailable = queriesToRun.some((query) => query.refId === condition); | ||||
|     const ruleCondition = isConditionAvailable ? condition : ''; | ||||
| 
 | ||||
|     this.subscription = runRequest(this.backendSrv, queriesToRun, ruleCondition).subscribe({ | ||||
|       next: (dataPerQuery) => { | ||||
|         const nextResult = applyChange(dataPerQuery, (refId, data) => { | ||||
|           const previous = this.lastResult[refId]; | ||||
|  | @ -65,6 +72,12 @@ export class AlertingQueryRunner { | |||
|           return setStructureRevision(preProcessed, previous); | ||||
|         }); | ||||
| 
 | ||||
|         // add link errors to the panelData and mark them as errors
 | ||||
|         const [_, linkErrors] = createDAGFromQueriesSafe(queries); | ||||
|         linkErrors.forEach((linkError) => { | ||||
|           nextResult[linkError.source] = createLinkErrorPanelData(linkError); | ||||
|         }); | ||||
| 
 | ||||
|         this.lastResult = nextResult; | ||||
|         this.subject.next(this.lastResult); | ||||
|       }, | ||||
|  | @ -78,17 +91,14 @@ export class AlertingQueryRunner { | |||
| 
 | ||||
|   // this function will omit any invalid queries and all of its descendants from the list of queries
 | ||||
|   // to do this we will convert the list of queries into a DAG and walk the invalid node's output edges recursively
 | ||||
|   async prepareQueries(queries: AlertQuery[]) { | ||||
|   async prepareQueries(queries: AlertQuery[]): Promise<AlertQuery[]> { | ||||
|     const queriesToExclude: string[] = []; | ||||
| 
 | ||||
|     // convert our list of queries to a graph
 | ||||
|     const queriesGraph = createDagFromQueries(queries); | ||||
| 
 | ||||
|     // find all invalid nodes and omit those and their child nodes from the final queries array
 | ||||
|     // ⚠️ also make sure all dependent nodes are omitted, otherwise we will be evaluating a broken graph with missing references
 | ||||
|     // find all invalid nodes and omit those
 | ||||
|     for (const query of queries) { | ||||
|       const refId = query.model.refId; | ||||
| 
 | ||||
|       // expression queries cannot be excluded / filtered out
 | ||||
|       if (isExpressionQuery(query.model)) { | ||||
|         continue; | ||||
|       } | ||||
|  | @ -100,12 +110,28 @@ export class AlertingQueryRunner { | |||
|         !dataSourceInstance.filterQuery(query.model); | ||||
| 
 | ||||
|       if (skipRunningQuery) { | ||||
|         const descendants = getDescendants(refId, queriesGraph); | ||||
|         queriesToExclude.push(refId, ...descendants); | ||||
|         queriesToExclude.push(refId); | ||||
|       } | ||||
|     } | ||||
| 
 | ||||
|     return reject(queries, (q) => queriesToExclude.includes(q.model.refId)); | ||||
|     // exclude nodes that failed to link and their child nodes from the final queries array by trying to parse the graph
 | ||||
|     // ⚠️ also make sure all dependent nodes are omitted, otherwise we will be evaluating a broken graph with missing references
 | ||||
|     const [cleanGraph] = createDAGFromQueriesSafe(queries); | ||||
|     const cleanNodes = Object.keys(cleanGraph.nodes); | ||||
| 
 | ||||
|     // find descendant nodes of data queries that have been excluded
 | ||||
|     queriesToExclude.forEach((refId) => { | ||||
|       const descendants = getDescendants(refId, cleanGraph); | ||||
|       queriesToExclude.push(...descendants); | ||||
|     }); | ||||
| 
 | ||||
|     // also exclude all nodes that aren't in cleanGraph, this means they point to other broken nodes
 | ||||
|     const nodesNotInGraph = queries.filter((query) => !cleanNodes.includes(query.refId)); | ||||
|     nodesNotInGraph.forEach((node) => { | ||||
|       queriesToExclude.push(node.refId); | ||||
|     }); | ||||
| 
 | ||||
|     return reject(queries, (query) => queriesToExclude.includes(query.refId)); | ||||
|   } | ||||
| 
 | ||||
|   cancel() { | ||||
|  | @ -219,7 +245,10 @@ const mapToPanelData = ( | |||
| const mapErrorToPanelData = (lastResult: Record<string, PanelData>, error: Error): Record<string, PanelData> => { | ||||
|   const queryError = toDataQueryError(error); | ||||
| 
 | ||||
|   return applyChange(lastResult, (refId, data) => { | ||||
|   return applyChange(lastResult, (_refId, data) => { | ||||
|     if (data.state === LoadingState.Error) { | ||||
|       return data; | ||||
|     } | ||||
|     return { | ||||
|       ...data, | ||||
|       state: LoadingState.Error, | ||||
|  | @ -240,3 +269,29 @@ const applyChange = ( | |||
| 
 | ||||
|   return nextResult; | ||||
| }; | ||||
| 
 | ||||
| const createLinkErrorPanelData = (error: LinkError): PanelData => ({ | ||||
|   series: [], | ||||
|   state: LoadingState.Error, | ||||
|   errors: [ | ||||
|     { | ||||
|       message: createLinkErrorMessage(error), | ||||
|     }, | ||||
|   ], | ||||
|   timeRange: getDefaultTimeRange(), | ||||
| }); | ||||
| 
 | ||||
| function createLinkErrorMessage(error: LinkError): string { | ||||
|   const isSelfReference = error.source === error.target; | ||||
| 
 | ||||
|   return isSelfReference | ||||
|     ? t('alerting.dag.self-reference', "You can't link an expression to itself") | ||||
|     : t( | ||||
|         'alerting.dag.missing-reference', | ||||
|         `Expression "{{source}}" failed to run because "{{target}}" is missing or also failed.`, | ||||
|         { | ||||
|           source: error.source, | ||||
|           target: error.target, | ||||
|         } | ||||
|       ); | ||||
| } | ||||
|  |  | |||
|  | @ -334,6 +334,10 @@ | |||
|       "label": "Contact point" | ||||
|     }, | ||||
|     "copy-to-clipboard": "Copy \"{{label}}\" to clipboard", | ||||
|     "dag": { | ||||
|       "missing-reference": "Expression \"{{source}}\" failed to run because \"{{target}}\" is missing or also failed.", | ||||
|       "self-reference": "You can't link an expression to itself" | ||||
|     }, | ||||
|     "export": { | ||||
|       "subtitle": { | ||||
|         "formats": "Select the format and download the file or copy the contents to clipboard", | ||||
|  | @ -566,6 +570,14 @@ | |||
|       "paused": "Paused", | ||||
|       "recording-rule": "Recording rule" | ||||
|     }, | ||||
|     "rule-view": { | ||||
|       "query": { | ||||
|         "datasources-na": { | ||||
|           "description": "Cannot display the query preview. Some of the data sources used in the queries are not available.", | ||||
|           "title": "Query not available" | ||||
|         } | ||||
|       } | ||||
|     }, | ||||
|     "rule-viewer": { | ||||
|       "prometheus-consistency-check": { | ||||
|         "alert-message": "Alert rule has been updated. Changes may take up to a minute to appear on the Alert rules list view.", | ||||
|  |  | |||
|  | @ -334,6 +334,10 @@ | |||
|       "label": "Cőʼnŧäčŧ pőįʼnŧ" | ||||
|     }, | ||||
|     "copy-to-clipboard": "Cőpy \"{{label}}\" ŧő čľįpþőäřđ", | ||||
|     "dag": { | ||||
|       "missing-reference": "Ēχpřęşşįőʼn \"{{source}}\" ƒäįľęđ ŧő řūʼn þęčäūşę \"{{target}}\" įş mįşşįʼnģ őř äľşő ƒäįľęđ.", | ||||
|       "self-reference": "Ÿőū čäʼn'ŧ ľįʼnĸ äʼn ęχpřęşşįőʼn ŧő įŧşęľƒ" | ||||
|     }, | ||||
|     "export": { | ||||
|       "subtitle": { | ||||
|         "formats": "Ŝęľęčŧ ŧĥę ƒőřmäŧ äʼnđ đőŵʼnľőäđ ŧĥę ƒįľę őř čőpy ŧĥę čőʼnŧęʼnŧş ŧő čľįpþőäřđ", | ||||
|  | @ -566,6 +570,14 @@ | |||
|       "paused": "Päūşęđ", | ||||
|       "recording-rule": "Ŗęčőřđįʼnģ řūľę" | ||||
|     }, | ||||
|     "rule-view": { | ||||
|       "query": { | ||||
|         "datasources-na": { | ||||
|           "description": "Cäʼnʼnőŧ đįşpľäy ŧĥę qūęřy přęvįęŵ. Ŝőmę őƒ ŧĥę đäŧä şőūřčęş ūşęđ įʼn ŧĥę qūęřįęş äřę ʼnőŧ äväįľäþľę.", | ||||
|           "title": "Qūęřy ʼnőŧ äväįľäþľę" | ||||
|         } | ||||
|       } | ||||
|     }, | ||||
|     "rule-viewer": { | ||||
|       "prometheus-consistency-check": { | ||||
|         "alert-message": "Åľęřŧ řūľę ĥäş þęęʼn ūpđäŧęđ. Cĥäʼnģęş mäy ŧäĸę ūp ŧő ä mįʼnūŧę ŧő äppęäř őʼn ŧĥę Åľęřŧ řūľęş ľįşŧ vįęŵ.", | ||||
|  |  | |||
		Loading…
	
		Reference in New Issue