diff --git a/public/app/plugins/datasource/prometheus/querybuilder/components/PromQueryEditorSelector.tsx b/public/app/plugins/datasource/prometheus/querybuilder/components/PromQueryEditorSelector.tsx index e7861247fd3..63108bfea33 100644 --- a/public/app/plugins/datasource/prometheus/querybuilder/components/PromQueryEditorSelector.tsx +++ b/public/app/plugins/datasource/prometheus/querybuilder/components/PromQueryEditorSelector.tsx @@ -96,8 +96,8 @@ export const PromQueryEditorSelector = React.memo((props) => { <> { changeEditorMode(query, QueryEditorMode.Builder, onChange); diff --git a/public/app/plugins/datasource/prometheus/querybuilder/parsing.test.ts b/public/app/plugins/datasource/prometheus/querybuilder/parsing.test.ts index c938cc0911a..08fa72b7b8a 100644 --- a/public/app/plugins/datasource/prometheus/querybuilder/parsing.test.ts +++ b/public/app/plugins/datasource/prometheus/querybuilder/parsing.test.ts @@ -71,6 +71,72 @@ describe('buildVisualQueryFromString', () => { ); }); + describe('nested binary operation errors in visual query editor', () => { + // Visual query builder does not currently have support for nested binary operations, for now we should throw an error in the UI letting users know that their query will be misinterpreted + it('throws error when visual query parse is ambiguous', () => { + expect( + buildVisualQueryFromString('topk(5, node_arp_entries / node_arp_entries{cluster="dev-eu-west-2"})') + ).toMatchObject({ + errors: [ + { + from: 8, + text: 'Query parsing is ambiguous.', + to: 68, + }, + ], + }); + }); + it('throws error when visual query parse with aggregation is ambiguous (scalar)', () => { + expect(buildVisualQueryFromString('topk(5, 1 / 2)')).toMatchObject({ + errors: [ + { + from: 8, + text: 'Query parsing is ambiguous.', + to: 13, + }, + ], + }); + }); + it('throws error when visual query parse with functionCall is ambiguous', () => { + expect( + buildVisualQueryFromString( + 'clamp_min(sum by(cluster)(rate(X{le="2.5"}[5m]))+sum by (cluster) (rate(X{le="5"}[5m])), 0.001)' + ) + ).toMatchObject({ + errors: [ + { + from: 10, + text: 'Query parsing is ambiguous.', + to: 87, + }, + ], + }); + }); + it('does not throw error when visual query parse is unambiguous', () => { + expect( + buildVisualQueryFromString('topk(5, node_arp_entries) / node_arp_entries{cluster="dev-eu-west-2"}') + ).toMatchObject({ + errors: [], + }); + }); + it('does not throw error when visual query parse is unambiguous (scalar)', () => { + // Note this topk query with scalars is not valid in prometheus, but it does not currently throw an error during parse + expect(buildVisualQueryFromString('topk(5, 1) / 2')).toMatchObject({ + errors: [], + }); + }); + it('does not throw error when visual query parse is unambiguous, function call', () => { + // Note this topk query with scalars is not valid in prometheus, but it does not currently throw an error during parse + expect( + buildVisualQueryFromString( + 'clamp_min(sum by(cluster) (rate(X{le="2.5"}[5m])), 0.001) + sum by(cluster) (rate(X{le="5"}[5m]))' + ) + ).toMatchObject({ + errors: [], + }); + }); + }); + it('parses query with rate and interval', () => { expect(buildVisualQueryFromString('rate(counters_logins{app="frontend"}[5m])')).toEqual( noErrors({ diff --git a/public/app/plugins/datasource/prometheus/querybuilder/parsing.ts b/public/app/plugins/datasource/prometheus/querybuilder/parsing.ts index f21c768685e..79792b8afe2 100644 --- a/public/app/plugins/datasource/prometheus/querybuilder/parsing.ts +++ b/public/app/plugins/datasource/prometheus/querybuilder/parsing.ts @@ -288,6 +288,16 @@ function handleAggregation(expr: string, node: SyntaxNode, context: Context) { const body = node.getChild(FunctionCallBody); const callArgs = body!.getChild(FunctionCallArgs); + const callArgsExprChild = callArgs?.getChild(Expr); + const binaryExpressionWithinAggregationArgs = callArgsExprChild?.getChild(BinaryExpr); + + if (binaryExpressionWithinAggregationArgs) { + context.errors.push({ + text: 'Query parsing is ambiguous.', + from: binaryExpressionWithinAggregationArgs.from, + to: binaryExpressionWithinAggregationArgs.to, + }); + } const op: QueryBuilderOperation = { id: funcName, params: [] }; visQuery.operations.unshift(op); @@ -318,10 +328,23 @@ function updateFunctionArgs(expr: string, node: SyntaxNode | null, context: Cont // FunctionCallArgs are nested bit weirdly as mentioned so we have to go one deeper in this case. case FunctionCallArgs: { let child = node.firstChild; + while (child) { + const callArgsExprChild = child.getChild(Expr); + const binaryExpressionWithinFunctionArgs = callArgsExprChild?.getChild(BinaryExpr); + + if (binaryExpressionWithinFunctionArgs) { + context.errors.push({ + text: 'Query parsing is ambiguous.', + from: binaryExpressionWithinFunctionArgs.from, + to: binaryExpressionWithinFunctionArgs.to, + }); + } + updateFunctionArgs(expr, child, context, op); child = child.nextSibling; } + break; }