[TSVB] Fix Rollup Search with auto interval for Rollup Jobs in calendar intervals (#36081) (#37066)

* auto interval

* [TSVB] Rollup Search - 'auto' interval is not working for Rollup Jobs which were created in Calendar intervals

* [TSVB] Rollup Search - 'auto' interval is not working for Rollup Jobs which were created in Calendar intervals

* fix issue

* fix broken tests

* fix pr comments

* fix pr comments
This commit is contained in:
Alexey Antonov 2019-05-24 13:22:12 +03:00 committed by GitHub
parent c03d1bbbae
commit 382970b14f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 349 additions and 72 deletions

View file

@ -23,6 +23,8 @@ import { relativeOptions } from '../../../../../ui/public/timepicker/relative_op
import { GTE_INTERVAL_RE, INTERVAL_STRING_RE } from '../../../common/interval_regexp';
export const AUTO_INTERVAL = 'auto';
export const unitLookup = {
s: i18n.translate('tsvb.getInterval.secondsLabel', { defaultMessage: 'seconds' }),
m: i18n.translate('tsvb.getInterval.minutesLabel', { defaultMessage: 'minutes' }),
@ -53,7 +55,7 @@ export const isGteInterval = (interval) => GTE_INTERVAL_RE.test(interval);
export const isIntervalValid = (interval) => {
return isString(interval) &&
(interval === 'auto' || INTERVAL_STRING_RE.test(interval) || isGteInterval(interval));
(interval === AUTO_INTERVAL || INTERVAL_STRING_RE.test(interval) || isGteInterval(interval));
};
export const getInterval = (visData, model) => {

View file

@ -22,7 +22,7 @@ import { get, isEqual } from 'lodash';
import { keyCodes, EuiFlexGroup, EuiFlexItem, EuiButton, EuiText, EuiSwitch } from '@elastic/eui';
import { getVisualizeLoader } from 'ui/visualize/loader/visualize_loader';
import { FormattedMessage, injectI18n } from '@kbn/i18n/react';
import { getInterval, convertIntervalIntoUnit, isIntervalValid, isGteInterval } from './lib/get_interval';
import { getInterval, convertIntervalIntoUnit, isIntervalValid, isGteInterval, AUTO_INTERVAL } from './lib/get_interval';
import { PANEL_TYPES } from '../../common/panel_types';
const MIN_CHART_HEIGHT = 250;
@ -71,7 +71,7 @@ class VisEditorVisualization extends Component {
timeRange,
appState,
savedObj,
onDataChange
onDataChange,
} = this.props;
this._handler = loader.embedVisualizationWithSavedObject(this._visEl.current, savedObj, {
@ -112,10 +112,11 @@ class VisEditorVisualization extends Component {
};
});
}
}
};
hasShowPanelIntervalValue() {
const type = get(this.props, 'model.type', '');
const interval = get(this.props, 'model.interval', AUTO_INTERVAL);
return [
PANEL_TYPES.METRIC,
@ -123,23 +124,14 @@ class VisEditorVisualization extends Component {
PANEL_TYPES.GAUGE,
PANEL_TYPES.MARKDOWN,
PANEL_TYPES.TABLE,
].includes(type);
].includes(type) && (interval === AUTO_INTERVAL
|| isGteInterval(interval) || !isIntervalValid(interval));
}
getFormattedPanelInterval() {
const interval = get(this.props, 'model.interval') || 'auto';
const isValid = isIntervalValid(interval);
const shouldShowActualInterval = interval === 'auto' || isGteInterval(interval);
const interval = convertIntervalIntoUnit(this.state.panelInterval, false);
if (shouldShowActualInterval || !isValid) {
const autoInterval = convertIntervalIntoUnit(this.state.panelInterval, false);
if (autoInterval) {
return `${autoInterval.unitValue}${autoInterval.unitString}`;
}
} else {
return interval;
}
return interval ? `${interval.unitValue}${interval.unitString}` : null;
}
componentWillUnmount() {
@ -173,7 +165,7 @@ class VisEditorVisualization extends Component {
title,
description,
onToggleAutoApply,
onCommit
onCommit,
} = this.props;
const style = { height: this.state.height };

View file

@ -66,7 +66,7 @@ export default function MetricsVisProvider(Private) {
}],
time_field: '',
index_pattern: '',
interval: 'auto',
interval: '',
axis_position: 'left',
axis_formatter: 'number',
axis_scale: 'normal',

View file

@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
import { convertIntervalToUnit, parseInterval } from '../vis_data/helpers/unit_to_seconds';
import { convertIntervalToUnit, parseInterval, getSuitableUnit } from '../vis_data/helpers/unit_to_seconds';
const getTimezoneFromRequest = request => {
return request.payload.timerange.timezone;
@ -63,6 +63,10 @@ export class DefaultSearchCapabilities {
return parseInterval(interval);
}
getSuitableUnit(intervalInSeconds) {
return getSuitableUnit(intervalInSeconds);
}
convertIntervalToUnit(intervalString, unit) {
const parsedInterval = this.parseInterval(intervalString);

View file

@ -19,7 +19,7 @@
import calculateAuto from './calculate_auto';
import moment from 'moment';
import unitToSeconds from './unit_to_seconds';
import { getUnitValue } from './unit_to_seconds';
import {
INTERVAL_STRING_RE,
GTE_INTERVAL_RE,
@ -29,7 +29,7 @@ const calculateBucketData = (timeInterval, capabilities) => {
const intervalString = capabilities ? capabilities.getValidTimeInterval(timeInterval) : timeInterval;
const intervalStringMatch = intervalString.match(INTERVAL_STRING_RE);
let bucketSize = Number(intervalStringMatch[1]) * unitToSeconds(intervalStringMatch[2]);
let bucketSize = Number(intervalStringMatch[1]) * getUnitValue(intervalStringMatch[2]);
// don't go too small
if (bucketSize < 1) {

View file

@ -28,7 +28,6 @@ import getSplits from './get_splits';
import getTimerange from './get_timerange';
import mapBucket from './map_bucket';
import parseSettings from './parse_settings';
import unitToSeconds from './unit_to_seconds';
export default {
bucketTransform,
@ -42,5 +41,4 @@ export default {
getTimerange,
mapBucket,
parseSettings,
unitToSeconds,
};

View file

@ -17,6 +17,7 @@
* under the License.
*/
import { INTERVAL_STRING_RE } from '../../../../common/interval_regexp';
import { sortBy, isNumber } from 'lodash';
const units = {
ms: 0.001,
@ -29,6 +30,8 @@ const units = {
y: 86400 * 7 * 4 * 12, // Leap year?
};
const sortedUnits = sortBy(Object.keys(units), key => units[key]);
export const parseInterval = (intervalString) => {
let value;
let unit;
@ -36,21 +39,35 @@ export const parseInterval = (intervalString) => {
if (intervalString) {
const matches = intervalString.match(INTERVAL_STRING_RE);
value = Number(matches[1]);
unit = matches[2];
if (matches) {
value = Number(matches[1]);
unit = matches[2];
}
}
return { value, unit };
};
export const convertIntervalToUnit = (intervalString, unit) => {
export const convertIntervalToUnit = (intervalString, newUnit) => {
const parsedInterval = parseInterval(intervalString);
const value = Number((parsedInterval.value * units[parsedInterval.unit] / units[unit]).toFixed(2));
let value;
let unit;
if (parsedInterval.value && units[newUnit]) {
value = Number((parsedInterval.value * units[parsedInterval.unit] / units[newUnit]).toFixed(2));
unit = newUnit;
}
return { value, unit };
};
export default (unit) => {
return units[unit];
};
export const getSuitableUnit = intervalInSeconds => sortedUnits.find((key, index, array) => {
const nextUnit = array[index + 1];
const isValidInput = isNumber(intervalInSeconds) && intervalInSeconds > 0;
const isLastItem = index + 1 === array.length;
return isValidInput && (intervalInSeconds >= units[key] && intervalInSeconds < units[nextUnit] || isLastItem);
});
export const getUnitValue = unit => units[unit];

View file

@ -0,0 +1,148 @@
/*
* 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 { getUnitValue, parseInterval, convertIntervalToUnit, getSuitableUnit } from './unit_to_seconds';
describe('unit_to_seconds', () => {
describe('parseInterval()', () => {
test('should parse "1m" interval (positive)', () =>
expect(parseInterval('1m')).toEqual({
value: 1,
unit: 'm',
}));
test('should parse "134d" interval (positive)', () =>
expect(parseInterval('134d')).toEqual({
value: 134,
unit: 'd',
}));
test('should parse "30M" interval (positive)', () =>
expect(parseInterval('30M')).toEqual({
value: 30,
unit: 'M',
}));
test('should not parse "gm" interval (negative)', () =>
expect(parseInterval('gm')).toEqual({
value: undefined,
unit: undefined,
}));
test('should not parse "-1d" interval (negative)', () =>
expect(parseInterval('-1d')).toEqual({
value: undefined,
unit: undefined,
}));
test('should not parse "M" interval (negative)', () =>
expect(parseInterval('M')).toEqual({
value: undefined,
unit: undefined,
}));
});
describe('convertIntervalToUnit()', () => {
test('should convert "30m" interval to "h" unit (positive)', () =>
expect(convertIntervalToUnit('30m', 'h')).toEqual({
value: 0.5,
unit: 'h',
}));
test('should convert "1h" interval to "m" unit (positive)', () =>
expect(convertIntervalToUnit('1h', 'm')).toEqual({
value: 60,
unit: 'm',
}));
test('should convert "1h" interval to "ms" unit (positive)', () =>
expect(convertIntervalToUnit('1h', 'ms')).toEqual({
value: 3600000,
unit: 'ms',
}));
test('should not convert "30m" interval to "0" unit (positive)', () =>
expect(convertIntervalToUnit('30m', 'o')).toEqual({
value: undefined,
unit: undefined,
}));
test('should not convert "m" interval to "s" unit (positive)', () =>
expect(convertIntervalToUnit('m', 's')).toEqual({
value: undefined,
unit: undefined,
}));
});
describe('getSuitableUnit()', () => {
test('should return "d" unit for oneDayInSeconds (positive)', () => {
const oneDayInSeconds = getUnitValue('d') * 1;
expect(getSuitableUnit(oneDayInSeconds)).toBe('d');
});
test('should return "d" unit for twoDaysInSeconds (positive)', () => {
const twoDaysInSeconds = getUnitValue('d') * 2;
expect(getSuitableUnit(twoDaysInSeconds)).toBe('d');
});
test('should return "w" unit for threeWeeksInSeconds (positive)', () => {
const threeWeeksInSeconds = getUnitValue('w') * 3;
expect(getSuitableUnit(threeWeeksInSeconds)).toBe('w');
});
test('should return "y" unit for aroundOneYearInSeconds (positive)', () => {
const aroundOneYearInSeconds = getUnitValue('d') * 370;
expect(getSuitableUnit(aroundOneYearInSeconds)).toBe('y');
});
test('should return "y" unit for twoYearsInSeconds (positive)', () => {
const twoYearsInSeconds = getUnitValue('y') * 2;
expect(getSuitableUnit(twoYearsInSeconds)).toBe('y');
});
test('should return "undefined" unit for negativeNumber (negative)', () => {
const negativeNumber = -12;
expect(getSuitableUnit(negativeNumber)).toBeUndefined();
});
test('should return "undefined" unit for string value (negative)', () => {
const stringValue = 'string';
expect(getSuitableUnit(stringValue)).toBeUndefined();
});
test('should return "undefined" unit for number string value (negative)', () => {
const stringValue = '-12';
expect(getSuitableUnit(stringValue)).toBeUndefined();
});
test('should return "undefined" in case of no input value(negative)', () =>
expect(getSuitableUnit()).toBeUndefined());
});
});

View file

@ -0,0 +1,10 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import { unitsMap } from '@elastic/datemath';
export const leastCommonInterval = (num = 0, base = 0) => Math.max(Math.ceil(num / base) * base, base);
export const isCalendarInterval = ({ unit, value }) => value === 1 && ['calendar', 'mixed'].includes(unitsMap[unit].type);

View file

@ -0,0 +1,80 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import { isCalendarInterval, leastCommonInterval } from './interval_helper';
describe('interval_helper', () => {
describe('isCalendarInterval', () => {
describe('calendar intervals', () => {
test('should return true for "w" intervals and value === 1', () =>
expect(isCalendarInterval({ value: 1, unit: 'w' })).toBeTruthy());
test('should return true for "M" intervals and value === 1', () =>
expect(isCalendarInterval({ value: 1, unit: 'M' })).toBeTruthy());
test('should return true for "y" intervals and value === 1', () =>
expect(isCalendarInterval({ value: 1, unit: 'y' })).toBeTruthy());
test('should return false for "w" intervals and value !== 1', () =>
expect(isCalendarInterval({ value: 2, unit: 'w' })).toBeFalsy());
test('should return false for "M" intervals and value !== 1', () =>
expect(isCalendarInterval({ value: 3, unit: 'M' })).toBeFalsy());
test('should return false for "y" intervals and value !== 1', () =>
expect(isCalendarInterval({ value: 4, unit: 'y' })).toBeFalsy());
});
describe('fixed intervals', () => {
test('should return false for "ms" intervals and value !== 1', () =>
expect(isCalendarInterval({ value: 2, unit: 'ms' })).toBeFalsy());
test('should return false for "s" intervals and value !== 1', () =>
expect(isCalendarInterval({ value: 3, unit: 's' })).toBeFalsy());
test('should return false for "s" intervals and value === 1', () =>
expect(isCalendarInterval({ value: 1, unit: 's' })).toBeFalsy());
});
describe('mixed intervals', () => {
test('should return true for "m" intervals and value === 1', () =>
expect(isCalendarInterval({ value: 1, unit: 'm' })).toBeTruthy());
test('should return true for "h" intervals and value === 1', () =>
expect(isCalendarInterval({ value: 1, unit: 'h' })).toBeTruthy());
test('should return true for "d" intervals and value === 1', () =>
expect(isCalendarInterval({ value: 1, unit: 'd' })).toBeTruthy());
test('should return false for "m" intervals and value !== 1', () =>
expect(isCalendarInterval({ value: 2, unit: 'm' })).toBeFalsy());
test('should return false for "h" intervals and value !== 1', () =>
expect(isCalendarInterval({ value: 3, unit: 'h' })).toBeFalsy());
test('should return false for "d" intervals and value !== 1', () =>
expect(isCalendarInterval({ value: 4, unit: 'd' })).toBeFalsy());
});
});
});
describe('leastCommonInterval', () => {
test('should return 1 as a least common interval for 0,1', () =>
expect(leastCommonInterval(0, 1)).toBe(1));
test('should return 3 as a least common interval for 1,3', () =>
expect(leastCommonInterval(1, 3)).toBe(3));
test('should return 15 as a least common interval for 12,5', () =>
expect(leastCommonInterval(12, 5)).toBe(15));
test('should return 7 as a least common interval for 4,7', () =>
expect(leastCommonInterval(4, 7)).toBe(7));
test('should not return least common interval (negative tests)', () =>
expect(leastCommonInterval(0, 0)).toBeNaN());
});

View file

@ -4,10 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/
import { get, has } from 'lodash';
import { unitsMap } from '@elastic/datemath';
const leastCommonInterval = (num = 0, base = 0) => Math.max(Math.ceil(num / base) * base, base);
const isCalendarInterval = ({ unit, value }) => value === 1 && ['calendar', 'mixed'].includes(unitsMap[unit].type);
import { leastCommonInterval, isCalendarInterval } from './lib/interval_helper';
export const getRollupSearchCapabilities = (DefaultSearchCapabilities) =>
(class RollupSearchCapabilities extends DefaultSearchCapabilities {
@ -25,7 +22,12 @@ export const getRollupSearchCapabilities = (DefaultSearchCapabilities) =>
}
get defaultTimeInterval() {
return get(this.dateHistogram, 'interval', null);
return this.dateHistogram.fixed_interval || this.dateHistogram.calendar_interval ||
/*
Deprecation: [interval] on [date_histogram] is deprecated, use [fixed_interval] or [calendar_interval] in the future.
We can remove the following line only for versions > 8.x
*/
this.dateHistogram.interval || null;
}
get searchTimezone() {
@ -58,17 +60,31 @@ export const getRollupSearchCapabilities = (DefaultSearchCapabilities) =>
getValidTimeInterval(userIntervalString) {
const parsedRollupJobInterval = this.parseInterval(this.defaultTimeInterval);
const parsedUserInterval = this.parseInterval(userIntervalString);
const inRollupJobUnit = this.convertIntervalToUnit(userIntervalString, parsedRollupJobInterval.unit);
let { unit } = parsedRollupJobInterval;
let { value } = this.convertIntervalToUnit(userIntervalString, unit);
const getValidCalendarInterval = () => {
let unit = parsedRollupJobInterval.unit;
if (isCalendarInterval(parsedRollupJobInterval) && isCalendarInterval(parsedUserInterval)) {
unit = value > 1 ? parsedUserInterval.unit : parsedRollupJobInterval.unit;
value = 1;
} else {
value = leastCommonInterval(value, parsedRollupJobInterval.value);
}
if (inRollupJobUnit.value > parsedRollupJobInterval.value) {
const inSeconds = this.convertIntervalToUnit(userIntervalString, 's');
unit = this.getSuitableUnit(inSeconds.value);
}
return {
value: 1,
unit,
};
};
const getValidFixedInterval = () => ({
value: leastCommonInterval(inRollupJobUnit.value, parsedRollupJobInterval.value),
unit: parsedRollupJobInterval.unit,
});
const { value, unit } = (
isCalendarInterval(parsedRollupJobInterval) ? getValidCalendarInterval : getValidFixedInterval
)();
return `${value}${unit}`;
}

View file

@ -56,60 +56,70 @@ describe('Rollup Search Capabilities', () => {
});
describe('getValidTimeInterval', () => {
let parsedDefaultInterval;
let parsedUserIntervalString;
let convertedIntervalIntoDefaultUnit;
let rollupJobInterval;
let userInterval;
let getSuitableUnit;
beforeEach(() => {
convertedIntervalIntoDefaultUnit = null;
rollupSearchCaps.parseInterval = jest.fn()
.mockImplementationOnce(() => parsedDefaultInterval)
.mockImplementationOnce(() => parsedUserIntervalString);
rollupSearchCaps.convertIntervalToUnit = jest
.fn(() => convertedIntervalIntoDefaultUnit || parsedUserIntervalString);
.mockImplementationOnce(() => rollupJobInterval)
.mockImplementationOnce(() => userInterval);
rollupSearchCaps.convertIntervalToUnit = jest.fn(() => userInterval);
rollupSearchCaps.getSuitableUnit = jest.fn(() => getSuitableUnit);
});
test('should return 1w as common interval for 1w(user interval) and 1d(rollup interval) - calendar intervals', () => {
parsedDefaultInterval = {
test('should return 1w as common interval for 5d(user interval) and 1d(rollup interval) - calendar intervals', () => {
rollupJobInterval = {
value: 1,
unit: 'd',
};
parsedUserIntervalString = {
value: 1,
unit: 'w',
userInterval = {
value: 5,
unit: 'd',
};
convertedIntervalIntoDefaultUnit = {
getSuitableUnit = 'd';
expect(rollupSearchCaps.getValidTimeInterval()).toBe('1d');
});
test('should return 1w as common interval for 7d(user interval) and 1d(rollup interval) - calendar intervals', () => {
rollupJobInterval = {
value: 1,
unit: 'd',
};
userInterval = {
value: 7,
unit: 'd',
};
getSuitableUnit = 'w';
expect(rollupSearchCaps.getValidTimeInterval()).toBe('1w');
});
test('should return 1w as common interval for 1d(user interval) and 1w(rollup interval) - calendar intervals', () => {
parsedDefaultInterval = {
rollupJobInterval = {
value: 1,
unit: 'w',
};
parsedUserIntervalString = {
userInterval = {
value: 1,
unit: 'd',
};
convertedIntervalIntoDefaultUnit = {
value: 1 / 7,
unit: 'w',
};
getSuitableUnit = 'w';
expect(rollupSearchCaps.getValidTimeInterval()).toBe('1w');
});
test('should return 2y as common interval for 0.1y(user interval) and 2y(rollup interval) - fixed intervals', () => {
parsedDefaultInterval = {
rollupJobInterval = {
value: 2,
unit: 'y',
};
parsedUserIntervalString = {
userInterval = {
value: 0.1,
unit: 'y',
};
@ -118,11 +128,11 @@ describe('Rollup Search Capabilities', () => {
});
test('should return 3h as common interval for 2h(user interval) and 3h(rollup interval) - fixed intervals', () => {
parsedDefaultInterval = {
rollupJobInterval = {
value: 3,
unit: 'h',
};
parsedUserIntervalString = {
userInterval = {
value: 2,
unit: 'h',
};
@ -131,11 +141,11 @@ describe('Rollup Search Capabilities', () => {
});
test('should return 6m as common interval for 4m(user interval) and 3m(rollup interval) - fixed intervals', () => {
parsedDefaultInterval = {
rollupJobInterval = {
value: 3,
unit: 'm',
};
parsedUserIntervalString = {
userInterval = {
value: 4,
unit: 'm',
};