Scale histogram aggregation interval to avoid crashing browser (#14157)

* update histogram agg to fetch min and max when search request started

* scale interval when too many buckets are created

* move min and max params into autoBounds param, remove typo in help text

* use decorated property instead of params to avoid changing agg state, add functional test

* remove sleep from functional test

* make args for onSearchRequest functions specific. Add getters and setters for autoBounds to AggConfig. Protect against divide by zero

* add unused arguments with eslint comment

src/core_plugins/kibana/public/discover/controllers/discover.js

@@ -679,8 +679,8 @@ function discoverController(
       aggs: visStateAggs
     });
 
-    $scope.searchSource.onRequestStart(() => {
-      return $scope.vis.requesting();
+    $scope.searchSource.onRequestStart((searchSource, searchRequest) => {
+      return $scope.vis.onSearchRequestStart(searchSource, searchRequest);
     });
 
     $scope.searchSource.aggs(function () {

src/core_plugins/kibana/public/visualize/saved_visualizations/_saved_vis.js

@@ -72,8 +72,8 @@ uiModules
       return self.vis ? self._updateVis() : self._createVis();
     })
     .then(function () {
-      self.searchSource.onRequestStart(() => {
-        return self.vis.requesting();
+      self.searchSource.onRequestStart((searchSource, searchRequest) => {
+        return self.vis.onSearchRequestStart(searchSource, searchRequest);
       });
 
       self.searchSource.aggs(function () {

src/ui/public/agg_types/__tests__/agg_param_writer.js

@@ -107,12 +107,6 @@ export default function AggParamWriterHelper(Private) {
       return aggConfig.type === self.aggType;
     });
 
-    aggConfig.type.params.forEach(function (param) {
-      if (param.onRequest) {
-        param.onRequest(aggConfig);
-      }
-    });
-
     return aggConfig.type.params.write(aggConfig);
   };
 

src/ui/public/agg_types/buckets/date_histogram.js

@@ -7,7 +7,7 @@ import { AggTypesBucketsBucketAggTypeProvider } from 'ui/agg_types/buckets/_buck
 import { TimeBucketsProvider } from 'ui/time_buckets';
 import { AggTypesBucketsCreateFilterDateHistogramProvider } from 'ui/agg_types/buckets/create_filter/date_histogram';
 import { AggTypesBucketsIntervalOptionsProvider } from 'ui/agg_types/buckets/_interval_options';
-import intervalTemplate from 'ui/agg_types/controls/interval.html';
+import intervalTemplate from 'ui/agg_types/controls/time_interval.html';
 
 export function AggTypesBucketsDateHistogramProvider(timefilter, config, Private) {
   const BucketAggType = Private(AggTypesBucketsBucketAggTypeProvider);
@@ -108,7 +108,7 @@ export function AggTypesBucketsDateHistogramProvider(timefilter, config, Private
         default: 'auto',
         options: intervalOptions,
         editor: intervalTemplate,
-        onRequest: function (agg) {
+        modifyAggConfigOnSearchRequestStart: function (agg) {
           setBounds(agg, true);
         },
         write: function (agg, output) {

src/ui/public/agg_types/buckets/histogram.js

@@ -1,15 +1,16 @@
+import _ from 'lodash';
+
 import 'ui/validate_date_interval';
 import { AggTypesBucketsBucketAggTypeProvider } from 'ui/agg_types/buckets/_bucket_agg_type';
 import { AggTypesBucketsCreateFilterHistogramProvider } from 'ui/agg_types/buckets/create_filter/histogram';
-import intervalTemplate from 'ui/agg_types/controls/interval.html';
+import intervalTemplate from 'ui/agg_types/controls/number_interval.html';
 import minDocCountTemplate from 'ui/agg_types/controls/min_doc_count.html';
 import extendedBoundsTemplate from 'ui/agg_types/controls/extended_bounds.html';
 
-export function AggTypesBucketsHistogramProvider(Private) {
+export function AggTypesBucketsHistogramProvider(Private, config) {
   const BucketAggType = Private(AggTypesBucketsBucketAggTypeProvider);
   const createFilter = Private(AggTypesBucketsCreateFilterHistogramProvider);
 
-
   return new BucketAggType({
     name: 'histogram',
     title: 'Histogram',
@@ -18,6 +19,22 @@ export function AggTypesBucketsHistogramProvider(Private) {
       return aggConfig.getFieldDisplayName();
     },
     createFilter: createFilter,
+    decorateAggConfig: function () {
+      let autoBounds;
+
+      return {
+        setAutoBounds: {
+          value(newValue) {
+            autoBounds = newValue;
+          }
+        },
+        getAutoBounds: {
+          value() {
+            return autoBounds;
+          }
+        }
+      };
+    },
     params: [
       {
         name: 'field',
@@ -27,8 +44,55 @@ export function AggTypesBucketsHistogramProvider(Private) {
       {
         name: 'interval',
         editor: intervalTemplate,
+        modifyAggConfigOnSearchRequestStart(aggConfig, searchSource) {
+          const field = aggConfig.getField();
+          const aggBody = field.scripted
+            ? { script: { inline: field.script, lang: field.lang } }
+            : { field: field.name };
+
+          return searchSource
+            .extend()
+            .size(0)
+            .aggs({
+              maxAgg: {
+                max: aggBody
+              },
+              minAgg: {
+                min: aggBody
+              }
+            })
+            .fetchAsRejectablePromise()
+            .then((resp) => {
+              aggConfig.setAutoBounds({
+                min: _.get(resp, 'aggregations.minAgg.value'),
+                max: _.get(resp, 'aggregations.maxAgg.value')
+              });
+            });
+        },
         write: function (aggConfig, output) {
-          output.params.interval = parseFloat(aggConfig.params.interval);
+          let interval = parseFloat(aggConfig.params.interval);
+          if (interval <= 0) {
+            interval = 1;
+          }
+
+          // ensure interval does not create too many buckets and crash browser
+          if (aggConfig.getAutoBounds()) {
+            const range = aggConfig.getAutoBounds().max - aggConfig.getAutoBounds().min;
+            const bars = range / interval;
+            if (bars > config.get('histogram:maxBars')) {
+              const minInterval = range / config.get('histogram:maxBars');
+              // 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 orderOfMaginute = Math.pow(10, Math.floor(Math.log10(minInterval)));
+              let roundInterval = orderOfMaginute;
+              while (roundInterval < minInterval) {
+                roundInterval += orderOfMaginute;
+              }
+              interval = roundInterval;
+            }
+          }
+
+          output.params.interval = interval;
         }
       },
 

src/ui/public/agg_types/controls/number_interval.html

@@ -0,0 +1,19 @@
+<div class="form-group">
+  <label for="visEditorInterval{{agg.id}}">
+    Minimum Interval
+    <kbn-info
+      placement="right"
+      info="Interval will be automatically scaled in the event that the provided value creates more buckets than specified by Advanced Setting's histogram:maxBars">
+    </kbn-info>
+  </label>
+  <input
+    id="visEditorInterval{{agg.id}}"
+    ng-model="agg.params.interval"
+    required
+    type="number"
+    class="form-control"
+    name="interval"
+    min="0"
+    input-number
+    >
+</div>

src/ui/public/agg_types/controls/interval.html → src/ui/public/agg_types/controls/time_interval.html

@@ -10,7 +10,6 @@
   </label>
   <select
     id="visEditorInterval{{agg.id}}"
-    ng-if="aggParam.options"
     ng-model="agg.params.interval"
     ng-change="agg.write()"
     required
@@ -29,15 +28,4 @@
     ng-if="agg.params.interval.val == 'custom'"
     class="form-control"
     required />
-  <input
-    id="visEditorInterval{{agg.id}}"
-    ng-if="!aggParam.options"
-    ng-model="agg.params.interval"
-    required
-    type="number"
-    class="form-control"
-    name="interval"
-    min="0"
-    input-number
-    >
 </div>

src/ui/public/agg_types/param_types/base.js

@@ -6,5 +6,19 @@ export function BaseParamTypeProvider() {
     _.assign(this, config);
   }
 
+  /**
+   *  A function that will be called before an aggConfig is serialized and sent to ES.
+   *  Allows aggConfig to retrieve values needed for serialization by creating a {SearchRequest}
+   *  Example usage: an aggregation needs to know the min/max of a field to determine an appropriate interval
+   *
+   *  @param {AggConfig} aggconfig
+   *  @param {Courier.SearchSource} searchSource
+   *  @param {Courier.SearchRequest} searchRequest
+   *  @returns {Promise<undefined>|undefined}
+   */
+  // eslint-disable-next-line no-unused-vars
+  BaseParamType.prototype.modifyAggConfigOnSearchRequestStart = function (aggconfig, searchSource, searchRequest) {
+  };
+
   return BaseParamType;
 }

src/ui/public/vis/agg_config.js

@@ -8,7 +8,7 @@
 import _ from 'lodash';
 import { RegistryFieldFormatsProvider } from 'ui/registry/field_formats';
 
-export function VisAggConfigProvider(Private) {
+export function VisAggConfigProvider(Private, Promise) {
   const fieldFormats = Private(RegistryFieldFormatsProvider);
 
   function AggConfig(vis, opts) {
@@ -183,16 +183,20 @@ export function VisAggConfigProvider(Private) {
   };
 
   /**
-   * Hook into param onRequest handling, and tell the aggConfig that it
-   * is being sent to elasticsearch.
-   *
-   * @return {[type]} [description]
+   *  Hook for pre-flight logic, see AggType#onSearchRequestStart
+   *  @param {Courier.SearchSource} searchSource
+   *  @param {Courier.SearchRequest} searchRequest
+   *  @return {Promise<undefined>}
    */
-  AggConfig.prototype.requesting = function () {
-    const self = this;
-    self.type && self.type.params.forEach(function (param) {
-      if (param.onRequest) param.onRequest(self);
-    });
+  AggConfig.prototype.onSearchRequestStart = function (searchSource, searchRequest) {
+    if (!this.type) {
+      return Promise.resolve();
+    }
+
+    return Promise.map(
+      this.type.params,
+      param => param.modifyAggConfigOnSearchRequestStart(this, searchSource, searchRequest)
+    );
   };
 
   /**

src/ui/public/vis/vis.js

@@ -23,7 +23,7 @@ import { SearchSourceProvider } from 'ui/courier/data_source/search_source';
 import { SavedObjectsClientProvider } from 'ui/saved_objects';
 import { FilterManagerProvider } from 'ui/filter_manager';
 
-export function VisProvider(Private, indexPatterns, timefilter, getAppState) {
+export function VisProvider(Private, Promise, indexPatterns, timefilter, getAppState) {
   const visTypes = Private(VisTypesRegistryProvider);
   const AggConfigs = Private(VisAggConfigsProvider);
   const brushEvent = Private(UtilsBrushEventProvider);
@@ -164,9 +164,17 @@ export function VisProvider(Private, indexPatterns, timefilter, getAppState) {
       return clonedVis;
     }
 
-    requesting() {
-      // Invoke requesting() on each agg. Aggs is an instance of AggConfigs.
-      _.invoke(this.aggs.getRequestAggs(), 'requesting');
+    /**
+     *  Hook for pre-flight logic, see AggType#onSearchRequestStart()
+     *  @param {Courier.SearchSource} searchSource
+     *  @param {Courier.SearchRequest} searchRequest
+     *  @return {Promise<undefined>}
+     */
+    onSearchRequestStart(searchSource, searchRequest) {
+      return Promise.map(
+        this.aggs.getRequestAggs(),
+        agg => agg.onSearchRequestStart(searchSource, searchRequest)
+      );
     }
 
     isHierarchical() {

test/functional/apps/visualize/_histogram_request_start.js

@@ -0,0 +1,61 @@
+import expect from 'expect.js';
+
+export default function ({ getService, getPageObjects }) {
+  const log = getService('log');
+  const PageObjects = getPageObjects(['common', 'visualize', 'header']);
+
+  describe('histogram agg onSearchRequestStart', function describeIndexTests() {
+    before(async function () {
+      const fromTime = '2015-09-19 06:31:44.000';
+      const toTime = '2015-09-23 18:31:44.000';
+
+      log.debug('navigateToApp visualize');
+      await PageObjects.common.navigateToUrl('visualize', 'new');
+      log.debug('clickDataTable');
+      await PageObjects.visualize.clickDataTable();
+      await PageObjects.visualize.clickNewSearch();
+      log.debug('Set absolute time range from \"' + fromTime + '\" to \"' + toTime + '\"');
+      await PageObjects.header.setAbsoluteRange(fromTime, toTime);
+      log.debug('Bucket = Split Rows');
+      await PageObjects.visualize.clickBucket('Split Rows');
+      log.debug('Aggregation = Histogram');
+      await PageObjects.visualize.selectAggregation('Histogram');
+      log.debug('Field = bytes');
+      await PageObjects.visualize.selectField('bytes');
+    });
+
+    describe('interval parameter uses autoBounds', function indexPatternCreation() {
+      it('should use provided value when number of generated buckets is less than histogram:maxBars', async function () {
+        const providedInterval = 2000;
+        log.debug(`Interval = ${providedInterval}`);
+        await PageObjects.visualize.setNumericInterval(providedInterval);
+        await PageObjects.visualize.clickGo();
+        await PageObjects.header.waitUntilLoadingHasFinished();
+
+        const data = await PageObjects.visualize.getDataTableData();
+        const dataArray = data.replace(/,/g, '').split('\n');
+        expect(dataArray.length).to.eql(20);
+        const bucketStart = parseInt(dataArray[0], 10);
+        const bucketEnd = parseInt(dataArray[2], 10);
+        const actualInterval = bucketEnd - bucketStart;
+        expect(actualInterval).to.eql(providedInterval);
+      });
+
+      it('should scale value to round number when number of generated buckets is greater than histogram:maxBars', async function () {
+        const providedInterval = 100;
+        log.debug(`Interval = ${providedInterval}`);
+        await PageObjects.visualize.setNumericInterval(providedInterval);
+        await PageObjects.visualize.clickGo();
+        await PageObjects.header.waitUntilLoadingHasFinished();
+
+        const data = await PageObjects.visualize.getDataTableData();
+        const dataArray = data.replace(/,/g, '').split('\n');
+        expect(dataArray.length).to.eql(20);
+        const bucketStart = parseInt(dataArray[0], 10);
+        const bucketEnd = parseInt(dataArray[2], 10);
+        const actualInterval = bucketEnd - bucketStart;
+        expect(actualInterval).to.eql(200);
+      });
+    });
+  });
+}

test/functional/apps/visualize/index.js

@@ -32,5 +32,6 @@ export default function ({ getService, loadTestFile }) {
     loadTestFile(require.resolve('./_tsvb_chart'));
     loadTestFile(require.resolve('./_shared_item'));
     loadTestFile(require.resolve('./_input_control_vis'));
+    loadTestFile(require.resolve('./_histogram_request_start'));
   });
 }

test/functional/page_objects/visualize_page.js

@@ -344,7 +344,8 @@ export function VisualizePageProvider({ getService, getPageObjects }) {
 
     async setNumericInterval(newValue) {
       const input = await find.byCssSelector('input[name="interval"]');
-      await input.type(newValue);
+      await input.clearValue();
+      await input.type(newValue + '');
     }
 
     async clickGo() {