Prometheus: Use single string expr as a state for the visual editor (#45232)

* Use just expr string as a state for whole editor

* Fix patterns

* Fix tests

public/app/plugins/datasource/prometheus/querybuilder/components/PromQueryBuilderContainer.test.tsx

@@ -0,0 +1,42 @@
+import React from 'react';
+import { render, screen } from '@testing-library/react';
+import { PromQueryBuilderContainer } from './PromQueryBuilderContainer';
+import { PrometheusDatasource } from '../../datasource';
+import { EmptyLanguageProviderMock } from '../../language_provider.mock';
+import PromQlLanguageProvider from '../../language_provider';
+import { addOperation } from '../shared/OperationList.testUtils';
+
+describe('PromQueryBuilderContainer', () => {
+  it('translates query between string and model', async () => {
+    const props = {
+      query: {
+        expr: 'metric_test{job="testjob"}',
+        refId: 'A',
+      },
+      datasource: new PrometheusDatasource(
+        {
+          id: 1,
+          uid: '',
+          type: 'prometheus',
+          name: 'prom-test',
+          access: 'proxy',
+          url: '',
+          jsonData: {},
+          meta: {} as any,
+        },
+        undefined,
+        undefined,
+        new EmptyLanguageProviderMock() as unknown as PromQlLanguageProvider
+      ),
+      onChange: jest.fn(),
+      onRunQuery: () => {},
+    };
+    render(<PromQueryBuilderContainer {...props} />);
+    expect(screen.getByText('metric_test')).toBeInTheDocument();
+    addOperation('Range functions', 'Rate');
+    expect(props.onChange).toBeCalledWith({
+      expr: 'rate(metric_test{job="testjob"}[$__rate_interval])',
+      refId: 'A',
+    });
+  });
+});

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

@@ -0,0 +1,43 @@
+import { CoreApp } from '@grafana/data';
+import React from 'react';
+
+import { PrometheusDatasource } from '../../datasource';
+import { PromQuery } from '../../types';
+import { buildVisualQueryFromString } from '../parsing';
+import { promQueryModeller } from '../PromQueryModeller';
+import { PromQueryBuilder } from './PromQueryBuilder';
+import { PromQueryBuilderOptions } from './PromQueryBuilderOptions';
+import { QueryPreview } from './QueryPreview';
+import { PromVisualQuery } from '../types';
+
+export interface Props {
+  query: PromQuery;
+  datasource: PrometheusDatasource;
+  onChange: (update: PromQuery) => void;
+  onRunQuery: () => void;
+  app?: CoreApp;
+}
+
+/**
+ * This component is here just to contain the translation logic between string query and the visual query builder model.
+ * @param props
+ * @constructor
+ */
+export function PromQueryBuilderContainer(props: Props) {
+  const { query, onChange, onRunQuery, datasource, app } = props;
+
+  const visQuery = buildVisualQueryFromString(query.expr || '').query;
+
+  const onVisQueryChange = (newVisQuery: PromVisualQuery) => {
+    const rendered = promQueryModeller.renderQuery(newVisQuery);
+    onChange({ ...query, expr: rendered });
+  };
+
+  return (
+    <>
+      <PromQueryBuilder query={visQuery} datasource={datasource} onChange={onVisQueryChange} onRunQuery={onRunQuery} />
+      {query.editorPreview && <QueryPreview query={query.expr} />}
+      <PromQueryBuilderOptions query={query} app={app} onChange={onChange} onRunQuery={onRunQuery} />
+    </>
+  );
+}

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

@@ -4,19 +4,25 @@ import { Stack } from '@grafana/experimental';
 import { promQueryModeller } from '../PromQueryModeller';
 import { OperationListExplained } from '../shared/OperationListExplained';
 import { OperationExplainedBox } from '../shared/OperationExplainedBox';
+import { buildVisualQueryFromString } from '../parsing';
 
 export interface Props {
-  query: PromVisualQuery;
+  query: string;
   nested?: boolean;
 }
 
 export const PromQueryBuilderExplained = React.memo<Props>(({ query, nested }) => {
+  const visQuery = buildVisualQueryFromString(query || '').query;
+
   return (
     <Stack gap={0} direction="column">
-      <OperationExplainedBox stepNumber={1} title={`${query.metric} ${promQueryModeller.renderLabels(query.labels)}`}>
+      <OperationExplainedBox
+        stepNumber={1}
+        title={`${visQuery.metric} ${promQueryModeller.renderLabels(visQuery.labels)}`}
+      >
         Fetch all series matching metric name and label filters.
       </OperationExplainedBox>
-      <OperationListExplained<PromVisualQuery> stepNumber={2} queryModeller={promQueryModeller} query={query} />
+      <OperationListExplained<PromVisualQuery> stepNumber={2} queryModeller={promQueryModeller} query={visQuery} />
     </Stack>
   );
 });

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

@@ -71,6 +71,7 @@ describe('PromQueryEditorSelector', () => {
 
   it('shows builder when builder mode is set', async () => {
     renderWithMode(QueryEditorMode.Builder);
+    screen.debug(undefined, 20000);
     expectBuilder();
   });
 
@@ -86,14 +87,6 @@ describe('PromQueryEditorSelector', () => {
       refId: 'A',
       expr: defaultQuery.expr,
       editorMode: QueryEditorMode.Builder,
-      visualQuery: {
-        labels: [
-          { label: 'label1', op: '=', value: 'foo' },
-          { label: 'label2', op: '=', value: 'bar' },
-        ],
-        metric: 'metric',
-        operations: [],
-      },
     });
   });
 
@@ -115,11 +108,7 @@ describe('PromQueryEditorSelector', () => {
     renderWithProps({
       editorPreview: true,
       editorMode: QueryEditorMode.Builder,
-      visualQuery: {
+      expr: 'my_metric',
-        metric: 'my_metric',
-        labels: [],
-        operations: [],
-      },
     });
     expect(screen.getByLabelText('selector').textContent).toBe('my_metric');
   });
@@ -187,7 +176,7 @@ function expectCodeEditor() {
 }
 
 function expectBuilder() {
-  expect(screen.getByText('Select metric')).toBeInTheDocument();
+  expect(screen.getByText('Metric')).toBeInTheDocument();
 }
 
 function expectExplain() {

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

@@ -2,7 +2,7 @@ import { css } from '@emotion/css';
 import { GrafanaTheme2, LoadingState } from '@grafana/data';
 import { EditorHeader, EditorRows, FlexItem, InlineSelect, Space } from '@grafana/experimental';
 import { Button, ConfirmModal, useStyles2 } from '@grafana/ui';
-import React, { SyntheticEvent, useCallback, useState } from 'react';
+import React, { SyntheticEvent, useCallback, useEffect, useState } from 'react';
 
 import { PromQueryEditor } from '../../components/PromQueryEditor';
 import { PromQueryEditorProps } from '../../components/types';
@@ -10,51 +10,32 @@ import { promQueryModeller } from '../PromQueryModeller';
 import { QueryEditorModeToggle } from '../shared/QueryEditorModeToggle';
 import { QueryHeaderSwitch } from '../shared/QueryHeaderSwitch';
 import { QueryEditorMode } from '../shared/types';
-import { getDefaultEmptyQuery, PromVisualQuery } from '../types';
-import { PromQueryBuilder } from './PromQueryBuilder';
 import { PromQueryBuilderExplained } from './PromQueryBuilderExplained';
-import { PromQueryBuilderOptions } from './PromQueryBuilderOptions';
-import { QueryPreview } from './QueryPreview';
 import { buildVisualQueryFromString } from '../parsing';
-import { PromQuery } from '../../types';
+import { PromQueryBuilderContainer } from './PromQueryBuilderContainer';
 
 export const PromQueryEditorSelector = React.memo<PromQueryEditorProps>((props) => {
   const { query, onChange, onRunQuery, data } = props;
   const styles = useStyles2(getStyles);
-  const [visualQuery, setVisualQuery] = useState<PromVisualQuery>(query.visualQuery ?? getDefaultEmptyQuery());
+
   const [parseModalOpen, setParseModalOpen] = useState(false);
-  const [pendingChange, setPendingChange] = useState<PromQuery | undefined>(undefined);
 
   const onEditorModeChange = useCallback(
     (newMetricEditorMode: QueryEditorMode) => {
       const change = { ...query, editorMode: newMetricEditorMode };
       if (newMetricEditorMode === QueryEditorMode.Builder) {
-        const result = buildVisualQueryFromString(query.expr);
+        const result = buildVisualQueryFromString(query.expr || '');
-        change.visualQuery = result.query;
         // If there are errors, give user a chance to decide if they want to go to builder as that can loose some data.
         if (result.errors.length) {
           setParseModalOpen(true);
-          setPendingChange(change);
           return;
         }
-        setVisualQuery(change.visualQuery);
       }
       onChange(change);
     },
     [onChange, query]
   );
 
-  const onChangeViewModel = (updatedQuery: PromVisualQuery) => {
-    setVisualQuery(updatedQuery);
-
-    onChange({
-      ...query,
-      expr: promQueryModeller.renderQuery(updatedQuery),
-      visualQuery: updatedQuery,
-      editorMode: QueryEditorMode.Builder,
-    });
-  };
-
   const onQueryPreviewChange = (event: SyntheticEvent<HTMLInputElement>) => {
     const isEnabled = event.currentTarget.checked;
     onChange({ ...query, editorPreview: isEnabled });
@@ -64,6 +45,12 @@ export const PromQueryEditorSelector = React.memo<PromQueryEditorProps>((props)
   // If no expr (ie new query) then default to builder
   const editorMode = query.editorMode ?? (query.expr ? QueryEditorMode.Code : QueryEditorMode.Builder);
 
+  useEffect(() => {
+    if (query.editorMode === undefined) {
+      onChange({ ...query, editorMode });
+    }
+  }, [editorMode, onChange, query]);
+
   return (
     <>
       <ConfirmModal
@@ -72,8 +59,7 @@ export const PromQueryEditorSelector = React.memo<PromQueryEditorProps>((props)
         body="There were errors while trying to parse the query. Continuing to visual builder may loose some parts of the query."
         confirmText="Continue"
         onConfirm={() => {
-          setVisualQuery(pendingChange!.visualQuery!);
+          onChange({ ...query, editorMode: QueryEditorMode.Builder });
-          onChange(pendingChange!);
           setParseModalOpen(false);
         }}
         onDismiss={() => setParseModalOpen(false)}
@@ -98,9 +84,13 @@ export const PromQueryEditorSelector = React.memo<PromQueryEditorProps>((props)
               placeholder="Query patterns"
               allowCustomValue
               onChange={({ value }) => {
-                onChangeViewModel({
+                // TODO: Bit convoluted as we don't have access to visualQuery model here. Maybe would make sense to
-                  ...visualQuery,
+                //  move it inside the editor?
-                  operations: value?.operations!,
+                const result = buildVisualQueryFromString(query.expr || '');
+                result.query.operations = value?.operations!;
+                onChange({
+                  ...query,
+                  expr: promQueryModeller.renderQuery(result.query),
                 });
               }}
               options={promQueryModeller.getQueryPatterns().map((x) => ({ label: x.name, value: x }))}
@@ -119,18 +109,14 @@ export const PromQueryEditorSelector = React.memo<PromQueryEditorProps>((props)
       <EditorRows>
         {editorMode === QueryEditorMode.Code && <PromQueryEditor {...props} />}
         {editorMode === QueryEditorMode.Builder && (
-          <>
+          <PromQueryBuilderContainer
-            <PromQueryBuilder
+            query={query}
-              query={visualQuery}
             datasource={props.datasource}
-              onChange={onChangeViewModel}
+            onChange={onChange}
             onRunQuery={props.onRunQuery}
           />
-            {query.editorPreview && <QueryPreview query={visualQuery} />}
-            <PromQueryBuilderOptions query={query} app={props.app} onChange={onChange} onRunQuery={onRunQuery} />
-          </>
         )}
-        {editorMode === QueryEditorMode.Explain && <PromQueryBuilderExplained query={visualQuery} />}
+        {editorMode === QueryEditorMode.Explain && <PromQueryBuilderExplained query={query.expr} />}
       </EditorRows>
     </>
   );

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

@@ -1,21 +1,19 @@
 import React from 'react';
-import { PromVisualQuery } from '../types';
 import { useTheme2 } from '@grafana/ui';
 import { GrafanaTheme2 } from '@grafana/data';
-import { promQueryModeller } from '../PromQueryModeller';
 import { css, cx } from '@emotion/css';
 import { EditorField, EditorFieldGroup, EditorRow } from '@grafana/experimental';
 import Prism from 'prismjs';
 import { promqlGrammar } from '../../promql';
 
 export interface Props {
-  query: PromVisualQuery;
+  query: string;
 }
 
 export function QueryPreview({ query }: Props) {
   const theme = useTheme2();
   const styles = getStyles(theme);
-  const hightlighted = Prism.highlight(promQueryModeller.renderQuery(query), promqlGrammar, 'promql');
+  const hightlighted = Prism.highlight(query, promqlGrammar, 'promql');
 
   return (
     <EditorRow>

public/app/plugins/datasource/prometheus/querybuilder/parsing.test.ts

@@ -148,6 +148,27 @@ describe('buildVisualQueryFromString', () => {
     );
   });
 
+  it('parses function with argument', () => {
+    expect(
+      buildVisualQueryFromString('histogram_quantile(0.99, rate(counters_logins{app="backend"}[$__rate_interval]))')
+    ).toEqual(
+      noErrors({
+        metric: 'counters_logins',
+        labels: [{ label: 'app', op: '=', value: 'backend' }],
+        operations: [
+          {
+            id: 'rate',
+            params: ['$__rate_interval'],
+          },
+          {
+            id: 'histogram_quantile',
+            params: [0.99],
+          },
+        ],
+      })
+    );
+  });
+
   it('parses function with multiple arguments', () => {
     expect(
       buildVisualQueryFromString(
@@ -327,6 +348,23 @@ describe('buildVisualQueryFromString', () => {
       },
     });
   });
+
+  it('parses query without metric', () => {
+    expect(buildVisualQueryFromString('label_replace(rate([$__rate_interval]), "", "$1", "", "(.*)")')).toEqual({
+      errors: [],
+      query: {
+        metric: '',
+        labels: [],
+        operations: [
+          { id: 'rate', params: ['$__rate_interval'] },
+          {
+            id: 'label_replace',
+            params: ['', '$1', '', '(.*)'],
+          },
+        ],
+      },
+    });
+  });
 });
 
 function noErrors(query: PromVisualQuery) {

public/app/plugins/datasource/prometheus/querybuilder/parsing.ts

@@ -56,7 +56,7 @@ function returnVariables(expr: string) {
 /**
  * Parses a PromQL query into a visual query model.
  *
- * It traverses the tree and uses sort of state machine to update update the query model. The query model is modified
+ * It traverses the tree and uses sort of state machine to update the query model. The query model is modified
  * during the traversal and sent to each handler as context.
  *
  * @param expr
@@ -98,7 +98,7 @@ const ErrorName = '⚠';
 
 /**
  * Handler for default state. It will traverse the tree and call the appropriate handler for each node. The node
- * handled here does not necessarily needs to be of type == Expr.
+ * handled here does not necessarily need to be of type == Expr.
  * @param expr
  * @param node
  * @param context
@@ -201,6 +201,7 @@ function handleFunction(expr: string, node: SyntaxNode, context: Context) {
   const body = node.getChild('FunctionCallBody');
   const callArgs = body!.getChild('FunctionCallArgs');
   const params = [];
+  let interval = '';
 
   // This is a bit of a shortcut to get the interval argument. Reasons are
   // - interval is not part of the function args per promQL grammar but we model it as argument for the function in
@@ -209,14 +210,23 @@ function handleFunction(expr: string, node: SyntaxNode, context: Context) {
   if (rangeFunctions.includes(funcName) || funcName.endsWith('_over_time')) {
     let match = getString(expr, node).match(/\[(.+)\]/);
     if (match?.[1]) {
-      params.push(returnVariables(match[1]));
+      interval = match[1];
+      params.push(match[1]);
     }
   }
 
   const op = { id: funcName, params };
   // We unshift operations to keep the more natural order that we want to have in the visual query editor.
   visQuery.operations.unshift(op);
-  updateFunctionArgs(expr, callArgs!, context, op);
+
+  if (callArgs) {
+    if (getString(expr, callArgs) === interval + ']') {
+      // This is a special case where we have a function with a single argument and it is the interval.
+      // This happens when you start adding operations in query builder and did not set a metric yet.
+      return;
+    }
+    updateFunctionArgs(expr, callArgs, context, op);
+  }
 }
 
 /**
@@ -284,7 +294,7 @@ function updateFunctionArgs(expr: string, node: SyntaxNode, context: Context, op
     }
 
     case 'NumberLiteral': {
-      op.params.push(parseInt(getString(expr, node), 10));
+      op.params.push(parseFloat(getString(expr, node)));
       break;
     }
 

public/app/plugins/datasource/prometheus/querybuilder/shared/OperationList.test.tsx

@@ -1,5 +1,5 @@
 import React from 'react';
-import { render, screen, fireEvent } from '@testing-library/react';
+import { render, screen } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 import { OperationList } from './OperationList';
 import { promQueryModeller } from '../PromQueryModeller';
@@ -8,6 +8,7 @@ import PromQlLanguageProvider from '../../language_provider';
 import { PromVisualQuery } from '../types';
 import { PrometheusDatasource } from '../../datasource';
 import { DataSourceApi } from '@grafana/data';
+import { addOperation } from './OperationList.testUtils';
 
 const defaultQuery: PromVisualQuery = {
   metric: 'random_metric',
@@ -79,17 +80,3 @@ function setup(query: PromVisualQuery = defaultQuery) {
   render(<OperationList {...props} query={query} />);
   return props;
 }
-
-function addOperation(section: string, op: string) {
-  const addOperationButton = screen.getByTitle('Add operation');
-  expect(addOperationButton).toBeInTheDocument();
-  userEvent.click(addOperationButton);
-  const sectionItem = screen.getByTitle(section);
-  expect(sectionItem).toBeInTheDocument();
-  // Weirdly the userEvent.click doesn't work here, it reports the item has pointer-events: none. Don't see that
-  // anywhere when debugging so not sure what style is it picking up.
-  fireEvent.click(sectionItem.children[0]);
-  const opItem = screen.getByTitle(op);
-  expect(opItem).toBeInTheDocument();
-  fireEvent.click(opItem);
-}

public/app/plugins/datasource/prometheus/querybuilder/shared/OperationList.testUtils.ts

@@ -0,0 +1,16 @@
+import { screen, fireEvent } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
+
+export function addOperation(section: string, op: string) {
+  const addOperationButton = screen.getByTitle('Add operation');
+  expect(addOperationButton).toBeInTheDocument();
+  userEvent.click(addOperationButton);
+  const sectionItem = screen.getByTitle(section);
+  expect(sectionItem).toBeInTheDocument();
+  // Weirdly the userEvent.click doesn't work here, it reports the item has pointer-events: none. Don't see that
+  // anywhere when debugging so not sure what style is it picking up.
+  fireEvent.click(sectionItem.children[0]);
+  const opItem = screen.getByTitle(op);
+  expect(opItem).toBeInTheDocument();
+  fireEvent.click(opItem);
+}

public/app/plugins/datasource/prometheus/types.ts

@@ -1,6 +1,5 @@
 import { DataQuery, DataSourceJsonData, QueryResultMeta, ScopedVars } from '@grafana/data';
 import { QueryEditorMode } from './querybuilder/shared/types';
-import { PromVisualQuery } from './querybuilder/types';
 
 export interface PromQuery extends DataQuery {
   expr: string;
@@ -22,8 +21,6 @@ export interface PromQuery extends DataQuery {
   editorMode?: QueryEditorMode;
   /** Controls if the query preview is shown */
   editorPreview?: boolean;
-  /** Temporary until we have a parser */
-  visualQuery?: PromVisualQuery;
 }
 
 export interface PromOptions extends DataSourceJsonData {