Skip to content

[Monitoring] Used fixed_interval explicitly for date_histogram aggregations - #37023

Merged
chrisronline merged 2 commits into
elastic:masterfrom
chrisronline:monitoring/interval_deprecated
May 24, 2019
Merged

chrisronline merged 2 commits into
elastic:masterfrom
chrisronline:monitoring/interval_deprecated

Conversation

@chrisronline

Copy link
Copy Markdown
Contributor

Relates to #27410

The interval field for date_histogram aggregations is deprecated and is replaced by either specifying fixed_interval or calendar_interval. This comment leads me to believe the monitoring code should be using fixed_interval, as our interval values are ns where n is derived bucketSize. This is actually a very simple change. Let's see how tests react.

@chrisronline chrisronline added the Team:Monitoring Stack Monitoring team label May 23, 2019
@chrisronline chrisronline self-assigned this May 23, 2019
@elasticmachine

Copy link
Copy Markdown
Contributor

Pinging @elastic/stack-monitoring

@elasticmachine

Copy link
Copy Markdown
Contributor

💔 Build Failed

@chrisronline

Copy link
Copy Markdown
Contributor Author

@pickypg Any thoughts on this fix? Seem accurate?

@pickypg pickypg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, this looks appropriate, but I wonder if there are other places in our code that require this change (thinking about Beats / Logstash in particular since they have semi-unique workflows).

It would probably be easiest to run with and without this change against the latest ES master to see what gets logged as deprecated.

@chrisronline
chrisronline marked this pull request as ready for review May 24, 2019 19:42
@elasticmachine

Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@chrisronline
chrisronline merged commit 34e6c90 into elastic:master May 24, 2019
@chrisronline
chrisronline deleted the monitoring/interval_deprecated branch May 24, 2019 21:28
chrisronline added a commit to chrisronline/kibana that referenced this pull request Aug 9, 2019
…ations (elastic#37023)

* Used fixed interval explicitly

* Fix tests
chrisronline added a commit to chrisronline/kibana that referenced this pull request Aug 9, 2019
…ations (elastic#37023)

* Used fixed interval explicitly

* Fix tests
chrisronline added a commit that referenced this pull request Aug 9, 2019
…ations (#37023) (#43045)

* Used fixed interval explicitly

* Fix tests
chrisronline added a commit that referenced this pull request Aug 9, 2019
…ations (#37023) (#43046)

* Used fixed interval explicitly

* Fix tests
@chrisronline

Copy link
Copy Markdown
Contributor Author

Backport:

7.x: db7f0e9
7.3: 57694bd

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants