Elasticsearch: Add generic support for template variables (#32762)

* Elasticsearch: Add generic support for template variables

* format MovingAverage settings as numbers

* Move formatting logic to query builder & forma serial_diff settings as numbers

* modify presence check

* add todo

* minor fixes

* transform string values to numbers

* Move casting logic

* Slightly cleaner implementation

* Add BE tests

* Leverage elastic validation when string doesn't resolve to a numeric value

* move newly introduced test to testify

* add FE query_builder tests

* check error

* Parse values to float instead of int

* Fix tests & ParseFloat bit size

public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/SettingsEditor/MovingAverageSettingsEditor.tsx

@@ -1,21 +1,24 @@
-import { Input, InlineField, Select, Switch } from '@grafana/ui';
+import { Input, InlineField, Select, InlineSwitch } from '@grafana/ui';
 import React, { FunctionComponent } from 'react';
 import { useDispatch } from '../../../../hooks/useStatelessReducer';
 import { movingAvgModelOptions } from '../../../../query_def';
 import { isEWMAMovingAverage, isHoltMovingAverage, isHoltWintersMovingAverage, MovingAverage } from '../aggregations';
 import { changeMetricSetting } from '../state/actions';
+import { SettingField } from './SettingField';
 
 interface Props {
   metric: MovingAverage;
 }
 
 // The way we handle changes for those settings is not ideal compared to the other components in the editor
+// FIXME: using `changeMetricSetting` will cause an error when switching from models that have different options
+// as they might be incompatible. We should clear all other options on model change.
 export const MovingAverageSettingsEditor: FunctionComponent<Props> = ({ metric }) => {
   const dispatch = useDispatch();
 
   return (
     <>
-      <InlineField label="Model">
+      <InlineField label="Model" labelWidth={16}>
         <Select
           onChange={(value) => dispatch(changeMetricSetting(metric, 'model', value.value!))}
           options={movingAvgModelOptions}
@@ -23,128 +26,64 @@ export const MovingAverageSettingsEditor: FunctionComponent<Props> = ({ metric }
         />
       </InlineField>
 
-      <InlineField label="Window">
-        <Input
-          onBlur={(e) => dispatch(changeMetricSetting(metric, 'window', parseInt(e.target.value!, 10)))}
-          defaultValue={metric.settings?.window}
-        />
-      </InlineField>
+      <SettingField label="Window" settingName="window" metric={metric} placeholder="5" />
 
-      <InlineField label="Predict">
-        <Input
-          onBlur={(e) => dispatch(changeMetricSetting(metric, 'predict', parseInt(e.target.value!, 10)))}
-          defaultValue={metric.settings?.predict}
-        />
-      </InlineField>
+      <SettingField label="Predict" settingName="predict" metric={metric} />
 
-      {isEWMAMovingAverage(metric) && (
-        <>
-          <InlineField label="Alpha">
-            <Input
-              onBlur={(e) => dispatch(changeMetricSetting(metric, 'alpha', parseInt(e.target.value!, 10)))}
-              defaultValue={metric.settings?.alpha}
-            />
-          </InlineField>
-
-          <InlineField label="Minimize">
-            <Switch
-              onChange={(e: React.ChangeEvent<HTMLInputElement>) =>
-                dispatch(changeMetricSetting(metric, 'minimize', e.target.checked))
-              }
-              checked={!!metric.settings?.minimize}
-            />
-          </InlineField>
-        </>
+      {(isEWMAMovingAverage(metric) || isHoltMovingAverage(metric) || isHoltWintersMovingAverage(metric)) && (
+        <InlineField label="Alpha" labelWidth={16}>
+          <Input
+            onBlur={(e) =>
+              dispatch(
+                changeMetricSetting(metric, 'settings', {
+                  ...metric.settings?.settings,
+                  alpha: e.target.value,
+                })
+              )
+            }
+            defaultValue={metric.settings?.settings?.alpha}
+          />
+        </InlineField>
       )}
 
-      {isHoltMovingAverage(metric) && (
-        <>
-          <InlineField label="Alpha">
-            <Input
-              onBlur={(e) =>
-                dispatch(
-                  changeMetricSetting(metric, 'settings', {
-                    ...metric.settings?.settings,
-                    alpha: parseInt(e.target.value!, 10),
-                  })
-                )
-              }
-              defaultValue={metric.settings?.settings?.alpha}
-            />
-          </InlineField>
-          <InlineField label="Beta">
-            <Input
-              onBlur={(e) =>
-                dispatch(
-                  changeMetricSetting(metric, 'settings', {
-                    ...metric.settings?.settings,
-                    beta: parseInt(e.target.value!, 10),
-                  })
-                )
-              }
-              defaultValue={metric.settings?.settings?.beta}
-            />
-          </InlineField>
-
-          <InlineField label="Minimize">
-            <Switch
-              onChange={(e: React.ChangeEvent<HTMLInputElement>) =>
-                dispatch(changeMetricSetting(metric, 'minimize', e.target.checked))
-              }
-              checked={!!metric.settings?.minimize}
-            />
-          </InlineField>
-        </>
+      {(isHoltMovingAverage(metric) || isHoltWintersMovingAverage(metric)) && (
+        <InlineField label="Beta" labelWidth={16}>
+          <Input
+            onBlur={(e) =>
+              dispatch(
+                changeMetricSetting(metric, 'settings', {
+                  ...metric.settings?.settings,
+                  beta: e.target.value,
+                })
+              )
+            }
+            defaultValue={metric.settings?.settings?.beta}
+          />
+        </InlineField>
       )}
 
       {isHoltWintersMovingAverage(metric) && (
         <>
-          <InlineField label="Alpha">
+          <InlineField label="Gamma" labelWidth={16}>
             <Input
               onBlur={(e) =>
                 dispatch(
                   changeMetricSetting(metric, 'settings', {
                     ...metric.settings?.settings,
-                    alpha: parseInt(e.target.value!, 10),
-                  })
-                )
-              }
-              defaultValue={metric.settings?.settings?.alpha}
-            />
-          </InlineField>
-          <InlineField label="Beta">
-            <Input
-              onBlur={(e) =>
-                dispatch(
-                  changeMetricSetting(metric, 'settings', {
-                    ...metric.settings?.settings,
-                    beta: parseInt(e.target.value!, 10),
-                  })
-                )
-              }
-              defaultValue={metric.settings?.settings?.beta}
-            />
-          </InlineField>
-          <InlineField label="Gamma">
-            <Input
-              onBlur={(e) =>
-                dispatch(
-                  changeMetricSetting(metric, 'settings', {
-                    ...metric.settings?.settings,
-                    gamma: parseInt(e.target.value!, 10),
+                    gamma: e.target.value,
                   })
                 )
               }
               defaultValue={metric.settings?.settings?.gamma}
             />
           </InlineField>
-          <InlineField label="Period">
+          <InlineField label="Period" labelWidth={16}>
             <Input
               onBlur={(e) =>
                 dispatch(
                   changeMetricSetting(metric, 'settings', {
                     ...metric.settings?.settings,
-                    period: parseInt(e.target.value!, 10),
+                    period: e.target.value!,
                   })
                 )
               }
@@ -152,8 +91,8 @@ export const MovingAverageSettingsEditor: FunctionComponent<Props> = ({ metric }
             />
           </InlineField>
 
-          <InlineField label="Pad">
-            <Switch
+          <InlineField label="Pad" labelWidth={16}>
+            <InlineSwitch
               onChange={(e: React.ChangeEvent<HTMLInputElement>) =>
                 dispatch(
                   changeMetricSetting(metric, 'settings', { ...metric.settings?.settings, pad: e.target.checked })
@@ -162,17 +101,19 @@ export const MovingAverageSettingsEditor: FunctionComponent<Props> = ({ metric }
               checked={!!metric.settings?.settings?.pad}
             />
           </InlineField>
-
-          <InlineField label="Minimize">
-            <Switch
-              onChange={(e: React.ChangeEvent<HTMLInputElement>) =>
-                dispatch(changeMetricSetting(metric, 'minimize', e.target.checked))
-              }
-              checked={!!metric.settings?.minimize}
-            />
-          </InlineField>
         </>
       )}
+
+      {(isEWMAMovingAverage(metric) || isHoltMovingAverage(metric) || isHoltWintersMovingAverage(metric)) && (
+        <InlineField label="Minimize" labelWidth={16}>
+          <InlineSwitch
+            onChange={(e: React.ChangeEvent<HTMLInputElement>) =>
+              dispatch(changeMetricSetting(metric, 'minimize', e.target.checked))
+            }
+            checked={!!metric.settings?.minimize}
+          />
+        </InlineField>
+      )}
     </>
   );
 };

public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/SettingsEditor/index.tsx

@@ -37,14 +37,7 @@ export const SettingsEditor: FunctionComponent<Props> = ({ metric, previousMetri
     <SettingsEditorContainer label={description} hidden={metric.hide}>
       {metric.type === 'derivative' && <SettingField label="Unit" metric={metric} settingName="unit" />}
 
-      {metric.type === 'serial_diff' && (
-        <InlineField label="Lag">
-          <Input
-            onBlur={(e) => dispatch(changeMetricSetting(metric, 'lag', parseInt(e.target.value, 10)))}
-            defaultValue={metric.settings?.lag}
-          />
-        </InlineField>
-      )}
+      {metric.type === 'serial_diff' && <SettingField label="Lag" metric={metric} settingName="lag" placeholder="1" />}
 
       {metric.type === 'cumulative_sum' && <SettingField label="Format" metric={metric} settingName="format" />}
 

public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/aggregations.ts

@@ -172,8 +172,8 @@ export interface MovingAverageModelOption {
 
 export interface BaseMovingAverageModelSettings {
   model: MovingAverageModel;
-  window: number;
-  predict: number;
+  window: string;
+  predict: string;
 }
 
 export interface MovingAverageSimpleModelSettings extends BaseMovingAverageModelSettings {
@@ -186,15 +186,17 @@ export interface MovingAverageLinearModelSettings extends BaseMovingAverageModel
 
 export interface MovingAverageEWMAModelSettings extends BaseMovingAverageModelSettings {
   model: 'ewma';
-  alpha: number;
+  settings?: {
+    alpha?: string;
+  };
   minimize: boolean;
 }
 
 export interface MovingAverageHoltModelSettings extends BaseMovingAverageModelSettings {
   model: 'holt';
   settings: {
-    alpha?: number;
-    beta?: number;
+    alpha?: string;
+    beta?: string;
   };
   minimize: boolean;
 }
@@ -202,10 +204,10 @@ export interface MovingAverageHoltModelSettings extends BaseMovingAverageModelSe
 export interface MovingAverageHoltWintersModelSettings extends BaseMovingAverageModelSettings {
   model: 'holt_winters';
   settings: {
-    alpha?: number;
-    beta?: number;
-    gamma?: number;
-    period?: number;
+    alpha?: string;
+    beta?: string;
+    gamma?: string;
+    period?: string;
     pad?: boolean;
   };
   minimize: boolean;
@@ -238,6 +240,11 @@ export const isHoltWintersMovingAverage = (
   metric: MovingAverage | MovingAverage<'holt_winters'>
 ): metric is MovingAverage<'holt_winters'> => metric.settings?.model === 'holt_winters';
 
+export const isMovingAverageWithModelSettings = (
+  metric: MovingAverage
+): metric is MovingAverage<'ewma'> | MovingAverage<'holt'> | MovingAverage<'holt_winters'> =>
+  ['holt', 'ewma', 'holt_winters'].includes(metric.settings?.model || '');
+
 export interface MovingFunction extends BasePipelineMetricAggregation {
   type: 'moving_fn';
   settings?: {
@@ -257,7 +264,7 @@ export interface Derivative extends BasePipelineMetricAggregation {
 export interface SerialDiff extends BasePipelineMetricAggregation {
   type: 'serial_diff';
   settings?: {
-    lag?: number;
+    lag?: string;
   };
 }
 

public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/utils.ts

@@ -121,7 +121,7 @@ export const metricAggregationConfig: MetricsConfiguration = {
     defaults: {
       settings: {
         model: 'simple',
-        window: 5,
+        window: '5',
       },
     },
   },
@@ -160,7 +160,11 @@ export const metricAggregationConfig: MetricsConfiguration = {
     hasSettings: true,
     supportsInlineScript: false,
     hasMeta: false,
-    defaults: {},
+    defaults: {
+      settings: {
+        lag: '1',
+      },
+    },
   },
   cumulative_sum: {
     label: 'Cumulative Sum',

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

@@ -34,7 +34,10 @@ import {
   Logs,
 } from './components/QueryEditor/MetricAggregationsEditor/aggregations';
 import { bucketAggregationConfig } from './components/QueryEditor/BucketAggregationsEditor/utils';
-import { isBucketAggregationWithField } from './components/QueryEditor/BucketAggregationsEditor/aggregations';
+import {
+  BucketAggregation,
+  isBucketAggregationWithField,
+} from './components/QueryEditor/BucketAggregationsEditor/aggregations';
 import { generate, Observable, of, throwError } from 'rxjs';
 import { catchError, first, map, mergeMap, skipWhile, throwIfEmpty } from 'rxjs/operators';
 import { getScriptValue } from './utils';
@@ -353,26 +356,39 @@ export class ElasticDatasource extends DataSourceApi<ElasticsearchQuery, Elastic
   }
 
   interpolateVariablesInQueries(queries: ElasticsearchQuery[], scopedVars: ScopedVars): ElasticsearchQuery[] {
-    let expandedQueries = queries;
-    if (queries && queries.length > 0) {
-      expandedQueries = queries.map((query) => {
-        const expandedQuery = {
-          ...query,
-          datasource: this.name,
-          query: this.interpolateLuceneQuery(query.query || '', scopedVars),
+    // 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),
+            })),
+          },
         };
+      }
 
-        for (let bucketAgg of query.bucketAggs || []) {
-          if (bucketAgg.type === 'filters') {
-            for (let filter of bucketAgg.settings?.filters || []) {
-              filter.query = this.interpolateLuceneQuery(filter.query, scopedVars);
-            }
-          }
-        }
-        return expandedQuery;
-      });
-    }
-    return expandedQueries;
+      return bucketAgg;
+    };
+
+    const expandedQueries = queries.map(
+      (query): ElasticsearchQuery => ({
+        ...query,
+        datasource: this.name,
+        query: this.interpolateLuceneQuery(query.query || '', scopedVars),
+        bucketAggs: query.bucketAggs?.map(interpolateBucketAgg),
+      })
+    );
+
+    const finalQueries: ElasticsearchQuery[] = JSON.parse(
+      this.templateSrv.replace(JSON.stringify(expandedQueries), scopedVars)
+    );
+
+    return finalQueries;
   }
 
   testDatasource() {

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

@@ -7,6 +7,7 @@ import {
 import {
   isMetricAggregationWithField,
   isMetricAggregationWithSettings,
+  isMovingAverageWithModelSettings,
   isPipelineAggregation,
   isPipelineAggregationWithMultipleBucketPaths,
   MetricAggregation,
@@ -346,6 +347,37 @@ export class ElasticQueryBuilder {
           .forEach(([k, v]) => {
             metricAgg[k] = k === 'script' ? getScriptValue(metric as MetricAggregationWithInlineScript) : v;
           });
+
+        // Elasticsearch isn't generally too picky about the data types in the request body,
+        // however some fields are required to be numeric.
+        // Users might have already created some of those with before, where the values were numbers.
+        if (metric.type === 'moving_avg') {
+          metricAgg = {
+            ...metricAgg,
+            ...(metricAgg?.window !== undefined && { window: this.toNumber(metricAgg.window) }),
+            ...(metricAgg?.predict !== undefined && { predict: this.toNumber(metricAgg.predict) }),
+            ...(isMovingAverageWithModelSettings(metric) && {
+              settings: {
+                ...metricAgg.settings,
+                ...Object.fromEntries(
+                  Object.entries(metricAgg.settings || {})
+                    // Only format properties that are required to be numbers
+                    .filter(([settingName]) => ['alpha', 'beta', 'gamma', 'period'].includes(settingName))
+                    // omitting undefined
+                    .filter(([_, stringValue]) => stringValue !== undefined)
+                    .map(([_, stringValue]) => [_, this.toNumber(stringValue)])
+                ),
+              },
+            }),
+          };
+        } else if (metric.type === 'serial_diff') {
+          metricAgg = {
+            ...metricAgg,
+            ...(metricAgg.lag !== undefined && {
+              lag: this.toNumber(metricAgg.lag),
+            }),
+          };
+        }
       }
 
       aggField[metric.type] = metricAgg;
@@ -355,6 +387,15 @@ export class ElasticQueryBuilder {
     return query;
   }
 
+  private toNumber(stringValue: unknown): unknown | number {
+    const parsedValue = parseFloat(`${stringValue}`);
+    if (isNaN(parsedValue)) {
+      return stringValue;
+    }
+
+    return parsedValue;
+  }
+
   getTermsQuery(queryDef: any) {
     const query: any = {
       size: 0,

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

@@ -495,7 +495,7 @@ describe('ElasticQueryBuilder', () => {
               type: 'serial_diff',
               field: '3',
               settings: {
-                lag: 5,
+                lag: '5',
               },
             },
           ],
@@ -749,4 +749,65 @@ describe('ElasticQueryBuilder', () => {
       });
     });
   });
+
+  describe('Value casting for settings', () => {
+    it('correctly casts values in moving_avg ', () => {
+      const query = builder7x.build({
+        refId: 'A',
+        metrics: [
+          { type: 'avg', id: '2' },
+          {
+            type: 'moving_avg',
+            id: '3',
+            field: '2',
+            settings: {
+              window: '5',
+              model: 'holt_winters',
+              predict: '10',
+              settings: {
+                alpha: '1',
+                beta: '2',
+                gamma: '3',
+                period: '4',
+              },
+            },
+          },
+        ],
+        timeField: '@timestamp',
+        bucketAggs: [{ type: 'date_histogram', field: '@timestamp', id: '1' }],
+      });
+
+      const movingAvg = query.aggs['1'].aggs['3'].moving_avg;
+
+      expect(movingAvg.window).toBe(5);
+      expect(movingAvg.predict).toBe(10);
+      expect(movingAvg.settings.alpha).toBe(1);
+      expect(movingAvg.settings.beta).toBe(2);
+      expect(movingAvg.settings.gamma).toBe(3);
+      expect(movingAvg.settings.period).toBe(4);
+    });
+
+    it('correctly casts values in serial_diff ', () => {
+      const query = builder7x.build({
+        refId: 'A',
+        metrics: [
+          { type: 'avg', id: '2' },
+          {
+            type: 'serial_diff',
+            id: '3',
+            field: '2',
+            settings: {
+              lag: '1',
+            },
+          },
+        ],
+        timeField: '@timestamp',
+        bucketAggs: [{ type: 'date_histogram', field: '@timestamp', id: '1' }],
+      });
+
+      const serialDiff = query.aggs['1'].aggs['3'].serial_diff;
+
+      expect(serialDiff.lag).toBe(1);
+    });
+  });
 });