From 9f9bf4650d3ec1d4b734d841bb3cf8a9c54425c1 Mon Sep 17 00:00:00 2001 From: Sven Grossmann Date: Thu, 22 Dec 2022 15:31:41 +0100 Subject: [PATCH] Loki: Fix missing parameters on Query Builder operations (#60677) * add missing mandatory params * improve naming * change let to const --- .../loki/querybuilder/operations.ts | 15 +++- .../loki/querybuilder/parsing.test.ts | 75 +++++++++++++++++++ .../datasource/loki/querybuilder/parsing.ts | 14 +++- 3 files changed, 101 insertions(+), 3 deletions(-) diff --git a/public/app/plugins/datasource/loki/querybuilder/operations.ts b/public/app/plugins/datasource/loki/querybuilder/operations.ts index 07a58338f44..0fb3d593d35 100644 --- a/public/app/plugins/datasource/loki/querybuilder/operations.ts +++ b/public/app/plugins/datasource/loki/querybuilder/operations.ts @@ -2,7 +2,7 @@ import { createAggregationOperation, createAggregationOperationWithParam, } from '../../prometheus/querybuilder/shared/operationUtils'; -import { QueryBuilderOperationDef } from '../../prometheus/querybuilder/shared/types'; +import { QueryBuilderOperationDef, QueryBuilderOperationParamValue } from '../../prometheus/querybuilder/shared/types'; import { binaryScalarOperations } from './binaryScalarOperations'; import { UnwrapParamEditor } from './components/UnwrapParamEditor'; @@ -480,3 +480,16 @@ export function explainOperator(id: LokiOperationId | string): string { // Strip markdown links return explain.replace(/\[(.*)\]\(.*\)/g, '$1'); } + +export function getDefinitionById(id: string): QueryBuilderOperationDef | undefined { + return definitions.find((x) => x.id === id); +} + +export function checkParamsAreValid(def: QueryBuilderOperationDef, params: QueryBuilderOperationParamValue[]): boolean { + // For now we only check if the operation has all the required params. + if (params.length < def.params.filter((param) => !param.optional).length) { + return false; + } + + return true; +} diff --git a/public/app/plugins/datasource/loki/querybuilder/parsing.test.ts b/public/app/plugins/datasource/loki/querybuilder/parsing.test.ts index 4ffe4957364..78649cce714 100644 --- a/public/app/plugins/datasource/loki/querybuilder/parsing.test.ts +++ b/public/app/plugins/datasource/loki/querybuilder/parsing.test.ts @@ -586,6 +586,81 @@ describe('buildVisualQueryFromString', () => { }) ); }); + + it('parses a regexp with empty string param', () => { + expect(buildVisualQueryFromString('{app="frontend"} | regexp "" ')).toEqual( + noErrors({ + labels: [ + { + op: '=', + value: 'frontend', + label: 'app', + }, + ], + operations: [{ id: LokiOperationId.Regexp, params: [''] }], + }) + ); + }); + + it('parses a regexp with no param', () => { + expect(buildVisualQueryFromString('{app="frontend"} | regexp ')).toEqual( + noErrors({ + labels: [ + { + op: '=', + value: 'frontend', + label: 'app', + }, + ], + operations: [{ id: LokiOperationId.Regexp, params: [''] }], + }) + ); + }); + + it('parses a pattern with empty string param', () => { + expect(buildVisualQueryFromString('{app="frontend"} | pattern "" ')).toEqual( + noErrors({ + labels: [ + { + op: '=', + value: 'frontend', + label: 'app', + }, + ], + operations: [{ id: LokiOperationId.Pattern, params: [''] }], + }) + ); + }); + + it('parses a pattern with no param', () => { + expect(buildVisualQueryFromString('{app="frontend"} | pattern ')).toEqual( + noErrors({ + labels: [ + { + op: '=', + value: 'frontend', + label: 'app', + }, + ], + operations: [{ id: LokiOperationId.Pattern, params: [''] }], + }) + ); + }); + + it('parses a json with no param', () => { + expect(buildVisualQueryFromString('{app="frontend"} | json ')).toEqual( + noErrors({ + labels: [ + { + op: '=', + value: 'frontend', + label: 'app', + }, + ], + operations: [{ id: LokiOperationId.Json, params: [] }], + }) + ); + }); }); function noErrors(query: LokiVisualQuery) { diff --git a/public/app/plugins/datasource/loki/querybuilder/parsing.ts b/public/app/plugins/datasource/loki/querybuilder/parsing.ts index cd428b02c1e..8e15b2b8641 100644 --- a/public/app/plugins/datasource/loki/querybuilder/parsing.ts +++ b/public/app/plugins/datasource/loki/querybuilder/parsing.ts @@ -51,9 +51,14 @@ import { makeError, replaceVariables, } from '../../prometheus/querybuilder/shared/parsingUtils'; -import { QueryBuilderLabelFilter, QueryBuilderOperation } from '../../prometheus/querybuilder/shared/types'; +import { + QueryBuilderLabelFilter, + QueryBuilderOperation, + QueryBuilderOperationParamValue, +} from '../../prometheus/querybuilder/shared/types'; import { binaryScalarDefs } from './binaryScalarOperations'; +import { checkParamsAreValid, getDefinitionById } from './operations'; import { LokiOperationId, LokiVisualQuery, LokiVisualQueryBinary } from './types'; interface Context { @@ -256,7 +261,12 @@ function getLabelParser(expr: string, node: SyntaxNode): QueryBuilderOperation { const parser = getString(expr, parserNode); const string = handleQuotes(getString(expr, node.getChild(String))); - const params = !!string ? [string] : []; + let params: QueryBuilderOperationParamValue[] = !!string ? [string] : []; + const opDef = getDefinitionById(parser); + if (opDef && !checkParamsAreValid(opDef, params)) { + params = opDef?.defaultParams || []; + } + return { id: parser, params,