Skip to content

Commit

Permalink
Loki: Fix filters not being added with multiple expressions and parse…
Browse files Browse the repository at this point in the history
…rs (#75152)

* determine last positions per expr

* fix lint
  • Loading branch information
svennergr committed Sep 20, 2023
1 parent 15e54df commit 480aa1c
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 15 deletions.
1 change: 1 addition & 0 deletions public/app/plugins/datasource/loki/modifyQuery.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 }) => {
Expand Down
54 changes: 39 additions & 15 deletions public/app/plugins/datasource/loki/modifyQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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;
}
},
Expand All @@ -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));
}
},
});
Expand All @@ -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;
}
},
Expand All @@ -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;
}
},
Expand All @@ -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;
}
},
Expand All @@ -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;
}

Expand Down Expand Up @@ -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)];
}
Expand All @@ -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));
}

0 comments on commit 480aa1c

Please sign in to comment.