Prometheus: Query builder - show warning when converting ambiguous order of operations (#75256)

* throw error when converting into visual query when parser detects that an aggregation expression node contains a function that contains a binary expression to detect queries that are ambiguously parsed

public/app/plugins/datasource/prometheus/querybuilder/components/PromQueryEditorSelector.tsx

@@ -96,8 +96,8 @@ export const PromQueryEditorSelector = React.memo<Props>((props) => {
     <>
       <ConfirmModal
         isOpen={parseModalOpen}
-        title="Query parsing"
-        body="There were errors while trying to parse the query. Continuing to visual builder may lose some parts of the query."
+        title="Parsing error: Switch to the builder mode?"
+        body="There is a syntax error, or the query structure cannot be visualized when switching to the builder mode. Parts of the query may be lost. "
         confirmText="Continue"
         onConfirm={() => {
           changeEditorMode(query, QueryEditorMode.Builder, onChange);

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({

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