Skip to content

Add index status to cluster state API response (under metadata metric)#33614

Closed
ycombinator wants to merge 2 commits intoelastic:masterfrom
ycombinator:cluster-state-index-status
Closed

Add index status to cluster state API response (under metadata metric)#33614
ycombinator wants to merge 2 commits intoelastic:masterfrom
ycombinator:cluster-state-index-status

Conversation

@ycombinator
Copy link
Copy Markdown
Contributor

@ycombinator ycombinator commented Sep 11, 2018

Currently, Elasticsearch Monitoring (the X-Pack feature) happens as a result of a handful of collectors running inside Elasticsearch. One such collector is the IndexStatsCollector which collects metrics about each index in the cluster. One of the metrics it collects is the index status: green, yellow, or red.

Longer term we want to externalize this collection process to an agent like Metricbeat, which could call Elasticsearch REST APIs to perform the collection. When building the equivalent of the IndexStatsCollector in Metricbeat, we ran into an issue with collecting the index status. Metricbeat calls the Cluster State REST API, which currently does not report index status. This means Metricbeat would have to compute the index status by inspecting the shards under the routing_table metric in the Cluster State REST API response, by doing something like this:

https://github.com/ycombinator/beats/blob/e33c0acb608011c56db8d4429ecbf3c9d7053e07/metricbeat/module/elasticsearch/index/data_xpack.go#L194-L237

Having Metricbeat compute index status this way seems a bit flimsy. It should be something that's opaque to Metricbeat (or any client for that matter). Ideally Elasticsearch would compute this field and return it as part of the Cluster State REST API response. That is what this PR attempts to do.

@ycombinator
Copy link
Copy Markdown
Contributor Author

ycombinator commented Sep 11, 2018

Note that I have deliberately not updated/added any tests or docs in this PR yet. I want to first make sure the Elasticsearch team is okay with this enhancement before proceeding further (hence the discuss and wip labels).

@rjernst rjernst added the :Core/Infra/Core Core issues without another label label Sep 11, 2018
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-core-infra

@pickypg
Copy link
Copy Markdown
Member

pickypg commented Sep 14, 2018

This makes total sense to me and it's a natural requirement given the external nature to avoid having Metricbeat have to join GET /_stats with otherwise avoidable APIs.

@jasontedor
Copy link
Copy Markdown
Member

I do not think that we should do this. I understand how it fits into the goals of Beats, but it is in opposition with how we think our APIs should function. I say "we" because we discussed this in Fix-it-Thursday and came to the same conclusion as I have: the get cluster state API is about the cluster state and now we exposing derivations on an index level in that API. We do not think we should do this.

We recognize this is at tension with the goals of Beats. We have seen this tension arise in other proposed or integrated changes as well, where the cluster UUID is being added to endpoints. We do not want to approach these piecemeal, we want to take a step back together and understand each other's philosophies and see if we can come up with a solution that works for both sides together. I do not think we can hash this out asynchronously, I think we need some face-to-face time. Can we discuss this in Dublin?

@ycombinator
Copy link
Copy Markdown
Contributor Author

Thanks for discussing this in Fix-it-Thursday. Starting with a more holistic in-person discussion about goals makes sense to me. I've put something on the Dublin cross-team agenda (1:30 - 2:45 PM) with you and I as hosts of the discussion.

Closing this PR unmerged for now...

@jasontedor
Copy link
Copy Markdown
Member

Thanks @ycombinator!

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

Labels

:Core/Infra/Core Core issues without another label discuss >enhancement WIP

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants