elastic · alexwizp · Sep 2, 2020 · Sep 1, 2020
diff --git a/src/plugins/data/common/search/aggs/buckets/_interval_options.ts b/src/plugins/data/common/search/aggs/buckets/_interval_options.ts
@@ -20,12 +20,15 @@
 import { i18n } from '@kbn/i18n';
 import { IBucketAggConfig } from './bucket_agg_type';
 
+export const autoInterval = 'auto';
+export const isAutoInterval = (value: unknown) => value === autoInterval;
+
 export const intervalOptions = [
   {
     display: i18n.translate('data.search.aggs.buckets.intervalOptions.autoDisplayName', {
       defaultMessage: 'Auto',
     }),
-    val: 'auto',
+    val: autoInterval,
     enabled(agg: IBucketAggConfig) {
       // not only do we need a time field, but the selected field needs
       // to be the time field. (see #3028)

diff --git a/src/plugins/data/common/search/aggs/buckets/create_filter/date_histogram.test.ts b/src/plugins/data/common/search/aggs/buckets/create_filter/date_histogram.test.ts
@@ -19,7 +19,7 @@
 
 import moment from 'moment';
 import { createFilterDateHistogram } from './date_histogram';
-import { intervalOptions } from '../_interval_options';
+import { intervalOptions, autoInterval } from '../_interval_options';
 import { AggConfigs } from '../../agg_configs';
 import { mockAggTypesRegistry } from '../../test_helpers';
 import { IBucketDateHistogramAggConfig } from '../date_histogram';
@@ -33,7 +33,10 @@ describe('AggConfig Filters', () => {
     let bucketStart: any;
     let field: any;
 
-    const init = (interval: string = 'auto', duration: any = moment.duration(15, 'minutes')) => {
+    const init = (
+      interval: string = autoInterval,
+      duration: any = moment.duration(15, 'minutes')
+    ) => {
       field = {
         name: 'date',
       };

diff --git a/src/plugins/data/common/search/aggs/buckets/date_histogram.ts b/src/plugins/data/common/search/aggs/buckets/date_histogram.ts
@@ -23,7 +23,7 @@ import { i18n } from '@kbn/i18n';
 
 import { KBN_FIELD_TYPES, TimeRange, TimeRangeBounds, UI_SETTINGS } from '../../../../common';
 
-import { intervalOptions } from './_interval_options';
+import { intervalOptions, autoInterval, isAutoInterval } from './_interval_options';
 import { createFilterDateHistogram } from './create_filter/date_histogram';
 import { BucketAggType, IBucketAggConfig } from './bucket_agg_type';
 import { BUCKET_TYPES } from './bucket_agg_types';
@@ -44,7 +44,7 @@ const updateTimeBuckets = (
   customBuckets?: IBucketDateHistogramAggConfig['buckets']
 ) => {
   const bounds =
-    agg.params.timeRange && (agg.fieldIsTimeField() || agg.params.interval === 'auto')
+    agg.params.timeRange && (agg.fieldIsTimeField() || isAutoInterval(agg.params.interval))
       ? calculateBounds(agg.params.timeRange)
       : undefined;
   const buckets = customBuckets || agg.buckets;
@@ -149,7 +149,7 @@ export const getDateHistogramBucketAgg = ({
           return agg.getIndexPattern().timeFieldName;
         },
         onChange(agg: IBucketDateHistogramAggConfig) {
-          if (get(agg, 'params.interval') === 'auto' && !agg.fieldIsTimeField()) {
+          if (isAutoInterval(get(agg, 'params.interval')) && !agg.fieldIsTimeField()) {
             delete agg.params.interval;
           }
         },
@@ -187,7 +187,7 @@ export const getDateHistogramBucketAgg = ({
           }
           return state;
         },
-        default: 'auto',
+        default: autoInterval,
         options: intervalOptions,
         write(agg, output, aggs) {
           updateTimeBuckets(agg, calculateBounds);

diff --git a/src/plugins/data/common/search/aggs/buckets/histogram.test.ts b/src/plugins/data/common/search/aggs/buckets/histogram.test.ts
@@ -103,7 +103,28 @@ describe('Histogram Agg', () => {
       });
     });
 
+    describe('maxBars', () => {
+      test('should not be written to the DSL', () => {
+        const aggConfigs = getAggConfigs({
+          maxBars: 50,
+          field: {
+            name: 'field',
+          },
+        });
+        const { [BUCKET_TYPES.HISTOGRAM]: params } = aggConfigs.aggs[0].toDsl();
+
+        expect(params).not.toHaveProperty('maxBars');
+      });
+    });
+
     describe('interval', () => {
+      test('accepts "auto" value', () => {
+        const params = getParams({
+          interval: 'auto',
+        });
+
+        expect(params).toHaveProperty('interval', 1);
+      });
       test('accepts a whole number', () => {
         const params = getParams({
           interval: 100,

diff --git a/src/plugins/data/common/search/aggs/buckets/histogram.ts b/src/plugins/data/common/search/aggs/buckets/histogram.ts
@@ -28,6 +28,8 @@ import { BucketAggType, IBucketAggConfig } from './bucket_agg_type';
 import { createFilterHistogram } from './create_filter/histogram';
 import { BUCKET_TYPES } from './bucket_agg_types';
 import { ExtendedBounds } from './lib/extended_bounds';
+import { isAutoInterval, autoInterval } from './_interval_options';
+import { calculateHistogramInterval } from './lib/histogram_calculate_interval';
 
 export interface AutoBounds {
   min: number;
@@ -47,6 +49,7 @@ export interface IBucketHistogramAggConfig extends IBucketAggConfig {
 export interface AggParamsHistogram extends BaseAggParams {
   field: string;
   interval: string;
+  maxBars?: number;
   intervalBase?: number;
   min_doc_count?: boolean;
   has_extended_bounds?: boolean;
@@ -102,6 +105,7 @@ export const getHistogramBucketAgg = ({
       },
       {
         name: 'interval',
+        default: autoInterval,
         modifyAggConfigOnSearchRequestStart(
           aggConfig: IBucketHistogramAggConfig,
           searchSource: any,
@@ -127,9 +131,12 @@ export const getHistogramBucketAgg = ({
           return childSearchSource
             .fetch(options)
             .then((resp: any) => {
+              const min = resp.aggregations?.minAgg?.value ?? 0;
+              const max = resp.aggregations?.maxAgg?.value ?? 0;
+
               aggConfig.setAutoBounds({
-                min: get(resp, 'aggregations.minAgg.value'),
-                max: get(resp, 'aggregations.maxAgg.value'),
+                min,
+                max,
               });
             })
             .catch((e: Error) => {
@@ -143,46 +150,24 @@ export const getHistogramBucketAgg = ({
             });
         },
         write(aggConfig, output) {
-          let interval = parseFloat(aggConfig.params.interval);
-          if (interval <= 0) {
-            interval = 1;
-          }
-          const autoBounds = aggConfig.getAutoBounds();
-
-          // ensure interval does not create too many buckets and crash browser
-          if (autoBounds) {
-            const range = autoBounds.max - autoBounds.min;
-            const bars = range / interval;
-
-            if (bars > getConfig(UI_SETTINGS.HISTOGRAM_MAX_BARS)) {
-              const minInterval = range / getConfig(UI_SETTINGS.HISTOGRAM_MAX_BARS);
-
-              // Round interval by order of magnitude to provide clean intervals
-              // Always round interval up so there will always be less buckets than histogram:maxBars
-              const orderOfMagnitude = Math.pow(10, Math.floor(Math.log10(minInterval)));
-              let roundInterval = orderOfMagnitude;
-
-              while (roundInterval < minInterval) {
-                roundInterval += orderOfMagnitude;
-              }
-              interval = roundInterval;
-            }
-          }
-          const base = aggConfig.params.intervalBase;
-
-          if (base) {
-            if (interval < base) {
-              // In case the specified interval is below the base, just increase it to it's base
-              interval = base;
-            } else if (interval % base !== 0) {
-              // In case the interval is not a multiple of the base round it to the next base
-              interval = Math.round(interval / base) * base;
-            }
-          }
-
-          output.params.interval = interval;
+          const values = aggConfig.getAutoBounds();
+
+          output.params.interval = calculateHistogramInterval({
+            values,
+            interval: aggConfig.params.interval,
+            maxBucketsUiSettings: getConfig(UI_SETTINGS.HISTOGRAM_MAX_BARS),
+            maxBucketsUserInput: aggConfig.params.maxBars,
+            intervalBase: aggConfig.params.intervalBase,
+          });
         },
       },
+      {
+        name: 'maxBars',
+        shouldShow(agg) {
+          return isAutoInterval(get(agg, 'params.interval'));
+        },
+        write: () => {},
+      },
       {
         name: 'min_doc_count',
         default: false,

diff --git a/src/plugins/data/common/search/aggs/buckets/histogram_fn.test.ts b/src/plugins/data/common/search/aggs/buckets/histogram_fn.test.ts
@@ -43,6 +43,7 @@ describe('agg_expression_functions', () => {
               "interval": "10",
               "intervalBase": undefined,
               "json": undefined,
+              "maxBars": undefined,
               "min_doc_count": undefined,
             },
             "schema": undefined,
@@ -55,8 +56,9 @@ describe('agg_expression_functions', () => {
     test('includes optional params when they are provided', () => {
       const actual = fn({
         field: 'field',
-        interval: '10',
+        interval: 'auto',
         intervalBase: 1,
+        maxBars: 25,
         min_doc_count: false,
         has_extended_bounds: false,
         extended_bounds: JSON.stringify({
@@ -77,9 +79,10 @@ describe('agg_expression_functions', () => {
             },
             "field": "field",
             "has_extended_bounds": false,
-            "interval": "10",
+            "interval": "auto",
             "intervalBase": 1,
             "json": undefined,
+            "maxBars": 25,
             "min_doc_count": false,
           },
           "schema": undefined,

diff --git a/src/plugins/data/common/search/aggs/buckets/histogram_fn.ts b/src/plugins/data/common/search/aggs/buckets/histogram_fn.ts
@@ -85,6 +85,12 @@ export const aggHistogram = (): FunctionDefinition => ({
         defaultMessage: 'Specifies whether to use min_doc_count for this aggregation',
       }),
     },
+    maxBars: {
+      types: ['number'],
+      help: i18n.translate('data.search.aggs.buckets.histogram.maxBars.help', {
+        defaultMessage: 'Calculate interval to get approximately this many bars',
+      }),
+    },
     has_extended_bounds: {
       types: ['boolean'],
       help: i18n.translate('data.search.aggs.buckets.histogram.hasExtendedBounds.help', {

diff --git a/src/plugins/data/common/search/aggs/buckets/lib/histogram_calculate_interval.test.ts b/src/plugins/data/common/search/aggs/buckets/lib/histogram_calculate_interval.test.ts
@@ -0,0 +1,144 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import {
+  calculateHistogramInterval,
+  CalculateHistogramIntervalParams,
+} from './histogram_calculate_interval';
+
+describe('calculateHistogramInterval', () => {
+  describe('auto calculating mode', () => {
+    let params: CalculateHistogramIntervalParams;
+
+    beforeEach(() => {
+      params = {
+        interval: 'auto',
+        intervalBase: undefined,
+        maxBucketsUiSettings: 100,
+        maxBucketsUserInput: undefined,
+        values: {
+          min: 0,
+          max: 1,
+        },
+      };
+    });
+
+    describe('maxBucketsUserInput is defined', () => {
+      test('should not set interval which more than largest possible', () => {
+        const p = {
+          ...params,
+          maxBucketsUserInput: 200,
+          values: {
+            min: 150,
+            max: 250,
+          },
+        };
+        expect(calculateHistogramInterval(p)).toEqual(1);
+      });
+
+      test('should correctly work for float numbers (small numbers)', () => {
+        expect(
+          calculateHistogramInterval({
+            ...params,
+            maxBucketsUserInput: 50,
+            values: {
+              min: 0.1,
+              max: 0.9,
+            },
+          })
+        ).toBe(0.02);
+      });
+
+      test('should correctly work for float numbers (big numbers)', () => {
+        expect(
+          calculateHistogramInterval({
+            ...params,
+            maxBucketsUserInput: 10,
+            values: {
+              min: 10.45,
+              max: 1000.05,
+            },
+          })
+        ).toBe(100);
+      });
+    });
+
+    describe('maxBucketsUserInput is not defined', () => {
+      test('should not set interval which more than largest possible', () => {
+        expect(
+          calculateHistogramInterval({
+            ...params,
+            values: {
+              min: 0,
+              max: 100,
+            },
+          })
+        ).toEqual(1);
+      });
+
+      test('should set intervals for integer numbers (diff less than maxBucketsUiSettings)', () => {
+        expect(
+          calculateHistogramInterval({
+            ...params,
+            values: {
+              min: 1,
+              max: 10,
+            },
+          })
+        ).toEqual(0.1);
+      });
+
+      test('should set intervals for integer numbers (diff more than maxBucketsUiSettings)', () => {
+        // diff === 44445; interval === 500; buckets === 89
+        expect(
+          calculateHistogramInterval({
+            ...params,
+            values: {
+              min: 45678,
+              max: 90123,
+            },
+          })
+        ).toEqual(500);
+      });
+
+      test('should set intervals the same for the same interval', () => {
+        // both diffs are the same
+        // diff === 1.655; interval === 0.02; buckets === 82
+        expect(
+          calculateHistogramInterval({
+            ...params,
+            values: {
+              min: 1.245,
+              max: 2.9,
+            },
+          })
+        ).toEqual(0.02);
+        expect(
+          calculateHistogramInterval({
+            ...params,
+            values: {
+              min: 0.5,
+              max: 2.3,
+            },
+          })
+        ).toEqual(0.02);
+      });
+    });
+  });
+});