From 3cf0103782109a64cc38fdc10c12c9643eaf9226 Mon Sep 17 00:00:00 2001 From: Giordano Ricci Date: Tue, 2 Mar 2021 09:10:41 +0000 Subject: [PATCH] Elasticsearch: Fix impossibility to perform non-logs queries after importing queries from loki or prometheus in explore (#31518) * Chore: remove isLogsQuery flag from ES datasource * Minore chores * Update public/app/plugins/datasource/elasticsearch/datasource.ts Co-authored-by: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com> Co-authored-by: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com> --- .../getAlertingValidationMessage.test.ts | 16 ++++++++-------- .../datasource/elasticsearch/datasource.test.ts | 4 +--- .../datasource/elasticsearch/datasource.ts | 9 +++++---- .../datasource/elasticsearch/elastic_response.ts | 2 +- .../elasticsearch/language_provider.test.ts | 1 - .../elasticsearch/language_provider.ts | 1 - .../datasource/elasticsearch/query_def.ts | 5 +++-- .../elasticsearch/specs/elastic_response.test.ts | 7 +------ .../plugins/datasource/elasticsearch/types.ts | 1 - 9 files changed, 19 insertions(+), 27 deletions(-) diff --git a/public/app/features/alerting/getAlertingValidationMessage.test.ts b/public/app/features/alerting/getAlertingValidationMessage.test.ts index ed505319381..218f364dea6 100644 --- a/public/app/features/alerting/getAlertingValidationMessage.test.ts +++ b/public/app/features/alerting/getAlertingValidationMessage.test.ts @@ -28,8 +28,8 @@ describe('getAlertingValidationMessage', () => { getInstanceSettings: (() => {}) as any, }; const targets: ElasticsearchQuery[] = [ - { refId: 'A', query: '@hostname:$hostname', isLogsQuery: false }, - { refId: 'B', query: '@instance:instance', isLogsQuery: false }, + { refId: 'A', query: '@hostname:$hostname' }, + { refId: 'B', query: '@instance:instance' }, ]; const transformations: DataTransformerConfig[] = []; @@ -95,8 +95,8 @@ describe('getAlertingValidationMessage', () => { }, }; const targets: ElasticsearchQuery[] = [ - { refId: 'A', query: '@hostname:$hostname', isLogsQuery: false }, - { refId: 'B', query: '@instance:$instance', isLogsQuery: false }, + { refId: 'A', query: '@hostname:$hostname' }, + { refId: 'B', query: '@instance:$instance' }, ]; const transformations: DataTransformerConfig[] = []; @@ -124,8 +124,8 @@ describe('getAlertingValidationMessage', () => { }, }; const targets: ElasticsearchQuery[] = [ - { refId: 'A', query: '@hostname:hostname', isLogsQuery: false }, - { refId: 'B', query: '@instance:instance', isLogsQuery: false }, + { refId: 'A', query: '@hostname:hostname' }, + { refId: 'B', query: '@instance:instance' }, ]; const transformations: DataTransformerConfig[] = []; @@ -153,8 +153,8 @@ describe('getAlertingValidationMessage', () => { }, }; const targets: ElasticsearchQuery[] = [ - { refId: 'A', query: '@hostname:hostname', isLogsQuery: false }, - { refId: 'B', query: '@instance:instance', isLogsQuery: false }, + { refId: 'A', query: '@hostname:hostname' }, + { refId: 'B', query: '@instance:instance' }, ]; const transformations: DataTransformerConfig[] = [{ id: 'A', options: null }]; diff --git a/public/app/plugins/datasource/elasticsearch/datasource.test.ts b/public/app/plugins/datasource/elasticsearch/datasource.test.ts index eb7c45ba586..9d69c76923e 100644 --- a/public/app/plugins/datasource/elasticsearch/datasource.test.ts +++ b/public/app/plugins/datasource/elasticsearch/datasource.test.ts @@ -226,9 +226,8 @@ describe('ElasticDatasource', function (this: any) { id: '2', }, ], - metrics: [{ type: 'count', id: '1' }], + metrics: [{ type: 'logs', id: '1' }], query: 'escape\\:test', - isLogsQuery: true, timeField: '@timestamp', }, ], @@ -957,7 +956,6 @@ const createElasticQuery = (): DataQueryRequest => { targets: [ { refId: '', - isLogsQuery: false, bucketAggs: [{ type: 'date_histogram', field: '@timestamp', id: '2' }], metrics: [{ type: 'count', id: '' }], query: 'test', diff --git a/public/app/plugins/datasource/elasticsearch/datasource.ts b/public/app/plugins/datasource/elasticsearch/datasource.ts index 17e10910804..08ce0e6331a 100644 --- a/public/app/plugins/datasource/elasticsearch/datasource.ts +++ b/public/app/plugins/datasource/elasticsearch/datasource.ts @@ -497,7 +497,7 @@ export class ElasticDatasource extends DataSourceApi hasMetricOfType(target, 'logs')); // add global adhoc filters to timeFilter const adhocFilters = this.templateSrv.getAdhocFilters(this.name); @@ -540,11 +541,10 @@ export class ElasticDatasource extends DataSourceApi { const er = new ElasticResponse(sentTargets, res); - if (sentTargets.some((target) => target.isLogsQuery)) { + // TODO: This needs to be revisited, it seems wrong to process ALL the sent queries as logs if only one of them was a log query + if (targetsContainsLogsQuery) { const response = er.getLogs(this.logMessageField, this.logLevelField); for (const dataFrame of response.data) { enhanceDataFrame(dataFrame, this.dataLinks); diff --git a/public/app/plugins/datasource/elasticsearch/elastic_response.ts b/public/app/plugins/datasource/elasticsearch/elastic_response.ts index 01f2d60751b..e7c13b44bbb 100644 --- a/public/app/plugins/datasource/elasticsearch/elastic_response.ts +++ b/public/app/plugins/datasource/elasticsearch/elastic_response.ts @@ -425,7 +425,7 @@ export class ElasticResponse { } getTimeSeries() { - if (this.targets.some((target) => target.metrics?.some((metric) => metric.type === 'raw_data'))) { + if (this.targets.some((target) => queryDef.hasMetricOfType(target, 'raw_data'))) { return this.processResponseToDataFrames(false); } return this.processResponseToSeries(); diff --git a/public/app/plugins/datasource/elasticsearch/language_provider.test.ts b/public/app/plugins/datasource/elasticsearch/language_provider.test.ts index 1a00a8cfc8d..c8b2c9f0c63 100644 --- a/public/app/plugins/datasource/elasticsearch/language_provider.test.ts +++ b/public/app/plugins/datasource/elasticsearch/language_provider.test.ts @@ -26,7 +26,6 @@ const dataSource = new ElasticDatasource( ); const baseLogsQuery: Partial = { - isLogsQuery: true, metrics: [{ type: 'logs', id: '1' }], bucketAggs: [{ ...defaultBucketAgg('2'), field: dataSource.timeField } as DateHistogram], }; diff --git a/public/app/plugins/datasource/elasticsearch/language_provider.ts b/public/app/plugins/datasource/elasticsearch/language_provider.ts index a9006564280..ea25957cdb4 100644 --- a/public/app/plugins/datasource/elasticsearch/language_provider.ts +++ b/public/app/plugins/datasource/elasticsearch/language_provider.ts @@ -116,7 +116,6 @@ export default class ElasticsearchLanguageProvider extends LanguageProvider { let prometheusQuery = query as PromQuery; const expr = getElasticsearchQuery(extractPrometheusLabels(prometheusQuery.expr)); return { - isLogsQuery: true, metrics: [ { id: '1', diff --git a/public/app/plugins/datasource/elasticsearch/query_def.ts b/public/app/plugins/datasource/elasticsearch/query_def.ts index bf212a4891d..6d7fb8377fa 100644 --- a/public/app/plugins/datasource/elasticsearch/query_def.ts +++ b/public/app/plugins/datasource/elasticsearch/query_def.ts @@ -6,6 +6,7 @@ import { MetricAggregationType, } from './components/QueryEditor/MetricAggregationsEditor/aggregations'; import { metricAggregationConfig, pipelineOptions } from './components/QueryEditor/MetricAggregationsEditor/utils'; +import { ElasticsearchQuery } from './types'; export const extendedStats: ExtendedStat[] = [ { label: 'Avg', value: 'avg' }, @@ -42,8 +43,8 @@ export function defaultBucketAgg(id = '1'): BucketAggregation { export const findMetricById = (metrics: MetricAggregation[], id: MetricAggregation['id']) => metrics.find((metric) => metric.id === id); -export function hasMetricOfType(target: any, type: string): boolean { - return target && target.metrics && target.metrics.some((m: any) => m.type === type); +export function hasMetricOfType(target: ElasticsearchQuery, type: MetricAggregationType): boolean { + return !!target?.metrics?.some((m) => m.type === type); } // Even if we have type guards when building a query, we currently have no way of getting this information from the response. diff --git a/public/app/plugins/datasource/elasticsearch/specs/elastic_response.test.ts b/public/app/plugins/datasource/elasticsearch/specs/elastic_response.test.ts index 149df1a4d67..e518e189fa1 100644 --- a/public/app/plugins/datasource/elasticsearch/specs/elastic_response.test.ts +++ b/public/app/plugins/datasource/elasticsearch/specs/elastic_response.test.ts @@ -1208,17 +1208,12 @@ describe('ElasticResponse', () => { }); describe('simple logs query and count', () => { - const targets: any = [ + const targets: ElasticsearchQuery[] = [ { refId: 'A', metrics: [{ type: 'count', id: '1' }], bucketAggs: [{ type: 'date_histogram', settings: { interval: 'auto' }, id: '2' }], - context: 'explore', - interval: '10s', - isLogsQuery: true, key: 'Q-1561369883389-0.7611823271062786-0', - liveStreaming: false, - maxDataPoints: 1620, query: 'hello AND message', timeField: '@timestamp', }, diff --git a/public/app/plugins/datasource/elasticsearch/types.ts b/public/app/plugins/datasource/elasticsearch/types.ts index fee88a5984a..6f9b6337537 100644 --- a/public/app/plugins/datasource/elasticsearch/types.ts +++ b/public/app/plugins/datasource/elasticsearch/types.ts @@ -59,7 +59,6 @@ export interface ElasticsearchAggregation { } export interface ElasticsearchQuery extends DataQuery { - isLogsQuery?: boolean; alias?: string; query?: string; bucketAggs?: BucketAggregation[];