From 9ba38b760ac3652857289da20d1982db959885a6 Mon Sep 17 00:00:00 2001 From: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com> Date: Thu, 23 Mar 2023 17:46:33 +0100 Subject: [PATCH] Elasticsearch: Unify adhoc variables processing (#65274) --- .betterer.results | 8 +- .../elasticsearch/QueryBuilder.test.ts | 50 ------------- .../datasource/elasticsearch/QueryBuilder.ts | 57 +------------- .../datasource/elasticsearch/datasource.ts | 75 +++++++++---------- 4 files changed, 41 insertions(+), 149 deletions(-) diff --git a/.betterer.results b/.betterer.results index bb39269fe37..c86406c9462 100644 --- a/.betterer.results +++ b/.betterer.results @@ -4618,13 +4618,9 @@ exports[`better eslint`] = { [0, 0, 0, "Unexpected any. Specify a different type.", "7"], [0, 0, 0, "Unexpected any. Specify a different type.", "8"], [0, 0, 0, "Unexpected any. Specify a different type.", "9"], - [0, 0, 0, "Unexpected any. Specify a different type.", "10"], + [0, 0, 0, "Do not use any type assertions.", "10"], [0, 0, 0, "Unexpected any. Specify a different type.", "11"], - [0, 0, 0, "Unexpected any. Specify a different type.", "12"], - [0, 0, 0, "Unexpected any. Specify a different type.", "13"], - [0, 0, 0, "Do not use any type assertions.", "14"], - [0, 0, 0, "Unexpected any. Specify a different type.", "15"], - [0, 0, 0, "Unexpected any. Specify a different type.", "16"] + [0, 0, 0, "Unexpected any. Specify a different type.", "12"] ], "public/app/plugins/datasource/elasticsearch/components/AddRemove.tsx:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"] diff --git a/public/app/plugins/datasource/elasticsearch/QueryBuilder.test.ts b/public/app/plugins/datasource/elasticsearch/QueryBuilder.test.ts index c9f8f06ae32..d2566708d63 100644 --- a/public/app/plugins/datasource/elasticsearch/QueryBuilder.test.ts +++ b/public/app/plugins/datasource/elasticsearch/QueryBuilder.test.ts @@ -604,36 +604,6 @@ describe('ElasticQueryBuilder', () => { expect(firstLevel.nested.path).toBe('nested_field'); }); - // This test wasn't migrated, as adhoc variables are going to be interpolated before - // Or we need to add this to backend query builder (TBD) - it('with adhoc filters', () => { - const query = builder.build( - { - refId: 'A', - metrics: [{ type: 'count', id: '0' }], - timeField: '@timestamp', - bucketAggs: [{ type: 'date_histogram', field: '@timestamp', id: '3' }], - }, - [ - { key: 'key1', operator: '=', value: 'value1', condition: '' }, - { key: 'key2', operator: '=', value: 'value2', condition: '' }, - { key: 'key2', operator: '!=', value: 'value2', condition: '' }, - { key: 'key3', operator: '<', value: 'value3', condition: '' }, - { key: 'key4', operator: '>', value: 'value4', condition: '' }, - { key: 'key5', operator: '=~', value: 'value5', condition: '' }, - { key: 'key6', operator: '!~', value: 'value6', condition: '' }, - ] - ); - - expect(query.query.bool.must[0].match_phrase['key1'].query).toBe('value1'); - expect(query.query.bool.must[1].match_phrase['key2'].query).toBe('value2'); - expect(query.query.bool.must_not[0].match_phrase['key2'].query).toBe('value2'); - expect(query.query.bool.filter[1].range['key3'].lt).toBe('value3'); - expect(query.query.bool.filter[2].range['key4'].gt).toBe('value4'); - expect(query.query.bool.filter[3].regexp['key5']).toBe('value5'); - expect(query.query.bool.filter[4].bool.must_not.regexp['key6']).toBe('value6'); - }); - describe('getTermsQuery', () => { function testGetTermsQuery(queryDef: TermsQuery) { const query = builder.getTermsQuery(queryDef); @@ -769,26 +739,6 @@ describe('ElasticQueryBuilder', () => { ).toBeFalsy(); }); }); - - it('with adhoc filters', () => { - // TODO: Types for AdHocFilters - const adhocFilters = [ - { key: 'key1', operator: '=', value: 'value1', condition: '' }, - { key: 'key2', operator: '!=', value: 'value2', condition: '' }, - { key: 'key3', operator: '<', value: 'value3', condition: '' }, - { key: 'key4', operator: '>', value: 'value4', condition: '' }, - { key: 'key5', operator: '=~', value: 'value5', condition: '' }, - { key: 'key6', operator: '!~', value: 'value6', condition: '' }, - ]; - const query = builder.getLogsQuery({ refId: 'A' }, 500, adhocFilters); - - expect(query.query.bool.must[0].match_phrase['key1'].query).toBe('value1'); - expect(query.query.bool.must_not[0].match_phrase['key2'].query).toBe('value2'); - expect(query.query.bool.filter[1].range['key3'].lt).toBe('value3'); - expect(query.query.bool.filter[2].range['key4'].gt).toBe('value4'); - expect(query.query.bool.filter[3].regexp['key5']).toBe('value5'); - expect(query.query.bool.filter[4].bool.must_not.regexp['key6']).toBe('value6'); - }); }); describe('Value casting for settings', () => { diff --git a/public/app/plugins/datasource/elasticsearch/QueryBuilder.ts b/public/app/plugins/datasource/elasticsearch/QueryBuilder.ts index abb1ae00f38..b55be456509 100644 --- a/public/app/plugins/datasource/elasticsearch/QueryBuilder.ts +++ b/public/app/plugins/datasource/elasticsearch/QueryBuilder.ts @@ -1,4 +1,4 @@ -import { AdHocVariableFilter, InternalTimeZones } from '@grafana/data'; +import { InternalTimeZones } from '@grafana/data'; import { isMetricAggregationWithField, @@ -154,54 +154,7 @@ export class ElasticQueryBuilder { return query; } - addAdhocFilters(query: any, adhocFilters: any) { - if (!adhocFilters) { - return; - } - - let i, filter, condition: any, queryCondition: any; - - for (i = 0; i < adhocFilters.length; i++) { - filter = adhocFilters[i]; - condition = {}; - condition[filter.key] = filter.value; - queryCondition = {}; - queryCondition[filter.key] = { query: filter.value }; - - switch (filter.operator) { - case '=': - if (!query.query.bool.must) { - query.query.bool.must = []; - } - query.query.bool.must.push({ match_phrase: queryCondition }); - break; - case '!=': - if (!query.query.bool.must_not) { - query.query.bool.must_not = []; - } - query.query.bool.must_not.push({ match_phrase: queryCondition }); - break; - case '<': - condition[filter.key] = { lt: filter.value }; - query.query.bool.filter.push({ range: condition }); - break; - case '>': - condition[filter.key] = { gt: filter.value }; - query.query.bool.filter.push({ range: condition }); - break; - case '=~': - query.query.bool.filter.push({ regexp: condition }); - break; - case '!~': - query.query.bool.filter.push({ - bool: { must_not: { regexp: condition } }, - }); - break; - } - } - } - - build(target: ElasticsearchQuery, adhocFilters?: AdHocVariableFilter[]) { + build(target: ElasticsearchQuery) { // make sure query has defaults; target.metrics = target.metrics || [defaultMetricAgg()]; target.bucketAggs = target.bucketAggs || [defaultBucketAgg()]; @@ -230,8 +183,6 @@ export class ElasticQueryBuilder { ]; } - this.addAdhocFilters(query, adhocFilters); - // If target doesn't have bucketAggs and type is not raw_document, it is invalid query. if (target.bucketAggs.length === 0) { metric = target.metrics[0]; @@ -483,7 +434,7 @@ export class ElasticQueryBuilder { return query; } - getLogsQuery(target: ElasticsearchQuery, limit: number, adhocFilters?: AdHocVariableFilter[]) { + getLogsQuery(target: ElasticsearchQuery, limit: number) { let query: any = { size: 0, query: { @@ -493,8 +444,6 @@ export class ElasticQueryBuilder { }, }; - this.addAdhocFilters(query, adhocFilters); - if (target.query) { query.query.bool.filter.push({ query_string: { diff --git a/public/app/plugins/datasource/elasticsearch/datasource.ts b/public/app/plugins/datasource/elasticsearch/datasource.ts index 1587848514a..c30e87d0c07 100644 --- a/public/app/plugins/datasource/elasticsearch/datasource.ts +++ b/public/app/plugins/datasource/elasticsearch/datasource.ts @@ -391,40 +391,8 @@ export class ElasticDatasource return this.templateSrv.replace(queryString, scopedVars, 'lucene'); } - interpolateVariablesInQueries(queries: ElasticsearchQuery[], scopedVars: ScopedVars): ElasticsearchQuery[] { - // We need a separate interpolation format for lucene queries, therefore we first interpolate any - // lucene query string and then everything else - const interpolateBucketAgg = (bucketAgg: BucketAggregation): BucketAggregation => { - if (bucketAgg.type === 'filters') { - return { - ...bucketAgg, - settings: { - ...bucketAgg.settings, - filters: bucketAgg.settings?.filters?.map((filter) => ({ - ...filter, - query: this.interpolateLuceneQuery(filter.query, scopedVars) || '*', - })), - }, - }; - } - - return bucketAgg; - }; - - const expandedQueries = queries.map( - (query): ElasticsearchQuery => ({ - ...query, - datasource: this.getRef(), - query: this.addAdHocFilters(this.interpolateLuceneQuery(query.query || '', scopedVars)), - bucketAggs: query.bucketAggs?.map(interpolateBucketAgg), - }) - ); - - const finalQueries: ElasticsearchQuery[] = JSON.parse( - this.templateSrv.replace(JSON.stringify(expandedQueries), scopedVars) - ); - - return finalQueries; + interpolateVariablesInQueries(queries: ElasticsearchQuery[], scopedVars: ScopedVars | {}): ElasticsearchQuery[] { + return queries.map((q) => this.applyTemplateVariables(q, scopedVars)); } testDatasource() { @@ -684,9 +652,6 @@ export class ElasticDatasource const sentTargets: ElasticsearchQuery[] = []; let targetsContainsLogsQuery = targets.some((target) => hasMetricOfType(target, 'logs')); - // add global adhoc filters to timeFilter - const adhocFilters = this.templateSrv.getAdhocFilters(this.name); - const logLimits: Array = []; for (const target of targets) { @@ -709,14 +674,14 @@ export class ElasticDatasource target.metrics = []; // Setting this for metrics queries that are typed as logs - queryObj = this.queryBuilder.getLogsQuery(target, limit, adhocFilters); + queryObj = this.queryBuilder.getLogsQuery(target, limit); } else { logLimits.push(); if (target.alias) { target.alias = this.interpolateLuceneQuery(target.alias, request.scopedVars); } - queryObj = this.queryBuilder.build(target, adhocFilters); + queryObj = this.queryBuilder.build(target); } const esQuery = JSON.stringify(queryObj); @@ -1039,6 +1004,38 @@ export class ElasticDatasource const finalQuery = [query, ...esFilters].filter((f) => f).join(' AND '); return finalQuery; } + + // Used when running queries through backend + applyTemplateVariables(query: ElasticsearchQuery, scopedVars: ScopedVars): ElasticsearchQuery { + // We need a separate interpolation format for lucene queries, therefore we first interpolate any + // lucene query string and then everything else + const interpolateBucketAgg = (bucketAgg: BucketAggregation): BucketAggregation => { + if (bucketAgg.type === 'filters') { + return { + ...bucketAgg, + settings: { + ...bucketAgg.settings, + filters: bucketAgg.settings?.filters?.map((filter) => ({ + ...filter, + query: this.interpolateLuceneQuery(filter.query, scopedVars) || '*', + })), + }, + }; + } + + return bucketAgg; + }; + + const expandedQuery = { + ...query, + datasource: this.getRef(), + query: this.addAdHocFilters(this.interpolateLuceneQuery(query.query || '', scopedVars)), + bucketAggs: query.bucketAggs?.map(interpolateBucketAgg), + }; + + const finalQuery = JSON.parse(this.templateSrv.replace(JSON.stringify(expandedQuery), scopedVars)); + return finalQuery; + } } /**