From fab6808d66acef66562ec19d91aa40981bae9533 Mon Sep 17 00:00:00 2001 From: spalger Date: Wed, 24 Oct 2018 19:50:20 -0700 Subject: [PATCH 1/4] [ui/timeBuckets] test calcAutoInterval module --- .../public/time_buckets/calc_auto_interval.js | 4 - .../time_buckets/calc_auto_interval.test.ts | 117 ++++++++++++++++++ 2 files changed, 117 insertions(+), 4 deletions(-) create mode 100644 src/ui/public/time_buckets/calc_auto_interval.test.ts diff --git a/src/ui/public/time_buckets/calc_auto_interval.js b/src/ui/public/time_buckets/calc_auto_interval.js index 4ed5d9cd22f16..a143f2f844d60 100644 --- a/src/ui/public/time_buckets/calc_auto_interval.js +++ b/src/ui/public/time_buckets/calc_auto_interval.js @@ -81,8 +81,4 @@ export const calcAutoInterval = { lessThan: find(revRoundingRules, function (bound, interval, target) { if (interval < target) return interval; }), - - atLeast: find(revRoundingRules, function atLeast(bound, interval, target) { - if (interval <= target) return interval; - }), }; diff --git a/src/ui/public/time_buckets/calc_auto_interval.test.ts b/src/ui/public/time_buckets/calc_auto_interval.test.ts new file mode 100644 index 0000000000000..b62b437a06be6 --- /dev/null +++ b/src/ui/public/time_buckets/calc_auto_interval.test.ts @@ -0,0 +1,117 @@ +/* + * 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 moment from 'moment'; + +// @ts-ignore +import { calcAutoInterval } from './calc_auto_interval'; + +describe('calcAutoInterval.near', () => { + test('100 buckets/1ms = 1ms buckets', () => { + const interval = calcAutoInterval.near(100, moment.duration(1, 'ms')); + expect(interval.asMilliseconds()).toBe(1); + }); + + test('100 buckets/200ms = 2ms buckets', () => { + const interval = calcAutoInterval.near(100, moment.duration(200, 'ms')); + expect(interval.asMilliseconds()).toBe(2); + }); + + test('1000 buckets/1s = 1ms buckets', () => { + const interval = calcAutoInterval.near(1000, moment.duration(1, 's')); + expect(interval.asMilliseconds()).toBe(1); + }); + + test('1000 buckets/1000h = 1h buckets', () => { + const interval = calcAutoInterval.near(1000, moment.duration(1000, 'hours')); + expect(interval.asHours()).toBe(1); + }); + + test('100 buckets/1h = 30s buckets', () => { + const interval = calcAutoInterval.near(100, moment.duration(1, 'hours')); + expect(interval.asSeconds()).toBe(30); + }); + + test('25 buckets/1d = 1h buckets', () => { + const interval = calcAutoInterval.near(25, moment.duration(1, 'day')); + expect(interval.asHours()).toBe(1); + }); + + test('1000 buckets/1y = 12h buckets', () => { + const interval = calcAutoInterval.near(1000, moment.duration(1, 'year')); + expect(interval.asHours()).toBe(12); + }); + + test('10000 buckets/1y = 1h buckets', () => { + const interval = calcAutoInterval.near(10000, moment.duration(1, 'year')); + expect(interval.asHours()).toBe(1); + }); + + test('100000 buckets/1y = 5m buckets', () => { + const interval = calcAutoInterval.near(100000, moment.duration(1, 'year')); + expect(interval.asMinutes()).toBe(5); + }); +}); + +describe('calcAutoInterval.lessThan', () => { + test('100 buckets/1ms = 1ms buckets', () => { + const interval = calcAutoInterval.lessThan(100, moment.duration(1, 'ms')); + expect(interval.asMilliseconds()).toBe(1); + }); + + test('100 buckets/200ms = 2ms buckets', () => { + const interval = calcAutoInterval.lessThan(100, moment.duration(200, 'ms')); + expect(interval.asMilliseconds()).toBe(2); + }); + + test('1000 buckets/1s = 1ms buckets', () => { + const interval = calcAutoInterval.lessThan(1000, moment.duration(1, 's')); + expect(interval.asMilliseconds()).toBe(1); + }); + + test('1000 buckets/1000h = 30m buckets', () => { + const interval = calcAutoInterval.lessThan(1000, moment.duration(1000, 'hours')); + expect(interval.asMinutes()).toBe(30); + }); + + test('100 buckets/1h = 30s buckets', () => { + const interval = calcAutoInterval.lessThan(100, moment.duration(1, 'hours')); + expect(interval.asSeconds()).toBe(30); + }); + + test('25 buckets/1d = 30m buckets', () => { + const interval = calcAutoInterval.lessThan(25, moment.duration(1, 'day')); + expect(interval.asMinutes()).toBe(30); + }); + + test('1000 buckets/1y = 3h buckets', () => { + const interval = calcAutoInterval.lessThan(1000, moment.duration(1, 'year')); + expect(interval.asHours()).toBe(3); + }); + + test('10000 buckets/1y = 30m buckets', () => { + const interval = calcAutoInterval.lessThan(10000, moment.duration(1, 'year')); + expect(interval.asMinutes()).toBe(30); + }); + + test('100000 buckets/1y = 5m buckets', () => { + const interval = calcAutoInterval.lessThan(100000, moment.duration(1, 'year')); + expect(interval.asMinutes()).toBe(5); + }); +}); From a071868fcebac6cc6c85b5cace31687dfbbdcce6 Mon Sep 17 00:00:00 2001 From: spalger Date: Wed, 24 Oct 2018 20:55:55 -0700 Subject: [PATCH 2/4] [ui/timeBuckets] refactor calcAutoInterval* methods --- .../public/time_buckets/calc_auto_interval.js | 84 ------------ .../time_buckets/calc_auto_interval.test.ts | 47 ++++--- .../public/time_buckets/calc_auto_interval.ts | 129 ++++++++++++++++++ src/ui/public/time_buckets/time_buckets.js | 6 +- 4 files changed, 155 insertions(+), 111 deletions(-) delete mode 100644 src/ui/public/time_buckets/calc_auto_interval.js create mode 100644 src/ui/public/time_buckets/calc_auto_interval.ts diff --git a/src/ui/public/time_buckets/calc_auto_interval.js b/src/ui/public/time_buckets/calc_auto_interval.js deleted file mode 100644 index a143f2f844d60..0000000000000 --- a/src/ui/public/time_buckets/calc_auto_interval.js +++ /dev/null @@ -1,84 +0,0 @@ -/* - * 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 moment from 'moment'; -const { duration: d } = moment; - -// these are the rounding rules used by roundInterval() - -const roundingRules = [ - [ d(500, 'ms'), d(100, 'ms') ], - [ d(5, 'second'), d(1, 'second') ], - [ d(7.5, 'second'), d(5, 'second') ], - [ d(15, 'second'), d(10, 'second') ], - [ d(45, 'second'), d(30, 'second') ], - [ d(3, 'minute'), d(1, 'minute') ], - [ d(9, 'minute'), d(5, 'minute') ], - [ d(20, 'minute'), d(10, 'minute') ], - [ d(45, 'minute'), d(30, 'minute') ], - [ d(2, 'hour'), d(1, 'hour') ], - [ d(6, 'hour'), d(3, 'hour') ], - [ d(24, 'hour'), d(12, 'hour') ], - [ d(1, 'week'), d(1, 'd') ], - [ d(3, 'week'), d(1, 'week') ], - [ d(1, 'year'), d(1, 'month') ], - [ Infinity, d(1, 'year') ] -]; - -const revRoundingRules = roundingRules.slice(0).reverse(); - -function find(rules, check, last) { - function pick(buckets, duration) { - const target = duration / buckets; - let lastResp; - - for (let i = 0; i < rules.length; i++) { - const rule = rules[i]; - const resp = check(rule[0], rule[1], target); - - if (resp == null) { - if (!last) continue; - if (lastResp) return lastResp; - break; - } - - if (!last) return resp; - lastResp = resp; - } - - // fallback to just a number of milliseconds, ensure ms is >= 1 - const ms = Math.max(Math.floor(target), 1); - return moment.duration(ms, 'ms'); - } - - return function (buckets, duration) { - const interval = pick(buckets, duration); - if (interval) return moment.duration(interval._data); - }; -} - -export const calcAutoInterval = { - near: find(revRoundingRules, function near(bound, interval, target) { - if (bound > target) return interval; - }, true), - - lessThan: find(revRoundingRules, function (bound, interval, target) { - if (interval < target) return interval; - }), -}; diff --git a/src/ui/public/time_buckets/calc_auto_interval.test.ts b/src/ui/public/time_buckets/calc_auto_interval.test.ts index b62b437a06be6..4adcc01e7e7ee 100644 --- a/src/ui/public/time_buckets/calc_auto_interval.test.ts +++ b/src/ui/public/time_buckets/calc_auto_interval.test.ts @@ -19,99 +19,98 @@ import moment from 'moment'; -// @ts-ignore -import { calcAutoInterval } from './calc_auto_interval'; +import { calcAutoIntervalLessThan, calcAutoIntervalNear } from './calc_auto_interval'; -describe('calcAutoInterval.near', () => { +describe('calcAutoIntervalNear', () => { test('100 buckets/1ms = 1ms buckets', () => { - const interval = calcAutoInterval.near(100, moment.duration(1, 'ms')); + const interval = calcAutoIntervalNear(100, moment.duration(1, 'ms')); expect(interval.asMilliseconds()).toBe(1); }); test('100 buckets/200ms = 2ms buckets', () => { - const interval = calcAutoInterval.near(100, moment.duration(200, 'ms')); + const interval = calcAutoIntervalNear(100, moment.duration(200, 'ms')); expect(interval.asMilliseconds()).toBe(2); }); test('1000 buckets/1s = 1ms buckets', () => { - const interval = calcAutoInterval.near(1000, moment.duration(1, 's')); + const interval = calcAutoIntervalNear(1000, moment.duration(1, 's')); expect(interval.asMilliseconds()).toBe(1); }); test('1000 buckets/1000h = 1h buckets', () => { - const interval = calcAutoInterval.near(1000, moment.duration(1000, 'hours')); + const interval = calcAutoIntervalNear(1000, moment.duration(1000, 'hours')); expect(interval.asHours()).toBe(1); }); test('100 buckets/1h = 30s buckets', () => { - const interval = calcAutoInterval.near(100, moment.duration(1, 'hours')); + const interval = calcAutoIntervalNear(100, moment.duration(1, 'hours')); expect(interval.asSeconds()).toBe(30); }); test('25 buckets/1d = 1h buckets', () => { - const interval = calcAutoInterval.near(25, moment.duration(1, 'day')); + const interval = calcAutoIntervalNear(25, moment.duration(1, 'day')); expect(interval.asHours()).toBe(1); }); test('1000 buckets/1y = 12h buckets', () => { - const interval = calcAutoInterval.near(1000, moment.duration(1, 'year')); + const interval = calcAutoIntervalNear(1000, moment.duration(1, 'year')); expect(interval.asHours()).toBe(12); }); test('10000 buckets/1y = 1h buckets', () => { - const interval = calcAutoInterval.near(10000, moment.duration(1, 'year')); + const interval = calcAutoIntervalNear(10000, moment.duration(1, 'year')); expect(interval.asHours()).toBe(1); }); test('100000 buckets/1y = 5m buckets', () => { - const interval = calcAutoInterval.near(100000, moment.duration(1, 'year')); + const interval = calcAutoIntervalNear(100000, moment.duration(1, 'year')); expect(interval.asMinutes()).toBe(5); }); }); -describe('calcAutoInterval.lessThan', () => { +describe('calcAutoIntervalLessThan', () => { test('100 buckets/1ms = 1ms buckets', () => { - const interval = calcAutoInterval.lessThan(100, moment.duration(1, 'ms')); + const interval = calcAutoIntervalLessThan(100, moment.duration(1, 'ms')); expect(interval.asMilliseconds()).toBe(1); }); test('100 buckets/200ms = 2ms buckets', () => { - const interval = calcAutoInterval.lessThan(100, moment.duration(200, 'ms')); + const interval = calcAutoIntervalLessThan(100, moment.duration(200, 'ms')); expect(interval.asMilliseconds()).toBe(2); }); test('1000 buckets/1s = 1ms buckets', () => { - const interval = calcAutoInterval.lessThan(1000, moment.duration(1, 's')); + const interval = calcAutoIntervalLessThan(1000, moment.duration(1, 's')); expect(interval.asMilliseconds()).toBe(1); }); - test('1000 buckets/1000h = 30m buckets', () => { - const interval = calcAutoInterval.lessThan(1000, moment.duration(1000, 'hours')); - expect(interval.asMinutes()).toBe(30); + test('1000 buckets/1000h = 1h buckets', () => { + const interval = calcAutoIntervalLessThan(1000, moment.duration(1000, 'hours')); + expect(interval.asHours()).toBe(1); }); test('100 buckets/1h = 30s buckets', () => { - const interval = calcAutoInterval.lessThan(100, moment.duration(1, 'hours')); + const interval = calcAutoIntervalLessThan(100, moment.duration(1, 'hours')); expect(interval.asSeconds()).toBe(30); }); test('25 buckets/1d = 30m buckets', () => { - const interval = calcAutoInterval.lessThan(25, moment.duration(1, 'day')); + const interval = calcAutoIntervalLessThan(25, moment.duration(1, 'day')); expect(interval.asMinutes()).toBe(30); }); test('1000 buckets/1y = 3h buckets', () => { - const interval = calcAutoInterval.lessThan(1000, moment.duration(1, 'year')); + const interval = calcAutoIntervalLessThan(1000, moment.duration(1, 'year')); expect(interval.asHours()).toBe(3); }); test('10000 buckets/1y = 30m buckets', () => { - const interval = calcAutoInterval.lessThan(10000, moment.duration(1, 'year')); + const interval = calcAutoIntervalLessThan(10000, moment.duration(1, 'year')); expect(interval.asMinutes()).toBe(30); }); test('100000 buckets/1y = 5m buckets', () => { - const interval = calcAutoInterval.lessThan(100000, moment.duration(1, 'year')); + const interval = calcAutoIntervalLessThan(100000, moment.duration(1, 'year')); expect(interval.asMinutes()).toBe(5); }); }); diff --git a/src/ui/public/time_buckets/calc_auto_interval.ts b/src/ui/public/time_buckets/calc_auto_interval.ts new file mode 100644 index 0000000000000..a36948313f841 --- /dev/null +++ b/src/ui/public/time_buckets/calc_auto_interval.ts @@ -0,0 +1,129 @@ +/* + * 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 moment, { Duration } from 'moment'; + +const rules = [ + { + bound: Infinity, + interval: moment.duration(1, 'year'), + }, + { + bound: moment.duration(1, 'year'), + interval: moment.duration(1, 'month'), + }, + { + bound: moment.duration(3, 'week'), + interval: moment.duration(1, 'week'), + }, + { + bound: moment.duration(1, 'week'), + interval: moment.duration(1, 'd'), + }, + { + bound: moment.duration(24, 'hour'), + interval: moment.duration(12, 'hour'), + }, + { + bound: moment.duration(6, 'hour'), + interval: moment.duration(3, 'hour'), + }, + { + bound: moment.duration(2, 'hour'), + interval: moment.duration(1, 'hour'), + }, + { + bound: moment.duration(45, 'minute'), + interval: moment.duration(30, 'minute'), + }, + { + bound: moment.duration(20, 'minute'), + interval: moment.duration(10, 'minute'), + }, + { + bound: moment.duration(9, 'minute'), + interval: moment.duration(5, 'minute'), + }, + { + bound: moment.duration(3, 'minute'), + interval: moment.duration(1, 'minute'), + }, + { + bound: moment.duration(45, 'second'), + interval: moment.duration(30, 'second'), + }, + { + bound: moment.duration(15, 'second'), + interval: moment.duration(10, 'second'), + }, + { + bound: moment.duration(7.5, 'second'), + interval: moment.duration(5, 'second'), + }, + { + bound: moment.duration(5, 'second'), + interval: moment.duration(1, 'second'), + }, + { + bound: moment.duration(500, 'ms'), + interval: moment.duration(100, 'ms'), + }, +]; + +function defaultInterval(targetMs: number) { + return moment.duration(Math.max(Math.floor(targetMs), 1), 'ms'); +} + +/** + * Using some simple rules we pick a "pretty" interval that will + * produce around the number of buckets desired given a time range. + * + * @param targetBucketCount desired number of buckets + * @param duration time range the agg covers + */ +export function calcAutoIntervalNear(targetBucketCount: number, duration: Duration) { + const targetMs = Number(duration) / targetBucketCount; + + for (let i = 0; i < rules.length - 1; i++) { + if (Number(rules[i + 1].bound) <= targetMs) { + return rules[i].interval.clone(); + } + } + + return defaultInterval(targetMs); +} + +/** + * Pick a "pretty" interval that produces no more than the maxBucketCount + * for the given time range. + * + * @param maxBucketCount maximum number of buckets to create + * @param duration amount of time covered by the agg + */ +export function calcAutoIntervalLessThan(maxBucketCount: number, duration: Duration) { + const maxMs = Number(duration) / maxBucketCount; + + for (const { interval } of rules) { + if (Number(interval) <= maxMs) { + return interval.clone(); + } + } + + return defaultInterval(maxMs); +} diff --git a/src/ui/public/time_buckets/time_buckets.js b/src/ui/public/time_buckets/time_buckets.js index a5af6c3294ed3..4c0e3220e6783 100644 --- a/src/ui/public/time_buckets/time_buckets.js +++ b/src/ui/public/time_buckets/time_buckets.js @@ -21,7 +21,7 @@ import _ from 'lodash'; import moment from 'moment'; import chrome from '../chrome'; import { parseInterval } from '../utils/parse_interval'; -import { calcAutoInterval } from './calc_auto_interval'; +import { calcAutoIntervalLessThan, calcAutoIntervalNear } from './calc_auto_interval'; import { convertDurationToNormalizedEsInterval, convertIntervalToEsInterval, @@ -231,7 +231,7 @@ TimeBuckets.prototype.getInterval = function (useNormalizedEsInterval = true) { function readInterval() { const interval = self._i; if (moment.isDuration(interval)) return interval; - return calcAutoInterval.near(config.get('histogram:barTarget'), duration); + return calcAutoIntervalNear(config.get('histogram:barTarget'), duration); } // check to see if the interval should be scaled, and scale it if so @@ -243,7 +243,7 @@ TimeBuckets.prototype.getInterval = function (useNormalizedEsInterval = true) { let scaled; if (approxLen > maxLength) { - scaled = calcAutoInterval.lessThan(maxLength, duration); + scaled = calcAutoIntervalLessThan(maxLength, duration); } else { return interval; } From 3ea56af11de0eb9545b9640692294c30c2e771c3 Mon Sep 17 00:00:00 2001 From: spalger Date: Fri, 26 Oct 2018 11:54:25 -0700 Subject: [PATCH 3/4] [calcAutoInterval] return 0ms when duration is invalid --- .../time_buckets/calc_auto_interval.test.ts | 20 +++++++++++++++++++ .../public/time_buckets/calc_auto_interval.ts | 11 +++++++--- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/src/ui/public/time_buckets/calc_auto_interval.test.ts b/src/ui/public/time_buckets/calc_auto_interval.test.ts index 4adcc01e7e7ee..e9723c6f483ed 100644 --- a/src/ui/public/time_buckets/calc_auto_interval.test.ts +++ b/src/ui/public/time_buckets/calc_auto_interval.test.ts @@ -22,6 +22,16 @@ import moment from 'moment'; import { calcAutoIntervalLessThan, calcAutoIntervalNear } from './calc_auto_interval'; describe('calcAutoIntervalNear', () => { + test('0 buckets/1h = 0ms buckets', () => { + const interval = calcAutoIntervalNear(0, moment.duration(1, 'h')); + expect(interval.asMilliseconds()).toBe(0); + }); + + test('100 buckets/undefined = 0ms buckets', () => { + const interval = calcAutoIntervalNear(0, undefined as any); + expect(interval.asMilliseconds()).toBe(0); + }); + test('100 buckets/1ms = 1ms buckets', () => { const interval = calcAutoIntervalNear(100, moment.duration(1, 'ms')); expect(interval.asMilliseconds()).toBe(1); @@ -69,6 +79,16 @@ describe('calcAutoIntervalNear', () => { }); describe('calcAutoIntervalLessThan', () => { + test('0 buckets/1h = 0ms buckets', () => { + const interval = calcAutoIntervalLessThan(0, moment.duration(1, 'h')); + expect(interval.asMilliseconds()).toBe(0); + }); + + test('100 buckets/undefined = 0ms buckets', () => { + const interval = calcAutoIntervalLessThan(0, undefined as any); + expect(interval.asMilliseconds()).toBe(0); + }); + test('100 buckets/1ms = 1ms buckets', () => { const interval = calcAutoIntervalLessThan(100, moment.duration(1, 'ms')); expect(interval.asMilliseconds()).toBe(1); diff --git a/src/ui/public/time_buckets/calc_auto_interval.ts b/src/ui/public/time_buckets/calc_auto_interval.ts index a36948313f841..61f6b1e1d06b4 100644 --- a/src/ui/public/time_buckets/calc_auto_interval.ts +++ b/src/ui/public/time_buckets/calc_auto_interval.ts @@ -86,8 +86,13 @@ const rules = [ }, ]; +function estimateBucketMs(count: number, duration: Duration) { + const ms = Number(duration) / count; + return isFinite(ms) ? ms : NaN; +} + function defaultInterval(targetMs: number) { - return moment.duration(Math.max(Math.floor(targetMs), 1), 'ms'); + return moment.duration(isNaN(targetMs) ? 0 : Math.max(Math.floor(targetMs), 1), 'ms'); } /** @@ -98,7 +103,7 @@ function defaultInterval(targetMs: number) { * @param duration time range the agg covers */ export function calcAutoIntervalNear(targetBucketCount: number, duration: Duration) { - const targetMs = Number(duration) / targetBucketCount; + const targetMs = estimateBucketMs(targetBucketCount, duration); for (let i = 0; i < rules.length - 1; i++) { if (Number(rules[i + 1].bound) <= targetMs) { @@ -117,7 +122,7 @@ export function calcAutoIntervalNear(targetBucketCount: number, duration: Durati * @param duration amount of time covered by the agg */ export function calcAutoIntervalLessThan(maxBucketCount: number, duration: Duration) { - const maxMs = Number(duration) / maxBucketCount; + const maxMs = estimateBucketMs(maxBucketCount, duration); for (const { interval } of rules) { if (Number(interval) <= maxMs) { From e38a0c53b9d900e59edca8b257cc801d8116af9f Mon Sep 17 00:00:00 2001 From: spalger Date: Fri, 26 Oct 2018 19:09:49 -0700 Subject: [PATCH 4/4] [calcAutoInterval] incorporate review feedback --- .../time_buckets/calc_auto_interval.test.ts | 84 ++++++------- .../public/time_buckets/calc_auto_interval.ts | 111 ++++++++++-------- src/ui/public/time_buckets/time_buckets.js | 4 +- 3 files changed, 105 insertions(+), 94 deletions(-) diff --git a/src/ui/public/time_buckets/calc_auto_interval.test.ts b/src/ui/public/time_buckets/calc_auto_interval.test.ts index e9723c6f483ed..7c95da6a74dd3 100644 --- a/src/ui/public/time_buckets/calc_auto_interval.test.ts +++ b/src/ui/public/time_buckets/calc_auto_interval.test.ts @@ -22,115 +22,115 @@ import moment from 'moment'; import { calcAutoIntervalLessThan, calcAutoIntervalNear } from './calc_auto_interval'; describe('calcAutoIntervalNear', () => { - test('0 buckets/1h = 0ms buckets', () => { - const interval = calcAutoIntervalNear(0, moment.duration(1, 'h')); + test('1h/0 buckets = 0ms buckets', () => { + const interval = calcAutoIntervalNear(0, Number(moment.duration(1, 'h'))); expect(interval.asMilliseconds()).toBe(0); }); - test('100 buckets/undefined = 0ms buckets', () => { + test('undefined/100 buckets = 0ms buckets', () => { const interval = calcAutoIntervalNear(0, undefined as any); expect(interval.asMilliseconds()).toBe(0); }); - test('100 buckets/1ms = 1ms buckets', () => { - const interval = calcAutoIntervalNear(100, moment.duration(1, 'ms')); + test('1ms/100 buckets = 1ms buckets', () => { + const interval = calcAutoIntervalNear(100, Number(moment.duration(1, 'ms'))); expect(interval.asMilliseconds()).toBe(1); }); - test('100 buckets/200ms = 2ms buckets', () => { - const interval = calcAutoIntervalNear(100, moment.duration(200, 'ms')); + test('200ms/100 buckets = 2ms buckets', () => { + const interval = calcAutoIntervalNear(100, Number(moment.duration(200, 'ms'))); expect(interval.asMilliseconds()).toBe(2); }); - test('1000 buckets/1s = 1ms buckets', () => { - const interval = calcAutoIntervalNear(1000, moment.duration(1, 's')); + test('1s/1000 buckets = 1ms buckets', () => { + const interval = calcAutoIntervalNear(1000, Number(moment.duration(1, 's'))); expect(interval.asMilliseconds()).toBe(1); }); - test('1000 buckets/1000h = 1h buckets', () => { - const interval = calcAutoIntervalNear(1000, moment.duration(1000, 'hours')); + test('1000h/1000 buckets = 1h buckets', () => { + const interval = calcAutoIntervalNear(1000, Number(moment.duration(1000, 'hours'))); expect(interval.asHours()).toBe(1); }); - test('100 buckets/1h = 30s buckets', () => { - const interval = calcAutoIntervalNear(100, moment.duration(1, 'hours')); + test('1h/100 buckets = 30s buckets', () => { + const interval = calcAutoIntervalNear(100, Number(moment.duration(1, 'hours'))); expect(interval.asSeconds()).toBe(30); }); - test('25 buckets/1d = 1h buckets', () => { - const interval = calcAutoIntervalNear(25, moment.duration(1, 'day')); + test('1d/25 buckets = 1h buckets', () => { + const interval = calcAutoIntervalNear(25, Number(moment.duration(1, 'day'))); expect(interval.asHours()).toBe(1); }); - test('1000 buckets/1y = 12h buckets', () => { - const interval = calcAutoIntervalNear(1000, moment.duration(1, 'year')); + test('1y/1000 buckets = 12h buckets', () => { + const interval = calcAutoIntervalNear(1000, Number(moment.duration(1, 'year'))); expect(interval.asHours()).toBe(12); }); - test('10000 buckets/1y = 1h buckets', () => { - const interval = calcAutoIntervalNear(10000, moment.duration(1, 'year')); + test('1y/10000 buckets = 1h buckets', () => { + const interval = calcAutoIntervalNear(10000, Number(moment.duration(1, 'year'))); expect(interval.asHours()).toBe(1); }); - test('100000 buckets/1y = 5m buckets', () => { - const interval = calcAutoIntervalNear(100000, moment.duration(1, 'year')); + test('1y/100000 buckets = 5m buckets', () => { + const interval = calcAutoIntervalNear(100000, Number(moment.duration(1, 'year'))); expect(interval.asMinutes()).toBe(5); }); }); describe('calcAutoIntervalLessThan', () => { - test('0 buckets/1h = 0ms buckets', () => { - const interval = calcAutoIntervalLessThan(0, moment.duration(1, 'h')); + test('1h/0 buckets = 0ms buckets', () => { + const interval = calcAutoIntervalLessThan(0, Number(moment.duration(1, 'h'))); expect(interval.asMilliseconds()).toBe(0); }); - test('100 buckets/undefined = 0ms buckets', () => { + test('undefined/100 buckets = 0ms buckets', () => { const interval = calcAutoIntervalLessThan(0, undefined as any); expect(interval.asMilliseconds()).toBe(0); }); - test('100 buckets/1ms = 1ms buckets', () => { - const interval = calcAutoIntervalLessThan(100, moment.duration(1, 'ms')); + test('1ms/100 buckets = 1ms buckets', () => { + const interval = calcAutoIntervalLessThan(100, Number(moment.duration(1, 'ms'))); expect(interval.asMilliseconds()).toBe(1); }); - test('100 buckets/200ms = 2ms buckets', () => { - const interval = calcAutoIntervalLessThan(100, moment.duration(200, 'ms')); + test('200ms/100 buckets = 2ms buckets', () => { + const interval = calcAutoIntervalLessThan(100, Number(moment.duration(200, 'ms'))); expect(interval.asMilliseconds()).toBe(2); }); - test('1000 buckets/1s = 1ms buckets', () => { - const interval = calcAutoIntervalLessThan(1000, moment.duration(1, 's')); + test('1s/1000 buckets = 1ms buckets', () => { + const interval = calcAutoIntervalLessThan(1000, Number(moment.duration(1, 's'))); expect(interval.asMilliseconds()).toBe(1); }); - test('1000 buckets/1000h = 1h buckets', () => { - const interval = calcAutoIntervalLessThan(1000, moment.duration(1000, 'hours')); + test('1000h/1000 buckets = 1h buckets', () => { + const interval = calcAutoIntervalLessThan(1000, Number(moment.duration(1000, 'hours'))); expect(interval.asHours()).toBe(1); }); - test('100 buckets/1h = 30s buckets', () => { - const interval = calcAutoIntervalLessThan(100, moment.duration(1, 'hours')); + test('1h/100 buckets = 30s buckets', () => { + const interval = calcAutoIntervalLessThan(100, Number(moment.duration(1, 'hours'))); expect(interval.asSeconds()).toBe(30); }); - test('25 buckets/1d = 30m buckets', () => { - const interval = calcAutoIntervalLessThan(25, moment.duration(1, 'day')); + test('1d/25 buckets = 30m buckets', () => { + const interval = calcAutoIntervalLessThan(25, Number(moment.duration(1, 'day'))); expect(interval.asMinutes()).toBe(30); }); - test('1000 buckets/1y = 3h buckets', () => { - const interval = calcAutoIntervalLessThan(1000, moment.duration(1, 'year')); + test('1y/1000 buckets = 3h buckets', () => { + const interval = calcAutoIntervalLessThan(1000, Number(moment.duration(1, 'year'))); expect(interval.asHours()).toBe(3); }); - test('10000 buckets/1y = 30m buckets', () => { - const interval = calcAutoIntervalLessThan(10000, moment.duration(1, 'year')); + test('1y/10000 buckets = 30m buckets', () => { + const interval = calcAutoIntervalLessThan(10000, Number(moment.duration(1, 'year'))); expect(interval.asMinutes()).toBe(30); }); - test('100000 buckets/1y = 5m buckets', () => { - const interval = calcAutoIntervalLessThan(100000, moment.duration(1, 'year')); + test('1y/100000 buckets = 5m buckets', () => { + const interval = calcAutoIntervalLessThan(100000, Number(moment.duration(1, 'year'))); expect(interval.asMinutes()).toBe(5); }); }); diff --git a/src/ui/public/time_buckets/calc_auto_interval.ts b/src/ui/public/time_buckets/calc_auto_interval.ts index 61f6b1e1d06b4..c3478772669c4 100644 --- a/src/ui/public/time_buckets/calc_auto_interval.ts +++ b/src/ui/public/time_buckets/calc_auto_interval.ts @@ -17,82 +17,83 @@ * under the License. */ -import moment, { Duration } from 'moment'; +import moment from 'moment'; -const rules = [ +const boundsDescending = [ { bound: Infinity, - interval: moment.duration(1, 'year'), + interval: Number(moment.duration(1, 'year')), }, { - bound: moment.duration(1, 'year'), - interval: moment.duration(1, 'month'), + bound: Number(moment.duration(1, 'year')), + interval: Number(moment.duration(1, 'month')), }, { - bound: moment.duration(3, 'week'), - interval: moment.duration(1, 'week'), + bound: Number(moment.duration(3, 'week')), + interval: Number(moment.duration(1, 'week')), }, { - bound: moment.duration(1, 'week'), - interval: moment.duration(1, 'd'), + bound: Number(moment.duration(1, 'week')), + interval: Number(moment.duration(1, 'd')), }, { - bound: moment.duration(24, 'hour'), - interval: moment.duration(12, 'hour'), + bound: Number(moment.duration(24, 'hour')), + interval: Number(moment.duration(12, 'hour')), }, { - bound: moment.duration(6, 'hour'), - interval: moment.duration(3, 'hour'), + bound: Number(moment.duration(6, 'hour')), + interval: Number(moment.duration(3, 'hour')), }, { - bound: moment.duration(2, 'hour'), - interval: moment.duration(1, 'hour'), + bound: Number(moment.duration(2, 'hour')), + interval: Number(moment.duration(1, 'hour')), }, { - bound: moment.duration(45, 'minute'), - interval: moment.duration(30, 'minute'), + bound: Number(moment.duration(45, 'minute')), + interval: Number(moment.duration(30, 'minute')), }, { - bound: moment.duration(20, 'minute'), - interval: moment.duration(10, 'minute'), + bound: Number(moment.duration(20, 'minute')), + interval: Number(moment.duration(10, 'minute')), }, { - bound: moment.duration(9, 'minute'), - interval: moment.duration(5, 'minute'), + bound: Number(moment.duration(9, 'minute')), + interval: Number(moment.duration(5, 'minute')), }, { - bound: moment.duration(3, 'minute'), - interval: moment.duration(1, 'minute'), + bound: Number(moment.duration(3, 'minute')), + interval: Number(moment.duration(1, 'minute')), }, { - bound: moment.duration(45, 'second'), - interval: moment.duration(30, 'second'), + bound: Number(moment.duration(45, 'second')), + interval: Number(moment.duration(30, 'second')), }, { - bound: moment.duration(15, 'second'), - interval: moment.duration(10, 'second'), + bound: Number(moment.duration(15, 'second')), + interval: Number(moment.duration(10, 'second')), }, { - bound: moment.duration(7.5, 'second'), - interval: moment.duration(5, 'second'), + bound: Number(moment.duration(7.5, 'second')), + interval: Number(moment.duration(5, 'second')), }, { - bound: moment.duration(5, 'second'), - interval: moment.duration(1, 'second'), + bound: Number(moment.duration(5, 'second')), + interval: Number(moment.duration(1, 'second')), }, { - bound: moment.duration(500, 'ms'), - interval: moment.duration(100, 'ms'), + bound: Number(moment.duration(500, 'ms')), + interval: Number(moment.duration(100, 'ms')), }, ]; -function estimateBucketMs(count: number, duration: Duration) { - const ms = Number(duration) / count; +function getPerBucketMs(count: number, duration: number) { + const ms = duration / count; return isFinite(ms) ? ms : NaN; } -function defaultInterval(targetMs: number) { - return moment.duration(isNaN(targetMs) ? 0 : Math.max(Math.floor(targetMs), 1), 'ms'); +function normalizeMinimumInterval(targetMs: number) { + const value = isNaN(targetMs) ? 0 : Math.max(Math.floor(targetMs), 1); + return moment.duration(value); } /** @@ -102,16 +103,24 @@ function defaultInterval(targetMs: number) { * @param targetBucketCount desired number of buckets * @param duration time range the agg covers */ -export function calcAutoIntervalNear(targetBucketCount: number, duration: Duration) { - const targetMs = estimateBucketMs(targetBucketCount, duration); +export function calcAutoIntervalNear(targetBucketCount: number, duration: number) { + const targetPerBucketMs = getPerBucketMs(targetBucketCount, duration); - for (let i = 0; i < rules.length - 1; i++) { - if (Number(rules[i + 1].bound) <= targetMs) { - return rules[i].interval.clone(); - } + // Find the first bound which is smaller than our target. + const lowerBoundIndex = boundsDescending.findIndex(({ bound }) => { + const boundMs = Number(bound); + return boundMs <= targetPerBucketMs; + }); + + // The bound immediately preceeding that lower bound contains the + // interval most closely matching our target. + if (lowerBoundIndex !== -1) { + const nearestInterval = boundsDescending[lowerBoundIndex - 1].interval; + return moment.duration(nearestInterval); } - return defaultInterval(targetMs); + // If the target is smaller than any of our bounds, then we'll use it for the interval as-is. + return normalizeMinimumInterval(targetPerBucketMs); } /** @@ -121,14 +130,16 @@ export function calcAutoIntervalNear(targetBucketCount: number, duration: Durati * @param maxBucketCount maximum number of buckets to create * @param duration amount of time covered by the agg */ -export function calcAutoIntervalLessThan(maxBucketCount: number, duration: Duration) { - const maxMs = estimateBucketMs(maxBucketCount, duration); +export function calcAutoIntervalLessThan(maxBucketCount: number, duration: number) { + const maxPerBucketMs = getPerBucketMs(maxBucketCount, duration); - for (const { interval } of rules) { - if (Number(interval) <= maxMs) { - return interval.clone(); + for (const { interval } of boundsDescending) { + // Find the highest interval which meets our per bucket limitation. + if (interval <= maxPerBucketMs) { + return moment.duration(interval); } } - return defaultInterval(maxMs); + // If the max is smaller than any of our intervals, then we'll use it for the interval as-is. + return normalizeMinimumInterval(maxPerBucketMs); } diff --git a/src/ui/public/time_buckets/time_buckets.js b/src/ui/public/time_buckets/time_buckets.js index 4c0e3220e6783..0da2fc868c977 100644 --- a/src/ui/public/time_buckets/time_buckets.js +++ b/src/ui/public/time_buckets/time_buckets.js @@ -231,7 +231,7 @@ TimeBuckets.prototype.getInterval = function (useNormalizedEsInterval = true) { function readInterval() { const interval = self._i; if (moment.isDuration(interval)) return interval; - return calcAutoIntervalNear(config.get('histogram:barTarget'), duration); + return calcAutoIntervalNear(config.get('histogram:barTarget'), Number(duration)); } // check to see if the interval should be scaled, and scale it if so @@ -243,7 +243,7 @@ TimeBuckets.prototype.getInterval = function (useNormalizedEsInterval = true) { let scaled; if (approxLen > maxLength) { - scaled = calcAutoIntervalLessThan(maxLength, duration); + scaled = calcAutoIntervalLessThan(maxLength, Number(duration)); } else { return interval; }