From 642f0a250d168ca5a9f9694fad6503a1e6daa11c Mon Sep 17 00:00:00 2001 From: Andrej Ocenas Date: Thu, 10 Feb 2022 16:55:44 +0100 Subject: [PATCH] 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 --- .../PromQueryBuilderContainer.test.tsx | 42 ++++++++++++ .../components/PromQueryBuilderContainer.tsx | 43 +++++++++++++ .../components/PromQueryBuilderExplained.tsx | 12 +++- .../PromQueryEditorSelector.test.tsx | 17 +---- .../components/PromQueryEditorSelector.tsx | 64 ++++++++----------- .../querybuilder/components/QueryPreview.tsx | 6 +- .../prometheus/querybuilder/parsing.test.ts | 38 +++++++++++ .../prometheus/querybuilder/parsing.ts | 20 ++++-- .../shared/OperationList.test.tsx | 17 +---- .../shared/OperationList.testUtils.ts | 16 +++++ .../plugins/datasource/prometheus/types.ts | 3 - 11 files changed, 195 insertions(+), 83 deletions(-) create mode 100644 public/app/plugins/datasource/prometheus/querybuilder/components/PromQueryBuilderContainer.test.tsx create mode 100644 public/app/plugins/datasource/prometheus/querybuilder/components/PromQueryBuilderContainer.tsx create mode 100644 public/app/plugins/datasource/prometheus/querybuilder/shared/OperationList.testUtils.ts diff --git a/public/app/plugins/datasource/prometheus/querybuilder/components/PromQueryBuilderContainer.test.tsx b/public/app/plugins/datasource/prometheus/querybuilder/components/PromQueryBuilderContainer.test.tsx new file mode 100644 index 00000000000..e8b7a7a7b4d --- /dev/null +++ b/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(); + expect(screen.getByText('metric_test')).toBeInTheDocument(); + addOperation('Range functions', 'Rate'); + expect(props.onChange).toBeCalledWith({ + expr: 'rate(metric_test{job="testjob"}[$__rate_interval])', + refId: 'A', + }); + }); +}); diff --git a/public/app/plugins/datasource/prometheus/querybuilder/components/PromQueryBuilderContainer.tsx b/public/app/plugins/datasource/prometheus/querybuilder/components/PromQueryBuilderContainer.tsx new file mode 100644 index 00000000000..9d900fe9d3c --- /dev/null +++ b/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 ( + <> + + {query.editorPreview && } + + + ); +} diff --git a/public/app/plugins/datasource/prometheus/querybuilder/components/PromQueryBuilderExplained.tsx b/public/app/plugins/datasource/prometheus/querybuilder/components/PromQueryBuilderExplained.tsx index d24b93bd05b..ad50f64a1ce 100644 --- a/public/app/plugins/datasource/prometheus/querybuilder/components/PromQueryBuilderExplained.tsx +++ b/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(({ query, nested }) => { + const visQuery = buildVisualQueryFromString(query || '').query; + return ( - + Fetch all series matching metric name and label filters. - stepNumber={2} queryModeller={promQueryModeller} query={query} /> + stepNumber={2} queryModeller={promQueryModeller} query={visQuery} /> ); }); diff --git a/public/app/plugins/datasource/prometheus/querybuilder/components/PromQueryEditorSelector.test.tsx b/public/app/plugins/datasource/prometheus/querybuilder/components/PromQueryEditorSelector.test.tsx index f2fe0a2d8ed..8ec6d21737e 100644 --- a/public/app/plugins/datasource/prometheus/querybuilder/components/PromQueryEditorSelector.test.tsx +++ b/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: { - metric: 'my_metric', - labels: [], - operations: [], - }, + expr: 'my_metric', }); 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() { diff --git a/public/app/plugins/datasource/prometheus/querybuilder/components/PromQueryEditorSelector.tsx b/public/app/plugins/datasource/prometheus/querybuilder/components/PromQueryEditorSelector.tsx index 660ec2dc9e3..3044361779f 100644 --- a/public/app/plugins/datasource/prometheus/querybuilder/components/PromQueryEditorSelector.tsx +++ b/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((props) => { const { query, onChange, onRunQuery, data } = props; const styles = useStyles2(getStyles); - const [visualQuery, setVisualQuery] = useState(query.visualQuery ?? getDefaultEmptyQuery()); + const [parseModalOpen, setParseModalOpen] = useState(false); - const [pendingChange, setPendingChange] = useState(undefined); const onEditorModeChange = useCallback( (newMetricEditorMode: QueryEditorMode) => { const change = { ...query, editorMode: newMetricEditorMode }; if (newMetricEditorMode === QueryEditorMode.Builder) { - const result = buildVisualQueryFromString(query.expr); - change.visualQuery = result.query; + const result = buildVisualQueryFromString(query.expr || ''); // 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) => { const isEnabled = event.currentTarget.checked; onChange({ ...query, editorPreview: isEnabled }); @@ -64,6 +45,12 @@ export const PromQueryEditorSelector = React.memo((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 ( <> ((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(pendingChange!); + onChange({ ...query, editorMode: QueryEditorMode.Builder }); setParseModalOpen(false); }} onDismiss={() => setParseModalOpen(false)} @@ -98,9 +84,13 @@ export const PromQueryEditorSelector = React.memo((props) placeholder="Query patterns" allowCustomValue onChange={({ value }) => { - onChangeViewModel({ - ...visualQuery, - operations: value?.operations!, + // TODO: Bit convoluted as we don't have access to visualQuery model here. Maybe would make sense to + // move it inside the editor? + 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((props) {editorMode === QueryEditorMode.Code && } {editorMode === QueryEditorMode.Builder && ( - <> - - {query.editorPreview && } - - + )} - {editorMode === QueryEditorMode.Explain && } + {editorMode === QueryEditorMode.Explain && } ); diff --git a/public/app/plugins/datasource/prometheus/querybuilder/components/QueryPreview.tsx b/public/app/plugins/datasource/prometheus/querybuilder/components/QueryPreview.tsx index d8cdd017be5..bdc0c7718de 100644 --- a/public/app/plugins/datasource/prometheus/querybuilder/components/QueryPreview.tsx +++ b/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 ( diff --git a/public/app/plugins/datasource/prometheus/querybuilder/parsing.test.ts b/public/app/plugins/datasource/prometheus/querybuilder/parsing.test.ts index 7d546246deb..08e735d2f97 100644 --- a/public/app/plugins/datasource/prometheus/querybuilder/parsing.test.ts +++ b/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) { diff --git a/public/app/plugins/datasource/prometheus/querybuilder/parsing.ts b/public/app/plugins/datasource/prometheus/querybuilder/parsing.ts index f1650e1c4b5..4953acc0dd8 100644 --- a/public/app/plugins/datasource/prometheus/querybuilder/parsing.ts +++ b/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; } diff --git a/public/app/plugins/datasource/prometheus/querybuilder/shared/OperationList.test.tsx b/public/app/plugins/datasource/prometheus/querybuilder/shared/OperationList.test.tsx index 3a8be9601cc..197b2b22589 100644 --- a/public/app/plugins/datasource/prometheus/querybuilder/shared/OperationList.test.tsx +++ b/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(); 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); -} diff --git a/public/app/plugins/datasource/prometheus/querybuilder/shared/OperationList.testUtils.ts b/public/app/plugins/datasource/prometheus/querybuilder/shared/OperationList.testUtils.ts new file mode 100644 index 00000000000..40440b72524 --- /dev/null +++ b/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); +} diff --git a/public/app/plugins/datasource/prometheus/types.ts b/public/app/plugins/datasource/prometheus/types.ts index 0ad839ce204..08a27ffe2d8 100644 --- a/public/app/plugins/datasource/prometheus/types.ts +++ b/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 {