Loki: Fix redundant escaping in adhoc filter with regex match (#56447)

* Loki: Fix redundant escaping in adhoc filter with regex match

* Update data.js

* Simplify test

* Simplify test

* Update

* Add more tests

devenv/docker/blocks/loki/data/data.js

@@ -125,8 +125,8 @@ async function main() {
     await sleep(getNextSineWaveSleepDuration());
     const timestampMs = new Date().getTime();
     const item = getRandomLogItem(step + 1)
-    lokiSendLogLine(timestampMs, JSON.stringify(item), {place:'moon', source: 'data'});
+    lokiSendLogLine(timestampMs, JSON.stringify(item), {place:'moon', source: 'data', instance: 'server\\1', job: '"grafana/data"'});
-    lokiSendLogLine(timestampMs, logFmtLine(item), {place:'luna', source: 'data'});
+    lokiSendLogLine(timestampMs, logFmtLine(item), {place:'luna', source: 'data', instance: 'server\\2', job: '"grafana/data"'});
   }
 }
 

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

@@ -201,7 +201,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])'
       );
     });
   });
@@ -665,10 +665,15 @@ describe('LokiDatasource', () => {
 
   describe('addAdHocFilters', () => {
     let ds: LokiDatasource;
-    let adHocFilters: AdHocFilter[];
+    const createTemplateSrvMock = (options: { adHocFilters: AdHocFilter[] }) => {
+      return {
+        getAdhocFilters: (): AdHocFilter[] => options.adHocFilters,
+        replace: (a: string) => a,
+      } as unknown as TemplateSrv;
+    };
     describe('when called with "=" operator', () => {
       beforeEach(() => {
-        adHocFilters = [
+        const defaultAdHocFilters: AdHocFilter[] = [
           {
             condition: '',
             key: 'job',
@@ -676,11 +681,7 @@ describe('LokiDatasource', () => {
             value: 'grafana',
           },
         ];
-        const templateSrvMock = {
+        ds = createLokiDatasource(createTemplateSrvMock({ adHocFilters: defaultAdHocFilters }));
-          getAdhocFilters: (): AdHocFilter[] => adHocFilters,
-          replace: (a: string) => a,
-        } as unknown as TemplateSrv;
-        ds = createLokiDatasource(templateSrvMock);
       });
       describe('and query has no parser', () => {
         it('then the correct label should be added for logs query', () => {
@@ -706,6 +707,21 @@ describe('LokiDatasource', () => {
         it('then the correct label should be added for metrics query with empty selector and variable', () => {
           assertAdHocFilters('rate({}[$__interval])', 'rate({job="grafana"}[$__interval])', ds);
         });
+        it('should correctly escape special characters in ad hoc filter', () => {
+          const ds = createLokiDatasource(
+            createTemplateSrvMock({
+              adHocFilters: [
+                {
+                  condition: '',
+                  key: 'instance',
+                  operator: '=',
+                  value: '"test"',
+                },
+              ],
+            })
+          );
+          assertAdHocFilters('{job="grafana"}', '{job="grafana", instance="\\"test\\""}', ds);
+        });
       });
       describe('and query has parser', () => {
         it('then the correct label should be added for logs query', () => {
@@ -719,7 +735,7 @@ describe('LokiDatasource', () => {
 
     describe('when called with "!=" operator', () => {
       beforeEach(() => {
-        adHocFilters = [
+        const defaultAdHocFilters: AdHocFilter[] = [
           {
             condition: '',
             key: 'job',
@@ -727,11 +743,7 @@ describe('LokiDatasource', () => {
             value: 'grafana',
           },
         ];
-        const templateSrvMock = {
+        ds = createLokiDatasource(createTemplateSrvMock({ adHocFilters: defaultAdHocFilters }));
-          getAdhocFilters: (): AdHocFilter[] => adHocFilters,
-          replace: (a: string) => a,
-        } as unknown as TemplateSrv;
-        ds = createLokiDatasource(templateSrvMock);
       });
       describe('and query has no parser', () => {
         it('then the correct label should be added for logs query', () => {
@@ -751,6 +763,23 @@ describe('LokiDatasource', () => {
         });
       });
     });
+
+    describe('when called with regex operator', () => {
+      beforeEach(() => {
+        const defaultAdHocFilters: AdHocFilter[] = [
+          {
+            condition: '',
+            key: 'instance',
+            operator: '=~',
+            value: '.*',
+          },
+        ];
+        ds = createLokiDatasource(createTemplateSrvMock({ adHocFilters: defaultAdHocFilters }));
+      });
+      it('should not escape special characters in ad hoc filter', () => {
+        assertAdHocFilters('{job="grafana"}', '{job="grafana", instance=~".*"}', ds);
+      });
+    });
   });
 
   describe('prepareLogRowContextQueryTarget', () => {

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

@@ -49,7 +49,7 @@ import LanguageProvider from './LanguageProvider';
 import { LiveStreams, LokiLiveTarget } from './LiveStreams';
 import { transformBackendResult } from './backendResultTransformer';
 import { LokiAnnotationsQueryEditor } from './components/AnnotationsQueryEditor';
-import { escapeLabelValueInSelector } from './languageUtils';
+import { escapeLabelValueInSelector, isRegexSelector } from './languageUtils';
 import { labelNamesRegex, labelValuesRegex } from './migrations/variableQueryMigrations';
 import {
   addLabelFormatToQuery,
@@ -456,13 +456,15 @@ export class LokiDatasource
     switch (action.type) {
       case 'ADD_FILTER': {
         if (action.options?.key && action.options?.value) {
-          expression = this.addLabelToQuery(expression, action.options.key, '=', action.options.value);
+          const value = escapeLabelValueInSelector(action.options.value);
+          expression = addLabelToQuery(expression, action.options.key, '=', value);
         }
         break;
       }
       case 'ADD_FILTER_OUT': {
         if (action.options?.key && action.options?.value) {
-          expression = this.addLabelToQuery(expression, action.options.key, '!=', action.options.value);
+          const value = escapeLabelValueInSelector(action.options.value);
+          expression = addLabelToQuery(expression, action.options.key, '!=', value);
         }
         break;
       }
@@ -747,18 +749,23 @@ export class LokiDatasource
     let expr = replaceVariables(queryExpr);
 
     expr = adhocFilters.reduce((acc: string, filter: { key: string; operator: string; value: string }) => {
-      const { key, operator, value } = filter;
+      const { key, operator } = filter;
-      return this.addLabelToQuery(acc, key, operator, value);
+      let { value } = filter;
+      if (isRegexSelector(operator)) {
+        // Adhoc filters don't support multiselect, therefore if user selects regex operator
+        // we are going to consider value to be regex filter and use lokiRegularEscape
+        // that does not escape regex special characters (e.g. .*test.* => .*test.*)
+        value = lokiRegularEscape(value);
+      } else {
+        // Otherwise, we want to escape special characters in value
+        value = escapeLabelValueInSelector(value, operator);
+      }
+      return addLabelToQuery(acc, key, operator, value);
     }, expr);
 
     return returnVariables(expr);
   }
 
-  addLabelToQuery(queryExpr: string, key: string, operator: string, value: string) {
-    const escapedValue = escapeLabelValueInSelector(value, operator);
-    return addLabelToQuery(queryExpr, key, operator, escapedValue);
-  }
-
   // Used when running queries through backend
   filterQuery(query: LokiQuery): boolean {
     if (query.hide || query.expr === '') {
@@ -794,6 +801,9 @@ export class LokiDatasource
   }
 }
 
+// NOTE: these two functions are very similar to the escapeLabelValueIn* functions
+// in language_utils.ts, but they are not exactly the same algorithm, and we found
+// no way to reuse one in the another or vice versa.
 export function lokiRegularEscape(value: any) {
   if (typeof value === 'string') {
     return value.replace(/'/g, "\\\\'");