Logs: Add toggle behavior support for "filter for" and "filter out" label within Logs Details (#70091)

* Datasource test: fix describe nesting

* Parsing: export handleQuotes function

* Modify query: add functions to detect the presence of a label and remove it

* Loki: add support to toggle filters if already present

* Datasource test: fix describe nesting

* Loki: add support to toggle filter out if present

* Remove label: handle escaped values

* Datasource: add test case for escaped label values

* Loki: remove = filter when applying !=

* Remove selector: add support for Selector node being far from Matcher

* Modify query: add unit tests

* Elasticsearch: create modifyQuery for elastic

* Elastic modify query: implement functions

* Elasticsearch: implement modifyQuery functions in datasource

* Elasticsearch: update datasource test

* Loki modify query: check for streamSelectorPositions length

* Elasticsearch query has filter: escape filter value in regex

* Remove unused type

* Modify query: use query modeller instance from module
This commit is contained in:
Matias Chomicki 2023-06-26 16:45:33 +02:00 committed by GitHub
parent 4ff0abd0d1
commit 4c4bd69eb6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 395 additions and 38 deletions

View File

@ -1198,12 +1198,24 @@ describe('modifyQuery', () => {
);
});
it('should toggle the filter', () => {
query.query = 'foo:"bar"';
expect(ds.modifyQuery(query, { type: 'ADD_FILTER', options: { key: 'foo', value: 'bar' } }).query).toBe('');
});
it('should add the negative filter', () => {
expect(ds.modifyQuery(query, { type: 'ADD_FILTER_OUT', options: { key: 'foo', value: 'bar' } }).query).toBe(
'-foo:"bar"'
);
});
it('should remove a positive filter to add a negative filter', () => {
query.query = 'foo:"bar"';
expect(ds.modifyQuery(query, { type: 'ADD_FILTER_OUT', options: { key: 'foo', value: 'bar' } }).query).toBe(
'-foo:"bar"'
);
});
it('should do nothing on unknown type', () => {
expect(ds.modifyQuery(query, { type: 'unknown', options: { key: 'foo', value: 'bar' } }).query).toBe(query.query);
});

View File

@ -52,6 +52,7 @@ import {
} from './components/QueryEditor/MetricAggregationsEditor/aggregations';
import { metricAggregationConfig } from './components/QueryEditor/MetricAggregationsEditor/utils';
import { isMetricAggregationWithMeta } from './guards';
import { addFilterToQuery, queryHasFilter, removeFilterFromQuery } from './modifyQuery';
import { trackAnnotationQuery, trackQuery } from './tracking';
import {
Logs,
@ -898,17 +899,22 @@ export class ElasticDatasource
let expression = query.query ?? '';
switch (action.type) {
case 'ADD_FILTER': {
if (expression.length > 0) {
expression += ' AND ';
}
expression += `${action.options.key}:"${action.options.value}"`;
// This gives the user the ability to toggle a filter on and off.
expression = queryHasFilter(expression, action.options.key, action.options.value)
? removeFilterFromQuery(expression, action.options.key, action.options.value)
: addFilterToQuery(expression, action.options.key, action.options.value);
break;
}
case 'ADD_FILTER_OUT': {
if (expression.length > 0) {
expression += ' AND ';
/**
* If there is a filter with the same key and value, remove it.
* This prevents the user from seeing no changes in the query when they apply
* this filter.
*/
if (queryHasFilter(expression, action.options.key, action.options.value)) {
expression = removeFilterFromQuery(expression, action.options.key, action.options.value);
}
expression += `-${action.options.key}:"${action.options.value}"`;
expression = addFilterToQuery(expression, action.options.key, action.options.value, '-');
break;
}
}

View File

@ -0,0 +1,74 @@
import { queryHasFilter, removeFilterFromQuery, addFilterToQuery } from './modifyQuery';
describe('queryHasFilter', () => {
it('should return true if the query contains the positive filter', () => {
expect(queryHasFilter('label:"value"', 'label', 'value')).toBe(true);
expect(queryHasFilter('label: "value"', 'label', 'value')).toBe(true);
expect(queryHasFilter('label : "value"', 'label', 'value')).toBe(true);
expect(queryHasFilter('label:value', 'label', 'value')).toBe(true);
expect(queryHasFilter('this:"that" AND label:value', 'label', 'value')).toBe(true);
expect(
queryHasFilter(
'message:"Jun 20 17:19:47 Xtorm syslogd[348]: ASL Sender Statistics"',
'message',
'Jun 20 17:19:47 Xtorm syslogd[348]: ASL Sender Statistics'
)
).toBe(true);
});
it('should return false if the query does not contain the positive filter', () => {
expect(queryHasFilter('label:"value"', 'label', 'otherValue')).toBe(false);
expect(queryHasFilter('-label:"value"', 'label', 'value')).toBe(false);
});
it('should return true if the query contains the negative filter', () => {
expect(queryHasFilter('-label:"value"', 'label', 'value', '-')).toBe(true);
expect(queryHasFilter('-label: "value"', 'label', 'value', '-')).toBe(true);
expect(queryHasFilter('-label : "value"', 'label', 'value', '-')).toBe(true);
expect(queryHasFilter('-label:value', 'label', 'value', '-')).toBe(true);
expect(queryHasFilter('this:"that" AND -label:value', 'label', 'value', '-')).toBe(true);
});
it('should return false if the query does not contain the negative filter', () => {
expect(queryHasFilter('label:"value"', 'label', 'otherValue', '-')).toBe(false);
expect(queryHasFilter('label:"value"', 'label', 'value', '-')).toBe(false);
});
});
describe('addFilterToQuery', () => {
it('should add a positive filter to the query', () => {
expect(addFilterToQuery('', 'label', 'value')).toBe('label:"value"');
});
it('should add a positive filter to the query with other filters', () => {
expect(addFilterToQuery('label2:"value2"', 'label', 'value')).toBe('label2:"value2" AND label:"value"');
});
it('should add a negative filter to the query', () => {
expect(addFilterToQuery('', 'label', 'value', '-')).toBe('-label:"value"');
});
it('should add a negative filter to the query with other filters', () => {
expect(addFilterToQuery('label2:"value2"', 'label', 'value', '-')).toBe('label2:"value2" AND -label:"value"');
});
});
describe('removeFilterFromQuery', () => {
it('should remove filter from query', () => {
const query = 'label:"value"';
expect(removeFilterFromQuery(query, 'label', 'value')).toBe('');
});
it('should remove filter from query with other filters', () => {
expect(removeFilterFromQuery('label:"value" AND label2:"value2"', 'label', 'value')).toBe('label2:"value2"');
expect(removeFilterFromQuery('label:value AND label2:"value2"', 'label', 'value')).toBe('label2:"value2"');
expect(removeFilterFromQuery('label : "value" OR label2:"value2"', 'label', 'value')).toBe('label2:"value2"');
expect(removeFilterFromQuery('test="test" OR label:"value" AND label2:"value2"', 'label', 'value')).toBe(
'test="test" AND label2:"value2"'
);
});
it('should not remove the wrong filter', () => {
expect(removeFilterFromQuery('-label:"value" AND label2:"value2"', 'label', 'value')).toBe(
'-label:"value" AND label2:"value2"'
);
expect(removeFilterFromQuery('label2:"value2" OR -label:value', 'label', 'value')).toBe(
'label2:"value2" OR -label:value'
);
expect(removeFilterFromQuery('-label : "value" OR label2:"value2"', 'label', 'value')).toBe(
'-label : "value" OR label2:"value2"'
);
});
});

View File

@ -0,0 +1,57 @@
import { escapeRegex } from '@grafana/data';
type ModifierType = '' | '-';
/**
* Checks for the presence of a given label:"value" filter in the query.
*/
export function queryHasFilter(query: string, key: string, value: string, modifier: ModifierType = ''): boolean {
const regex = getFilterRegex(key, value);
const matches = query.matchAll(regex);
for (const match of matches) {
if (modifier === '-' && match[0].startsWith(modifier)) {
return true;
}
if (modifier === '' && !match[0].startsWith('-')) {
return true;
}
}
return false;
}
/**
* Adds a label:"value" expression to the query.
*/
export function addFilterToQuery(query: string, key: string, value: string, modifier: ModifierType = ''): string {
if (queryHasFilter(query, key, value, modifier)) {
return query;
}
const filter = `${modifier}${key}:"${value}"`;
return query === '' ? filter : `${query} AND ${filter}`;
}
function getFilterRegex(key: string, value: string) {
return new RegExp(`[-]{0,1}\\s*${key}\\s*:\\s*["']{0,1}${escapeRegex(value)}["']{0,1}`, 'ig');
}
/**
* Removes a label:"value" expression from the query.
*/
export function removeFilterFromQuery(query: string, key: string, value: string, modifier: ModifierType = ''): string {
const regex = getFilterRegex(key, value);
const matches = query.matchAll(regex);
const opRegex = new RegExp(`\\s+(?:AND|OR)\\s*$|^\\s*(?:AND|OR)\\s+`, 'ig');
for (const match of matches) {
if (modifier === '-' && match[0].startsWith(modifier)) {
query = query.replace(regex, '').replace(opRegex, '');
}
if (modifier === '' && !match[0].startsWith('-')) {
query = query.replace(regex, '').replace(opRegex, '');
}
}
query = query.replace(/AND\s+OR/gi, 'OR');
query = query.replace(/OR\s+AND/gi, 'AND');
return query;
}

View File

@ -637,22 +637,55 @@ 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' };
describe('and the filter is already present', () => {
it('then it should remove the filter', () => {
const query: LokiQuery = { refId: 'A', expr: '{bar="baz", job="grafana"}' };
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`');
expect(result.expr).toEqual('{bar="baz"}');
});
it('then the correct label should be added for metrics query', () => {
const query: LokiQuery = { refId: 'A', expr: 'rate({bar="baz"} | logfmt [5m])' };
it('then it should remove the filter with escaped value', () => {
const query: LokiQuery = { refId: 'A', expr: '{place="luna", job="\\"grafana/data\\""}' };
const action = { options: { key: 'job', value: '"grafana/data"' }, type: 'ADD_FILTER' };
const result = ds.modifyQuery(query, action);
expect(result.refId).toEqual('A');
expect(result.expr).toEqual('{place="luna"}');
});
});
});
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('and the filter is already present', () => {
it('then it should remove the filter', () => {
const query: LokiQuery = { refId: 'A', expr: '{bar="baz"} | logfmt | job="grafana"' };
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])');
expect(result.expr).toEqual('{bar="baz"} | logfmt');
});
});
});
@ -691,28 +724,52 @@ 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);
});
it('then the correct label should be added for logs query', () => {
const query: LokiQuery = { refId: 'A', expr: '{bar="baz"} | logfmt' };
describe('and the opposite filter is present', () => {
it('then it should remove the filter', () => {
const query: LokiQuery = { refId: 'A', expr: '{bar="baz", job="grafana"}' };
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", job!="grafana"}');
});
});
});
describe('and query has 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"} | 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('rate({bar="baz"} | logfmt | job!=`grafana` [5m])');
});
describe('and the filter is already present', () => {
it('then it should remove the filter', () => {
const query: LokiQuery = { refId: 'A', expr: '{bar="baz"} | logfmt | job="grafana"' };
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])');
});
});
});
});

View File

@ -65,6 +65,8 @@ import {
addLineFilter,
findLastPosition,
getLabelFilterPositions,
queryHasFilter,
removeLabelFromQuery,
} from './modifyQuery';
import { getQueryHints } from './queryHints';
import { runSplitQuery } from './querySplitting';
@ -614,13 +616,27 @@ export class LokiDatasource
case 'ADD_FILTER': {
if (action.options?.key && action.options?.value) {
const value = escapeLabelValueInSelector(action.options.value);
expression = addLabelToQuery(expression, action.options.key, '=', value);
// This gives the user the ability to toggle a filter on and off.
expression = queryHasFilter(expression, action.options.key, '=', value)
? removeLabelFromQuery(expression, action.options.key, '=', value)
: addLabelToQuery(expression, action.options.key, '=', value);
}
break;
}
case 'ADD_FILTER_OUT': {
if (action.options?.key && action.options?.value) {
const value = escapeLabelValueInSelector(action.options.value);
/**
* If there is a filter with the same key and value, remove it.
* This prevents the user from seeing no changes in the query when they apply
* this filter.
*/
if (queryHasFilter(expression, action.options.key, '=', value)) {
expression = removeLabelFromQuery(expression, action.options.key, '=', value);
}
expression = addLabelToQuery(expression, action.options.key, '!=', value);
}
break;

View File

@ -6,7 +6,9 @@ import {
addNoPipelineErrorToQuery,
addParserToQuery,
NodePosition,
queryHasFilter,
removeCommentsFromQuery,
removeLabelFromQuery,
} from './modifyQuery';
describe('addLabelToQuery()', () => {
@ -246,3 +248,47 @@ describe('NodePosition', () => {
});
});
});
describe('queryHasFilter', () => {
it.each([
['{job="grafana"}', 'grafana'],
['{job="grafana", foo="bar"}', 'grafana'],
['{foo="bar", job="grafana"}', 'grafana'],
['{job="\\"grafana\\""}', '"grafana"'],
['{foo="bar"} | logfmt | job=`grafana`', 'grafana'],
])('should return true if query has a positive filter', (query: string, value: string) => {
expect(queryHasFilter(query, 'job', '=', value)).toBe(true);
});
it.each([
['{job!="grafana"}', 'grafana'],
['{job!="grafana", foo="bar"}', 'grafana'],
['{foo="bar", job!="grafana"}', 'grafana'],
['{job!="\\"grafana\\""}', '"grafana"'],
['{foo="bar"} | logfmt | job!=`grafana`', 'grafana'],
])('should return true if query has a negative filter', (query: string, value: string) => {
expect(queryHasFilter(query, 'job', '!=', value)).toBe(true);
});
});
describe('removeLabelFromQuery', () => {
it.each([
['{job="grafana"}', 'grafana', '{}'],
['{job="grafana", foo="bar"}', 'grafana', '{foo="bar"}'],
['{foo="bar", job="grafana"}', 'grafana', '{foo="bar"}'],
['{job="\\"grafana\\""}', '"grafana"', '{}'],
['{foo="bar"} | logfmt | job=`grafana`', 'grafana', '{foo="bar"} | logfmt'],
])('should remove a positive label matcher from the query', (query: string, value: string, expected: string) => {
expect(removeLabelFromQuery(query, 'job', '=', value)).toBe(expected);
});
it.each([
['{job!="grafana"}', 'grafana', '{}'],
['{job!="grafana", foo="bar"}', 'grafana', '{foo="bar"}'],
['{foo="bar", job!="grafana"}', 'grafana', '{foo="bar"}'],
['{job!="\\"grafana\\""}', '"grafana"', '{}'],
['{foo="bar"} | logfmt | job!=`grafana`', 'grafana', '{foo="bar"} | logfmt'],
])('should remove a negative label matcher from the query', (query: string, value: string, expected: string) => {
expect(removeLabelFromQuery(query, 'job', '!=', value)).toBe(expected);
});
});

View File

@ -2,6 +2,7 @@ import { NodeType, SyntaxNode } from '@lezer/common';
import { sortBy } from 'lodash';
import {
Identifier,
JsonExpressionParser,
LabelFilter,
LabelParser,
@ -14,13 +15,14 @@ import {
PipelineExpr,
Selector,
UnwrapExpr,
String,
} from '@grafana/lezer-logql';
import { QueryBuilderLabelFilter } from '../prometheus/querybuilder/shared/types';
import { unescapeLabelValue } from './languageUtils';
import { LokiQueryModeller } from './querybuilder/LokiQueryModeller';
import { buildVisualQueryFromString } from './querybuilder/parsing';
import { lokiQueryModeller as modeller } from './querybuilder/LokiQueryModeller';
import { buildVisualQueryFromString, handleQuotes } from './querybuilder/parsing';
export class NodePosition {
from: number;
@ -45,6 +47,88 @@ export class NodePosition {
return query.substring(this.from, this.to);
}
}
/**
* Checks for the presence of a given label=value filter in any Matcher expression in the query.
*/
export function queryHasFilter(query: string, key: string, operator: string, value: string): boolean {
const matchers = getMatchersWithFilter(query, key, operator, value);
return matchers.length > 0;
}
/**
* Removes a label=value Matcher expression from the query.
*/
export function removeLabelFromQuery(query: string, key: string, operator: string, value: string): string {
const matchers = getMatchersWithFilter(query, key, operator, value);
for (const matcher of matchers) {
query =
matcher.parent?.type.id === LabelFilter ? removeLabelFilter(query, matcher) : removeSelector(query, matcher);
}
return query;
}
function removeLabelFilter(query: string, matcher: SyntaxNode): string {
const pipelineStage = matcher.parent?.parent;
if (!pipelineStage) {
return query;
}
return (query.substring(0, pipelineStage.from) + query.substring(pipelineStage.to)).trim();
}
function removeSelector(query: string, matcher: SyntaxNode): string {
let selector: SyntaxNode | null = matcher;
do {
selector = selector.parent;
} while (selector && selector.type.id !== Selector);
const label = matcher.getChild(Identifier);
if (!selector || !label) {
return query;
}
const labelName = query.substring(label.from, label.to);
const prefix = query.substring(0, selector.from);
const suffix = query.substring(selector.to);
const matchVisQuery = buildVisualQueryFromString(query.substring(selector.from, selector.to));
matchVisQuery.query.labels = matchVisQuery.query.labels.filter((label) => label.label !== labelName);
return prefix + modeller.renderQuery(matchVisQuery.query) + suffix;
}
function getMatchersWithFilter(query: string, key: string, operator: string, value: string): SyntaxNode[] {
const tree = parser.parse(query);
const matchers: SyntaxNode[] = [];
tree.iterate({
enter: ({ type, node }): void => {
if (type.id === Matcher) {
matchers.push(node);
}
},
});
return matchers.filter((matcher) => {
const labelNode = matcher.getChild(Identifier);
const opNode = labelNode?.nextSibling;
const valueNode = matcher.getChild(String);
if (!labelNode || !opNode || !valueNode) {
return false;
}
const labelName = query.substring(labelNode.from, labelNode.to);
if (labelName !== key) {
return false;
}
const labelValue = query.substring(valueNode.from, valueNode.to);
if (handleQuotes(labelValue) !== unescapeLabelValue(value)) {
return false;
}
const labelOperator = query.substring(opNode.from, opNode.to);
if (labelOperator !== operator) {
return false;
}
return true;
});
}
/**
* Adds label filter to existing query. Useful for query modification for example for ad hoc filters.
*
@ -65,6 +149,10 @@ export function addLabelToQuery(query: string, key: string, operator: string, va
}
const streamSelectorPositions = getStreamSelectorPositions(query);
if (!streamSelectorPositions.length) {
return query;
}
const hasStreamSelectorMatchers = getMatcherInStreamPositions(query);
const everyStreamSelectorHasMatcher = streamSelectorPositions.every((streamSelectorPosition) =>
hasStreamSelectorMatchers.some(
@ -74,9 +162,6 @@ export function addLabelToQuery(query: string, key: string, operator: string, va
);
const parserPositions = getParserPositions(query);
const labelFilterPositions = getLabelFilterPositions(query);
if (!streamSelectorPositions.length) {
return query;
}
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.
@ -103,6 +188,9 @@ export function addParserToQuery(query: string, parser: string): string {
return addParser(query, lineFilterPositions, parser);
} else {
const streamSelectorPositions = getStreamSelectorPositions(query);
if (!streamSelectorPositions.length) {
return query;
}
return addParser(query, streamSelectorPositions, parser);
}
}
@ -302,13 +390,11 @@ function addFilterToStreamSelector(
vectorSelectorPositions: NodePosition[],
filter: QueryBuilderLabelFilter
): string {
const modeller = new LokiQueryModeller();
let newQuery = '';
let prev = 0;
for (let i = 0; i < vectorSelectorPositions.length; i++) {
// This is basically just doing splice on a string for each matched vector selector.
const match = vectorSelectorPositions[i];
const isLast = i === vectorSelectorPositions.length - 1;
@ -421,6 +507,9 @@ function addLabelFormat(
export function addLineFilter(query: string): string {
const streamSelectorPositions = getStreamSelectorPositions(query);
if (!streamSelectorPositions.length) {
return query;
}
const streamSelectorEnd = streamSelectorPositions[0].to;
const newQueryExpr = query.slice(0, streamSelectorEnd) + ' |= ``' + query.slice(streamSelectorEnd);

View File

@ -597,7 +597,7 @@ function isIntervalVariableError(node: SyntaxNode) {
return node?.parent?.type.id === Range;
}
function handleQuotes(string: string) {
export function handleQuotes(string: string) {
if (string[0] === `"` && string[string.length - 1] === `"`) {
return string
.substring(1, string.length - 1)