From 76c9ad2d9b2d2f96c690f8eb096b8a514bc251f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Wed, 23 Mar 2022 07:42:25 +0100 Subject: [PATCH] Loki: Improve operation ordering when adding new operations (#46598) --- .../querybuilder/LokiQueryModeller.test.ts | 42 ++++++---- .../loki/querybuilder/operations.ts | 76 +++++++++---------- .../datasource/loki/querybuilder/types.ts | 10 +++ .../prometheus/querybuilder/shared/types.ts | 2 + 4 files changed, 77 insertions(+), 53 deletions(-) diff --git a/public/app/plugins/datasource/loki/querybuilder/LokiQueryModeller.test.ts b/public/app/plugins/datasource/loki/querybuilder/LokiQueryModeller.test.ts index 86074e1484c..1d8459ec941 100644 --- a/public/app/plugins/datasource/loki/querybuilder/LokiQueryModeller.test.ts +++ b/public/app/plugins/datasource/loki/querybuilder/LokiQueryModeller.test.ts @@ -128,12 +128,12 @@ describe('LokiQueryModeller', () => { it('When adding function without range vector param should automatically add rate after existing pipe operation', () => { const query = { labels: [], - operations: [{ id: 'json', params: [] }], + operations: [{ id: LokiOperationId.Json, params: [] }], }; const def = modeller.getOperationDef('sum')!; const result = def.addOperationHandler(def, query, modeller); - expect(result.operations[0].id).toBe('json'); + expect(result.operations[0].id).toBe(LokiOperationId.Json); expect(result.operations[1].id).toBe('rate'); expect(result.operations[2].id).toBe('sum'); }); @@ -144,45 +144,59 @@ describe('LokiQueryModeller', () => { operations: [{ id: 'rate', params: [] }], }; - const def = modeller.getOperationDef('json')!; + const def = modeller.getOperationDef(LokiOperationId.Json)!; const result = def.addOperationHandler(def, query, modeller); - expect(result.operations[0].id).toBe('json'); + expect(result.operations[0].id).toBe(LokiOperationId.Json); expect(result.operations[1].id).toBe('rate'); }); it('When adding a pipe operation after a line filter operation', () => { const query = { labels: [], - operations: [{ id: '__line_contains', params: ['error'] }], + operations: [{ id: LokiOperationId.LineContains, params: ['error'] }], }; - const def = modeller.getOperationDef('json')!; + const def = modeller.getOperationDef(LokiOperationId.Json)!; const result = def.addOperationHandler(def, query, modeller); - expect(result.operations[0].id).toBe('__line_contains'); - expect(result.operations[1].id).toBe('json'); + expect(result.operations[0].id).toBe(LokiOperationId.LineContains); + expect(result.operations[1].id).toBe(LokiOperationId.Json); }); it('When adding a line filter operation after format operation', () => { const query = { labels: [], - operations: [{ id: 'json', params: [] }], + operations: [{ id: LokiOperationId.Json, params: [] }], }; - const def = modeller.getOperationDef('__line_contains')!; + const def = modeller.getOperationDef(LokiOperationId.LineContains)!; const result = def.addOperationHandler(def, query, modeller); - expect(result.operations[0].id).toBe('__line_contains'); - expect(result.operations[1].id).toBe('json'); + expect(result.operations[0].id).toBe(LokiOperationId.LineContains); + expect(result.operations[1].id).toBe(LokiOperationId.Json); }); it('When adding a rate it should not add another rate', () => { const query = { labels: [], - operations: [], + operations: [{ id: LokiOperationId.Rate, params: [] }], }; - const def = modeller.getOperationDef('rate')!; + const def = modeller.getOperationDef(LokiOperationId.Rate)!; const result = def.addOperationHandler(def, query, modeller); expect(result.operations.length).toBe(1); }); + + it('When adding unwrap it should be added after format and error filter', () => { + const query = { + labels: [], + operations: [ + { id: LokiOperationId.Json, params: [] }, + { id: LokiOperationId.LabelFilterNoErrors, params: [] }, + ], + }; + + const def = modeller.getOperationDef(LokiOperationId.Unwrap)!; + const result = def.addOperationHandler(def, query, modeller); + expect(result.operations[1].id).toBe(LokiOperationId.Unwrap); + }); }); }); diff --git a/public/app/plugins/datasource/loki/querybuilder/operations.ts b/public/app/plugins/datasource/loki/querybuilder/operations.ts index 614fab36960..32f49d07d68 100644 --- a/public/app/plugins/datasource/loki/querybuilder/operations.ts +++ b/public/app/plugins/datasource/loki/querybuilder/operations.ts @@ -9,7 +9,7 @@ import { VisualQueryModeller, } from '../../prometheus/querybuilder/shared/types'; import { FUNCTIONS } from '../syntax'; -import { LokiOperationId, LokiVisualQuery, LokiVisualQueryOperationCategory } from './types'; +import { LokiOperationId, LokiOperationOrder, LokiVisualQuery, LokiVisualQueryOperationCategory } from './types'; export function getOperationDefintions(): QueryBuilderOperationDef[] { const list: QueryBuilderOperationDef[] = [ @@ -30,6 +30,7 @@ export function getOperationDefintions(): QueryBuilderOperationDef[] { defaultParams: [], alternativesKey: 'format', category: LokiVisualQueryOperationCategory.Formats, + orderRank: LokiOperationOrder.LineFormats, renderer: pipelineRenderer, addOperationHandler: addLokiOperation, }, @@ -40,6 +41,7 @@ export function getOperationDefintions(): QueryBuilderOperationDef[] { defaultParams: [], alternativesKey: 'format', category: LokiVisualQueryOperationCategory.Formats, + orderRank: LokiOperationOrder.LineFormats, renderer: pipelineRenderer, addOperationHandler: addLokiOperation, explainHandler: () => @@ -61,6 +63,7 @@ export function getOperationDefintions(): QueryBuilderOperationDef[] { defaultParams: [''], alternativesKey: 'line filter', category: LokiVisualQueryOperationCategory.LineFilters, + orderRank: LokiOperationOrder.LineFilters, renderer: getLineFilterRenderer('|='), addOperationHandler: addLokiOperation, explainHandler: (op) => `Return log lines that contain string \`${op.params[0]}\`.`, @@ -81,6 +84,7 @@ export function getOperationDefintions(): QueryBuilderOperationDef[] { defaultParams: [''], alternativesKey: 'line filter', category: LokiVisualQueryOperationCategory.LineFilters, + orderRank: LokiOperationOrder.LineFilters, renderer: getLineFilterRenderer('!='), addOperationHandler: addLokiOperation, explainHandler: (op) => `Return log lines that does not contain string \`${op.params[0]}\`.`, @@ -101,6 +105,7 @@ export function getOperationDefintions(): QueryBuilderOperationDef[] { defaultParams: [''], alternativesKey: 'line filter', category: LokiVisualQueryOperationCategory.LineFilters, + orderRank: LokiOperationOrder.LineFilters, renderer: getLineFilterRenderer('|~'), addOperationHandler: addLokiOperation, explainHandler: (op) => `Return log lines that match regex \`${op.params[0]}\`.`, @@ -121,6 +126,7 @@ export function getOperationDefintions(): QueryBuilderOperationDef[] { defaultParams: [''], alternativesKey: 'line filter', category: LokiVisualQueryOperationCategory.LineFilters, + orderRank: LokiOperationOrder.LineFilters, renderer: getLineFilterRenderer('!~'), addOperationHandler: addLokiOperation, explainHandler: (op) => `Return log lines that does not match regex \`${op.params[0]}\`.`, @@ -135,6 +141,7 @@ export function getOperationDefintions(): QueryBuilderOperationDef[] { ], defaultParams: ['', '=', ''], category: LokiVisualQueryOperationCategory.LabelFilters, + orderRank: LokiOperationOrder.LabelFilters, renderer: labelFilterRenderer, addOperationHandler: addLokiOperation, explainHandler: () => `Label expression filter allows filtering using original and extracted labels.`, @@ -145,6 +152,7 @@ export function getOperationDefintions(): QueryBuilderOperationDef[] { params: [], defaultParams: [], category: LokiVisualQueryOperationCategory.LabelFilters, + orderRank: LokiOperationOrder.NoErrors, renderer: (model, def, innerExpr) => `${innerExpr} | __error__=""`, addOperationHandler: addLokiOperation, explainHandler: () => `Filter out all formatting and parsing errors.`, @@ -155,6 +163,7 @@ export function getOperationDefintions(): QueryBuilderOperationDef[] { params: [{ name: 'Identifier', type: 'string', hideName: true, minWidth: 16, placeholder: 'Label key' }], defaultParams: [''], category: LokiVisualQueryOperationCategory.Formats, + orderRank: LokiOperationOrder.Unwrap, renderer: (op, def, innerExpr) => `${innerExpr} | unwrap ${op.params[0]}`, addOperationHandler: addLokiOperation, explainHandler: (op) => { @@ -175,6 +184,7 @@ function createRangeOperation(name: string): QueryBuilderOperationDef { defaultParams: ['$__interval'], alternativesKey: 'range function', category: LokiVisualQueryOperationCategory.RangeFunctions, + orderRank: LokiOperationOrder.RangeVectorFunction, renderer: operationWithRangeVectorRenderer, addOperationHandler: addLokiOperation, explainHandler: (op, def) => { @@ -197,6 +207,7 @@ function createAggregationOperation(name: string): QueryBuilderOperationDef { defaultParams: [], alternativesKey: 'plain aggregation', category: LokiVisualQueryOperationCategory.Aggregations, + orderRank: LokiOperationOrder.Last, renderer: functionRendererLeft, addOperationHandler: addLokiOperation, explainHandler: (op, def) => { @@ -280,58 +291,45 @@ export function addLokiOperation( const operations = [...query.operations]; + const existingRangeVectorFunction = operations.find((x) => { + const opDef = modeller.getOperationDef(x.id); + if (!opDef) { + return false; + } + return isRangeVectorFunction(opDef); + }); + switch (def.category) { case LokiVisualQueryOperationCategory.Aggregations: - case LokiVisualQueryOperationCategory.Functions: { - const rangeVectorFunction = operations.find((x) => { - const opDef = modeller.getOperationDef(x.id); - if (!opDef) { - return false; - } - return isRangeVectorFunction(opDef); - }); - + case LokiVisualQueryOperationCategory.Functions: // If we are adding a function but we have not range vector function yet add one - if (!rangeVectorFunction) { + if (!existingRangeVectorFunction) { const placeToInsert = getIndexOfOrLast( operations, modeller, (def) => def.category === LokiVisualQueryOperationCategory.Functions ); - operations.splice(placeToInsert, 0, { id: 'rate', params: ['auto'] }); + operations.splice(placeToInsert, 0, { id: LokiOperationId.Rate, params: ['auto'] }); } - operations.push(newOperation); break; - } case LokiVisualQueryOperationCategory.RangeFunctions: - // Add range functions after any formats, line filters and label filters - const placeToInsert = getIndexOfOrLast(operations, modeller, (x) => { - return ( - x.category !== LokiVisualQueryOperationCategory.Formats && - x.category !== LokiVisualQueryOperationCategory.LineFilters && - x.category !== LokiVisualQueryOperationCategory.LabelFilters - ); - }); + // If adding a range function and range function is already added replace it + if (existingRangeVectorFunction) { + const index = operations.indexOf(existingRangeVectorFunction); + operations[index] = newOperation; + break; + } + + // Add range functions after any formats, line filters and label filters + default: + const placeToInsert = getIndexOfOrLast( + operations, + modeller, + (x) => (def.orderRank ?? 100) < (x.orderRank ?? 100) + ); operations.splice(placeToInsert, 0, newOperation); break; - case LokiVisualQueryOperationCategory.Formats: - case LokiVisualQueryOperationCategory.LineFilters: { - const placeToInsert = getIndexOfOrLast(operations, modeller, (x) => { - return x.category !== LokiVisualQueryOperationCategory.LineFilters; - }); - operations.splice(placeToInsert, 0, newOperation); - break; - } - case LokiVisualQueryOperationCategory.LabelFilters: { - const placeToInsert = getIndexOfOrLast(operations, modeller, (x) => { - return ( - x.category !== LokiVisualQueryOperationCategory.LineFilters && - x.category !== LokiVisualQueryOperationCategory.Formats - ); - }); - operations.splice(placeToInsert, 0, newOperation); - } } return { diff --git a/public/app/plugins/datasource/loki/querybuilder/types.ts b/public/app/plugins/datasource/loki/querybuilder/types.ts index fb9e9e3e62c..ecb6868d0b5 100644 --- a/public/app/plugins/datasource/loki/querybuilder/types.ts +++ b/public/app/plugins/datasource/loki/querybuilder/types.ts @@ -50,6 +50,16 @@ export enum LokiOperationId { Unwrap = 'unwrap', } +export enum LokiOperationOrder { + LineFilters = 1, + LineFormats = 2, + LabelFilters = 3, + Unwrap = 4, + NoErrors = 5, + RangeVectorFunction = 5, + Last = 6, +} + export function getDefaultEmptyQuery(): LokiVisualQuery { return { labels: [], diff --git a/public/app/plugins/datasource/prometheus/querybuilder/shared/types.ts b/public/app/plugins/datasource/prometheus/querybuilder/shared/types.ts index f399702c285..d15f438c4e8 100644 --- a/public/app/plugins/datasource/prometheus/querybuilder/shared/types.ts +++ b/public/app/plugins/datasource/prometheus/querybuilder/shared/types.ts @@ -27,6 +27,8 @@ export interface QueryBuilderOperationDef extends RegistryItem { category: string; hideFromList?: boolean; alternativesKey?: string; + /** Can be used to control operation placement when adding a new operations, lower are placed first */ + orderRank?: number; renderer: QueryBuilderOperationRenderer; addOperationHandler: QueryBuilderAddOperationHandler; paramChangedHandler?: QueryBuilderOnParamChangedHandler;