[Discover] fix histogram min interval#53979
Merged
nickofthyme merged 11 commits intoelastic:masterfrom Jan 9, 2020
Merged
Conversation
Contributor
Author
|
@elasticmachine merge upstream |
Contributor
|
Pinging @elastic/kibana-app (Team:KibanaApp) |
Contributor
Author
|
@elasticmachine merge upstream |
Contributor
|
@elasticmachine merge upstream |
Contributor
|
@elasticmachine merge upstream |
nickofthyme
approved these changes
Jan 7, 2020
Contributor
nickofthyme
left a comment
There was a problem hiding this comment.
LGTM, All scenarios are working as expected.
…s below. - For some reason discover will error on getFieldsForWildcard without logstash_functional - Loading long_window_logstash doesn't create an index pattern - No other tests using long_window_logstash manually creates an index pattern - No results found for long_window_logstash using abs time, until a relative time is selected in discover
Contributor
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
nickofthyme
pushed a commit
to nickofthyme/kibana
that referenced
this pull request
Jan 9, 2020
- Fixes issues involving min intervals for leap years and DST
gmmorris
added a commit
to gmmorris/kibana
that referenced
this pull request
Jan 9, 2020
* master: (23 commits) [Vis: Default editor] Reactify the timelion editor (elastic#52990) [Discover] fix histogram min interval (elastic#53979) [Telemetry] [Monitoring] Only retry fetching usage once monito… (elastic#54309) [docs][APM] Add runtime index config documentation (elastic#53907) [SIEM] Detection engine timeline (elastic#53783) Filter scripted fields preview field list to source fields (elastic#53826) Management - New platform api (elastic#52579) Reset region and Account when switching inventory (elastic#54287) [SIEM] [Case] Case workflow api schema (elastic#51535) Code coverage setup on CI (elastic#49003) [ML] DF Analytics Results: adds link to docs (elastic#54189) Update schemas boolean, byteSize, and duration to coerce strings (elastic#54177) [Metrics UI] Pass relevant shouldAllowEdit capabilities into SettingsPage (elastic#49781) [Canvas] Fixes bugs with autoplay and refresh (elastic#53149) [ML] DF Analytics Classification: ensure confusion matrix can be fetched (elastic#53629) Fix Vega react eslint errors (elastic#54259) Remove non existing codeowners (elastic#54274) use correct type (elastic#54244) [Dashboard] Removing 100% as dshDashboardViewport height (elastic#54263) add `examples/` to no-restricted-path config (elastic#54252) ...
nickofthyme
added a commit
that referenced
this pull request
Jan 9, 2020
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.






Summary
This commit fix the missing bars on the histogram of Discover.
Discover is passing a minInterval parameter in milliseconds that is not
the the minimum interval in milliseconds between the buckets. Calendar intervals
are not fixed and this value change depending on time range and the time zone.
To avoid patching the elastic-chart and upgrading the dependency we applied a temporary patch
to the histogram code. For the feature version this patch will be removed
and a correct minInterval computation will be done directly in elastic-charts.
fix #52152
to test:
add some data that spans multiple years (possibly leap years, last one was 2016)
create the index pattern for that index:
logstash-*orlong_window_logstash*check the behaviour on histogram when the timerange spans several years (including leap years) with different interval configuration. The issue is visible when:
visualizing a timerange with monthly intervals that contains data in the February bucket
timezone:
alltime range:
Nov 1, 2017 @ 01:00:00.000 - Mar 21, 2018 @ 02:00:00.000intervals:
Monthlyvisualizing a timerange with a DST time change within the range. The DST change needs to be a positive change like when the time is moved forward (= that day last less than 24 hours) like in the following situation:
timezone:
Europe/Berlin(doesn't happen with UTC timezone)time range:
Mar 1, 2018 @ 00:00:00.000 - May 1, 2018 @ 00:00:00.000intervals:
all except Monthly and Yearlyvisualizing a timerange with a DST time change with
autointerval on a fixed/multi days interval:timezone:
Europe/Berlintime range:
Last 15 years - nowintervals: every interval smaller than month (be sure that the scaled interval is scaled to 30days)
Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.[ ] This was checked for cross-browser compatibility, including a check against IE11[ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support[ ] Documentation was added for features that require explanation or tutorials[ ] This was checked for keyboard-only and screenreader accessibilityFor maintainers
[ ] This was checked for breaking API changes and was labeled appropriately[ ] This includes a feature addition or change that requires a release note and was labeled appropriately