From cf29df808a581759212a1a0417cd2fa081c500f0 Mon Sep 17 00:00:00 2001
From: "Grot (@grafanabot)" <43478413+grafanabot@users.noreply.github.com>
Date: Fri, 5 Mar 2021 13:36:35 +0000
Subject: [PATCH] Elasticsearch: Fix script fields in query editor (#31681)
 (#31727)

* Elasticsearch: Fix script fields in query editor

* properly name bucke_script deries

(cherry picked from commit 64a8514e47fdbcd22262cc43b8147e5237cd3c6f)

Co-authored-by: Giordano Ricci <gio.ricci@grafana.com>
---
 .../elasticsearch/components/MetricPicker.tsx | 22 ++++++++------
 .../SettingsEditor/SettingField.tsx           | 11 +++++--
 .../MetricAggregationsEditor/aggregations.ts  | 19 ++++++------
 .../datasource/elasticsearch/datasource.ts    |  3 +-
 .../elasticsearch/elastic_response.ts         |  6 ++--
 .../datasource/elasticsearch/query_builder.ts | 11 +++++--
 .../plugins/datasource/elasticsearch/utils.ts | 29 +++++++++++++++++++
 7 files changed, 74 insertions(+), 27 deletions(-)

diff --git a/public/app/plugins/datasource/elasticsearch/components/MetricPicker.tsx b/public/app/plugins/datasource/elasticsearch/components/MetricPicker.tsx
index 6660bea5a89..c1b7f86d8d8 100644
--- a/public/app/plugins/datasource/elasticsearch/components/MetricPicker.tsx
+++ b/public/app/plugins/datasource/elasticsearch/components/MetricPicker.tsx
@@ -23,12 +23,16 @@ interface Props {
   value?: string;
 }
 
-export const MetricPicker: FunctionComponent<Props> = ({ options, onChange, className, value }) => (
-  <Segment
-    className={cx(className, noWrap)}
-    options={toOptions(options)}
-    onChange={onChange}
-    placeholder="Select Metric"
-    value={!!value ? toOption(options.find((option) => option.id === value)!) : null}
-  />
-);
+export const MetricPicker: FunctionComponent<Props> = ({ options, onChange, className, value }) => {
+  const selectedOption = options.find((option) => option.id === value);
+
+  return (
+    <Segment
+      className={cx(className, noWrap)}
+      options={toOptions(options)}
+      onChange={onChange}
+      placeholder="Select Metric"
+      value={!!selectedOption ? toOption(selectedOption) : null}
+    />
+  );
+};
diff --git a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/SettingsEditor/SettingField.tsx b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/SettingsEditor/SettingField.tsx
index ff5bc5438ca..01a9370ccaa 100644
--- a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/SettingsEditor/SettingField.tsx
+++ b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/SettingsEditor/SettingField.tsx
@@ -4,8 +4,9 @@ import { useDispatch } from '../../../../hooks/useStatelessReducer';
 import { changeMetricSetting } from '../state/actions';
 import { ChangeMetricSettingAction } from '../state/types';
 import { SettingKeyOf } from '../../../types';
-import { MetricAggregationWithSettings } from '../aggregations';
+import { MetricAggregationWithInlineScript, MetricAggregationWithSettings } from '../aggregations';
 import { uniqueId } from 'lodash';
+import { getScriptValue } from 'app/plugins/datasource/elasticsearch/utils';
 
 interface Props<T extends MetricAggregationWithSettings, K extends SettingKeyOf<T>> {
   label: string;
@@ -26,13 +27,19 @@ export function SettingField<T extends MetricAggregationWithSettings, K extends
   const [id] = useState(uniqueId(`es-field-id-`));
   const settings = metric.settings;
 
+  let defaultValue = settings?.[settingName as keyof typeof settings] || '';
+
+  if (settingName === 'script') {
+    defaultValue = getScriptValue(metric as MetricAggregationWithInlineScript);
+  }
+
   return (
     <InlineField label={label} labelWidth={16} tooltip={tooltip}>
       <Input
         id={id}
         placeholder={placeholder}
         onBlur={(e) => dispatch(changeMetricSetting(metric, settingName, e.target.value as any))}
-        defaultValue={settings?.[settingName as keyof typeof settings]}
+        defaultValue={defaultValue}
       />
     </InlineField>
   );
diff --git a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/aggregations.ts b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/aggregations.ts
index b56fedd6480..e833fdf5c56 100644
--- a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/aggregations.ts
+++ b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/aggregations.ts
@@ -43,9 +43,10 @@ export interface MetricAggregationWithMissingSupport extends BaseMetricAggregati
   };
 }
 
+type InlineScript = string | { inline?: string };
 export interface MetricAggregationWithInlineScript extends BaseMetricAggregation {
   settings?: {
-    script?: string;
+    script?: InlineScript;
   };
 }
 
@@ -59,7 +60,7 @@ export interface Average
     MetricAggregationWithInlineScript {
   type: 'avg';
   settings?: {
-    script?: string;
+    script?: InlineScript;
     missing?: string;
   };
 }
@@ -67,7 +68,7 @@ export interface Average
 export interface Sum extends MetricAggregationWithField, MetricAggregationWithInlineScript {
   type: 'sum';
   settings?: {
-    script?: string;
+    script?: InlineScript;
     missing?: string;
   };
 }
@@ -75,7 +76,7 @@ export interface Sum extends MetricAggregationWithField, MetricAggregationWithIn
 export interface Max extends MetricAggregationWithField, MetricAggregationWithInlineScript {
   type: 'max';
   settings?: {
-    script?: string;
+    script?: InlineScript;
     missing?: string;
   };
 }
@@ -83,7 +84,7 @@ export interface Max extends MetricAggregationWithField, MetricAggregationWithIn
 export interface Min extends MetricAggregationWithField, MetricAggregationWithInlineScript {
   type: 'min';
   settings?: {
-    script?: string;
+    script?: InlineScript;
     missing?: string;
   };
 }
@@ -105,7 +106,7 @@ export interface ExtendedStat {
 export interface ExtendedStats extends MetricAggregationWithField, MetricAggregationWithInlineScript {
   type: 'extended_stats';
   settings?: {
-    script?: string;
+    script?: InlineScript;
     missing?: string;
     sigma?: string;
   };
@@ -118,7 +119,7 @@ export interface Percentiles extends MetricAggregationWithField, MetricAggregati
   type: 'percentiles';
   settings?: {
     percents?: string[];
-    script?: string;
+    script?: InlineScript;
     missing?: string;
   };
 }
@@ -238,7 +239,7 @@ export interface MovingFunction extends BasePipelineMetricAggregation {
   type: 'moving_fn';
   settings?: {
     window?: string;
-    script?: string;
+    script?: InlineScript;
     shift?: string;
   };
 }
@@ -267,7 +268,7 @@ export interface CumulativeSum extends BasePipelineMetricAggregation {
 export interface BucketScript extends PipelineMetricAggregationWithMultipleBucketPaths {
   type: 'bucket_script';
   settings?: {
-    script?: string;
+    script?: InlineScript;
   };
 }
 
diff --git a/public/app/plugins/datasource/elasticsearch/datasource.ts b/public/app/plugins/datasource/elasticsearch/datasource.ts
index 08ce0e6331a..19fccfaf4ed 100644
--- a/public/app/plugins/datasource/elasticsearch/datasource.ts
+++ b/public/app/plugins/datasource/elasticsearch/datasource.ts
@@ -36,6 +36,7 @@ import { bucketAggregationConfig } from './components/QueryEditor/BucketAggregat
 import { 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';
 
 // Those are metadata fields as defined in https://www.elastic.co/guide/en/elasticsearch/reference/current/mapping-fields.html#_identity_metadata_fields.
 // custom fields can start with underscores, therefore is not safe to exclude anything that starts with one.
@@ -425,7 +426,7 @@ export class ElasticDatasource extends DataSourceApi<ElasticsearchQuery, Elastic
         text += metric.field;
       }
       if (isPipelineAggregationWithMultipleBucketPaths(metric)) {
-        text += metric.settings?.script?.replace(new RegExp('params.', 'g'), '');
+        text += getScriptValue(metric).replace(new RegExp('params.', 'g'), '');
       }
       text += '), ';
 
diff --git a/public/app/plugins/datasource/elasticsearch/elastic_response.ts b/public/app/plugins/datasource/elasticsearch/elastic_response.ts
index e7c13b44bbb..470c0645723 100644
--- a/public/app/plugins/datasource/elasticsearch/elastic_response.ts
+++ b/public/app/plugins/datasource/elasticsearch/elastic_response.ts
@@ -15,7 +15,7 @@ import {
   ExtendedStatMetaType,
   isMetricAggregationWithField,
 } from './components/QueryEditor/MetricAggregationsEditor/aggregations';
-import { describeMetric } from './utils';
+import { describeMetric, getScriptValue } from './utils';
 import { metricAggregationConfig } from './components/QueryEditor/MetricAggregationsEditor/utils';
 
 const HIGHLIGHT_TAGS_EXP = `${queryDef.highlightTags.pre}([^@]+)${queryDef.highlightTags.post}`;
@@ -207,7 +207,7 @@ export class ElasticResponse {
 
               if (metric.type === 'bucket_script') {
                 //Use the formula in the column name
-                metricName = metric.settings?.script || '';
+                metricName = getScriptValue(metric);
               }
             }
 
@@ -306,7 +306,7 @@ export class ElasticResponse {
       if (series.metric && queryDef.isPipelineAggWithMultipleBucketPaths(series.metric)) {
         const agg: any = _.find(target.metrics, { id: series.metricId });
         if (agg && agg.settings.script) {
-          metricName = agg.settings.script;
+          metricName = getScriptValue(agg);
 
           for (const pv of agg.pipelineVariables) {
             const appliedAgg: any = _.find(target.metrics, { id: pv.pipelineAgg });
diff --git a/public/app/plugins/datasource/elasticsearch/query_builder.ts b/public/app/plugins/datasource/elasticsearch/query_builder.ts
index 56f4fdccc20..256359b6cc3 100644
--- a/public/app/plugins/datasource/elasticsearch/query_builder.ts
+++ b/public/app/plugins/datasource/elasticsearch/query_builder.ts
@@ -9,10 +9,12 @@ import {
   isMetricAggregationWithSettings,
   isPipelineAggregation,
   isPipelineAggregationWithMultipleBucketPaths,
+  MetricAggregation,
+  MetricAggregationWithInlineScript,
 } from './components/QueryEditor/MetricAggregationsEditor/aggregations';
 import { defaultBucketAgg, defaultMetricAgg, findMetricById, highlightTags } from './query_def';
 import { ElasticsearchQuery } from './types';
-import { convertOrderByToMetricId } from './utils';
+import { convertOrderByToMetricId, getScriptValue } from './utils';
 
 export class ElasticQueryBuilder {
   timeField: string;
@@ -204,8 +206,9 @@ export class ElasticQueryBuilder {
     target.metrics = target.metrics || [defaultMetricAgg()];
     target.bucketAggs = target.bucketAggs || [defaultBucketAgg()];
     target.timeField = this.timeField;
+    let metric: MetricAggregation;
 
-    let i, j, pv, nestedAggs, metric;
+    let i, j, pv, nestedAggs;
     const query = {
       size: 0,
       query: {
@@ -340,7 +343,9 @@ export class ElasticQueryBuilder {
       if (isMetricAggregationWithSettings(metric)) {
         Object.entries(metric.settings || {})
           .filter(([_, v]) => v !== null)
-          .forEach(([k, v]) => (metricAgg[k] = v));
+          .forEach(([k, v]) => {
+            metricAgg[k] = k === 'script' ? getScriptValue(metric as MetricAggregationWithInlineScript) : v;
+          });
       }
 
       aggField[metric.type] = metricAgg;
diff --git a/public/app/plugins/datasource/elasticsearch/utils.ts b/public/app/plugins/datasource/elasticsearch/utils.ts
index f74195c2c30..ad6de6a7249 100644
--- a/public/app/plugins/datasource/elasticsearch/utils.ts
+++ b/public/app/plugins/datasource/elasticsearch/utils.ts
@@ -1,6 +1,7 @@
 import {
   isMetricAggregationWithField,
   MetricAggregation,
+  MetricAggregationWithInlineScript,
 } from './components/QueryEditor/MetricAggregationsEditor/aggregations';
 import { metricAggregationConfig } from './components/QueryEditor/MetricAggregationsEditor/utils';
 
@@ -62,3 +63,31 @@ export const convertOrderByToMetricId = (orderBy: string): string | undefined =>
   const metricIdMatches = orderBy.match(/^(\d+)/);
   return metricIdMatches ? metricIdMatches[1] : void 0;
 };
+
+/** Gets the actual script value for metrics that support inline scripts.
+ *
+ *  This is needed because the `script` is a bit polymorphic.
+ *  when creating a query with Grafana < 7.4 it was stored as:
+ * ```json
+ * {
+ *    "settings": {
+ *      "script": {
+ *        "inline": "value"
+ *      }
+ *    }
+ * }
+ * ```
+ *
+ * while from 7.4 it's stored as
+ * ```json
+ * {
+ *    "settings": {
+ *      "script": "value"
+ *    }
+ * }
+ * ```
+ *
+ * This allows us to access both formats and support both queries created before 7.4 and after.
+ */
+export const getScriptValue = (metric: MetricAggregationWithInlineScript) =>
+  (typeof metric.settings?.script === 'object' ? metric.settings?.script?.inline : metric.settings?.script) || '';