mirror of
https://github.com/grafana/grafana.git
synced 2025-02-25 18:55:37 -06:00
Loki: Fix ad hoc filters adding stream selectors to stream selectors and line filters (#90626)
* fix: fix case where we are adding stream selectors to both stream selectors and line filters --------- Co-authored-by: Sven Grossmann <sven.grossmann@grafana.com>
This commit is contained in:
Galen Kistler
GitHub
parent
8aecf9bd0d
commit
6fa25df37f
@ -281,16 +281,18 @@ describe('LokiDatasource', () => {
|
||||
};
|
||||
const query: LokiQuery = { expr: originalQuery, refId: 'A' };
|
||||
const ds = createLokiDatasource(templateSrv);
|
||||
const adhocFilters = [
|
||||
const adhocFilters: AdHocFilter[] = [
|
||||
{
|
||||
key: 'k1',
|
||||
operator: '=',
|
||||
value: 'v1',
|
||||
condition: '',
|
||||
},
|
||||
{
|
||||
key: 'k2',
|
||||
operator: '!=',
|
||||
value: 'v2',
|
||||
condition: '',
|
||||
},
|
||||
];
|
||||
jest.spyOn(ds, 'addAdHocFilters');
|
||||
@ -303,6 +305,13 @@ describe('LokiDatasource', () => {
|
||||
expect(ds.applyTemplateVariables(query, {}, adhocFilters).expr).toBe(
|
||||
'rate({bar="baz", job="foo", k1="v1", k2!="v2"} |= "bar" [5m])'
|
||||
);
|
||||
|
||||
assertAdHocFilters(
|
||||
originalQuery,
|
||||
'rate({bar="baz", job="foo", k1="v1", k2!="v2"} |= "bar" [$__auto])',
|
||||
ds,
|
||||
adhocFilters
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@ -1285,6 +1294,36 @@ describe('LokiDatasource', () => {
|
||||
assertAdHocFilters('{job="grafana"}', '{job="grafana", instance=~".*"}', ds, defaultAdHocFilters);
|
||||
});
|
||||
});
|
||||
|
||||
describe('bug', () => {
|
||||
beforeEach(() => {
|
||||
ds = createLokiDatasource();
|
||||
});
|
||||
const defaultAdHocFilters: AdHocFilter[] = [
|
||||
{
|
||||
key: 'service_name',
|
||||
operator: '=',
|
||||
value: 'grafana/hosted-grafana-gateway',
|
||||
condition: '',
|
||||
},
|
||||
];
|
||||
it('should not add indexed fields twice as index filter and line filter, backtick', () => {
|
||||
assertAdHocFilters(
|
||||
'{service_name=`grafana/hosted-grafana-gateway`} | logfmt',
|
||||
'{service_name="grafana/hosted-grafana-gateway"} | logfmt',
|
||||
ds,
|
||||
defaultAdHocFilters
|
||||
);
|
||||
});
|
||||
it('should not add indexed fields twice as index filter and line filter, quotes', () => {
|
||||
assertAdHocFilters(
|
||||
'{service_name="grafana/hosted-grafana-gateway"} | logfmt',
|
||||
'{service_name="grafana/hosted-grafana-gateway"} | logfmt',
|
||||
ds,
|
||||
defaultAdHocFilters
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('logs volume data provider', () => {
|
||||
|
||||
@ -1043,7 +1043,7 @@ export class LokiDatasource
|
||||
* @todo this.templateSrv.getAdhocFilters() is deprecated
|
||||
*/
|
||||
addAdHocFilters(queryExpr: string, adhocFilters?: AdHocVariableFilter[]) {
|
||||
if (!adhocFilters) {
|
||||
if (!adhocFilters?.length) {
|
||||
return queryExpr;
|
||||
}
|
||||
|
||||
|
||||
@ -6,6 +6,8 @@ import {
|
||||
addLineFilter,
|
||||
addNoPipelineErrorToQuery,
|
||||
addParserToQuery,
|
||||
getIdentifierInStreamPositions,
|
||||
getStreamSelectorPositions,
|
||||
NodePosition,
|
||||
queryHasFilter,
|
||||
removeCommentsFromQuery,
|
||||
@ -53,11 +55,20 @@ describe('addLabelToQuery()', () => {
|
||||
${'{} | logfmt | x="y"'} | ${'query without stream selector and with parser and label filter'} | ${'bar'} | ${'='} | ${'baz'} | ${'{bar="baz"}| logfmt | x="y"'}
|
||||
${'sum(rate({x="y"} [5m])) + sum(rate({} | logfmt [5m]))'} | ${'metric query with 1 empty and 1 not empty stream selector with parser'} | ${'bar'} | ${'='} | ${'baz'} | ${'sum(rate({x="y", bar="baz"} [5m])) + sum(rate({bar="baz"}| logfmt [5m]))'}
|
||||
${'sum(rate({x="y"} | logfmt [5m])) + sum(rate({} [5m]))'} | ${'metric query with 1 non-empty and 1 not empty stream selector with parser'} | ${'bar'} | ${'='} | ${'baz'} | ${'sum(rate({x="y", bar="baz"} | logfmt [5m])) + sum(rate({bar="baz"}[5m]))'}
|
||||
${'sum(rate({x="y", bar="baz"} | logfmt [5m])) + sum(rate({x="y", bar="baz"} [5m]))'} | ${'metric query with two duplicate stream selectors'} | ${'x'} | ${'='} | ${'y'} | ${'sum(rate({x="y", bar="baz"} | logfmt [5m])) + sum(rate({x="y", bar="baz"} [5m]))'}
|
||||
${'{x="yy"}'} | ${'simple query with escaped value'} | ${'bar'} | ${'='} | ${'"baz"'} | ${'{x="yy", bar=""baz""}'}
|
||||
${'{x="yy"}'} | ${'simple query with escaped value'} | ${'bar'} | ${'='} | ${'\\"baz\\"'} | ${'{x="yy", bar="\\"baz\\""}'}
|
||||
${'{x="yy"}'} | ${'simple query with an other escaped value'} | ${'bar'} | ${'='} | ${'baz\\\\'} | ${'{x="yy", bar="baz\\\\"}'}
|
||||
${'{x="yy"}'} | ${'simple query with escaped value and regex operator'} | ${'bar'} | ${'~='} | ${'baz\\\\'} | ${'{x="yy", bar~="baz\\\\"}'}
|
||||
${'{foo="bar"} | logfmt'} | ${'query with parser with escaped value'} | ${'bar'} | ${'='} | ${'\\"baz\\"'} | ${'{foo="bar"} | logfmt | bar=`"baz"`'}
|
||||
${'{foo=`"bar"`} | logfmt'} | ${'query with label already added to stream selector, doublequotes/backticks'} | ${'foo'} | ${'='} | ${`"bar"`} | ${'{foo=""bar""} | logfmt'}
|
||||
${'{foo="\\"bar\\""`} | logfmt'} | ${'query with label already added to stream selector, doublequotes/escaped'} | ${'foo'} | ${'='} | ${'\\"bar\\"'} | ${'{foo="\\"bar\\""} | logfmt'}
|
||||
${'{foo="bar"} | logfmt'} | ${'query with label already added to stream selector, doublequotes unescaped'} | ${'foo'} | ${'='} | ${'bar'} | ${'{foo="bar"} | logfmt'}
|
||||
${'{foo=`bar`} | logfmt'} | ${'query with label already added to stream selector, backticks'} | ${'foo'} | ${'='} | ${'bar'} | ${'{foo="bar"} | logfmt'}
|
||||
${'{service_name=`grafana/hosted-grafana-gateway`} | logfmt | caller!=`handler.go:637` '} | ${'query with parser and line filter, backticks'} | ${'service_name'} | ${'='} | ${'grafana/hosted-grafana-gateway'} | ${'{service_name="grafana/hosted-grafana-gateway"} | logfmt | caller!=`handler.go:637` '}
|
||||
${'{service_name=`grafana/hosted-grafana-gateway`, pod_template_hash!=`5fd76866f4`} | logfmt | caller!=`handler.go:637`'} | ${'query with parser and line filter, multiple stream selectors'} | ${'service_name'} | ${'='} | ${'grafana/hosted-grafana-gateway'} | ${'{service_name="grafana/hosted-grafana-gateway", pod_template_hash!="5fd76866f4"} | logfmt | caller!=`handler.go:637`'}
|
||||
${'{service_name=`grafana/hosted-grafana-gateway`, x!=`y`} | logfmt | caller!=`handler.go:637`'} | ${'query with parser and line filter, multiple stream selectors, value2'} | ${'x'} | ${'!='} | ${'y'} | ${'{service_name="grafana/hosted-grafana-gateway", x!="y"} | logfmt | caller!=`handler.go:637`'}
|
||||
${'{service_name="grafana/hosted-grafana-gateway"} | logfmt | caller!=`handler.go:637` '} | ${'query with parser and line filter, doublequotes'} | ${'service_name'} | ${'='} | ${'grafana/hosted-grafana-gateway'} | ${'{service_name="grafana/hosted-grafana-gateway"} | logfmt | caller!=`handler.go:637` '}
|
||||
${'{foo="bar"} | logfmt'} | ${'query with parser with an other escaped value'} | ${'bar'} | ${'='} | ${'baz\\\\'} | ${'{foo="bar"} | logfmt | bar=`baz\\`'}
|
||||
${'{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 with escaped value and regex operator'} | ${'bar'} | ${'~='} | ${'\\"baz\\"'} | ${'{foo="bar"} | logfmt | bar~=`"baz"`'}
|
||||
@ -353,3 +364,88 @@ describe.each(['|=', '!='])('addLineFilter type %s', (op: string) => {
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('getStreamSelectorPositions', () => {
|
||||
it('should parse position of stream selectors', () => {
|
||||
expect(
|
||||
getStreamSelectorPositions('sum(rate({x="y", bar="baz"} | logfmt [5m])) + sum(rate({x="y", bar="baz"} [5m]))')
|
||||
).toEqual([
|
||||
{
|
||||
from: 9,
|
||||
to: 27,
|
||||
type: {
|
||||
name: 'Selector',
|
||||
props: {},
|
||||
id: 40,
|
||||
flags: 0,
|
||||
},
|
||||
},
|
||||
{
|
||||
from: 55,
|
||||
to: 73,
|
||||
type: {
|
||||
name: 'Selector',
|
||||
props: {},
|
||||
id: 40,
|
||||
flags: 0,
|
||||
},
|
||||
},
|
||||
]);
|
||||
});
|
||||
});
|
||||
describe('getIdentifierInStreamPositions', () => {
|
||||
it('should parse position of stream selectors', () => {
|
||||
const indexedKeys = ['x', 'bar'];
|
||||
const expr = `sum(rate({${indexedKeys[0]}="y", ${indexedKeys[1]}="baz"} | logfmt | x |= "x=y" |= "bar=baz" [5m])) + sum(rate({${indexedKeys[0]}="y", ${indexedKeys[1]}="baz"} [5m]))`;
|
||||
const identifiers = getIdentifierInStreamPositions(expr);
|
||||
identifiers.forEach((identifier, index) => {
|
||||
expect(identifier.getExpression(expr)).toEqual(indexedKeys[index % 2]);
|
||||
});
|
||||
expect(identifiers).toEqual([
|
||||
//x1
|
||||
{
|
||||
from: 10,
|
||||
to: 11,
|
||||
type: {
|
||||
name: 'Identifier',
|
||||
props: {},
|
||||
id: 43,
|
||||
flags: 0,
|
||||
},
|
||||
},
|
||||
//bar1
|
||||
{
|
||||
from: 17,
|
||||
to: 20,
|
||||
type: {
|
||||
name: 'Identifier',
|
||||
props: {},
|
||||
id: 43,
|
||||
flags: 0,
|
||||
},
|
||||
},
|
||||
//x2
|
||||
{
|
||||
from: 82,
|
||||
to: 83,
|
||||
type: {
|
||||
name: 'Identifier',
|
||||
props: {},
|
||||
id: 43,
|
||||
flags: 0,
|
||||
},
|
||||
},
|
||||
//bar2
|
||||
{
|
||||
from: 89,
|
||||
to: 92,
|
||||
type: {
|
||||
name: 'Identifier',
|
||||
props: {},
|
||||
id: 43,
|
||||
flags: 0,
|
||||
},
|
||||
},
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
@ -165,6 +165,16 @@ export function addLabelToQuery(
|
||||
const hasStreamSelectorMatchers = getMatcherInStreamPositions(query);
|
||||
// For non-indexed labels we want to add them after label_format to, for example, allow ad-hoc filters to use formatted labels
|
||||
const labelFormatPositions = getNodePositionsFromQuery(query, [LabelFormatExpr]);
|
||||
|
||||
// If the label type wasn't passed in from the calling function, we can use lezer to figure out if this label is already in the stream selectors
|
||||
if (!labelType) {
|
||||
const identifierSelectorMatchers = getIdentifierInStreamPositions(query);
|
||||
const indexedKeys = identifierSelectorMatchers.map((match) => match.getExpression(query));
|
||||
if (indexedKeys.includes(key)) {
|
||||
labelType = LabelType.Indexed;
|
||||
}
|
||||
}
|
||||
|
||||
const everyStreamSelectorHasMatcher = streamSelectorPositions.every((streamSelectorPosition) =>
|
||||
hasStreamSelectorMatchers.some(
|
||||
(matcherPosition) =>
|
||||
@ -621,3 +631,16 @@ function getMatcherInStreamPositions(query: string): NodePosition[] {
|
||||
});
|
||||
return positions;
|
||||
}
|
||||
|
||||
export function getIdentifierInStreamPositions(query: string): NodePosition[] {
|
||||
const tree = parser.parse(query);
|
||||
const positions: NodePosition[] = [];
|
||||
tree.iterate({
|
||||
enter: ({ node }): false | void => {
|
||||
if (node.type.id === Selector) {
|
||||
positions.push(...getAllPositionsInNodeByType(node, Identifier));
|
||||
}
|
||||
},
|
||||
});
|
||||
return positions;
|
||||
}
|
||||
|
||||
Loading…
Reference in New Issue
Block a user