From 903cf3e30d3ce8daf6c624900a3c7c787ffcfebd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A1bor=20Farkas?= Date: Wed, 10 May 2023 08:50:59 +0200 Subject: [PATCH] elastic: refactor: simplify query-type handling (#67921) * elastic: refactor: simplify query-type handling * improved comment Co-authored-by: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com> --------- Co-authored-by: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com> --- .../state/reducer.test.ts | 4 ++-- .../BucketAggregationsEditor/state/reducer.ts | 2 +- .../MetricAggregationsEditor/MetricEditor.tsx | 2 +- .../MetricAggregationsEditor/index.tsx | 2 +- .../state/reducer.test.ts | 2 +- .../MetricAggregationsEditor/state/reducer.ts | 6 +++-- .../MetricAggregationsEditor/utils.ts | 22 ++++++++++++++++--- .../QueryEditor/QueryTypeSelector.tsx | 18 +++------------ .../components/QueryEditor/index.tsx | 2 +- .../plugins/datasource/elasticsearch/types.ts | 4 +++- 10 files changed, 36 insertions(+), 28 deletions(-) diff --git a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/BucketAggregationsEditor/state/reducer.test.ts b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/BucketAggregationsEditor/state/reducer.test.ts index 95052d9d229..b39166c1093 100644 --- a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/BucketAggregationsEditor/state/reducer.test.ts +++ b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/BucketAggregationsEditor/state/reducer.test.ts @@ -114,10 +114,10 @@ describe('Bucket Aggregations Reducer', () => { reducerTester() .givenReducer(createReducer('@timestamp'), initialState) - // If the new metric aggregation is `isSingleMetric` we should remove all bucket aggregations. + // If the new metric aggregation is non-metric, we should remove all bucket aggregations. .whenActionIsDispatched(changeMetricType({ id: 'Some id', type: 'raw_data' })) .thenStatePredicateShouldEqual((newState) => newState?.length === 0) - // Switching back to another aggregation that is NOT `isSingleMetric` should bring back a bucket aggregation + // Switching back to another aggregation that is metric should bring back a bucket aggregation .whenActionIsDispatched(changeMetricType({ id: 'Some id', type: 'max' })) .thenStatePredicateShouldEqual((newState) => newState?.length === 1) // When none of the above is true state shouldn't change. diff --git a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/BucketAggregationsEditor/state/reducer.ts b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/BucketAggregationsEditor/state/reducer.ts index 6bfc9a6ee39..d289f7dc32f 100644 --- a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/BucketAggregationsEditor/state/reducer.ts +++ b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/BucketAggregationsEditor/state/reducer.ts @@ -75,7 +75,7 @@ export const createReducer = if (changeMetricType.match(action)) { // If we are switching to a metric which requires the absence of bucket aggregations // we remove all of them. - if (metricAggregationConfig[action.payload.type].isSingleMetric) { + if (metricAggregationConfig[action.payload.type].impliedQueryType !== 'metrics') { return []; } else if (state!.length === 0) { // Else, if there are no bucket aggregations we restore a default one. diff --git a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/MetricEditor.tsx b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/MetricEditor.tsx index a476ff13069..a520ae0fbbe 100644 --- a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/MetricEditor.tsx +++ b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/MetricEditor.tsx @@ -48,7 +48,7 @@ const getTypeOptions = ( return ( Object.entries(metricAggregationConfig) - .filter(([_, config]) => !config.isSingleMetric) + .filter(([_, config]) => config.impliedQueryType === 'metrics') // Only showing metrics type supported by the version of ES. // if we cannot determine the version, we assume it is suitable. .filter(([_, { versionRange = '*' }]) => (esVersion != null ? satisfies(esVersion, versionRange) : true)) diff --git a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/index.tsx b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/index.tsx index e104465fb31..7051d3a3f05 100644 --- a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/index.tsx +++ b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/index.tsx @@ -48,7 +48,7 @@ export const MetricAggregationsEditor = ({ nextId }: Props) => { > - {!metricAggregationConfig[metric.type].isSingleMetric && index === 0 && ( + {metricAggregationConfig[metric.type].impliedQueryType === 'metrics' && index === 0 && ( dispatch(addMetric(nextId))} label="add" /> )} diff --git a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/state/reducer.test.ts b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/state/reducer.test.ts index d91c28ca65c..246642b7b5e 100644 --- a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/state/reducer.test.ts +++ b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/state/reducer.test.ts @@ -84,7 +84,7 @@ describe('Metric Aggregations Reducer', () => { .thenStateShouldEqual([firstAggregation, { ...secondAggregation, type: expectedSecondAggregation.type }]); }); - it('Should remove all other aggregations when the newly selected one is `isSingleMetric`', () => { + it('Should remove all other aggregations when the newly selected one is not metric', () => { const firstAggregation: MetricAggregation = { id: '1', type: 'count', diff --git a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/state/reducer.ts b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/state/reducer.ts index f42baf0998c..e3da5ccd2e6 100644 --- a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/state/reducer.ts +++ b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/state/reducer.ts @@ -36,9 +36,11 @@ export const reducer = (state: ElasticsearchQuery['metrics'], action: Action): E if (changeMetricType.match(action)) { return state! .filter((metric) => - // When the new metric type is `isSingleMetric` we remove all other metrics from the query + // When the new query type is not `metrics` we remove all other metrics from the query // leaving only the current one. - !!metricAggregationConfig[action.payload.type].isSingleMetric ? metric.id === action.payload.id : true + metricAggregationConfig[action.payload.type].impliedQueryType === 'metrics' + ? true + : metric.id === action.payload.id ) .map((metric) => { if (metric.id !== action.payload.id) { diff --git a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/utils.ts b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/utils.ts index b940e0cfb6e..ad27d6aebea 100644 --- a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/utils.ts +++ b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/utils.ts @@ -9,6 +9,7 @@ import { isMetricAggregationWithField, isPipelineAggregationWithMultipleBucketPa export const metricAggregationConfig: MetricsConfiguration = { count: { label: 'Count', + impliedQueryType: 'metrics', requiresField: false, isPipelineAgg: false, supportsMissing: false, @@ -20,6 +21,7 @@ export const metricAggregationConfig: MetricsConfiguration = { }, avg: { label: 'Average', + impliedQueryType: 'metrics', requiresField: true, supportsInlineScript: true, supportsMissing: true, @@ -31,6 +33,7 @@ export const metricAggregationConfig: MetricsConfiguration = { }, sum: { label: 'Sum', + impliedQueryType: 'metrics', requiresField: true, supportsInlineScript: true, supportsMissing: true, @@ -42,6 +45,7 @@ export const metricAggregationConfig: MetricsConfiguration = { }, max: { label: 'Max', + impliedQueryType: 'metrics', requiresField: true, supportsInlineScript: true, supportsMissing: true, @@ -53,6 +57,7 @@ export const metricAggregationConfig: MetricsConfiguration = { }, min: { label: 'Min', + impliedQueryType: 'metrics', requiresField: true, supportsInlineScript: true, supportsMissing: true, @@ -64,6 +69,7 @@ export const metricAggregationConfig: MetricsConfiguration = { }, extended_stats: { label: 'Extended Stats', + impliedQueryType: 'metrics', requiresField: true, supportsMissing: true, supportsInlineScript: true, @@ -80,6 +86,7 @@ export const metricAggregationConfig: MetricsConfiguration = { }, percentiles: { label: 'Percentiles', + impliedQueryType: 'metrics', requiresField: true, supportsMissing: true, supportsInlineScript: true, @@ -95,6 +102,7 @@ export const metricAggregationConfig: MetricsConfiguration = { }, cardinality: { label: 'Unique Count', + impliedQueryType: 'metrics', requiresField: true, supportsMissing: true, isPipelineAgg: false, @@ -108,6 +116,7 @@ export const metricAggregationConfig: MetricsConfiguration = { // deprecated in 6.4.0, removed in 8.0.0, // recommended replacement is moving_fn label: 'Moving Average', + impliedQueryType: 'metrics', requiresField: true, isPipelineAgg: true, versionRange: '<8.0.0', @@ -126,6 +135,7 @@ export const metricAggregationConfig: MetricsConfiguration = { moving_fn: { // TODO: Check this label: 'Moving Function', + impliedQueryType: 'metrics', requiresField: true, isPipelineAgg: true, supportsMultipleBucketPaths: false, @@ -137,6 +147,7 @@ export const metricAggregationConfig: MetricsConfiguration = { }, derivative: { label: 'Derivative', + impliedQueryType: 'metrics', requiresField: true, isPipelineAgg: true, supportsMissing: false, @@ -148,6 +159,7 @@ export const metricAggregationConfig: MetricsConfiguration = { }, serial_diff: { label: 'Serial Difference', + impliedQueryType: 'metrics', requiresField: true, isPipelineAgg: true, supportsMissing: false, @@ -163,6 +175,7 @@ export const metricAggregationConfig: MetricsConfiguration = { }, cumulative_sum: { label: 'Cumulative Sum', + impliedQueryType: 'metrics', requiresField: true, isPipelineAgg: true, supportsMissing: false, @@ -174,6 +187,7 @@ export const metricAggregationConfig: MetricsConfiguration = { }, bucket_script: { label: 'Bucket Script', + impliedQueryType: 'metrics', requiresField: false, isPipelineAgg: true, supportsMissing: false, @@ -188,7 +202,7 @@ export const metricAggregationConfig: MetricsConfiguration = { raw_document: { label: 'Raw Document (deprecated)', requiresField: false, - isSingleMetric: true, + impliedQueryType: 'raw_document', isPipelineAgg: false, supportsMissing: false, supportsMultipleBucketPaths: false, @@ -204,7 +218,7 @@ export const metricAggregationConfig: MetricsConfiguration = { raw_data: { label: 'Raw Data', requiresField: false, - isSingleMetric: true, + impliedQueryType: 'raw_data', isPipelineAgg: false, supportsMissing: false, supportsMultipleBucketPaths: false, @@ -224,7 +238,7 @@ export const metricAggregationConfig: MetricsConfiguration = { supportsMissing: false, supportsMultipleBucketPaths: false, hasSettings: true, - isSingleMetric: true, + impliedQueryType: 'logs', supportsInlineScript: false, hasMeta: false, defaults: { @@ -235,6 +249,7 @@ export const metricAggregationConfig: MetricsConfiguration = { }, top_metrics: { label: 'Top Metrics', + impliedQueryType: 'metrics', requiresField: false, isPipelineAgg: false, supportsMissing: false, @@ -250,6 +265,7 @@ export const metricAggregationConfig: MetricsConfiguration = { }, rate: { label: 'Rate', + impliedQueryType: 'metrics', requiresField: true, isPipelineAgg: false, supportsMissing: false, diff --git a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/QueryTypeSelector.tsx b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/QueryTypeSelector.tsx index 9b8ade09bfa..7dfa3fd2ec6 100644 --- a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/QueryTypeSelector.tsx +++ b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/QueryTypeSelector.tsx @@ -4,12 +4,11 @@ import { SelectableValue } from '@grafana/data'; import { RadioButtonGroup } from '@grafana/ui'; import { useDispatch } from '../../hooks/useStatelessReducer'; -import { MetricAggregation } from '../../types'; +import { MetricAggregation, QueryType } from '../../types'; import { useQuery } from './ElasticsearchQueryContext'; import { changeMetricType } from './MetricAggregationsEditor/state/actions'; - -type QueryType = 'metrics' | 'logs' | 'raw_data' | 'raw_document'; +import { metricAggregationConfig } from './MetricAggregationsEditor/utils'; const OPTIONS: Array> = [ { value: 'metrics', label: 'Metrics' }, @@ -18,17 +17,6 @@ const OPTIONS: Array> = [ { value: 'raw_document', label: 'Raw Document' }, ]; -function metricTypeToQueryType(type: MetricAggregation['type']): QueryType { - switch (type) { - case 'logs': - case 'raw_data': - case 'raw_document': - return type; - default: - return 'metrics'; - } -} - function queryTypeToMetricType(type: QueryType): MetricAggregation['type'] { switch (type) { case 'logs': @@ -54,7 +42,7 @@ export const QueryTypeSelector = () => { return null; } - const queryType = metricTypeToQueryType(firstMetric.type); + const queryType = metricAggregationConfig[firstMetric.type].impliedQueryType; const onChange = (newQueryType: QueryType) => { dispatch(changeMetricType({ id: firstMetric.id, type: queryTypeToMetricType(newQueryType) })); diff --git a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/index.tsx b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/index.tsx index 2e60a71865e..d9030d47b29 100644 --- a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/index.tsx +++ b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/index.tsx @@ -104,7 +104,7 @@ const QueryEditorForm = ({ value }: Props) => { const isTimeSeriesQuery = value?.bucketAggs?.slice(-1)[0]?.type === 'date_histogram'; const showBucketAggregationsEditor = value.metrics?.every( - (metric) => !metricAggregationConfig[metric.type].isSingleMetric + (metric) => metricAggregationConfig[metric.type].impliedQueryType === 'metrics' ); return ( diff --git a/public/app/plugins/datasource/elasticsearch/types.ts b/public/app/plugins/datasource/elasticsearch/types.ts index ecb44373041..c7f7e68df62 100644 --- a/public/app/plugins/datasource/elasticsearch/types.ts +++ b/public/app/plugins/datasource/elasticsearch/types.ts @@ -65,6 +65,8 @@ export interface ElasticsearchOptions extends DataSourceJsonData { index?: string; } +export type QueryType = 'metrics' | 'logs' | 'raw_data' | 'raw_document'; + interface MetricConfiguration { label: string; requiresField: boolean; @@ -77,7 +79,7 @@ interface MetricConfiguration { */ versionRange?: string; supportsMultipleBucketPaths: boolean; - isSingleMetric?: boolean; + impliedQueryType: QueryType; hasSettings: boolean; hasMeta: boolean; defaults: Omit, 'id' | 'type'>;