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.

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

Interval field on DateHistograms will be deprecated #27410

Closed
4 tasks done
polyfractal opened this issue Dec 18, 2018 · 11 comments
Closed
4 tasks done

Interval field on DateHistograms will be deprecated #27410

polyfractal opened this issue Dec 18, 2018 · 11 comments
Labels
Feature:Aggregations Aggregation infrastructure (AggConfig, esaggs, ...) Feature:Rollups release_note:breaking Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more Team:Monitoring Stack Monitoring team Team:Visualizations Visualization editors, elastic-charts and infrastructure

Comments

@polyfractal
Copy link
Contributor

polyfractal commented Dec 18, 2018

Today, Elasticsearch's date_histogram aggregation's interval field supports both "fixed" and "calendar" time simultaneously, choosing between the two depending on the interval's syntax (e.g. 1d is calendar, 2d is fixed). This is super confusing for users even if they read the documentation, and a very subtle bug for everyone else because intervals don't always work as they expect.

Elasticsearch is working on a PR to deprecate this functionality, and replace it with two explicit fields: fixed_interval and calendar_interval. This way the user knows exactly what they are getting.

The plan as it stands today:

  • Add fixed_interval / calendar_interval fields to date_histogram aggregation.
  • Deprecate interval field in 7.0+
  • Remove interval field in 8.0
  • All three fields are mutually exclusive
  • Changes also affect rollup configurations and composite aggs (since they both use the date_histo)

More details can be found in the PR itself (elastic/elasticsearch#33727) but I wanted to open an issue in Kibana to track the deprecation since it'll affect anything touching a date histo.

Note: we're targeting 7.0, but there's no requirement it has to make 7.0 so it could slip to 7.x

TODOs

@polyfractal polyfractal added the Feature:Aggregations Aggregation infrastructure (AggConfig, esaggs, ...) label Dec 18, 2018
@timroes timroes added the Team:Visualizations Visualization editors, elastic-charts and infrastructure label Mar 28, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app

@polyfractal
Copy link
Contributor Author

Note: this merged into master, backport for 7.x is in-progress. I'll update when that lands as well.

@cjcenizal cjcenizal added the Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more label May 8, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/es-ui

@elasticmachine
Copy link
Contributor

Pinging @elastic/stack-monitoring

@polyfractal
Copy link
Contributor Author

Final note, elastic/elasticsearch#41906 just merged into 7.x and backporting is now done on the ES side.

@timroes
Copy link
Contributor

timroes commented May 23, 2019

Update: We currently have plans to provide a utility function that autodetects this into the new data core plugin, which all teams can use. I'll update here (and via mail) once this is available.

@chrisronline
Copy link
Contributor

I'm not quite sure I understand how this works.

Today, Elasticsearch's date_histogram aggregation's interval field supports both "fixed" and "calendar" time simultaneously, choosing between the two depending on the interval's syntax (e.g. 1d is calendar, 2d is fixed).

You're saying 1d is calendar, but 2d is fixed? I don't know if I understand how the ES code differentiates those two as separate interval types entirely?

@polyfractal
Copy link
Contributor Author

You're saying 1d is calendar, but 2d is fixed? I don't know if I understand how the ES code differentiates those two as separate interval types entirely?

This is correct. In the code, there's a map of "calendar" units and it literally looks to see if the supplied interval is one of the accepted calendar units (1d, 1w, etc). If it matches, it uses calendar-aware rounding for all the date math.

If the map doesn't contain the specified interval, it is assumed to be "fixed" and tries to parse it with the allowed non-calendar units. If this parsing works, fixed time rounding is used for the rest of the date math.

If that parsing fails, it throws an exception (either because the unit is an illegal fixed time like 2w, or just invalid foobar).

This is why we wanted explicit fields to disambiguate. Having two distinctly different behaviors overlapping with the same set of semi-shared syntax is complicated and confusing to everyone :)

@chrisronline
Copy link
Contributor

@polyfractal

I don't think it's exactly related, but do you know if this interval is also affected by this change?

@polyfractal
Copy link
Contributor Author

@chrisronline No, I don't think so. Should only affect things that touch the date_histogram interval directly (so the agg, composite agg, and rollups). Looks like that interval is always parsed as fixed time

@timroes
Copy link
Contributor

timroes commented Sep 10, 2020

I will be closing this issue since all linked TODOs are done. If any team still is using interval in their code, please open a separate issue for your team (so we can also have properly assigned team labels), reference this issue, and mark yoru issue as a blocker for 8.0. Thanks everyone.

@timroes timroes closed this as completed Sep 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Aggregations Aggregation infrastructure (AggConfig, esaggs, ...) Feature:Rollups release_note:breaking Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more Team:Monitoring Stack Monitoring team Team:Visualizations Visualization editors, elastic-charts and infrastructure
Projects
None yet
Development

No branches or pull requests

7 participants