Loki: Fix filtering with structured metadata (#73955)

* check languageProvider to work with non-indexed metadata

* change loki devenv to work with non-indexed metadata

* trigger ci

* add forced labels after parsers

* add comment

* Update public/app/plugins/datasource/loki/modifyQuery.ts

Co-authored-by: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com>

---------

Co-authored-by: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com>
This commit is contained in:
Sven Grossmann 2023-08-31 15:24:03 +02:00 committed by GitHub
parent e3c0bc7f5c
commit 423a451858
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 109 additions and 53 deletions

View File

@ -47,12 +47,12 @@ async function sleep(duration) {
});
}
async function lokiSendLogLine(timestampNs, line, tags) {
async function lokiSendLogLine(timestampNs, line, tags, nonIndexed = {}) {
const data = {
streams: [
{
stream: tags,
values: [[timestampNs, line]],
values: [[timestampNs, line, nonIndexed]],
},
],
};
@ -187,8 +187,8 @@ async function sendNewLogs() {
const nowMs = new Date().getTime();
const timestampNs = `${nowMs}${getRandomNanosecPart()}`;
const item = getRandomLogItem(globalCounter)
await lokiSendLogLine(timestampNs, JSON.stringify(item), {age:'new', place:'moon', ...sharedLabels});
await lokiSendLogLine(timestampNs, logFmtLine(item), {age:'new', place:'luna', ...sharedLabels});
await lokiSendLogLine(timestampNs, JSON.stringify(item), {age:'new', place:'moon', ...sharedLabels}, {nonIndexed: 'value'});
await lokiSendLogLine(timestampNs, logFmtLine(item), {age:'new', place:'luna', ...sharedLabels}, {nonIndexed: 'value'});
const sleepDuration = 200 + Math.random() * 800; // between 0.2 and 1 seconds
await sleep(sleepDuration);
}

View File

@ -1,5 +1,5 @@
loki:
image: grafana/loki:latest
image: grafana/loki:main
command: -config.file=/etc/loki/loki-config.yaml
volumes:
- ./docker/blocks/loki/loki-config.yaml:/etc/loki/loki-config.yaml

View File

@ -21,7 +21,7 @@ schema_config:
- from: 2020-10-24
store: tsdb
object_store: filesystem
schema: v11
schema: v13
index:
prefix: index_
period: 24h

View File

@ -609,6 +609,7 @@ describe('LokiDatasource', () => {
let ds: LokiDatasource;
beforeEach(() => {
ds = createLokiDatasource(templateSrvStub);
ds.languageProvider.labelKeys = ['bar', 'job'];
});
describe('and query has no parser', () => {
@ -638,34 +639,44 @@ describe('LokiDatasource', () => {
expect(result.refId).toEqual('A');
expect(result.expr).toEqual('rate({bar="baz", job="grafana"}[5m])');
});
describe('and query has parser', () => {
it('then the correct label should be added for logs query', () => {
const query: LokiQuery = { refId: 'A', expr: '{bar="baz"} | logfmt' };
const action = { options: { key: 'job', value: 'grafana' }, type: 'ADD_FILTER' };
const result = ds.modifyQuery(query, action);
expect(result.refId).toEqual('A');
expect(result.expr).toEqual('{bar="baz"} | logfmt | job=`grafana`');
});
it('then the correct label should be added for metrics query', () => {
const query: LokiQuery = { refId: 'A', expr: 'rate({bar="baz"} | logfmt [5m])' };
const action = { options: { key: 'job', value: 'grafana' }, type: 'ADD_FILTER' };
const result = ds.modifyQuery(query, action);
it('then the correct label should be added for non-indexed metadata as LabelFilter', () => {
const query: LokiQuery = { refId: 'A', expr: '{bar="baz"}' };
const action = { options: { key: 'job', value: 'grafana' }, type: 'ADD_FILTER' };
ds.languageProvider.labelKeys = ['bar'];
const result = ds.modifyQuery(query, action);
expect(result.refId).toEqual('A');
expect(result.expr).toEqual('rate({bar="baz"} | logfmt | job=`grafana` [5m])');
});
expect(result.refId).toEqual('A');
expect(result.expr).toEqual('{bar="baz"} | job=`grafana`');
});
});
describe('and query has parser', () => {
it('then the correct label should be added for logs query', () => {
const query: LokiQuery = { refId: 'A', expr: '{bar="baz"} | logfmt' };
const action = { options: { key: 'job', value: 'grafana' }, type: 'ADD_FILTER' };
const result = ds.modifyQuery(query, action);
expect(result.refId).toEqual('A');
expect(result.expr).toEqual('{bar="baz"} | logfmt | job=`grafana`');
});
it('then the correct label should be added for metrics query', () => {
const query: LokiQuery = { refId: 'A', expr: 'rate({bar="baz"} | logfmt [5m])' };
const action = { options: { key: 'job', value: 'grafana' }, type: 'ADD_FILTER' };
const result = ds.modifyQuery(query, action);
expect(result.refId).toEqual('A');
expect(result.expr).toEqual('rate({bar="baz"} | logfmt | job=`grafana` [5m])');
});
});
});
describe('when called with ADD_FILTER_OUT', () => {
let ds: LokiDatasource;
beforeEach(() => {
ds = createLokiDatasource(templateSrvStub);
ds.languageProvider.labelKeys = ['bar', 'job'];
});
describe('and query has no parser', () => {
let ds: LokiDatasource;
beforeEach(() => {
ds = createLokiDatasource(templateSrvStub);
});
it('then the correct label should be added for logs query', () => {
const query: LokiQuery = { refId: 'A', expr: '{bar="baz"}' };
const action = { options: { key: 'job', value: 'grafana' }, type: 'ADD_FILTER_OUT' };
@ -692,28 +703,29 @@ describe('LokiDatasource', () => {
expect(result.refId).toEqual('A');
expect(result.expr).toEqual('rate({bar="baz", job!="grafana"}[5m])');
});
describe('and query has parser', () => {
let ds: LokiDatasource;
beforeEach(() => {
ds = createLokiDatasource(templateSrvStub);
});
});
describe('and query has parser', () => {
let ds: LokiDatasource;
beforeEach(() => {
ds = createLokiDatasource(templateSrvStub);
ds.languageProvider.labelKeys = ['bar', 'job'];
});
it('then the correct label should be added for logs query', () => {
const query: LokiQuery = { refId: 'A', expr: '{bar="baz"} | logfmt' };
const action = { options: { key: 'job', value: 'grafana' }, type: 'ADD_FILTER_OUT' };
const result = ds.modifyQuery(query, action);
it('then the correct label should be added for logs query', () => {
const query: LokiQuery = { refId: 'A', expr: '{bar="baz"} | logfmt' };
const action = { options: { key: 'job', value: 'grafana' }, type: 'ADD_FILTER_OUT' };
const result = ds.modifyQuery(query, action);
expect(result.refId).toEqual('A');
expect(result.expr).toEqual('{bar="baz"} | logfmt | job!=`grafana`');
});
it('then the correct label should be added for metrics query', () => {
const query: LokiQuery = { refId: 'A', expr: 'rate({bar="baz"} | logfmt [5m])' };
const action = { options: { key: 'job', value: 'grafana' }, type: 'ADD_FILTER_OUT' };
const result = ds.modifyQuery(query, action);
expect(result.refId).toEqual('A');
expect(result.expr).toEqual('{bar="baz"} | logfmt | job!=`grafana`');
});
it('then the correct label should be added for metrics query', () => {
const query: LokiQuery = { refId: 'A', expr: 'rate({bar="baz"} | logfmt [5m])' };
const action = { options: { key: 'job', value: 'grafana' }, type: 'ADD_FILTER_OUT' };
const result = ds.modifyQuery(query, action);
expect(result.refId).toEqual('A');
expect(result.expr).toEqual('rate({bar="baz"} | logfmt | job!=`grafana` [5m])');
});
expect(result.refId).toEqual('A');
expect(result.expr).toEqual('rate({bar="baz"} | logfmt | job!=`grafana` [5m])');
});
});
});

View File

@ -659,18 +659,34 @@ export class LokiDatasource
modifyQuery(query: LokiQuery, action: QueryFixAction): LokiQuery {
let expression = query.expr ?? '';
// NB: Usually the labelKeys should be fetched and cached in the datasource,
// but there might be some edge cases where this wouldn't be the case.
// However the changed would make this method `async`.
const allLabels = this.languageProvider.getLabelKeys();
switch (action.type) {
case 'ADD_FILTER': {
if (action.options?.key && action.options?.value) {
const value = escapeLabelValueInSelector(action.options.value);
expression = addLabelToQuery(expression, action.options.key, '=', value);
expression = addLabelToQuery(
expression,
action.options.key,
'=',
value,
allLabels.includes(action.options.key) === false
);
}
break;
}
case 'ADD_FILTER_OUT': {
if (action.options?.key && action.options?.value) {
const value = escapeLabelValueInSelector(action.options.value);
expression = addLabelToQuery(expression, action.options.key, '!=', value);
expression = addLabelToQuery(
expression,
action.options.key,
'!=',
value,
allLabels.includes(action.options.key) === false
);
}
break;
}

View File

@ -71,6 +71,18 @@ describe('addLabelToQuery()', () => {
}
}
);
it('should always add label as labelFilter if force flag is given', () => {
expect(addLabelToQuery('{foo="bar"}', 'forcedLabel', '=', 'value', true)).toEqual(
'{foo="bar"} | forcedLabel=`value`'
);
});
it('should always add label as labelFilter if force flag is given with a parser', () => {
expect(addLabelToQuery('{foo="bar"} | logfmt', 'forcedLabel', '=', 'value', true)).toEqual(
'{foo="bar"} | logfmt | forcedLabel=`value`'
);
});
});
describe('addParserToQuery', () => {

View File

@ -139,12 +139,19 @@ function getMatchersWithFilter(query: string, label: string, operator: string, v
* This operates on substrings of the query with labels and operates just on those. This makes this
* more robust and can alter even invalid queries, and preserves in general the query structure and whitespace.
*
* @param query
* @param key
* @param value
* @param operator
* @param {string} query
* @param {string} key
* @param {string} operator
* @param {string} value
* @param {boolean} [forceAsLabelFilter=false] - if true, it will add a LabelFilter expression even if there is no parser in the query
*/
export function addLabelToQuery(query: string, key: string, operator: string, value: string): string {
export function addLabelToQuery(
query: string,
key: string,
operator: string,
value: string,
forceAsLabelFilter = false
): string {
if (!key || !value) {
throw new Error('Need label to add to query.');
}
@ -167,7 +174,16 @@ export function addLabelToQuery(query: string, key: string, operator: string, va
const filter = toLabelFilter(key, value, operator);
// 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)) {
if (forceAsLabelFilter) {
// `forceAsLabelFilter` is mostly used for structured metadata labels. Those are not
// very well distinguishable from real labels, but need to be added as label
// filters after the last stream selector, parser or label filter. This is
// just a quickfix for now and still has edge-cases where it can fail.
// TODO: improve this once we have a better API in Loki to distinguish
// between the origins of labels.
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);
} else {