Skip to content

Commit e56a7a2

Browse files
[Discover] fix histogram min interval (#53979) (#54324)
- Fixes issues involving min intervals for leap years and DST Co-authored-by: Marco Vettorello <[email protected]>
1 parent 4fbf9c7 commit e56a7a2

File tree

8 files changed

+180
-10
lines changed

8 files changed

+180
-10
lines changed

src/legacy/core_plugins/kibana/public/discover/np_ready/angular/directives/histogram.tsx

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
import { EuiFlexGroup, EuiFlexItem, EuiIcon, EuiSpacer } from '@elastic/eui';
2121
import moment from 'moment-timezone';
22+
import { unitOfTime } from 'moment';
2223
import React, { Component } from 'react';
2324
import PropTypes from 'prop-types';
2425
import lightEuiTheme from '@elastic/eui/dist/eui_theme_light.json';
@@ -53,6 +54,58 @@ interface DiscoverHistogramState {
5354
chartsTheme: EuiChartThemeType['theme'];
5455
}
5556

57+
function findIntervalFromDuration(
58+
dateValue: number,
59+
esValue: number,
60+
esUnit: unitOfTime.Base,
61+
timeZone: string
62+
) {
63+
const date = moment.tz(dateValue, timeZone);
64+
const startOfDate = moment.tz(date, timeZone).startOf(esUnit);
65+
const endOfDate = moment
66+
.tz(date, timeZone)
67+
.startOf(esUnit)
68+
.add(esValue, esUnit);
69+
return endOfDate.valueOf() - startOfDate.valueOf();
70+
}
71+
72+
function getIntervalInMs(
73+
value: number,
74+
esValue: number,
75+
esUnit: unitOfTime.Base,
76+
timeZone: string
77+
): number {
78+
switch (esUnit) {
79+
case 's':
80+
return 1000 * esValue;
81+
case 'ms':
82+
return 1 * esValue;
83+
default:
84+
return findIntervalFromDuration(value, esValue, esUnit, timeZone);
85+
}
86+
}
87+
88+
export function findMinInterval(
89+
xValues: number[],
90+
esValue: number,
91+
esUnit: string,
92+
timeZone: string
93+
): number {
94+
return xValues.reduce((minInterval, currentXvalue, index) => {
95+
let currentDiff = minInterval;
96+
if (index > 0) {
97+
currentDiff = Math.abs(xValues[index - 1] - currentXvalue);
98+
}
99+
const singleUnitInterval = getIntervalInMs(
100+
currentXvalue,
101+
esValue,
102+
esUnit as unitOfTime.Base,
103+
timeZone
104+
);
105+
return Math.min(minInterval, singleUnitInterval, currentDiff);
106+
}, Number.MAX_SAFE_INTEGER);
107+
}
108+
56109
export class DiscoverHistogram extends Component<DiscoverHistogramProps, DiscoverHistogramState> {
57110
public static propTypes = {
58111
chartData: PropTypes.object,
@@ -154,7 +207,7 @@ export class DiscoverHistogram extends Component<DiscoverHistogramProps, Discove
154207
* see https://github.com/elastic/kibana/issues/27410
155208
* TODO: Once the Discover query has been update, we should change the below to use the new field
156209
*/
157-
const xInterval = chartData.ordered.interval;
210+
const { intervalESValue, intervalESUnit, interval: xInterval } = chartData.ordered;
158211

159212
const xValues = chartData.xAxisOrderedValues;
160213
const lastXValue = xValues[xValues.length - 1];
@@ -169,7 +222,7 @@ export class DiscoverHistogram extends Component<DiscoverHistogramProps, Discove
169222
const xDomain = {
170223
min: domainMin,
171224
max: domainMax,
172-
minInterval: xInterval,
225+
minInterval: findMinInterval(xValues, intervalESValue, intervalESUnit, timeZone),
173226
};
174227

175228
// Domain end of 'now' will be milliseconds behind current time, so we extend time by 1 minute and check if

src/legacy/core_plugins/visualizations/public/np_ready/public/legacy/build_pipeline.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -445,6 +445,8 @@ export const buildVislibDimensions = async (
445445
dimensions.x.params.date = true;
446446
const { esUnit, esValue } = xAgg.buckets.getInterval();
447447
dimensions.x.params.interval = moment.duration(esValue, esUnit);
448+
dimensions.x.params.intervalESValue = esValue;
449+
dimensions.x.params.intervalESUnit = esUnit;
448450
dimensions.x.params.format = xAgg.buckets.getScaledDateFormat();
449451
dimensions.x.params.bounds = xAgg.buckets.getBounds();
450452
} else if (xAgg.type.name === 'histogram') {

src/legacy/ui/public/agg_response/point_series/__tests__/_init_x_axis.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,8 @@ describe('initXAxis', function() {
104104

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

114116
expect(moment.isDuration(chart.ordered.interval)).to.be(true);
115117
expect(chart.ordered.interval.toISOString()).to.eql('P1D');
118+
expect(chart.ordered.intervalESValue).to.be(1);
119+
expect(chart.ordered.intervalESUnit).to.be('d');
116120
});
117121

118122
it('reads the numeric interval param from the x agg', function() {

src/legacy/ui/public/agg_response/point_series/_init_x_axis.js

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,19 @@ export function initXAxis(chart, table) {
2828
chart.xAxisFormat = format;
2929
chart.xAxisLabel = title;
3030

31-
if (params.interval) {
32-
chart.ordered = {
33-
interval: params.date ? moment.duration(params.interval) : params.interval,
34-
};
31+
const { interval, date } = params;
32+
if (interval) {
33+
if (date) {
34+
const { intervalESUnit, intervalESValue } = params;
35+
chart.ordered = {
36+
interval: moment.duration(interval),
37+
intervalESUnit: intervalESUnit,
38+
intervalESValue: intervalESValue,
39+
};
40+
} else {
41+
chart.ordered = {
42+
interval,
43+
};
44+
}
3545
}
3646
}
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
/*
2+
* Licensed to Elasticsearch B.V. under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch B.V. licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
import expect from '@kbn/expect';
21+
22+
export default function({ getService, getPageObjects }) {
23+
const log = getService('log');
24+
const esArchiver = getService('esArchiver');
25+
const browser = getService('browser');
26+
const kibanaServer = getService('kibanaServer');
27+
const PageObjects = getPageObjects(['settings', 'common', 'discover', 'header', 'timePicker']);
28+
const defaultSettings = {
29+
defaultIndex: 'long-window-logstash-*',
30+
'dateFormat:tz': 'Europe/Berlin',
31+
};
32+
33+
describe('discover histogram', function describeIndexTests() {
34+
before(async function() {
35+
log.debug('load kibana index with default index pattern');
36+
await PageObjects.common.navigateToApp('home');
37+
await esArchiver.loadIfNeeded('logstash_functional');
38+
await esArchiver.load('long_window_logstash');
39+
await esArchiver.load('visualize');
40+
await esArchiver.load('discover');
41+
42+
log.debug('create long_window_logstash index pattern');
43+
// NOTE: long_window_logstash load does NOT create index pattern
44+
await PageObjects.settings.createIndexPattern('long-window-logstash-');
45+
await kibanaServer.uiSettings.replace(defaultSettings);
46+
await browser.refresh();
47+
48+
log.debug('discover');
49+
await PageObjects.common.navigateToApp('discover');
50+
await PageObjects.discover.selectIndexPattern('long-window-logstash-*');
51+
// NOTE: For some reason without setting this relative time, the abs times will not fetch data.
52+
await PageObjects.timePicker.setCommonlyUsedTime('superDatePickerCommonlyUsed_Last_1 year');
53+
});
54+
after(async () => {
55+
await esArchiver.unload('long_window_logstash');
56+
await esArchiver.unload('visualize');
57+
await esArchiver.unload('discover');
58+
});
59+
60+
it('should visualize monthly data with different day intervals', async () => {
61+
//Nov 1, 2017 @ 01:00:00.000 - Mar 21, 2018 @ 02:00:00.000
62+
const fromTime = '2017-11-01 00:00:00.000';
63+
const toTime = '2018-03-21 00:00:00.000';
64+
await PageObjects.timePicker.setAbsoluteRange(fromTime, toTime);
65+
await PageObjects.discover.setChartInterval('Monthly');
66+
await PageObjects.header.waitUntilLoadingHasFinished();
67+
const chartCanvasExist = await PageObjects.discover.chartCanvasExist();
68+
expect(chartCanvasExist).to.be(true);
69+
});
70+
it('should visualize weekly data with within DST changes', async () => {
71+
//Nov 1, 2017 @ 01:00:00.000 - Mar 21, 2018 @ 02:00:00.000
72+
const fromTime = '2018-03-01 00:00:00.000';
73+
const toTime = '2018-05-01 00:00:00.000';
74+
await PageObjects.timePicker.setAbsoluteRange(fromTime, toTime);
75+
await PageObjects.discover.setChartInterval('Weekly');
76+
await PageObjects.header.waitUntilLoadingHasFinished();
77+
const chartCanvasExist = await PageObjects.discover.chartCanvasExist();
78+
expect(chartCanvasExist).to.be(true);
79+
});
80+
it('should visualize monthly data with different years Scaled to 30d', async () => {
81+
//Nov 1, 2017 @ 01:00:00.000 - Mar 21, 2018 @ 02:00:00.000
82+
const fromTime = '2010-01-01 00:00:00.000';
83+
const toTime = '2018-03-21 00:00:00.000';
84+
await PageObjects.timePicker.setAbsoluteRange(fromTime, toTime);
85+
await PageObjects.discover.setChartInterval('Daily');
86+
await PageObjects.header.waitUntilLoadingHasFinished();
87+
const chartCanvasExist = await PageObjects.discover.chartCanvasExist();
88+
expect(chartCanvasExist).to.be(true);
89+
});
90+
});
91+
}

test/functional/apps/discover/index.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ export default function({ getService, loadTestFile }) {
3434

3535
loadTestFile(require.resolve('./_saved_queries'));
3636
loadTestFile(require.resolve('./_discover'));
37+
loadTestFile(require.resolve('./_discover_histogram'));
3738
loadTestFile(require.resolve('./_filter_editor'));
3839
loadTestFile(require.resolve('./_errors'));
3940
loadTestFile(require.resolve('./_field_data'));

test/functional/apps/management/_handle_alias.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ export default function({ getService, getPageObjects }) {
7474
const toTime = 'Nov 19, 2016 @ 05:00:00.000';
7575

7676
await PageObjects.common.navigateToApp('discover');
77-
await PageObjects.discover.selectIndexPattern('alias2');
77+
await PageObjects.discover.selectIndexPattern('alias2*');
7878
await PageObjects.timePicker.setAbsoluteRange(fromTime, toTime);
7979

8080
await retry.try(async function() {

test/functional/page_objects/discover_page.js

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,8 +117,16 @@ export function DiscoverPageProvider({ getService, getPageObjects }) {
117117
await testSubjects.click('discoverOpenButton');
118118
}
119119

120+
async getChartCanvas() {
121+
return await find.byCssSelector('.echChart canvas:last-of-type');
122+
}
123+
124+
async chartCanvasExist() {
125+
return await find.existsByCssSelector('.echChart canvas:last-of-type');
126+
}
127+
120128
async clickHistogramBar() {
121-
const el = await find.byCssSelector('.echChart canvas:last-of-type');
129+
const el = await this.getChartCanvas();
122130

123131
await browser
124132
.getActions()
@@ -128,7 +136,8 @@ export function DiscoverPageProvider({ getService, getPageObjects }) {
128136
}
129137

130138
async brushHistogram() {
131-
const el = await find.byCssSelector('.echChart canvas:last-of-type');
139+
const el = await this.getChartCanvas();
140+
132141
await browser.dragAndDrop(
133142
{ location: el, offset: { x: 200, y: 20 } },
134143
{ location: el, offset: { x: 400, y: 30 } }
@@ -279,7 +288,7 @@ export function DiscoverPageProvider({ getService, getPageObjects }) {
279288
async selectIndexPattern(indexPattern) {
280289
await testSubjects.click('indexPattern-switch-link');
281290
await find.clickByCssSelector(
282-
`[data-test-subj="indexPattern-switcher"] [title="${indexPattern}*"]`
291+
`[data-test-subj="indexPattern-switcher"] [title="${indexPattern}"]`
283292
);
284293
await PageObjects.header.waitUntilLoadingHasFinished();
285294
}

0 commit comments

Comments
 (0)