Loki: Fix adding of adhoc filters to stream selector when query with empty stream selector (#57280) (#57286)
* disable double stringify
* Refactor test for addLabelToQuery
* Add tests (TDD for expected behaviour)
* Fix adding ad hoc filters to correct place when no stream selector
* Update
* Update comment
* Fix getAllPositionsInNodeByType
Co-authored-by: Sven Grossmann <svennergr@gmail.com>
(cherry picked from commit 7928f170ce)
Co-authored-by: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com>
This commit is contained in:
Grot (@grafanabot)
committed by
GitHub
GitHub
parent
a991be81da
commit
613d3b5e17
@@ -7,157 +7,53 @@ import {
|
||||
} from './modifyQuery';
|
||||
|
||||
describe('addLabelToQuery()', () => {
|
||||
it('should add label to simple query', () => {
|
||||
expect(() => {
|
||||
addLabelToQuery('foo', '', '=', '');
|
||||
}).toThrow();
|
||||
expect(addLabelToQuery('{}', 'bar', '=', 'baz')).toBe('{bar="baz"}');
|
||||
expect(addLabelToQuery('{x="yy"}', 'bar', '=', 'baz')).toBe('{x="yy", bar="baz"}');
|
||||
});
|
||||
|
||||
it('should add custom operator', () => {
|
||||
expect(addLabelToQuery('{}', 'bar', '!=', 'baz')).toBe('{bar!="baz"}');
|
||||
expect(addLabelToQuery('{x="yy"}', 'bar', '!=', 'baz')).toBe('{x="yy", bar!="baz"}');
|
||||
});
|
||||
|
||||
it('should not modify ranges', () => {
|
||||
expect(addLabelToQuery('rate({}[1m])', 'foo', '=', 'bar')).toBe('rate({foo="bar"}[1m])');
|
||||
});
|
||||
|
||||
it('should detect in-order function use', () => {
|
||||
expect(addLabelToQuery('sum by (host) (rate({} [1m]))', 'bar', '=', 'baz')).toBe(
|
||||
'sum by (host) (rate({bar="baz"}[1m]))'
|
||||
);
|
||||
});
|
||||
|
||||
it('should handle selectors with punctuation', () => {
|
||||
expect(addLabelToQuery('{instance="my-host.com:9100"}', 'bar', '=', 'baz')).toBe(
|
||||
'{instance="my-host.com:9100", bar="baz"}'
|
||||
);
|
||||
expect(addLabelToQuery('{list="a,b,c"}', 'bar', '=', 'baz')).toBe('{list="a,b,c", bar="baz"}');
|
||||
});
|
||||
|
||||
it('should work on arithmetical expressions', () => {
|
||||
expect(addLabelToQuery('{} + {}', 'bar', '=', 'baz')).toBe('{bar="baz"}+ {bar="baz"}');
|
||||
expect(addLabelToQuery('avg(rate({x="y"} [$__interval]))+ sum(rate({}[5m]))', 'bar', '=', 'baz')).toBe(
|
||||
'avg(rate({x="y", bar="baz"} [$__interval]))+ sum(rate({bar="baz"}[5m]))'
|
||||
);
|
||||
expect(addLabelToQuery('{x="yy"} * {y="zz",a="bb"} * {}', 'bar', '=', 'baz')).toBe(
|
||||
'{x="yy", bar="baz"} * {y="zz", a="bb", bar="baz"} * {bar="baz"}'
|
||||
);
|
||||
});
|
||||
|
||||
it('should not add duplicate labels to a query', () => {
|
||||
expect(addLabelToQuery(addLabelToQuery('{x="yy"}', 'bar', '!=', 'baz'), 'bar', '!=', 'baz')).toBe(
|
||||
'{x="yy", bar!="baz"}'
|
||||
);
|
||||
expect(addLabelToQuery(addLabelToQuery('rate({}[1m])', 'foo', '=', 'bar'), 'foo', '=', 'bar')).toBe(
|
||||
'rate({foo="bar"}[1m])'
|
||||
);
|
||||
expect(addLabelToQuery(addLabelToQuery('{list="a,b,c"}', 'bar', '=', 'baz'), 'bar', '=', 'baz')).toBe(
|
||||
'{list="a,b,c", bar="baz"}'
|
||||
);
|
||||
expect(
|
||||
addLabelToQuery(
|
||||
addLabelToQuery('avg(rate({bar="baz"} [$__interval]))+ sum(rate({bar="baz"}[5m]))', 'bar', '=', 'baz'),
|
||||
'bar',
|
||||
'=',
|
||||
'baz'
|
||||
)
|
||||
).toBe('avg(rate({bar="baz"} [$__interval]))+ sum(rate({bar="baz"}[5m]))');
|
||||
});
|
||||
|
||||
it('should not remove filters', () => {
|
||||
expect(addLabelToQuery('{x="y"} |="yy"', 'bar', '=', 'baz')).toBe('{x="y", bar="baz"} |="yy"');
|
||||
expect(addLabelToQuery('{x="y"} |="yy" !~"xx"', 'bar', '=', 'baz')).toBe('{x="y", bar="baz"} |="yy" !~"xx"');
|
||||
});
|
||||
|
||||
it('should add label to query properly with Loki datasource', () => {
|
||||
expect(addLabelToQuery('{job="grafana"} |= "foo-bar"', 'filename', '=', 'test.txt')).toBe(
|
||||
'{job="grafana", filename="test.txt"} |= "foo-bar"'
|
||||
);
|
||||
});
|
||||
|
||||
it('should add labels to metrics with logical operators', () => {
|
||||
expect(addLabelToQuery('{x="y"} or {}', 'bar', '=', 'baz')).toBe('{x="y", bar="baz"} or {bar="baz"}');
|
||||
expect(addLabelToQuery('{x="y"} and {}', 'bar', '=', 'baz')).toBe('{x="y", bar="baz"} and {bar="baz"}');
|
||||
});
|
||||
|
||||
it('should not add ad-hoc filter to template variables', () => {
|
||||
expect(addLabelToQuery('sum(rate({job="foo"}[2m])) by (value $variable)', 'bar', '=', 'baz')).toBe(
|
||||
'sum(rate({job="foo", bar="baz"}[2m])) by (value $variable)'
|
||||
);
|
||||
});
|
||||
|
||||
it('should not add ad-hoc filter to range', () => {
|
||||
expect(addLabelToQuery('avg(rate(({job="foo"} > 0)[3h:])) by (label)', 'bar', '=', 'baz')).toBe(
|
||||
'avg(rate(({job="foo", bar="baz"} > 0)[3h:])) by (label)'
|
||||
);
|
||||
});
|
||||
it('should not add ad-hoc filter to labels in label list provided with the group modifier', () => {
|
||||
expect(
|
||||
addLabelToQuery(
|
||||
'max by (id, name, type) ({type=~"foo|bar|baz-test"}) * on(id) group_right(id, type, name) sum by (id) (rate({} [5m])) * 1000',
|
||||
'bar',
|
||||
'=',
|
||||
'baz'
|
||||
)
|
||||
).toBe(
|
||||
'max by (id, name, type) ({type=~"foo|bar|baz-test", bar="baz"}) * on(id) group_right(id, type, name) sum by (id) (rate({bar="baz"}[5m])) * 1000'
|
||||
);
|
||||
});
|
||||
it('should not add ad-hoc filter to labels in label list provided with the group modifier', () => {
|
||||
expect(addLabelToQuery('rate({x="y"}[${__range_s}s])', 'bar', '=', 'baz')).toBe(
|
||||
'rate({x="y", bar="baz"}[${__range_s}s])'
|
||||
);
|
||||
});
|
||||
it('should not add ad-hoc filter to labels to math operations', () => {
|
||||
expect(addLabelToQuery('count({job!="foo"} < (5*1024*1024*1024) or vector(0)) - 1', 'bar', '=', 'baz')).toBe(
|
||||
'count({job!="foo", bar="baz"} < (5*1024*1024*1024) or vector(0)) - 1'
|
||||
);
|
||||
});
|
||||
|
||||
describe('should add label as label filter is query with parser', () => {
|
||||
it('should add label filter after parser', () => {
|
||||
expect(addLabelToQuery('{foo="bar"} | logfmt', 'bar', '=', 'baz')).toBe('{foo="bar"} | logfmt | bar=`baz`');
|
||||
});
|
||||
it('should add label filter after last parser when multiple parsers', () => {
|
||||
expect(addLabelToQuery('{foo="bar"} | logfmt | json', 'bar', '=', 'baz')).toBe(
|
||||
'{foo="bar"} | logfmt | json | bar=`baz`'
|
||||
);
|
||||
});
|
||||
it('should add label filter after last label filter when multiple label filters', () => {
|
||||
expect(addLabelToQuery('{foo="bar"} | logfmt | x="y"', 'bar', '=', 'baz')).toBe(
|
||||
'{foo="bar"} | logfmt | x="y" | bar=`baz`'
|
||||
);
|
||||
});
|
||||
it('should add label filter in metric query', () => {
|
||||
expect(addLabelToQuery('rate({foo="bar"} | logfmt [5m])', 'bar', '=', 'baz')).toBe(
|
||||
'rate({foo="bar"} | logfmt | bar=`baz` [5m])'
|
||||
);
|
||||
});
|
||||
it('should add label filter in complex metric query', () => {
|
||||
expect(
|
||||
addLabelToQuery(
|
||||
'sum by(host) (rate({foo="bar"} | logfmt | x="y" | line_format "{{.status}}" [5m]))',
|
||||
'bar',
|
||||
'=',
|
||||
'baz'
|
||||
)
|
||||
).toBe('sum by(host) (rate({foo="bar"} | logfmt | x="y" | bar=`baz` | line_format "{{.status}}" [5m]))');
|
||||
});
|
||||
it('should not add adhoc filter to line_format expressions', () => {
|
||||
expect(addLabelToQuery('{foo="bar"} | logfmt | line_format "{{.status}}"', 'bar', '=', 'baz')).toBe(
|
||||
'{foo="bar"} | logfmt | bar=`baz` | line_format "{{.status}}"'
|
||||
);
|
||||
});
|
||||
|
||||
it('should not add adhoc filter to line_format expressions', () => {
|
||||
expect(addLabelToQuery('{foo="bar"} | logfmt | line_format "{{status}}"', 'bar', '=', 'baz')).toBe(
|
||||
'{foo="bar"} | logfmt | bar=`baz` | line_format "{{status}}"'
|
||||
);
|
||||
});
|
||||
});
|
||||
it.each`
|
||||
query | description | label | operator | value | expectedResult
|
||||
${'{x="y"}'} | ${'no label and value'} | ${''} | ${'='} | ${''} | ${''}
|
||||
${'{x="yy"}'} | ${'simple query'} | ${'bar'} | ${'='} | ${'baz'} | ${'{x="yy", bar="baz"}'}
|
||||
${'{x="yy"}'} | ${'simple query'} | ${'bar'} | ${'='} | ${'baz'} | ${'{x="yy", bar="baz"}'}
|
||||
${'{x="yy"}'} | ${'custom operator'} | ${'bar'} | ${'!='} | ${'baz'} | ${'{x="yy", bar!="baz"}'}
|
||||
${'rate({}[1m])'} | ${'do not modify ranges'} | ${'bar'} | ${'='} | ${'baz'} | ${'rate({bar="baz"}[1m])'}
|
||||
${'sum by (host) (rate({} [1m]))'} | ${'detect in-order function use'} | ${'bar'} | ${'='} | ${'baz'} | ${'sum by (host) (rate({bar="baz"}[1m]))'}
|
||||
${'{instance="my-host.com:9100"}'} | ${'selectors with punctuation'} | ${'bar'} | ${'='} | ${'baz'} | ${'{instance="my-host.com:9100", bar="baz"}'}
|
||||
${'{list="a,b,c"}'} | ${'selectors with punctuation'} | ${'bar'} | ${'='} | ${'baz'} | ${'{list="a,b,c", bar="baz"}'}
|
||||
${'rate({}[5m]) + rate({}[5m])'} | ${'arithmetical expressions'} | ${'bar'} | ${'='} | ${'baz'} | ${'rate({bar="baz"}[5m]) + rate({bar="baz"}[5m])'}
|
||||
${'avg(rate({x="y"} [$__interval]))+ sum(rate({}[5m]))'} | ${'arithmetical expressions'} | ${'bar'} | ${'='} | ${'baz'} | ${'avg(rate({x="y", bar="baz"} [$__interval]))+ sum(rate({bar="baz"}[5m]))'}
|
||||
${'rate({x="yy"}[5m]) * rate({y="zz",a="bb"}[5m]) * rate({}[5m])'} | ${'arithmetical expressions'} | ${'bar'} | ${'='} | ${'baz'} | ${'rate({x="yy", bar="baz"}[5m]) * rate({y="zz", a="bb", bar="baz"}[5m]) * rate({bar="baz"}[5m])'}
|
||||
${'{x="yy", bar!="baz"}'} | ${'do not add duplicate labels'} | ${'bar'} | ${'!='} | ${'baz'} | ${'{x="yy", bar!="baz"}'}
|
||||
${'rate({bar="baz"}[1m])'} | ${'do not add duplicate labels'} | ${'bar'} | ${'='} | ${'baz'} | ${'rate({bar="baz"}[1m])'}
|
||||
${'{list="a,b,c", bar="baz"}'} | ${'do not add duplicate labels'} | ${'bar'} | ${'='} | ${'baz'} | ${'{list="a,b,c", bar="baz"}'}
|
||||
${'avg(rate({bar="baz"} [$__interval]))+ sum(rate({bar="baz"}[5m]))'} | ${'do not add duplicate labels'} | ${'bar'} | ${'='} | ${'baz'} | ${'avg(rate({bar="baz"} [$__interval]))+ sum(rate({bar="baz"}[5m]))'}
|
||||
${'{x="y"} |="yy"'} | ${'do not remove filters'} | ${'bar'} | ${'='} | ${'baz'} | ${'{x="y", bar="baz"} |="yy"'}
|
||||
${'{x="y"} |="yy" !~"xx"'} | ${'do not remove filters'} | ${'bar'} | ${'='} | ${'baz'} | ${'{x="y", bar="baz"} |="yy" !~"xx"'}
|
||||
${'{x="y"} or {}'} | ${'metrics with logical operators'} | ${'bar'} | ${'='} | ${'baz'} | ${'{x="y", bar="baz"} or {bar="baz"}'}
|
||||
${'{x="y"} and {}'} | ${'metrics with logical operators'} | ${'bar'} | ${'='} | ${'baz'} | ${'{x="y", bar="baz"} and {bar="baz"}'}
|
||||
${'sum(rate({job="foo"}[2m])) by (value $variable)'} | ${'template variables'} | ${'bar'} | ${'='} | ${'baz'} | ${'sum(rate({job="foo", bar="baz"}[2m])) by (value $variable)'}
|
||||
${'rate({x="y"}[${__range_s}s])'} | ${'metrics query with range grafana variable'} | ${'bar'} | ${'='} | ${'baz'} | ${'rate({x="y", bar="baz"}[${__range_s}s])'}
|
||||
${'max by (id, name, type) ({type=~"foo|bar|baz-test"}) * on(id) group_right(id, type, name) sum by (id) (rate({} [5m])) * 1000'} | ${'metrics query with labels in label list with the group modifier'} | ${'bar'} | ${'='} | ${'baz'} | ${'max by (id, name, type) ({type=~"foo|bar|baz-test", bar="baz"}) * on(id) group_right(id, type, name) sum by (id) (rate({bar="baz"}[5m])) * 1000'}
|
||||
${'{foo="bar"} | logfmt'} | ${'query with parser'} | ${'bar'} | ${'='} | ${'baz'} | ${'{foo="bar"} | logfmt | bar=`baz`'}
|
||||
${'{foo="bar"} | logfmt | json'} | ${'query with multiple parsers'} | ${'bar'} | ${'='} | ${'baz'} | ${'{foo="bar"} | logfmt | json | bar=`baz`'}
|
||||
${'{foo="bar"} | logfmt | x="y"'} | ${'query with parser and label filter'} | ${'bar'} | ${'='} | ${'baz'} | ${'{foo="bar"} | logfmt | x="y" | bar=`baz`'}
|
||||
${'rate({foo="bar"} | logfmt [5m])'} | ${'metrics query with parser'} | ${'bar'} | ${'='} | ${'baz'} | ${'rate({foo="bar"} | logfmt | bar=`baz` [5m])'}
|
||||
${'sum by(host) (rate({foo="bar"} | logfmt | x="y" | line_format "{{.status}}" [5m]))'} | ${'metrics query with parser'} | ${'bar'} | ${'='} | ${'baz'} | ${'sum by(host) (rate({foo="bar"} | logfmt | x="y" | bar=`baz` | line_format "{{.status}}" [5m]))'}
|
||||
${'{foo="bar"} | logfmt | line_format "{{.status}}"'} | ${'do not add filter to line_format expressions in query with parser'} | ${'bar'} | ${'='} | ${'baz'} | ${'{foo="bar"} | logfmt | bar=`baz` | line_format "{{.status}}"'}
|
||||
${'{foo="bar"} | logfmt | line_format "{{status}}"'} | ${'do not add filter to line_format expressions in query with parser'} | ${'bar'} | ${'='} | ${'baz'} | ${'{foo="bar"} | logfmt | bar=`baz` | line_format "{{status}}"'}
|
||||
${'{}'} | ${'query without stream selector'} | ${'bar'} | ${'='} | ${'baz'} | ${'{bar="baz"}'}
|
||||
${'{} | logfmt'} | ${'query without stream selector and with parser'} | ${'bar'} | ${'='} | ${'baz'} | ${'{bar="baz"}| logfmt'}
|
||||
${'{} | x="y"'} | ${'query without stream selector and with label filter'} | ${'bar'} | ${'='} | ${'baz'} | ${'{bar="baz"}| x="y"'}
|
||||
${'{} | logfmt | x="y"'} | ${'query without stream selector and with parser and label filter'} | ${'bar'} | ${'='} | ${'baz'} | ${'{bar="baz"}| logfmt | x="y"'}
|
||||
`(
|
||||
'should add label to query: $query, description: $description',
|
||||
({ query, description, label, operator, value, expectedResult }) => {
|
||||
if (description === 'no label and value') {
|
||||
expect(() => {
|
||||
addLabelToQuery(query, label, operator, value);
|
||||
}).toThrow();
|
||||
} else {
|
||||
expect(addLabelToQuery(query, label, operator, value)).toEqual(expectedResult);
|
||||
}
|
||||
}
|
||||
);
|
||||
});
|
||||
|
||||
describe('addParserToQuery', () => {
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import { SyntaxNode } from '@lezer/common';
|
||||
import { sortBy } from 'lodash';
|
||||
|
||||
import {
|
||||
@@ -8,6 +9,7 @@ import {
|
||||
LineFilters,
|
||||
LogExpr,
|
||||
LogRangeExpr,
|
||||
Matcher,
|
||||
parser,
|
||||
PipelineExpr,
|
||||
Selector,
|
||||
@@ -40,6 +42,7 @@ export function addLabelToQuery(query: string, key: string, operator: string, va
|
||||
}
|
||||
|
||||
const streamSelectorPositions = getStreamSelectorPositions(query);
|
||||
const hasStreamSelectorMatchers = getMatcherInStreamPositions(query).length > 0;
|
||||
const parserPositions = getParserPositions(query);
|
||||
const labelFilterPositions = getLabelFilterPositions(query);
|
||||
if (!streamSelectorPositions.length) {
|
||||
@@ -47,8 +50,8 @@ export function addLabelToQuery(query: string, key: string, operator: string, va
|
||||
}
|
||||
|
||||
const filter = toLabelFilter(key, value, operator);
|
||||
// If we have label filters or parser, we want to add new label filter after the last one
|
||||
if (labelFilterPositions.length || parserPositions.length) {
|
||||
// If we have non-empty stream selector and parser/label filter, we want to add a new label filter after the last one.
|
||||
if (hasStreamSelectorMatchers && (labelFilterPositions.length || parserPositions.length)) {
|
||||
const positionToAdd = findLastPosition([...labelFilterPositions, ...parserPositions]);
|
||||
return addFilterAsLabelFilter(query, [positionToAdd], filter);
|
||||
} else {
|
||||
@@ -145,6 +148,19 @@ function getStreamSelectorPositions(query: string): Position[] {
|
||||
return positions;
|
||||
}
|
||||
|
||||
function getMatcherInStreamPositions(query: string): Position[] {
|
||||
const tree = parser.parse(query);
|
||||
const positions: Position[] = [];
|
||||
tree.iterate({
|
||||
enter: ({ node }): false | void => {
|
||||
if (node.type.id === Selector) {
|
||||
positions.push(...getAllPositionsInNodeByType(query, node, Matcher));
|
||||
}
|
||||
},
|
||||
});
|
||||
return positions;
|
||||
}
|
||||
|
||||
/**
|
||||
* Parse the string and get all LabelParser positions in the query.
|
||||
* @param query
|
||||
@@ -397,3 +413,19 @@ function labelExists(labels: QueryBuilderLabelFilter[], filter: QueryBuilderLabe
|
||||
function findLastPosition(positions: Position[]): Position {
|
||||
return positions.reduce((prev, current) => (prev.to > current.to ? prev : current));
|
||||
}
|
||||
|
||||
function getAllPositionsInNodeByType(query: string, node: SyntaxNode, type: number): Position[] {
|
||||
if (node.type.id === type) {
|
||||
return [{ from: node.from, to: node.to }];
|
||||
}
|
||||
|
||||
const positions: Position[] = [];
|
||||
let pos = 0;
|
||||
let child = node.childAfter(pos);
|
||||
while (child) {
|
||||
positions.push(...getAllPositionsInNodeByType(query, child, type));
|
||||
pos = child.to;
|
||||
child = node.childAfter(pos);
|
||||
}
|
||||
return positions;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user