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

Update Cluster Alerts watches for hits.total change #39143

Closed
ycombinator opened this issue Feb 19, 2019 · 3 comments
Closed

Update Cluster Alerts watches for hits.total change #39143

ycombinator opened this issue Feb 19, 2019 · 3 comments

Comments

@ycombinator
Copy link
Contributor

Monitoring creates watches for its Cluster Alerts feature. The definitions for these watches can be found in this folder: https://github.com/elastic/elasticsearch/tree/master/x-pack/plugin/monitoring/src/main/resources/monitoring/watches.

Some of these watches rely on hits.total being a scalar value. For example:

"source": "ctx.vars.fails_check = ctx.payload.check.hits.total != 0 && ctx.payload.check.hits.hits[0]._source.cluster_state.status != 'green';ctx.vars.not_resolved = ctx.payload.alert.hits.total == 1 && ctx.payload.alert.hits.hits[0]._source.resolved_timestamp == null;return ctx.vars.fails_check || ctx.vars.not_resolved"

Starting 7.0, Elasticsearch will default to hits.total being an object instead of a scalar value. To account for this, two sets of fixes ought to be made to the Cluster Alerts watches:

  1. In the 6.7 branch, these watches should include rest_total_hits_as_int in their search queries. This will allow a Cluster Alert watch created by a 6.7 cluster to function properly either on a 6.7 or 7.0 monitoring cluster.

  2. In the 7.0, 7.x, and master branches, these watches should be updated so any references to hits.total refer to hits.total.value instead.

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features

@jakelandis
Copy link
Contributor

For 7.0 we will send rest_total_hits_as_int per #36697, and plan to address removing this during `7.x via #38387.

The change here is essentially taking the change on #36697 back to v6.7.0 to allow 6.7 data clusters to talk to a 7.0 monitoring cluster correctly. (cherry-pick won't work due to differences in base)

@jakelandis
Copy link
Contributor

jakelandis commented Feb 20, 2019

Looking at this again today and this isn't an issue due to the way rest_total_hits_as_int is implemented in 7.x. The parameter is always sent irregardless if it is defined inside the search template (defaulted to true). So in 7.x a user can explicitly set it to false it per watch, but if they do nothing it will always be sent with a true value.

This means that both 6.7 and 7.0 monitoring will always use the scalar value irregardless of where the watch originated. #38387 will change this behavior before 8.0.

Also, tested locally and 6.7 data -> 7.0 monitoring cluster with alerts works (lots of deprecation notices...but that is expected)

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

No branches or pull requests

4 participants