Elasticsearch: Fix query initialization logic & query transformation …

…from Promethous/Loki (#31322) * Elasticsearch: Fix query initialization logic * Only import prometheus & loki queries as log queries
grafana · Feb 24, 2021 · eac6f81 · eac6f81
1 parent 7506a04
commit eac6f81
Show file tree

Hide file tree

Showing 11 changed files with 189 additions and 63 deletions.
diff --git a/...datasource/elasticsearch/components/QueryEditor/BucketAggregationsEditor/state/reducer.ts b/...datasource/elasticsearch/components/QueryEditor/BucketAggregationsEditor/state/reducer.ts
@@ -16,7 +16,7 @@ import { bucketAggregationConfig } from '../utils';
 import { removeEmpty } from '../../../../utils';
 
 export const reducer = (
-  state: BucketAggregation[],
+  state: ElasticsearchQuery['bucketAggs'],
   action: BucketAggregationAction | ChangeMetricTypeAction | InitAction
 ): ElasticsearchQuery['bucketAggs'] => {
   switch (action.type) {
@@ -28,18 +28,18 @@ export const reducer = (
       };
 
       // If the last bucket aggregation is a `date_histogram` we add the new one before it.
-      const lastAgg = state[state.length - 1];
+      const lastAgg = state![state!.length - 1];
       if (lastAgg?.type === 'date_histogram') {
-        return [...state.slice(0, state.length - 1), newAgg, lastAgg];
+        return [...state!.slice(0, state!.length - 1), newAgg, lastAgg];
       }
 
-      return [...state, newAgg];
+      return [...state!, newAgg];
 
     case REMOVE_BUCKET_AGG:
-      return state.filter((bucketAgg) => bucketAgg.id !== action.payload.id);
+      return state!.filter((bucketAgg) => bucketAgg.id !== action.payload.id);
 
     case CHANGE_BUCKET_AGG_TYPE:
-      return state.map((bucketAgg) => {
+      return state!.map((bucketAgg) => {
         if (bucketAgg.id !== action.payload.id) {
           return bucketAgg;
         }
@@ -58,7 +58,7 @@ export const reducer = (
       });
 
     case CHANGE_BUCKET_AGG_FIELD:
-      return state.map((bucketAgg) => {
+      return state!.map((bucketAgg) => {
         if (bucketAgg.id !== action.payload.id) {
           return bucketAgg;
         }
@@ -74,7 +74,7 @@ export const reducer = (
       // we remove all of them.
       if (metricAggregationConfig[action.payload.type].isSingleMetric) {
         return [];
-      } else if (state.length === 0) {
+      } else if (state!.length === 0) {
         // Else, if there are no bucket aggregations we restore a default one.
         // This happens when switching from a metric that requires the absence of bucket aggregations to
         // one that requires it.
@@ -83,7 +83,7 @@ export const reducer = (
       return state;
 
     case CHANGE_BUCKET_AGG_SETTING:
-      return state.map((bucketAgg) => {
+      return state!.map((bucketAgg) => {
         if (bucketAgg.id !== action.payload.bucketAgg.id) {
           return bucketAgg;
         }
@@ -102,6 +102,9 @@ export const reducer = (
       });
 
     case INIT:
+      if (state?.length || 0 > 0) {
+        return state;
+      }
       return [defaultBucketAgg('2')];
 
     default:

diff --git a/...lugins/datasource/elasticsearch/components/QueryEditor/ElasticsearchQueryContext.test.tsx b/...lugins/datasource/elasticsearch/components/QueryEditor/ElasticsearchQueryContext.test.tsx
@@ -7,6 +7,7 @@ import { ElasticDatasource } from '../../datasource';
 
 const query: ElasticsearchQuery = {
   refId: 'A',
+  query: '',
   metrics: [{ id: '1', type: 'count' }],
   bucketAggs: [{ type: 'date_histogram', id: '2' }],
 };

diff --git a/...app/plugins/datasource/elasticsearch/components/QueryEditor/ElasticsearchQueryContext.tsx b/...app/plugins/datasource/elasticsearch/components/QueryEditor/ElasticsearchQueryContext.tsx
@@ -48,7 +48,7 @@ export const ElasticsearchProvider: FunctionComponent<Props> = ({
 
   // This initializes the query by dispatching an init action to each reducer.
   // useStatelessReducer will then call `onChange` with the newly generated query
-  if (!query.metrics && !query.bucketAggs) {
+  if (!query.metrics || !query.bucketAggs || query.query === undefined) {
     dispatch(initQuery());
 
     return null;

diff --git a/...asticsearch/components/QueryEditor/MetricAggregationsEditor/SettingsEditor/index.test.tsx b/...asticsearch/components/QueryEditor/MetricAggregationsEditor/SettingsEditor/index.test.tsx
@@ -12,6 +12,7 @@ describe('Settings Editor', () => {
       const initialSize = '500';
       const query: ElasticsearchQuery = {
         refId: 'A',
+        query: '',
         metrics: [
           {
             id: metricId,
@@ -21,6 +22,7 @@ describe('Settings Editor', () => {
             },
           },
         ],
+        bucketAggs: [],
       };
 
       const onChange = jest.fn();

diff --git a/...datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/state/reducer.ts b/...datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/state/reducer.ts
@@ -22,24 +22,26 @@ import {
 } from './types';
 
 export const reducer = (
-  state: MetricAggregation[],
+  state: ElasticsearchQuery['metrics'],
   action: MetricAggregationAction | InitAction
 ): ElasticsearchQuery['metrics'] => {
   switch (action.type) {
     case ADD_METRIC:
-      return [...state, defaultMetricAgg(action.payload.id)];
+      return [...state!, defaultMetricAgg(action.payload.id)];
 
     case REMOVE_METRIC:
-      const metricToRemove = state.find((m) => m.id === action.payload.id)!;
-      const metricsToRemove = [metricToRemove, ...getChildren(metricToRemove, state)];
-      const resultingMetrics = state.filter((metric) => !metricsToRemove.some((toRemove) => toRemove.id === metric.id));
+      const metricToRemove = state!.find((m) => m.id === action.payload.id)!;
+      const metricsToRemove = [metricToRemove, ...getChildren(metricToRemove, state!)];
+      const resultingMetrics = state!.filter(
+        (metric) => !metricsToRemove.some((toRemove) => toRemove.id === metric.id)
+      );
       if (resultingMetrics.length === 0) {
         return [defaultMetricAgg('1')];
       }
       return resultingMetrics;
 
     case CHANGE_METRIC_TYPE:
-      return state
+      return state!
         .filter((metric) =>
           // When the new metric type is `isSingleMetric` we remove all other metrics from the query
           // leaving only the current one.
@@ -64,7 +66,7 @@ export const reducer = (
         });
 
     case CHANGE_METRIC_FIELD:
-      return state.map((metric) => {
+      return state!.map((metric) => {
         if (metric.id !== action.payload.id) {
           return metric;
         }
@@ -82,7 +84,7 @@ export const reducer = (
       });
 
     case TOGGLE_METRIC_VISIBILITY:
-      return state.map((metric) => {
+      return state!.map((metric) => {
         if (metric.id !== action.payload.id) {
           return metric;
         }
@@ -94,7 +96,7 @@ export const reducer = (
       });
 
     case CHANGE_METRIC_SETTING:
-      return state.map((metric) => {
+      return state!.map((metric) => {
         if (metric.id !== action.payload.metric.id) {
           return metric;
         }
@@ -119,7 +121,7 @@ export const reducer = (
       });
 
     case CHANGE_METRIC_META:
-      return state.map((metric) => {
+      return state!.map((metric) => {
         if (metric.id !== action.payload.metric.id) {
           return metric;
         }
@@ -140,7 +142,7 @@ export const reducer = (
       });
 
     case CHANGE_METRIC_ATTRIBUTE:
-      return state.map((metric) => {
+      return state!.map((metric) => {
         if (metric.id !== action.payload.metric.id) {
           return metric;
         }
@@ -152,6 +154,9 @@ export const reducer = (
       });
 
     case INIT:
+      if (state?.length || 0 > 0) {
+        return state;
+      }
       return [defaultMetricAgg('1')];
 
     default:

diff --git a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/index.test.tsx b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/index.test.tsx
@@ -10,6 +10,7 @@ describe('QueryEditor', () => {
       const alias = '{{metric}}';
       const query: ElasticsearchQuery = {
         refId: 'A',
+        query: '',
         alias,
         metrics: [
           {

diff --git a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/state.test.ts b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/state.test.ts
@@ -1,8 +1,29 @@
 import { reducerTester } from 'test/core/redux/reducerTester';
 import { ElasticsearchQuery } from '../../types';
-import { aliasPatternReducer, changeAliasPattern, changeQuery, queryReducer } from './state';
+import { aliasPatternReducer, changeAliasPattern, changeQuery, initQuery, queryReducer } from './state';
 
 describe('Query Reducer', () => {
+  describe('On Init', () => {
+    it('Should maintain the previous `query` if present', () => {
+      const initialQuery: ElasticsearchQuery['query'] = 'Some lucene query';
+
+      reducerTester()
+        .givenReducer(queryReducer, initialQuery)
+        .whenActionIsDispatched(initQuery())
+        .thenStateShouldEqual(initialQuery);
+    });
+
+    it('Should set an empty `query` if it is not already set', () => {
+      const initialQuery: ElasticsearchQuery['query'] = undefined;
+      const expectedQuery = '';
+
+      reducerTester()
+        .givenReducer(queryReducer, initialQuery)
+        .whenActionIsDispatched(initQuery())
+        .thenStateShouldEqual(expectedQuery);
+    });
+  });
+
   it('Should correctly set `query`', () => {
     const expectedQuery: ElasticsearchQuery['query'] = 'Some lucene query';
 

diff --git a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/state.ts b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/state.ts
@@ -1,4 +1,5 @@
 import { Action } from '../../hooks/useStatelessReducer';
+import { ElasticsearchQuery } from '../../types';
 
 export const INIT = 'init';
 const CHANGE_QUERY = 'change_query';
@@ -18,6 +19,10 @@ interface ChangeAliasPatternAction extends Action<typeof CHANGE_ALIAS_PATTERN> {
   };
 }
 
+/**
+ * When the `initQuery` Action is dispatched, the query gets populated with default values where values are not present.
+ * This means it won't override any existing value in place, but just ensure the query is in a "runnable" state.
+ */
 export const initQuery = (): InitAction => ({ type: INIT });
 
 export const changeQuery = (query: string): ChangeQueryAction => ({
@@ -34,26 +39,29 @@ export const changeAliasPattern = (aliasPattern: string): ChangeAliasPatternActi
   },
 });
 
-export const queryReducer = (prevQuery: string, action: ChangeQueryAction | InitAction) => {
+export const queryReducer = (prevQuery: ElasticsearchQuery['query'], action: ChangeQueryAction | InitAction) => {
   switch (action.type) {
     case CHANGE_QUERY:
       return action.payload.query;
 
     case INIT:
-      return '';
+      return prevQuery || '';
 
     default:
       return prevQuery;
   }
 };
 
-export const aliasPatternReducer = (prevAliasPattern: string, action: ChangeAliasPatternAction | InitAction) => {
+export const aliasPatternReducer = (
+  prevAliasPattern: ElasticsearchQuery['alias'],
+  action: ChangeAliasPatternAction | InitAction
+) => {
   switch (action.type) {
     case CHANGE_ALIAS_PATTERN:
       return action.payload.aliasPattern;
 
     case INIT:
-      return '';
+      return prevAliasPattern || '';
 
     default:
       return prevAliasPattern;

diff --git a/public/app/plugins/datasource/elasticsearch/hooks/useNextId.test.tsx b/public/app/plugins/datasource/elasticsearch/hooks/useNextId.test.tsx
@@ -8,6 +8,7 @@ describe('useNextId', () => {
   it('Should return the next available id', () => {
     const query: ElasticsearchQuery = {
       refId: 'A',
+      query: '',
       metrics: [{ id: '1', type: 'avg' }],
       bucketAggs: [{ id: '2', type: 'date_histogram' }],
     };