From 0b367182a3fbc8fccc48835d7663551e8f47e265 Mon Sep 17 00:00:00 2001 From: Sven Grossmann Date: Wed, 20 Sep 2023 17:14:20 +0200 Subject: [PATCH] [v10.1.x] Loki: Fix filters not being added with multiple expressions and parsers (#75172) Loki: Fix filters not being added with multiple expressions and parsers (#75152) * determine last positions per expr * fix lint (cherry picked from commit 480aa1ccca03f267907c685a0f2358a9793ff677) --- .../datasource/loki/modifyQuery.test.ts | 1 + .../plugins/datasource/loki/modifyQuery.ts | 54 +++++++++++++------ 2 files changed, 40 insertions(+), 15 deletions(-) diff --git a/public/app/plugins/datasource/loki/modifyQuery.test.ts b/public/app/plugins/datasource/loki/modifyQuery.test.ts index 6d831690fd3..63a4c3d30d0 100644 --- a/public/app/plugins/datasource/loki/modifyQuery.test.ts +++ b/public/app/plugins/datasource/loki/modifyQuery.test.ts @@ -59,6 +59,7 @@ describe('addLabelToQuery()', () => { ${'{foo="bar"} | logfmt'} | ${'query with parser with escaped value and regex operator'} | ${'bar'} | ${'~='} | ${'\\"baz\\"'} | ${'{foo="bar"} | logfmt | bar~=`"baz"`'} ${'{foo="bar"} | logfmt'} | ${'query with parser, > operator and number value'} | ${'bar'} | ${'>'} | ${'5'} | ${'{foo="bar"} | logfmt | bar>5'} ${'{foo="bar"} | logfmt'} | ${'query with parser, < operator and non-number value'} | ${'bar'} | ${'<'} | ${'5KiB'} | ${'{foo="bar"} | logfmt | bar<`5KiB`'} + ${'sum(rate({x="y"} | logfmt [5m])) + sum(rate({x="z"} | logfmt [5m]))'} | ${'metric query with non empty selectors and parsers'} | ${'bar'} | ${'='} | ${'baz'} | ${'sum(rate({x="y"} | logfmt | bar=`baz` [5m])) + sum(rate({x="z"} | logfmt | bar=`baz` [5m]))'} `( 'should add label to query: $query, description: $description', ({ query, description, label, operator, value, expectedResult }) => { diff --git a/public/app/plugins/datasource/loki/modifyQuery.ts b/public/app/plugins/datasource/loki/modifyQuery.ts index 3678a723ca8..9e53de50d96 100644 --- a/public/app/plugins/datasource/loki/modifyQuery.ts +++ b/public/app/plugins/datasource/loki/modifyQuery.ts @@ -17,11 +17,13 @@ import { UnwrapExpr, String, PipelineStage, + Expr, } from '@grafana/lezer-logql'; import { QueryBuilderLabelFilter } from '../prometheus/querybuilder/shared/types'; import { unescapeLabelValue } from './languageUtils'; +import { getNodePositionsFromQuery } from './queryUtils'; import { lokiQueryModeller as modeller } from './querybuilder/LokiQueryModeller'; import { buildVisualQueryFromString, handleQuotes } from './querybuilder/parsing'; @@ -168,8 +170,20 @@ export function addLabelToQuery(query: string, key: string, operator: string, va // If we have non-empty stream selector and parser/label filter, we want to add a new label filter after the last one. // If some of the stream selectors don't have matchers, we want to add new matcher to the all stream selectors. if (everyStreamSelectorHasMatcher && (labelFilterPositions.length || parserPositions.length)) { - const positionToAdd = findLastPosition([...labelFilterPositions, ...parserPositions]); - return addFilterAsLabelFilter(query, [positionToAdd], filter); + // in case we are not adding the label to stream selectors we need to find the last position to add in each expression + const subExpressions = findLeaves(getNodePositionsFromQuery(query, [Expr])); + const parserFilterPositions = [...parserPositions, ...labelFilterPositions]; + + // find last position for each subexpression + const lastPositionsPerExpression = subExpressions.map((subExpression) => { + return findLastPosition( + parserFilterPositions.filter((p) => { + return subExpression.contains(p); + }) + ); + }); + + return addFilterAsLabelFilter(query, lastPositionsPerExpression, filter); } else { return addFilterToStreamSelector(query, streamSelectorPositions, filter); } @@ -255,9 +269,9 @@ export function getStreamSelectorPositions(query: string): NodePosition[] { const tree = parser.parse(query); const positions: NodePosition[] = []; tree.iterate({ - enter: ({ type, from, to }): false | void => { + enter: ({ type, node }): false | void => { if (type.id === Selector) { - positions.push(new NodePosition(from, to, type)); + positions.push(NodePosition.fromNode(node)); return false; } }, @@ -271,7 +285,7 @@ function getMatcherInStreamPositions(query: string): NodePosition[] { tree.iterate({ enter: ({ node }): false | void => { if (node.type.id === Selector) { - positions.push(...getAllPositionsInNodeByType(query, node, Matcher)); + positions.push(...getAllPositionsInNodeByType(node, Matcher)); } }, }); @@ -286,9 +300,9 @@ export function getParserPositions(query: string): NodePosition[] { const tree = parser.parse(query); const positions: NodePosition[] = []; tree.iterate({ - enter: ({ type, from, to }): false | void => { + enter: ({ type, node }): false | void => { if (type.id === LabelParser || type.id === JsonExpressionParser) { - positions.push(new NodePosition(from, to, type)); + positions.push(NodePosition.fromNode(node)); return false; } }, @@ -304,9 +318,9 @@ export function getLabelFilterPositions(query: string): NodePosition[] { const tree = parser.parse(query); const positions: NodePosition[] = []; tree.iterate({ - enter: ({ type, from, to }): false | void => { + enter: ({ type, node }): false | void => { if (type.id === LabelFilter) { - positions.push(new NodePosition(from, to, type)); + positions.push(NodePosition.fromNode(node)); return false; } }, @@ -322,9 +336,9 @@ function getLineFiltersPositions(query: string): NodePosition[] { const tree = parser.parse(query); const positions: NodePosition[] = []; tree.iterate({ - enter: ({ type, from, to }): false | void => { + enter: ({ type, node }): false | void => { if (type.id === LineFilters) { - positions.push(new NodePosition(from, to, type)); + positions.push(NodePosition.fromNode(node)); return false; } }, @@ -340,9 +354,9 @@ function getLogQueryPositions(query: string): NodePosition[] { const tree = parser.parse(query); const positions: NodePosition[] = []; tree.iterate({ - enter: ({ type, from, to, node }): false | void => { + enter: ({ type, node }): false | void => { if (type.id === LogExpr) { - positions.push(new NodePosition(from, to, type)); + positions.push(NodePosition.fromNode(node)); return false; } @@ -549,7 +563,7 @@ export function findLastPosition(positions: NodePosition[]): NodePosition { return positions.reduce((prev, current) => (prev.to > current.to ? prev : current)); } -function getAllPositionsInNodeByType(query: string, node: SyntaxNode, type: number): NodePosition[] { +function getAllPositionsInNodeByType(node: SyntaxNode, type: number): NodePosition[] { if (node.type.id === type) { return [NodePosition.fromNode(node)]; } @@ -558,9 +572,19 @@ function getAllPositionsInNodeByType(query: string, node: SyntaxNode, type: numb let pos = 0; let child = node.childAfter(pos); while (child) { - positions.push(...getAllPositionsInNodeByType(query, child, type)); + positions.push(...getAllPositionsInNodeByType(child, type)); pos = child.to; child = node.childAfter(pos); } return positions; } + +/** + * Gets all leaves of the nodes given. Leaves are nodes that don't contain any other nodes. + * + * @param {NodePosition[]} nodes + * @return + */ +function findLeaves(nodes: NodePosition[]): NodePosition[] { + return nodes.filter((node) => nodes.every((n) => node.contains(n) === false || node === n)); +}