Skip to content

Commit

Permalink
[calcAutoInterval] incorporate review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
spalger committed Oct 27, 2018
1 parent b4f8a79 commit e38a0c5
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 94 deletions.
84 changes: 42 additions & 42 deletions src/ui/public/time_buckets/calc_auto_interval.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
111 changes: 61 additions & 50 deletions src/ui/public/time_buckets/calc_auto_interval.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand All @@ -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);
}

/**
Expand All @@ -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);
}
4 changes: 2 additions & 2 deletions src/ui/public/time_buckets/time_buckets.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
}
Expand Down

0 comments on commit e38a0c5

Please sign in to comment.