From 480aa1ccca03f267907c685a0f2358a9793ff677 Mon Sep 17 00:00:00 2001 From: Sven Grossmann Date: Wed, 20 Sep 2023 16:54:24 +0200 Subject: [PATCH] Loki: Fix filters not being added with multiple expressions and parsers (#75152) * determine last positions per expr * fix lint --- .../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 38284968b53..9b22d518965 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 61edcc93029..4c2b02c41a9 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'; @@ -184,8 +186,20 @@ export function addLabelToQuery( const positionToAdd = findLastPosition([...streamSelectorPositions, ...labelFilterPositions, ...parserPositions]); return addFilterAsLabelFilter(query, [positionToAdd], filter); } else 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); } @@ -271,9 +285,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; } }, @@ -287,7 +301,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)); } }, }); @@ -302,9 +316,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; } }, @@ -320,9 +334,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; } }, @@ -338,9 +352,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; } }, @@ -356,9 +370,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; } @@ -565,7 +579,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)]; } @@ -574,9 +588,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)); +}