Skip to content

Commit

Permalink
[Discover] fix histogram min interval (#53979)
Browse files Browse the repository at this point in the history
- Fixes issues involving min intervals for leap years and DST
  • Loading branch information
markov00 authored and nickofthyme committed Jan 9, 2020
1 parent 70870e2 commit 71e79b0
Show file tree
Hide file tree
Showing 8 changed files with 180 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import { EuiFlexGroup, EuiFlexItem, EuiIcon, EuiSpacer } from '@elastic/eui';
import moment from 'moment-timezone';
import { unitOfTime } from 'moment';
import React, { Component } from 'react';
import PropTypes from 'prop-types';
import lightEuiTheme from '@elastic/eui/dist/eui_theme_light.json';
Expand Down Expand Up @@ -53,6 +54,58 @@ interface DiscoverHistogramState {
chartsTheme: EuiChartThemeType['theme'];
}

function findIntervalFromDuration(
dateValue: number,
esValue: number,
esUnit: unitOfTime.Base,
timeZone: string
) {
const date = moment.tz(dateValue, timeZone);
const startOfDate = moment.tz(date, timeZone).startOf(esUnit);
const endOfDate = moment
.tz(date, timeZone)
.startOf(esUnit)
.add(esValue, esUnit);
return endOfDate.valueOf() - startOfDate.valueOf();
}

function getIntervalInMs(
value: number,
esValue: number,
esUnit: unitOfTime.Base,
timeZone: string
): number {
switch (esUnit) {
case 's':
return 1000 * esValue;
case 'ms':
return 1 * esValue;
default:
return findIntervalFromDuration(value, esValue, esUnit, timeZone);
}
}

export function findMinInterval(
xValues: number[],
esValue: number,
esUnit: string,
timeZone: string
): number {
return xValues.reduce((minInterval, currentXvalue, index) => {
let currentDiff = minInterval;
if (index > 0) {
currentDiff = Math.abs(xValues[index - 1] - currentXvalue);
}
const singleUnitInterval = getIntervalInMs(
currentXvalue,
esValue,
esUnit as unitOfTime.Base,
timeZone
);
return Math.min(minInterval, singleUnitInterval, currentDiff);
}, Number.MAX_SAFE_INTEGER);
}

export class DiscoverHistogram extends Component<DiscoverHistogramProps, DiscoverHistogramState> {
public static propTypes = {
chartData: PropTypes.object,
Expand Down Expand Up @@ -154,7 +207,7 @@ export class DiscoverHistogram extends Component<DiscoverHistogramProps, Discove
* see https://github.com/elastic/kibana/issues/27410
* TODO: Once the Discover query has been update, we should change the below to use the new field
*/
const xInterval = chartData.ordered.interval;
const { intervalESValue, intervalESUnit, interval: xInterval } = chartData.ordered;

const xValues = chartData.xAxisOrderedValues;
const lastXValue = xValues[xValues.length - 1];
Expand All @@ -169,7 +222,7 @@ export class DiscoverHistogram extends Component<DiscoverHistogramProps, Discove
const xDomain = {
min: domainMin,
max: domainMax,
minInterval: xInterval,
minInterval: findMinInterval(xValues, intervalESValue, intervalESUnit, timeZone),
};

// Domain end of 'now' will be milliseconds behind current time, so we extend time by 1 minute and check if
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,8 @@ export const buildVislibDimensions = async (
dimensions.x.params.date = true;
const { esUnit, esValue } = xAgg.buckets.getInterval();
dimensions.x.params.interval = moment.duration(esValue, esUnit);
dimensions.x.params.intervalESValue = esValue;
dimensions.x.params.intervalESUnit = esUnit;
dimensions.x.params.format = xAgg.buckets.getScaledDateFormat();
dimensions.x.params.bounds = xAgg.buckets.getBounds();
} else if (xAgg.type.name === 'histogram') {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ describe('initXAxis', function() {

it('reads the date interval param from the x agg', function() {
chart.aspects.x[0].params.interval = 'P1D';
chart.aspects.x[0].params.intervalESValue = 1;
chart.aspects.x[0].params.intervalESUnit = 'd';
chart.aspects.x[0].params.date = true;
initXAxis(chart, table);
expect(chart)
Expand All @@ -113,6 +115,8 @@ describe('initXAxis', function() {

expect(moment.isDuration(chart.ordered.interval)).to.be(true);
expect(chart.ordered.interval.toISOString()).to.eql('P1D');
expect(chart.ordered.intervalESValue).to.be(1);
expect(chart.ordered.intervalESUnit).to.be('d');
});

it('reads the numeric interval param from the x agg', function() {
Expand Down
18 changes: 14 additions & 4 deletions src/legacy/ui/public/agg_response/point_series/_init_x_axis.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,19 @@ export function initXAxis(chart, table) {
chart.xAxisFormat = format;
chart.xAxisLabel = title;

if (params.interval) {
chart.ordered = {
interval: params.date ? moment.duration(params.interval) : params.interval,
};
const { interval, date } = params;
if (interval) {
if (date) {
const { intervalESUnit, intervalESValue } = params;
chart.ordered = {
interval: moment.duration(interval),
intervalESUnit: intervalESUnit,
intervalESValue: intervalESValue,
};
} else {
chart.ordered = {
interval,
};
}
}
}
91 changes: 91 additions & 0 deletions test/functional/apps/discover/_discover_histogram.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
/*
* 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 expect from '@kbn/expect';

export default function({ getService, getPageObjects }) {
const log = getService('log');
const esArchiver = getService('esArchiver');
const browser = getService('browser');
const kibanaServer = getService('kibanaServer');
const PageObjects = getPageObjects(['settings', 'common', 'discover', 'header', 'timePicker']);
const defaultSettings = {
defaultIndex: 'long-window-logstash-*',
'dateFormat:tz': 'Europe/Berlin',
};

describe('discover histogram', function describeIndexTests() {
before(async function() {
log.debug('load kibana index with default index pattern');
await PageObjects.common.navigateToApp('home');
await esArchiver.loadIfNeeded('logstash_functional');
await esArchiver.load('long_window_logstash');
await esArchiver.load('visualize');
await esArchiver.load('discover');

log.debug('create long_window_logstash index pattern');
// NOTE: long_window_logstash load does NOT create index pattern
await PageObjects.settings.createIndexPattern('long-window-logstash-');
await kibanaServer.uiSettings.replace(defaultSettings);
await browser.refresh();

log.debug('discover');
await PageObjects.common.navigateToApp('discover');
await PageObjects.discover.selectIndexPattern('long-window-logstash-*');
// NOTE: For some reason without setting this relative time, the abs times will not fetch data.
await PageObjects.timePicker.setCommonlyUsedTime('superDatePickerCommonlyUsed_Last_1 year');
});
after(async () => {
await esArchiver.unload('long_window_logstash');
await esArchiver.unload('visualize');
await esArchiver.unload('discover');
});

it('should visualize monthly data with different day intervals', async () => {
//Nov 1, 2017 @ 01:00:00.000 - Mar 21, 2018 @ 02:00:00.000
const fromTime = '2017-11-01 00:00:00.000';
const toTime = '2018-03-21 00:00:00.000';
await PageObjects.timePicker.setAbsoluteRange(fromTime, toTime);
await PageObjects.discover.setChartInterval('Monthly');
await PageObjects.header.waitUntilLoadingHasFinished();
const chartCanvasExist = await PageObjects.discover.chartCanvasExist();
expect(chartCanvasExist).to.be(true);
});
it('should visualize weekly data with within DST changes', async () => {
//Nov 1, 2017 @ 01:00:00.000 - Mar 21, 2018 @ 02:00:00.000
const fromTime = '2018-03-01 00:00:00.000';
const toTime = '2018-05-01 00:00:00.000';
await PageObjects.timePicker.setAbsoluteRange(fromTime, toTime);
await PageObjects.discover.setChartInterval('Weekly');
await PageObjects.header.waitUntilLoadingHasFinished();
const chartCanvasExist = await PageObjects.discover.chartCanvasExist();
expect(chartCanvasExist).to.be(true);
});
it('should visualize monthly data with different years Scaled to 30d', async () => {
//Nov 1, 2017 @ 01:00:00.000 - Mar 21, 2018 @ 02:00:00.000
const fromTime = '2010-01-01 00:00:00.000';
const toTime = '2018-03-21 00:00:00.000';
await PageObjects.timePicker.setAbsoluteRange(fromTime, toTime);
await PageObjects.discover.setChartInterval('Daily');
await PageObjects.header.waitUntilLoadingHasFinished();
const chartCanvasExist = await PageObjects.discover.chartCanvasExist();
expect(chartCanvasExist).to.be(true);
});
});
}
1 change: 1 addition & 0 deletions test/functional/apps/discover/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export default function({ getService, loadTestFile }) {

loadTestFile(require.resolve('./_saved_queries'));
loadTestFile(require.resolve('./_discover'));
loadTestFile(require.resolve('./_discover_histogram'));
loadTestFile(require.resolve('./_filter_editor'));
loadTestFile(require.resolve('./_errors'));
loadTestFile(require.resolve('./_field_data'));
Expand Down
2 changes: 1 addition & 1 deletion test/functional/apps/management/_handle_alias.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export default function({ getService, getPageObjects }) {
const toTime = 'Nov 19, 2016 @ 05:00:00.000';

await PageObjects.common.navigateToApp('discover');
await PageObjects.discover.selectIndexPattern('alias2');
await PageObjects.discover.selectIndexPattern('alias2*');
await PageObjects.timePicker.setAbsoluteRange(fromTime, toTime);

await retry.try(async function() {
Expand Down
15 changes: 12 additions & 3 deletions test/functional/page_objects/discover_page.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,16 @@ export function DiscoverPageProvider({ getService, getPageObjects }) {
await testSubjects.click('discoverOpenButton');
}

async getChartCanvas() {
return await find.byCssSelector('.echChart canvas:last-of-type');
}

async chartCanvasExist() {
return await find.existsByCssSelector('.echChart canvas:last-of-type');
}

async clickHistogramBar() {
const el = await find.byCssSelector('.echChart canvas:last-of-type');
const el = await this.getChartCanvas();

await browser
.getActions()
Expand All @@ -128,7 +136,8 @@ export function DiscoverPageProvider({ getService, getPageObjects }) {
}

async brushHistogram() {
const el = await find.byCssSelector('.echChart canvas:last-of-type');
const el = await this.getChartCanvas();

await browser.dragAndDrop(
{ location: el, offset: { x: 200, y: 20 } },
{ location: el, offset: { x: 400, y: 30 } }
Expand Down Expand Up @@ -279,7 +288,7 @@ export function DiscoverPageProvider({ getService, getPageObjects }) {
async selectIndexPattern(indexPattern) {
await testSubjects.click('indexPattern-switch-link');
await find.clickByCssSelector(
`[data-test-subj="indexPattern-switcher"] [title="${indexPattern}*"]`
`[data-test-subj="indexPattern-switcher"] [title="${indexPattern}"]`
);
await PageObjects.header.waitUntilLoadingHasFinished();
}
Expand Down

0 comments on commit 71e79b0

Please sign in to comment.