Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Simplify date histogram meta and apply interval scaling to all levels #95206

Merged
merged 9 commits into from
Mar 31, 2021
Merged

Simplify date histogram meta and apply interval scaling to all levels #95206

merged 9 commits into from
Mar 31, 2021

Conversation

flash1293
Copy link
Contributor

Fixes #95078
Fixes #93960

This PR does two things:

  • Making sure the date interval is scaled up for all bound time fields of the current request, not just for the default time field of the index pattern
  • Store the used interval and time zone of a date histogram in serialize-only params to pass it to the renderer instead of running the actualy logic

@flash1293 flash1293 added v8.0.0 Team:AppServices release_note:skip Skip the PR/issue when compiling release notes v7.13.0 auto-backport Deprecated - use backport:version if exact versions are needed labels Mar 23, 2021
@flash1293 flash1293 marked this pull request as ready for review March 24, 2021 12:22
@flash1293 flash1293 requested a review from a team March 24, 2021 12:22
@flash1293 flash1293 requested a review from a team as a code owner March 24, 2021 12:22
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-services (Team:AppServices)

ppisljar
ppisljar reviewed Mar 24, 2021
View reviewed changes
src/plugins/data/common/search/aggs/agg_config.ts
const defaultTimeFields = defaultTimeField ? [defaultTimeField] : [];
const allTimeFields =
this.aggConfigs.timeFields && this.aggConfigs.timeFields.length > 0
? this.aggConfigs.timeFields
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will this contain the default one? or we don't want the default one if this was provided ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It overwrites the default one - this is the same logic as esaggs uses for building the filter

ppisljar
ppisljar approved these changes Mar 24, 2021
View reviewed changes
Copy link
Member

@ppisljar ppisljar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

app services changes LGTM

@flash1293
Copy link
Contributor Author

@elasticmachine merge upstream

@flash1293
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
lens 958.5KB 958.6KB +154.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
data 802.8KB 803.2KB +434.0B
lens 33.5KB 33.4KB -66.0B
total +368.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

wylieconlon
wylieconlon approved these changes Mar 30, 2021
View reviewed changes
Copy link
Contributor

@wylieconlon wylieconlon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this keeps the current behavior on both Lens and Visualize, since Lens will provide timefields but Visualize does not. LGTM

@flash1293 flash1293 merged commit a9d0b6f into elastic:master Mar 31, 2021
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Mar 31, 2021
@flash1293 @kibanamachine
Simplify date histogram meta and apply interval scaling to all levels (…
73e9004 
…elastic#95206)
@kibanamachine kibanamachine mentioned this pull request Mar 31, 2021
Merged
@kibanamachine
Copy link
Contributor

💚 Backport successful

✅ 7.x / #95905

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Mar 31, 2021
@kibanamachine @flash1293
Simplify date histogram meta and apply interval scaling to all levels (…
1cb8946 
…#95206) (#95905)

Co-authored-by: Joe Reuter <johannes.reuter@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wylieconlon wylieconlon wylieconlon approved these changes

@ppisljar ppisljar ppisljar approved these changes

Assignees
No one assigned
Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes v7.13.0 v8.0.0
Projects
None yet
Milestone
No milestone
5 participants
@flash1293 @elasticmachine @kibanamachine @wylieconlon @ppisljar