Loki: Fix support of ad-hoc filters for specific queries (#51232)

* Loki: Refactor ad-hoc filters to use parser

* Remove renaming of files for easier review

* Update

* Update

* Add previously buggy test

* Fix tests

* Fix typos

* Update, improve typing

* Move reused code up

* Update order

* Update betterer statss

.betterer.results

@@ -5919,7 +5919,7 @@ exports[`no explicit any`] = {
       [58, 64, 3, "Unexpected any. Specify a different type.", "193409811"],
       [65, 17, 3, "Unexpected any. Specify a different type.", "193409811"]
     ],
-    "public/app/plugins/datasource/loki/datasource.test.ts:821497219": [
+    "public/app/plugins/datasource/loki/datasource.test.ts:371314051": [
       [42, 39, 3, "Unexpected any. Specify a different type.", "193409811"],
       [43, 40, 3, "Unexpected any. Specify a different type.", "193409811"],
       [135, 20, 3, "Unexpected any. Specify a different type.", "193409811"],
@@ -5936,18 +5936,15 @@ exports[`no explicit any`] = {
       [853, 45, 3, "Unexpected any. Specify a different type.", "193409811"],
       [870, 9, 3, "Unexpected any. Specify a different type.", "193409811"]
     ],
-    "public/app/plugins/datasource/loki/datasource.ts:1442011327": [
+    "public/app/plugins/datasource/loki/datasource.ts:979697429": [
       [219, 23, 3, "Unexpected any. Specify a different type.", "193409811"],
       [355, 30, 3, "Unexpected any. Specify a different type.", "193409811"],
       [359, 30, 3, "Unexpected any. Specify a different type.", "193409811"],
       [359, 45, 3, "Unexpected any. Specify a different type.", "193409811"],
       [373, 40, 3, "Unexpected any. Specify a different type.", "193409811"],
       [560, 33, 3, "Unexpected any. Specify a different type.", "193409811"],
-      [641, 61, 3, "Unexpected any. Specify a different type.", "193409811"],
+      [685, 41, 3, "Unexpected any. Specify a different type.", "193409811"],
-      [641, 77, 3, "Unexpected any. Specify a different type.", "193409811"],
+      [692, 46, 3, "Unexpected any. Specify a different type.", "193409811"]
-      [641, 90, 3, "Unexpected any. Specify a different type.", "193409811"],
-      [699, 41, 3, "Unexpected any. Specify a different type.", "193409811"],
-      [706, 46, 3, "Unexpected any. Specify a different type.", "193409811"]
     ],
     "public/app/plugins/datasource/loki/getDerivedFields.ts:1557842937": [
       [37, 87, 3, "Unexpected any. Specify a different type.", "193409811"]

public/app/plugins/datasource/loki/add_label_to_query.test.ts

@@ -1,121 +1,155 @@
-import { addLabelToQuery, addLabelToSelector } from './add_label_to_query';
+import { addLabelToQuery } from './add_label_to_query';
 
 describe('addLabelToQuery()', () => {
   it('should add label to simple query', () => {
     expect(() => {
-      addLabelToQuery('foo', '', '');
+      addLabelToQuery('foo', '', '=', '');
     }).toThrow();
-    expect(addLabelToQuery('{}', 'bar', 'baz')).toBe('{bar="baz"}');
+    expect(addLabelToQuery('{}', 'bar', '=', 'baz')).toBe('{bar="baz"}');
-    expect(addLabelToQuery('{x="yy"}', 'bar', 'baz')).toBe('{bar="baz",x="yy"}');
+    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('{}', 'bar', '!=', 'baz')).toBe('{bar!="baz"}');
-    expect(addLabelToQuery('{x="yy"}', 'bar', 'baz', '!=')).toBe('{bar!="baz",x="yy"}');
+    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])');
+    expect(addLabelToQuery('rate({}[1m])', 'foo', '=', 'bar')).toBe('rate({foo="bar"}[1m])');
   });
 
   it('should detect in-order function use', () => {
-    expect(addLabelToQuery('sum by (xx) ({})', 'bar', 'baz')).toBe('sum by (xx) ({bar="baz"})');
+    expect(addLabelToQuery('sum by (host) (rate({} [1m]))', 'bar', '=', 'baz')).toBe(
-  });
+      'sum by (host) (rate({bar="baz"} [1m]))'
-
+    );
-  it('should convert number Infinity to +Inf', () => {
-    expect(addLabelToQuery('sum(rate({}[5m])) by (le)', 'le', Infinity)).toBe('sum(rate({le="+Inf"}[5m])) by (le)');
   });
 
   it('should handle selectors with punctuation', () => {
-    expect(addLabelToQuery('{instance="my-host.com:9100"}', 'bar', 'baz')).toBe(
+    expect(addLabelToQuery('{instance="my-host.com:9100"}', 'bar', '=', 'baz')).toBe(
-      '{bar="baz",instance="my-host.com:9100"}'
+      '{instance="my-host.com:9100", bar="baz"}'
     );
-    expect(addLabelToQuery('{list="a,b,c"}', 'bar', 'baz')).toBe('{bar="baz",list="a,b,c"}');
+    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('{} + {}', 'bar', '=', 'baz')).toBe('{bar="baz"} + {bar="baz"}');
-    expect(addLabelToQuery('avg({}) + sum({})', 'bar', 'baz')).toBe('avg({bar="baz"}) + sum({bar="baz"})');
+    expect(addLabelToQuery('avg(rate({x="y"} [$__interval]))+ sum(rate({}[5m]))', 'bar', '=', 'baz')).toBe(
-    expect(addLabelToQuery('{x="yy"} * {y="zz",a="bb"} * {}', 'bar', 'baz')).toBe(
+      'avg(rate({x="y", bar="baz"} [$__interval]))+ sum(rate({bar="baz"}[5m]))'
-      '{bar="baz",x="yy"} * {a="bb",bar="baz",y="zz"} * {bar="baz"}'
+    );
+    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(
+    expect(addLabelToQuery(addLabelToQuery('{x="yy"}', 'bar', '!=', 'baz'), 'bar', '!=', 'baz')).toBe(
-      '{bar!="baz",x="yy"}'
+      '{x="yy", bar!="baz"}'
     );
-    expect(addLabelToQuery(addLabelToQuery('rate({}[1m])', 'foo', 'bar'), 'foo', 'bar')).toBe('rate({foo="bar"}[1m])');
+    expect(addLabelToQuery(addLabelToQuery('rate({}[1m])', 'foo', '=', 'bar'), 'foo', '=', 'bar')).toBe(
-    expect(addLabelToQuery(addLabelToQuery('{list="a,b,c"}', 'bar', 'baz'), 'bar', 'baz')).toBe(
+      'rate({foo="bar"}[1m])'
-      '{bar="baz",list="a,b,c"}'
     );
-    expect(addLabelToQuery(addLabelToQuery('avg({}) + sum({})', 'bar', 'baz'), 'bar', 'baz')).toBe(
+    expect(addLabelToQuery(addLabelToQuery('{list="a,b,c"}', 'bar', '=', 'baz'), 'bar', '=', 'baz')).toBe(
-      'avg({bar="baz"}) + sum({bar="baz"})'
+      '{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('{bar="baz",x="y"} |="yy"');
+    expect(addLabelToQuery('{x="y"} |="yy"', 'bar', '=', 'baz')).toBe('{x="y", bar="baz"} |="yy"');
-    expect(addLabelToQuery('{x="y"} |="yy" !~"xx"', 'bar', 'baz')).toBe('{bar="baz",x="y"} |="yy" !~"xx"');
+    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', undefined, true)).toBe(
+    expect(addLabelToQuery('{job="grafana"} |= "foo-bar"', 'filename', '=', 'test.txt')).toBe(
-      '{filename="test.txt",job="grafana"} |= "foo-bar"'
+      '{job="grafana", filename="test.txt"} |= "foo-bar"'
     );
   });
 
   it('should add labels to metrics with logical operators', () => {
-    expect(addLabelToQuery('{} or {}', 'bar', 'baz')).toBe('{bar="baz"} or {bar="baz"}');
+    expect(addLabelToQuery('{x="y"} or {}', 'bar', '=', 'baz')).toBe('{x="y", bar="baz"} or {bar="baz"}');
-    expect(addLabelToQuery('{} and {}', 'bar', 'baz')).toBe('{bar="baz"} and {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(
+    expect(addLabelToQuery('sum(rate({job="foo"}[2m])) by (value $variable)', 'bar', '=', 'baz')).toBe(
-      'sum(rate({bar="baz",job="foo"}[2m])) by (value $variable)'
+      '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(
+    expect(addLabelToQuery('avg(rate(({job="foo"} > 0)[3h:])) by (label)', 'bar', '=', 'baz')).toBe(
-      'avg(rate(({bar="baz",job="foo"} > 0)[3h:])) by (label)'
+      '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) ({}) * 1000',
+        '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) ({bar="baz",type=~"foo|bar|baz-test"}) * on(id) group_right(id, type, name) sum by (id) ({bar="baz"}) * 1000'
+      '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({}[${__range_s}s])', 'bar', 'baz')).toBe('rate({bar="baz"}[${__range_s}s])');
+    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(
+    expect(addLabelToQuery('count({job!="foo"} < (5*1024*1024*1024) or vector(0)) - 1', 'bar', '=', 'baz')).toBe(
-      'count({bar="baz",job!="foo"} < (5*1024*1024*1024) or vector(0)) - 1'
+      'count({job!="foo", bar="baz"} < (5*1024*1024*1024) or vector(0)) - 1'
     );
   });
-  it('should not add adhoc filter to line_format expressions', () => {
-    expect(addLabelToQuery('{foo="bar"} | logfmt | line_format {{.status}}', 'bar', 'baz')).toBe(
-      '{bar="baz",foo="bar"} | logfmt | line_format {{.status}}'
-    );
-  });
-});
 
-describe('addLabelToSelector()', () => {
+  describe('should add label as label filter is query with parser', () => {
-  test('should add a label to an empty selector', () => {
+    it('should add label filter after parser', () => {
-    expect(addLabelToSelector('{}', 'foo', 'bar')).toBe('{foo="bar"}');
+      expect(addLabelToQuery('{foo="bar"} | logfmt', 'bar', '=', 'baz')).toBe('{foo="bar"} | logfmt | bar=`baz`');
-    expect(addLabelToSelector('', 'foo', 'bar')).toBe('{foo="bar"}');
+    });
-  });
+    it('should add label filter after multiple parsers', () => {
-  test('should add a label to a selector', () => {
+      expect(addLabelToQuery('{foo="bar"} | logfmt | json', 'bar', '=', 'baz')).toBe(
-    expect(addLabelToSelector('{foo="bar"}', 'baz', '42')).toBe('{baz="42",foo="bar"}');
+        '{foo="bar"} | logfmt | bar=`baz` | json | bar=`baz`'
-  });
+      );
-  test('should add a label to a selector with custom operator', () => {
+    });
-    expect(addLabelToSelector('{}', 'baz', '42', '!=')).toBe('{baz!="42"}');
+    it('should add label filter after parser when multiple label filters', () => {
+      expect(addLabelToQuery('{foo="bar"} | logfmt | x="y"', 'bar', '=', 'baz')).toBe(
+        '{foo="bar"} | logfmt | bar=`baz` | x="y"'
+      );
+    });
+    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 | bar=`baz` | x="y" | 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}}"'
+      );
+    });
   });
 });

public/app/plugins/datasource/loki/add_label_to_query.ts

@@ -1,140 +1,159 @@
-import { chain, isEqual } from 'lodash';
+import { parser } from '@grafana/lezer-logql';
 
-import { PROM_KEYWORDS, OPERATORS, LOGICAL_OPERATORS } from 'app/plugins/datasource/prometheus/promql';
+import { QueryBuilderLabelFilter } from '../prometheus/querybuilder/shared/types';
 
-import { LOKI_KEYWORDS } from './syntax';
+import { LokiQueryModeller } from './querybuilder/LokiQueryModeller';
+import { buildVisualQueryFromString } from './querybuilder/parsing';
+import { LokiVisualQuery } from './querybuilder/types';
 
-const builtInWords = [...PROM_KEYWORDS, ...OPERATORS, ...LOGICAL_OPERATORS, ...LOKI_KEYWORDS];
+/**
-
+ * Adds label filter to existing query. Useful for query modification for example for ad hoc filters.
-// We want to extract all possible metrics and also keywords
+ *
-const metricsAndKeywordsRegexp = /([A-Za-z:][\w:]*)\b(?![\]{=!",])/g;
+ * It uses LogQL parser to find instances of labels, alters them and then splices them back into the query.
-
+ * In a case when we have parser, instead of adding new instance of label it adds label filter after the parser.
-export function addLabelToQuery(
+ *
-  query: string,
+ * This operates on substrings of the query with labels and operates just on those. This makes this
-  key: string,
+ * more robust and can alter even invalid queries, and preserves in general the query structure and whitespace.
-  value: string | number,
+ *
-  operator?: string,
+ * @param query
-  hasNoMetrics?: boolean
+ * @param key
-): string {
+ * @param value
+ * @param operator
+ */
+export function addLabelToQuery(query: string, key: string, operator: string, value: string): string {
   if (!key || !value) {
     throw new Error('Need label to add to query.');
   }
 
-  // We need to make sure that we convert the value back to string because it may be a number
+  const streamSelectorPositions = getStreamSelectorPositions(query);
-  const transformedValue = value === Infinity ? '+Inf' : value.toString();
+  const parserPositions = getParserPositions(query);
+  if (!streamSelectorPositions.length) {
+    return query;
+  }
 
-  // Add empty selectors to bare metric names
+  const filter = toLabelFilter(key, value, operator);
-  let previousWord: string;
+  if (!parserPositions.length) {
+    return addFilterToStreamSelector(query, streamSelectorPositions, filter);
+  } else {
+    return addFilterAsLabelFilter(query, parserPositions, filter);
+  }
+}
 
-  query = query.replace(metricsAndKeywordsRegexp, (match, word, offset) => {
+type StreamSelectorPosition = { from: number; to: number; query: LokiVisualQuery };
-    const isMetric = isWordMetric(query, word, offset, previousWord, hasNoMetrics);
+type PipelineStagePosition = { from: number; to: number };
-    previousWord = word;
 
-    return isMetric ? `${word}{}` : word;
+/**
+ * Parse the string and get all Selector positions in the query together with parsed representation of the
+ * selector.
+ * @param query
+ */
+function getStreamSelectorPositions(query: string): StreamSelectorPosition[] {
+  const tree = parser.parse(query);
+  const positions: StreamSelectorPosition[] = [];
+  tree.iterate({
+    enter: (type, from, to, get): false | void => {
+      if (type.name === 'Selector') {
+        const visQuery = buildVisualQueryFromString(query.substring(from, to));
+        positions.push({ query: visQuery.query, from, to });
+        return false;
+      }
+    },
   });
+  return positions;
+}
 
-  //This is a RegExp for stream selector - e.g. {job="grafana"}
+/**
-  const selectorRegexp = /(\$)?{([^{]*)}/g;
+ * Parse the string and get all LabelParser positions in the query.
-  const parts = [];
+ * @param query
-  let lastIndex = 0;
+ */
-  let suffix = '';
+function getParserPositions(query: string): PipelineStagePosition[] {
+  const tree = parser.parse(query);
+  const positions: PipelineStagePosition[] = [];
+  tree.iterate({
+    enter: (type, from, to, get): false | void => {
+      if (type.name === 'LabelParser') {
+        positions.push({ from, to });
+        return false;
+      }
+    },
+  });
+  return positions;
+}
 
-  let match = selectorRegexp.exec(query);
+function toLabelFilter(key: string, value: string, operator: string): QueryBuilderLabelFilter {
-  /* 
+  // We need to make sure that we convert the value back to string because it may be a number
-    There are 2 possible false positive scenarios: 
+  return { label: key, op: operator, value };
-    
+}
-    1. We match Grafana's variables with ${ syntax - such as${__rate_s}. To filter these out we could use negative lookbehind,
-    but Safari browser currently doesn't support it. Therefore we need to hack this by creating 2 matching groups. 
-    (\$) is for the Grafana's variables and if we match it, we know this is not a stream selector and we don't want to add label.
 
-    2. Log queries can include {{.label}} syntax when line_format is used. We need to filter these out by checking
+/**
-    if match starts with "{."
+ * Add filter as to stream selectors
-  */
+ * @param query
-  while (match) {
+ * @param vectorSelectorPositions
-    const prefix = query.slice(lastIndex, match.index);
+ * @param filter
-    lastIndex = match.index + match[2].length + 2;
+ */
-    suffix = query.slice(match.index + match[0].length);
+function addFilterToStreamSelector(
+  query: string,
+  vectorSelectorPositions: StreamSelectorPosition[],
+  filter: QueryBuilderLabelFilter
+): string {
+  const modeller = new LokiQueryModeller();
+  let newQuery = '';
+  let prev = 0;
 
-    // Filtering our false positives
+  for (let i = 0; i < vectorSelectorPositions.length; i++) {
-    if (match[0].startsWith('{.') || match[1]) {
+    // This is basically just doing splice on a string for each matched vector selector.
-      parts.push(prefix);
+
-      parts.push(match[0]);
+    const match = vectorSelectorPositions[i];
-    } else {
+    const isLast = i === vectorSelectorPositions.length - 1;
-      // If we didn't match first group, we are inside selector and we want to add labels
+
-      const selector = match[2];
+    const start = query.substring(prev, match.from);
-      const selectorWithLabel = addLabelToSelector(selector, key, transformedValue, operator);
+    const end = isLast ? query.substring(match.to) : '';
-      parts.push(prefix, selectorWithLabel);
+
+    if (!labelExists(match.query.labels, filter)) {
+      // We don't want to add duplicate labels.
+      match.query.labels.push(filter);
     }
-
+    const newLabels = modeller.renderQuery(match.query);
-    match = selectorRegexp.exec(query);
+    newQuery += start + newLabels + end;
+    prev = match.to;
   }
-
+  return newQuery;
-  parts.push(suffix);
-  return parts.join('');
 }
 
-const labelRegexp = /(\w+)\s*(=|!=|=~|!~)\s*("[^"]*")/g;
+/**
+ * Add filter as label filter after the parsers
+ * @param query
+ * @param parserPositions
+ * @param filter
+ */
+function addFilterAsLabelFilter(
+  query: string,
+  parserPositions: PipelineStagePosition[],
+  filter: QueryBuilderLabelFilter
+): string {
+  let newQuery = '';
+  let prev = 0;
 
-export function addLabelToSelector(selector: string, labelKey: string, labelValue: string, labelOperator?: string) {
+  for (let i = 0; i < parserPositions.length; i++) {
-  const parsedLabels = [];
+    // This is basically just doing splice on a string for each matched vector selector.
+    const match = parserPositions[i];
+    const isLast = i === parserPositions.length - 1;
 
-  // Split selector into labels
+    const start = query.substring(prev, match.to);
-  if (selector) {
+    const end = isLast ? query.substring(match.to) : '';
-    let match = labelRegexp.exec(selector);
+
-    while (match) {
+    const labelFilter = ` | ${filter.label}${filter.op}\`${filter.value}\``;
-      parsedLabels.push({ key: match[1], operator: match[2], value: match[3] });
+    newQuery += start + labelFilter + end;
-      match = labelRegexp.exec(selector);
+    prev = match.to;
-    }
   }
-
+  return newQuery;
-  // Add new label
-  const operatorForLabelKey = labelOperator || '=';
-  parsedLabels.push({ key: labelKey, operator: operatorForLabelKey, value: `"${labelValue}"` });
-
-  // Sort labels by key and put them together
-  const formatted = chain(parsedLabels)
-    .uniqWith(isEqual)
-    .compact()
-    .sortBy('key')
-    .map(({ key, operator, value }) => `${key}${operator}${value}`)
-    .value()
-    .join(',');
-
-  return `{${formatted}}`;
 }
 
-function isPositionInsideChars(text: string, position: number, openChar: string, closeChar: string) {
+/**
-  const nextSelectorStart = text.slice(position).indexOf(openChar);
+ * Check if label exists in the list of labels but ignore the operator.
-  const nextSelectorEnd = text.slice(position).indexOf(closeChar);
+ * @param labels
-  return nextSelectorEnd > -1 && (nextSelectorStart === -1 || nextSelectorStart > nextSelectorEnd);
+ * @param filter
+ */
+function labelExists(labels: QueryBuilderLabelFilter[], filter: QueryBuilderLabelFilter) {
+  return labels.find((label) => label.label === filter.label && label.value === filter.value);
 }
-
-function isWordMetric(query: string, word: string, offset: number, previousWord: string, hasNoMetrics?: boolean) {
-  const insideSelector = isPositionInsideChars(query, offset, '{', '}');
-  // Handle "sum by (key) (metric)"
-  const previousWordIsKeyWord = previousWord && OPERATORS.indexOf(previousWord) > -1;
-  // Check for colon as as "word boundary" symbol
-  const isColonBounded = word.endsWith(':');
-  // Check for words that start with " which means that they are not metrics
-  const startsWithQuote = query[offset - 1] === '"';
-  // Check for template variables
-  const isTemplateVariable = query[offset - 1] === '$';
-  // Check for time units
-  const isTimeUnit = ['s', 'm', 'h', 'd', 'w'].includes(word) && Boolean(Number(query[offset - 1]));
-
-  if (
-    !hasNoMetrics &&
-    !insideSelector &&
-    !isColonBounded &&
-    !previousWordIsKeyWord &&
-    !startsWithQuote &&
-    !isTemplateVariable &&
-    !isTimeUnit &&
-    builtInWords.indexOf(word) === -1
-  ) {
-    return true;
-  }
-  return false;
-}
-
-export default addLabelToQuery;

public/app/plugins/datasource/loki/datasource.test.ts

@@ -206,7 +206,7 @@ describe('LokiDatasource', () => {
       ]);
 
       expect(ds.applyTemplateVariables(query, {}).expr).toBe(
-        'rate({bar="baz",job="foo",k1="v1",k2!="v2"} |= "bar" [5m])'
+        'rate({bar="baz", job="foo", k1="v1", k2!="v2"} |= "bar" [5m])'
       );
     });
 
@@ -224,7 +224,7 @@ describe('LokiDatasource', () => {
         },
       ]);
       expect(ds.applyTemplateVariables(query, {}).expr).toBe(
-        'rate({bar="baz",job="foo",k1=~"v\\\\.\\\\*",k2=~"v\'\\\\.\\\\*"} |= "bar" [5m])'
+        'rate({bar="baz", job="foo", k1=~"v\\\\.\\\\*", k2=~"v\'\\\\.\\\\*"} |= "bar" [5m])'
       );
     });
   });
@@ -452,7 +452,7 @@ describe('LokiDatasource', () => {
         });
       });
       describe('When textFormat is set', () => {
-        it('should fromat the text accordingly', async () => {
+        it('should format the text accordingly', async () => {
           const res = await getTestContext(testFrame, { textFormat: 'hello {{label2}}', stepInterval: '15s' });
 
           expect(res.length).toBe(1);
@@ -460,7 +460,7 @@ describe('LokiDatasource', () => {
         });
       });
       describe('When titleFormat is set', () => {
-        it('should fromat the title accordingly', async () => {
+        it('should format the title accordingly', async () => {
           const res = await getTestContext(testFrame, { titleFormat: 'Title {{label2}}', stepInterval: '15s' });
 
           expect(res.length).toBe(1);
@@ -527,7 +527,7 @@ describe('LokiDatasource', () => {
           const result = ds.modifyQuery(query, action);
 
           expect(result.refId).toEqual('A');
-          expect(result.expr).toEqual('{bar="baz",job="grafana"}');
+          expect(result.expr).toEqual('{bar="baz", job="grafana"}');
         });
 
         it('then the correctly escaped label should be added for logs query', () => {
@@ -537,7 +537,7 @@ describe('LokiDatasource', () => {
           const result = ds.modifyQuery(query, action);
 
           expect(result.refId).toEqual('A');
-          expect(result.expr).toEqual('{bar="baz",job="\\\\test"}');
+          expect(result.expr).toEqual('{bar="baz", job="\\\\test"}');
         });
 
         it('then the correct label should be added for metrics query', () => {
@@ -547,7 +547,7 @@ describe('LokiDatasource', () => {
           const result = ds.modifyQuery(query, action);
 
           expect(result.refId).toEqual('A');
-          expect(result.expr).toEqual('rate({bar="baz",job="grafana"}[5m])');
+          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', () => {
@@ -557,7 +557,7 @@ describe('LokiDatasource', () => {
             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"} | 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])' };
@@ -566,7 +566,7 @@ describe('LokiDatasource', () => {
             const result = ds.modifyQuery(query, action);
 
             expect(result.refId).toEqual('A');
-            expect(result.expr).toEqual('rate({bar="baz",job="grafana"} | logfmt [5m])');
+            expect(result.expr).toEqual('rate({bar="baz"} | logfmt | job=`grafana` [5m])');
           });
         });
       });
@@ -581,7 +581,7 @@ describe('LokiDatasource', () => {
           const result = ds.modifyQuery(query, action);
 
           expect(result.refId).toEqual('A');
-          expect(result.expr).toEqual('{bar="baz",job!="grafana"}');
+          expect(result.expr).toEqual('{bar="baz", job!="grafana"}');
         });
 
         it('then the correctly escaped label should be added for logs query', () => {
@@ -591,7 +591,7 @@ describe('LokiDatasource', () => {
           const result = ds.modifyQuery(query, action);
 
           expect(result.refId).toEqual('A');
-          expect(result.expr).toEqual('{bar="baz",job!="\\"test"}');
+          expect(result.expr).toEqual('{bar="baz", job!="\\"test"}');
         });
 
         it('then the correct label should be added for metrics query', () => {
@@ -601,7 +601,7 @@ describe('LokiDatasource', () => {
           const result = ds.modifyQuery(query, action);
 
           expect(result.refId).toEqual('A');
-          expect(result.expr).toEqual('rate({bar="baz",job!="grafana"}[5m])');
+          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', () => {
@@ -611,7 +611,7 @@ describe('LokiDatasource', () => {
             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"} | 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])' };
@@ -620,7 +620,7 @@ describe('LokiDatasource', () => {
             const result = ds.modifyQuery(query, action);
 
             expect(result.refId).toEqual('A');
-            expect(result.expr).toEqual('rate({bar="baz",job!="grafana"} | logfmt [5m])');
+            expect(result.expr).toEqual('rate({bar="baz"} | logfmt | job!=`grafana` [5m])');
           });
         });
       });
@@ -648,19 +648,19 @@ describe('LokiDatasource', () => {
       });
       describe('and query has no parser', () => {
         it('then the correct label should be added for logs query', () => {
-          assertAdHocFilters('{bar="baz"}', '{bar="baz",job="grafana"}', ds);
+          assertAdHocFilters('{bar="baz"}', '{bar="baz", job="grafana"}', ds);
         });
 
         it('then the correct label should be added for metrics query', () => {
-          assertAdHocFilters('rate({bar="baz"}[5m])', 'rate({bar="baz",job="grafana"}[5m])', ds);
+          assertAdHocFilters('rate({bar="baz"}[5m])', 'rate({bar="baz", job="grafana"}[5m])', ds);
         });
       });
       describe('and query has parser', () => {
         it('then the correct label should be added for logs query', () => {
-          assertAdHocFilters('{bar="baz"} | logfmt', '{bar="baz",job="grafana"} | logfmt', ds);
+          assertAdHocFilters('{bar="baz"} | logfmt', '{bar="baz"} | logfmt | job=`grafana`', ds);
         });
         it('then the correct label should be added for metrics query', () => {
-          assertAdHocFilters('rate({bar="baz"} | logfmt [5m])', 'rate({bar="baz",job="grafana"} | logfmt [5m])', ds);
+          assertAdHocFilters('rate({bar="baz"} | logfmt [5m])', 'rate({bar="baz"} | logfmt | job=`grafana` [5m])', ds);
         });
       });
     });
@@ -683,19 +683,19 @@ describe('LokiDatasource', () => {
       });
       describe('and query has no parser', () => {
         it('then the correct label should be added for logs query', () => {
-          assertAdHocFilters('{bar="baz"}', '{bar="baz",job!="grafana"}', ds);
+          assertAdHocFilters('{bar="baz"}', '{bar="baz", job!="grafana"}', ds);
         });
 
         it('then the correct label should be added for metrics query', () => {
-          assertAdHocFilters('rate({bar="baz"}[5m])', 'rate({bar="baz",job!="grafana"}[5m])', ds);
+          assertAdHocFilters('rate({bar="baz"}[5m])', 'rate({bar="baz", job!="grafana"}[5m])', ds);
         });
       });
       describe('and query has parser', () => {
         it('then the correct label should be added for logs query', () => {
-          assertAdHocFilters('{bar="baz"} | logfmt', '{bar="baz",job!="grafana"} | logfmt', ds);
+          assertAdHocFilters('{bar="baz"} | logfmt', '{bar="baz"} | logfmt | job!=`grafana`', ds);
         });
         it('then the correct label should be added for metrics query', () => {
-          assertAdHocFilters('rate({bar="baz"} | logfmt [5m])', 'rate({bar="baz",job!="grafana"} | logfmt [5m])', ds);
+          assertAdHocFilters('rate({bar="baz"} | logfmt [5m])', 'rate({bar="baz"} | logfmt | job!=`grafana` [5m])', ds);
         });
       });
     });

public/app/plugins/datasource/loki/datasource.ts

@@ -50,7 +50,7 @@ import { LokiAnnotationsQueryEditor } from './components/AnnotationsQueryEditor'
 import LanguageProvider from './language_provider';
 import { escapeLabelValueInSelector } from './language_utils';
 import { LiveStreams, LokiLiveTarget } from './live_streams';
-import { addParsedLabelToQuery, getNormalizedLokiQuery, queryHasPipeParser } from './query_utils';
+import { getNormalizedLokiQuery } from './query_utils';
 import { sortDataFrameByTime } from './sortDataFrame';
 import { doLokiChannelStream } from './streaming';
 import syntax from './syntax';
@@ -375,11 +375,11 @@ export class LokiDatasource
     let expression = query.expr ?? '';
     switch (action.type) {
       case 'ADD_FILTER': {
-        expression = this.addLabelToQuery(expression, action.key, action.value, '=');
+        expression = this.addLabelToQuery(expression, action.key, '=', action.value);
         break;
       }
       case 'ADD_FILTER_OUT': {
-        expression = this.addLabelToQuery(expression, action.key, action.value, '!=');
+        expression = this.addLabelToQuery(expression, action.key, '!=', action.value);
         break;
       }
       default:
@@ -639,31 +639,17 @@ export class LokiDatasource
     const adhocFilters = this.templateSrv.getAdhocFilters(this.name);
     let expr = queryExpr;
 
-    expr = adhocFilters.reduce((acc: string, filter: { key?: any; operator?: any; value?: any }) => {
+    expr = adhocFilters.reduce((acc: string, filter: { key: string; operator: string; value: string }) => {
-      const { key, operator } = filter;
+      const { key, operator, value } = filter;
-      let { value } = filter;
+      return this.addLabelToQuery(acc, key, operator, value);
-      return this.addLabelToQuery(acc, key, value, operator, true);
     }, expr);
 
     return expr;
   }
 
-  addLabelToQuery(
+  addLabelToQuery(queryExpr: string, key: string, operator: string, value: string) {
-    queryExpr: string,
+    const escapedValue = escapeLabelValueInSelector(value, operator);
-    key: string,
+    return addLabelToQuery(queryExpr, key, operator, escapedValue);
-    value: string | number,
-    operator: string,
-    // Override to make sure that we use label as actual label and not parsed label
-    notParsedLabelOverride?: boolean
-  ) {
-    let escapedValue = escapeLabelValueInSelector(value.toString(), operator);
-
-    if (queryHasPipeParser(queryExpr) && !isMetricsQuery(queryExpr) && !notParsedLabelOverride) {
-      // If query has parser, we treat all labels as parsed and use | key="value" syntax
-      return addParsedLabelToQuery(queryExpr, key, escapedValue, operator);
-    } else {
-      return addLabelToQuery(queryExpr, key, escapedValue, operator, true);
-    }
   }
 
   // Used when running queries through backend

public/app/plugins/datasource/loki/query_utils.ts

@@ -1,6 +1,5 @@
 import { escapeRegExp } from 'lodash';
 
-import { PIPE_PARSERS } from './syntax';
 import { LokiQuery, LokiQueryType } from './types';
 
 export function formatQuery(selector: string | undefined): string {
@@ -64,16 +63,6 @@ export function getHighlighterExpressionsFromQuery(input: string): string[] {
   return results;
 }
 
-export function queryHasPipeParser(expr: string): boolean {
-  const parsers = PIPE_PARSERS.map((parser) => `${parser.label}`).join('|');
-  const regexp = new RegExp(`\\\|\\\s?(${parsers})`);
-  return regexp.test(expr);
-}
-
-export function addParsedLabelToQuery(expr: string, key: string, value: string | number, operator: string) {
-  return expr + ` | ${key}${operator}"${value.toString()}"`;
-}
-
 // we are migrating from `.instant` and `.range` to `.queryType`
 // this function returns a new query object that:
 // - has `.queryType`