Elasticsearch: move adhoc filters code to modifyQuery module and refactor (#76529)

* Elasticsearch: move addhoc filters to modifyQuery and refactor

* Elasticsearch: add support for number-value adhoc filters

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

@@ -1307,8 +1307,14 @@ describe('queryHasFilter()', () => {
 
 describe('addAdhocFilters', () => {
   describe('with invalid filters', () => {
+    let ds: ElasticDatasource, templateSrv: TemplateSrv;
+    beforeEach(() => {
+      const context = getTestContext();
+      ds = context.ds;
+      templateSrv = context.templateSrv;
+    });
+
     it('should filter out ad hoc filter without key', () => {
-      const { ds, templateSrv } = getTestContext();
       jest.mocked(templateSrv.getAdhocFilters).mockReturnValue([{ key: '', operator: '=', value: 'a', condition: '' }]);
 
       const query = ds.addAdHocFilters('foo:"bar"');
@@ -1316,7 +1322,6 @@ describe('addAdhocFilters', () => {
     });
 
     it('should filter out ad hoc filter without value', () => {
-      const { ds, templateSrv } = getTestContext();
       jest.mocked(templateSrv.getAdhocFilters).mockReturnValue([{ key: 'a', operator: '=', value: '', condition: '' }]);
 
       const query = ds.addAdHocFilters('foo:"bar"');
@@ -1324,7 +1329,6 @@ describe('addAdhocFilters', () => {
     });
 
     it('should filter out filter ad hoc filter with invalid operator', () => {
-      const { ds, templateSrv } = getTestContext();
       jest.mocked(templateSrv.getAdhocFilters).mockReturnValue([{ key: 'a', operator: 'A', value: '', condition: '' }]);
 
       const query = ds.addAdHocFilters('foo:"bar"');
@@ -1349,10 +1353,50 @@ describe('addAdhocFilters', () => {
     });
 
     it('should correctly add 1 ad hoc filter when query is empty', () => {
-      const query = ds.addAdHocFilters('');
+      expect(ds.addAdHocFilters('')).toBe('test:"test1"');
-      expect(query).toBe('test:"test1"');
+      expect(ds.addAdHocFilters(' ')).toBe('test:"test1"');
+      expect(ds.addAdHocFilters('  ')).toBe('test:"test1"');
     });
 
+    it('should not fail if the filter value is a number', () => {
+      jest
+        .mocked(templateSrvMock.getAdhocFilters)
+        // @ts-expect-error
+        .mockReturnValue([{ key: 'key', operator: '=', value: 1, condition: '' }]);
+      expect(ds.addAdHocFilters('')).toBe('key:"1"');
+    });
+
+    it.each(['=', '!=', '=~', '!~', '>', '<', '', ''])(
+      `should properly build queries with '%s' filters`,
+      (operator: string) => {
+        jest
+          .mocked(templateSrvMock.getAdhocFilters)
+          .mockReturnValue([{ key: 'key', operator, value: 'value', condition: '' }]);
+
+        const query = ds.addAdHocFilters('foo:"bar"');
+        switch (operator) {
+          case '=':
+            expect(query).toBe('foo:"bar" AND key:"value"');
+            break;
+          case '!=':
+            expect(query).toBe('foo:"bar" AND -key:"value"');
+            break;
+          case '=~':
+            expect(query).toBe('foo:"bar" AND key:/value/');
+            break;
+          case '!~':
+            expect(query).toBe('foo:"bar" AND -key:/value/');
+            break;
+          case '>':
+            expect(query).toBe('foo:"bar" AND key:>value');
+            break;
+          case '<':
+            expect(query).toBe('foo:"bar" AND key:<value');
+            break;
+        }
+      }
+    );
+
     it('should escape characters in filter keys', () => {
       jest
         .mocked(templateSrvMock.getAdhocFilters)

public/app/plugins/datasource/elasticsearch/datasource.ts

@@ -56,13 +56,7 @@ import {
 } from './components/QueryEditor/MetricAggregationsEditor/aggregations';
 import { metricAggregationConfig } from './components/QueryEditor/MetricAggregationsEditor/utils';
 import { isMetricAggregationWithMeta } from './guards';
-import {
+import { addAddHocFilter, addFilterToQuery, queryHasFilter, removeFilterFromQuery } from './modifyQuery';
-  addFilterToQuery,
-  escapeFilter,
-  escapeFilterValue,
-  queryHasFilter,
-  removeFilterFromQuery,
-} from './modifyQuery';
 import { trackAnnotationQuery, trackQuery } from './tracking';
 import {
   Logs,
@@ -955,35 +949,11 @@ export class ElasticDatasource
     if (adhocFilters.length === 0) {
       return query;
     }
-    const esFilters = adhocFilters.map((filter) => {
+    let finalQuery = query;
-      let { key, operator, value } = filter;
+    adhocFilters.forEach((filter) => {
-      if (!key || !value) {
+      finalQuery = addAddHocFilter(finalQuery, filter);
-        return;
-      }
-      /**
-       * Keys and values in ad hoc filters may contain characters such as
-       * colons, which needs to be escaped.
-       */
-      key = escapeFilter(key);
-      value = escapeFilterValue(value);
-      switch (operator) {
-        case '=':
-          return `${key}:"${value}"`;
-        case '!=':
-          return `-${key}:"${value}"`;
-        case '=~':
-          return `${key}:/${value}/`;
-        case '!~':
-          return `-${key}:/${value}/`;
-        case '>':
-          return `${key}:>${value}`;
-        case '<':
-          return `${key}:<${value}`;
-      }
-      return;
     });
 
-    const finalQuery = [query, ...esFilters].filter((f) => f).join(' AND ');
     return finalQuery;
   }
 

public/app/plugins/datasource/elasticsearch/modifyQuery.ts

@@ -1,6 +1,8 @@
 import { isEqual } from 'lodash';
 import lucene, { AST, BinaryAST, LeftOnlyAST, NodeTerm } from 'lucene';
 
+import { AdHocVariableFilter } from '@grafana/data';
+
 type ModifierType = '' | '-';
 
 /**
@@ -65,7 +67,59 @@ export function addFilterToQuery(query: string, key: string, value: string, modi
   value = lucene.phrase.escape(value);
   const filter = `${modifier}${key}:"${value}"`;
 
-  return query === '' ? filter : `${query} AND ${filter}`;
+  return concatenate(query, filter);
+}
+
+/**
+ * Merge a query with a filter.
+ */
+function concatenate(query: string, filter: string, condition = 'AND'): string {
+  if (!filter) {
+    return query;
+  }
+  return query.trim() === '' ? filter : `${query} ${condition} ${filter}`;
+}
+
+/**
+ * Adds a label:"value" expression to the query.
+ */
+export function addAddHocFilter(query: string, filter: AdHocVariableFilter): string {
+  if (!filter.key || !filter.value) {
+    return query;
+  }
+
+  filter = {
+    ...filter,
+    // Type is defined as string, but it can be a number.
+    value: filter.value.toString(),
+  };
+
+  const equalityFilters = ['=', '!='];
+  if (equalityFilters.includes(filter.operator)) {
+    return addFilterToQuery(query, filter.key, filter.value, filter.operator === '=' ? '' : '-');
+  }
+  /**
+   * Keys and values in ad hoc filters may contain characters such as
+   * colons, which needs to be escaped.
+   */
+  const key = escapeFilter(filter.key);
+  const value = escapeFilterValue(filter.value);
+  let addHocFilter = '';
+  switch (filter.operator) {
+    case '=~':
+      addHocFilter = `${key}:/${value}/`;
+      break;
+    case '!~':
+      addHocFilter = `-${key}:/${value}/`;
+      break;
+    case '>':
+      addHocFilter = `${key}:>${value}`;
+      break;
+    case '<':
+      addHocFilter = `${key}:<${value}`;
+      break;
+  }
+  return concatenate(query, addHocFilter);
 }
 
 /**