From 91398a173f51fa4b3ef5d44c0fb1fc48b7416cd7 Mon Sep 17 00:00:00 2001 From: Matias Chomicki Date: Tue, 31 Oct 2023 16:30:02 +0100 Subject: [PATCH] Query Validation: accept any parser to validate query and integrate with Prom query editor (#77081) * Validation: make it generic for any parser * Prometheus: integrate query validator with editor * Prom validation: add unit test * Run prettier --- .../monaco-query-field/MonacoQueryField.tsx | 4 +- .../validation.test.ts | 20 +++-- .../monaco-completion-provider/validation.ts | 12 +-- .../prometheus/components/PromQueryField.tsx | 1 + .../monaco-query-field/MonacoQueryField.tsx | 32 ++++++- .../MonacoQueryFieldProps.ts | 2 + .../validation.test.ts | 85 +++++++++++++++++++ .../datasource/prometheus/datasource.ts | 4 +- 8 files changed, 142 insertions(+), 18 deletions(-) create mode 100644 public/app/plugins/datasource/prometheus/components/monaco-query-field/monaco-completion-provider/validation.test.ts diff --git a/public/app/plugins/datasource/loki/components/monaco-query-field/MonacoQueryField.tsx b/public/app/plugins/datasource/loki/components/monaco-query-field/MonacoQueryField.tsx index 45cf68b77fc..1ce1c8ed76a 100644 --- a/public/app/plugins/datasource/loki/components/monaco-query-field/MonacoQueryField.tsx +++ b/public/app/plugins/datasource/loki/components/monaco-query-field/MonacoQueryField.tsx @@ -6,6 +6,7 @@ import { v4 as uuidv4 } from 'uuid'; import { GrafanaTheme2 } from '@grafana/data'; import { selectors } from '@grafana/e2e-selectors'; +import { parser } from '@grafana/lezer-logql'; import { languageConfiguration, monarchlanguage } from '@grafana/monaco-logql'; import { useTheme2, ReactMonacoEditor, Monaco, monacoTypes, MonacoEditor } from '@grafana/ui'; @@ -187,7 +188,8 @@ const MonacoQueryField = ({ history, onBlur, onRunQuery, initialValue, datasourc validateQuery( query, datasource.interpolateString(query, placeHolderScopedVars), - model.getLinesContent() + model.getLinesContent(), + parser ) || []; const markers = errors.map(({ error, ...boundary }) => ({ diff --git a/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/validation.test.ts b/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/validation.test.ts index 837775a5c72..d11938aff2a 100644 --- a/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/validation.test.ts +++ b/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/validation.test.ts @@ -1,18 +1,20 @@ +import { parser } from '@grafana/lezer-logql'; + import { validateQuery } from './validation'; describe('Monaco Query Validation', () => { test('Identifies empty queries as valid', () => { - expect(validateQuery('', '', [])).toBeFalsy(); + expect(validateQuery('', '', [], parser)).toBeFalsy(); }); test('Identifies valid queries', () => { const query = '{place="luna"}'; - expect(validateQuery(query, query, [])).toBeFalsy(); + expect(validateQuery(query, query, [], parser)).toBeFalsy(); }); test('Validates logs queries', () => { let query = '{place="incomplete"'; - expect(validateQuery(query, query, [query])).toEqual([ + expect(validateQuery(query, query, [query], parser)).toEqual([ { endColumn: 20, endLineNumber: 1, @@ -23,7 +25,7 @@ describe('Monaco Query Validation', () => { ]); query = '{place="luna"} | notaparser'; - expect(validateQuery(query, query, [query])).toEqual([ + expect(validateQuery(query, query, [query], parser)).toEqual([ { endColumn: 28, endLineNumber: 1, @@ -34,7 +36,7 @@ describe('Monaco Query Validation', () => { ]); query = '{place="luna"} | logfmt |'; - expect(validateQuery(query, query, [query])).toEqual([ + expect(validateQuery(query, query, [query], parser)).toEqual([ { endColumn: 26, endLineNumber: 1, @@ -47,7 +49,7 @@ describe('Monaco Query Validation', () => { test('Validates metric queries', () => { let query = 'sum(count_over_time({place="luna" | unwrap request_time [5m])) by (level)'; - expect(validateQuery(query, query, [query])).toEqual([ + expect(validateQuery(query, query, [query], parser)).toEqual([ { endColumn: 35, endLineNumber: 1, @@ -58,7 +60,7 @@ describe('Monaco Query Validation', () => { ]); query = 'sum(count_over_time({place="luna"} | unwrap [5m])) by (level)'; - expect(validateQuery(query, query, [query])).toEqual([ + expect(validateQuery(query, query, [query], parser)).toEqual([ { endColumn: 45, endLineNumber: 1, @@ -69,7 +71,7 @@ describe('Monaco Query Validation', () => { ]); query = 'sum()'; - expect(validateQuery(query, query, [query])).toEqual([ + expect(validateQuery(query, query, [query], parser)).toEqual([ { endColumn: 5, endLineNumber: 1, @@ -88,7 +90,7 @@ describe('Monaco Query Validation', () => { unpack fail |= "a"`; const queryLines = query.split('\n'); - expect(validateQuery(query, query, queryLines)).toEqual([ + expect(validateQuery(query, query, queryLines, parser)).toEqual([ { endColumn: 12, endLineNumber: 5, diff --git a/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/validation.ts b/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/validation.ts index 117c012876a..59b4572a767 100644 --- a/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/validation.ts +++ b/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/validation.ts @@ -1,6 +1,6 @@ import { SyntaxNode } from '@lezer/common'; +import { LRParser } from '@lezer/lr'; -import { parser } from '@grafana/lezer-logql'; import { ErrorId } from 'app/plugins/datasource/prometheus/querybuilder/shared/parsingUtils'; interface ParserErrorBoundary { @@ -26,7 +26,8 @@ interface ParseError { export function validateQuery( query: string, interpolatedQuery: string, - queryLines: string[] + queryLines: string[], + parser: LRParser ): ParserErrorBoundary[] | false { if (!query) { return false; @@ -39,14 +40,14 @@ export function validateQuery( * have different lengths. With this, we also exclude irrelevant parser errors that are produced by * lezer not understanding $variables and $__variables, which usually generate 2 or 3 error SyntaxNode. */ - const interpolatedErrors: ParseError[] = parseQuery(interpolatedQuery); + const interpolatedErrors: ParseError[] = parseQuery(interpolatedQuery, parser); if (!interpolatedErrors.length) { return false; } let parseErrors: ParseError[] = interpolatedErrors; if (query !== interpolatedQuery) { - const queryErrors: ParseError[] = parseQuery(query); + const queryErrors: ParseError[] = parseQuery(query, parser); parseErrors = interpolatedErrors.flatMap( (interpolatedError) => queryErrors.filter((queryError) => interpolatedError.text === queryError.text) || interpolatedError @@ -56,7 +57,7 @@ export function validateQuery( return parseErrors.map((parseError) => findErrorBoundary(query, queryLines, parseError)).filter(isErrorBoundary); } -function parseQuery(query: string) { +function parseQuery(query: string, parser: LRParser) { const parseErrors: ParseError[] = []; const tree = parser.parse(query); tree.iterate({ @@ -115,6 +116,7 @@ function isErrorBoundary(boundary: ParserErrorBoundary | null): boundary is Pars export const placeHolderScopedVars = { __interval: { text: '1s', value: '1s' }, + __rate_interval: { text: '1s', value: '1s' }, __auto: { text: '1s', value: '1s' }, __interval_ms: { text: '1000', value: 1000 }, __range_ms: { text: '1000', value: 1000 }, diff --git a/public/app/plugins/datasource/prometheus/components/PromQueryField.tsx b/public/app/plugins/datasource/prometheus/components/PromQueryField.tsx index 4162dfca2e4..97e010b0b5f 100644 --- a/public/app/plugins/datasource/prometheus/components/PromQueryField.tsx +++ b/public/app/plugins/datasource/prometheus/components/PromQueryField.tsx @@ -243,6 +243,7 @@ class PromQueryField extends React.PureComponent diff --git a/public/app/plugins/datasource/prometheus/components/monaco-query-field/MonacoQueryField.tsx b/public/app/plugins/datasource/prometheus/components/monaco-query-field/MonacoQueryField.tsx index d79208b1efb..c9d72392276 100644 --- a/public/app/plugins/datasource/prometheus/components/monaco-query-field/MonacoQueryField.tsx +++ b/public/app/plugins/datasource/prometheus/components/monaco-query-field/MonacoQueryField.tsx @@ -1,4 +1,5 @@ import { css } from '@emotion/css'; +import { parser } from '@prometheus-io/lezer-promql'; import { debounce } from 'lodash'; import { promLanguageDefinition } from 'monaco-promql'; import React, { useRef, useEffect } from 'react'; @@ -8,6 +9,10 @@ import { v4 as uuidv4 } from 'uuid'; import { GrafanaTheme2 } from '@grafana/data'; import { selectors } from '@grafana/e2e-selectors'; import { useTheme2, ReactMonacoEditor, Monaco, monacoTypes } from '@grafana/ui'; +import { + placeHolderScopedVars, + validateQuery, +} from 'app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/validation'; import { Props } from './MonacoQueryFieldProps'; import { getOverrideServices } from './getOverrideServices'; @@ -95,7 +100,7 @@ const MonacoQueryField = (props: Props) => { // we need only one instance of `overrideServices` during the lifetime of the react component const overrideServicesRef = useRef(getOverrideServices()); const containerRef = useRef(null); - const { languageProvider, history, onBlur, onRunQuery, initialValue, placeholder, onChange } = props; + const { languageProvider, history, onBlur, onRunQuery, initialValue, placeholder, onChange, datasource } = props; const lpRef = useLatest(languageProvider); const historyRef = useLatest(history); @@ -281,6 +286,31 @@ const MonacoQueryField = (props: Props) => { checkDecorators(); editor.onDidChangeModelContent(checkDecorators); + + editor.onDidChangeModelContent((e) => { + const model = editor.getModel(); + if (!model) { + return; + } + const query = model.getValue(); + const errors = + validateQuery( + query, + datasource.interpolateString(query, placeHolderScopedVars), + model.getLinesContent(), + parser + ) || []; + + const markers = errors.map(({ error, ...boundary }) => ({ + message: `${ + error ? `Error parsing "${error}"` : 'Parse error' + }. The query appears to be incorrect and could fail to be executed.`, + severity: monaco.MarkerSeverity.Error, + ...boundary, + })); + + monaco.editor.setModelMarkers(model, 'owner', markers); + }); } }} /> diff --git a/public/app/plugins/datasource/prometheus/components/monaco-query-field/MonacoQueryFieldProps.ts b/public/app/plugins/datasource/prometheus/components/monaco-query-field/MonacoQueryFieldProps.ts index 3c5f17fa23d..1383b565828 100644 --- a/public/app/plugins/datasource/prometheus/components/monaco-query-field/MonacoQueryFieldProps.ts +++ b/public/app/plugins/datasource/prometheus/components/monaco-query-field/MonacoQueryFieldProps.ts @@ -1,5 +1,6 @@ import { HistoryItem } from '@grafana/data'; +import { PrometheusDatasource } from '../../datasource'; import type PromQlLanguageProvider from '../../language_provider'; import { PromQuery } from '../../types'; @@ -16,4 +17,5 @@ export type Props = { onBlur: (value: string) => void; // onChange will never initiate a query, it just denotes that a query value has been changed onChange: (value: string) => void; + datasource: PrometheusDatasource; }; diff --git a/public/app/plugins/datasource/prometheus/components/monaco-query-field/monaco-completion-provider/validation.test.ts b/public/app/plugins/datasource/prometheus/components/monaco-query-field/monaco-completion-provider/validation.test.ts new file mode 100644 index 00000000000..2fa5706412d --- /dev/null +++ b/public/app/plugins/datasource/prometheus/components/monaco-query-field/monaco-completion-provider/validation.test.ts @@ -0,0 +1,85 @@ +import { parser } from '@prometheus-io/lezer-promql'; + +import { validateQuery } from 'app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/validation'; + +describe('Monaco Query Validation', () => { + test('Identifies empty queries as valid', () => { + expect(validateQuery('', '', [], parser)).toBeFalsy(); + }); + + test.each([ + 'access_evaluation_duration_sum{job="grafana"}', + 'http_requests_total{job="apiserver", handler="/api/comments"}[5m]', + 'http_requests_total{job=~".*server"}', + 'rate(http_requests_total[5m])[30m:1m]', + 'max_over_time(deriv(rate(distance_covered_total[5s])[30s:5s])[10m:])', + 'rate(http_requests_total[5m])', + 'topk(3, sum by (app, proc) (rate(instance_cpu_time_ns[5m])))', + ])('Identifies valid queries', (query: string) => { + expect(validateQuery(query, query, [], parser)).toBeFalsy(); + }); + + test('Identifies invalid queries', () => { + // Missing } at the end + let query = 'access_evaluation_duration_sum{job="grafana"'; + expect(validateQuery(query, query, [query], parser)).toEqual([ + { + endColumn: 45, + endLineNumber: 1, + error: '{job="grafana"', + startColumn: 31, + startLineNumber: 1, + }, + ]); + + // Missing handler="value" + query = 'http_requests_total{job="apiserver", handler}[5m]'; + expect(validateQuery(query, query, [query], parser)).toEqual([ + { + endColumn: 45, + endLineNumber: 1, + error: 'handler', + startColumn: 38, + startLineNumber: 1, + }, + ]); + + // Missing : in [30s:5s] + query = 'max_over_time(deriv(rate(distance_covered_total[5s])[30s5s])[10m:])'; + expect(validateQuery(query, query, [query], parser)).toEqual([ + { + endColumn: 60, + endLineNumber: 1, + error: 'rate(distance_covered_total[5s])[30s5s]', + startColumn: 21, + startLineNumber: 1, + }, + ]); + }); + + test('Identifies valid multi-line queries', () => { + const query = ` +sum by (job) ( + rate(http_requests_total[5m]) +)`; + const queryLines = query.split('\n'); + expect(validateQuery(query, query, queryLines, parser)).toBeFalsy(); + }); + + test('Identifies invalid multi-line queries', () => { + const query = ` +sum by (job) ( + rate(http_requests_total[]) +)`; + const queryLines = query.split('\n'); + expect(validateQuery(query, query, queryLines, parser)).toEqual([ + { + endColumn: 30, + endLineNumber: 3, + error: '', + startColumn: 30, + startLineNumber: 3, + }, + ]); + }); +}); diff --git a/public/app/plugins/datasource/prometheus/datasource.ts b/public/app/plugins/datasource/prometheus/datasource.ts index 12fdd812864..ccecbe67f29 100644 --- a/public/app/plugins/datasource/prometheus/datasource.ts +++ b/public/app/plugins/datasource/prometheus/datasource.ts @@ -1203,8 +1203,8 @@ export class PrometheusDatasource return this.templateSrv.getVariables().map((v) => `$${v.name}`); } - interpolateString(string: string) { - return this.templateSrv.replace(string, undefined, this.interpolateQueryExpr); + interpolateString(string: string, scopedVars?: ScopedVars) { + return this.templateSrv.replace(string, scopedVars, this.interpolateQueryExpr); } getDebounceTimeInMilliseconds(): number {