SpringBoot Add support for reactive Elasticsearch healthcheck

Hello,

I am working a lot with spring-data-elasticsearch using reactive repositories. I noticed that even the newest version still supports only healthchecks for ReactiveElasticsearchClient. More over the default healthcheck still wants to run even if all of the clients are reactive - it uses the default one created by autoconfiguration that fails.

I checked how it was done for Mongo that supports reactive indicators as well as basing on the existing ElasticSearch health indicator I implemented a simple proposal. The generated output would be as follows:

"elasticsearch": {
            "status": "UP",
            "components": {
                "reactiveElasticsearchClient": {
                    "status": "UP",
                    "details": {
                        "192.168.78.150": "ONLINE",
                        "192.168.78.160": "OFFLINE"
                    }
                }
            }
        }

This is my first contribution to the project so please forgive me any mistakes, I tried my best to follow the guidelines. Any comments appreciated. Thank you.

Comment From: aleksanderlech

any ideas why CI failed? Running locally succeeds :no_mouth:

gradle clean --no-build-cache :spring-boot-project:spring-boot-actuator-autoconfigure:test

Comment From: philwebb

@aleksanderlech Those failures look unrelated to your changes. We've had some issues with diskspace on the CI box, so it might be that.

Comment From: aleksanderlech

Hello, any feedback here? :)

Comment From: bclozel

Thanks for your contribution! I believe we can't merge this right now as there are several questions we need to answer first.

I agree that we should align here with what we're doing with MongoDB (reactive vs. non-reactive). This changes the name of the contributor beans, which is a breaking change. We should consider that in a minor release (at least), but we're already quite late in the 2.3 cycle with RC1 already being released.

Also, it bothers me that the health response for the reactive case is very different from the one with the base REST client. Your proposal shows a global "UP"/"DOWN" status and specific "ONLINE"/"OFFLINE" node details. The existing indicator queries the "/_cluster/health/" endpoint and reports the health of the cluster itself ("UP", "OUT_OF_SERVICE", "DOWN") with the actual JSON response as details. I'd rather try and align both implementations - I think the actuator health indicator should provide information about the cluster health, rather than the online/offline status of existing nodes.

@christophstrobl and @mp911de might disagree with that point, since the client interface only exposes a client.status() which does not seem to do the same thing. Is there a way to get the cluster health with a ReactiveElasticsearchClient? Should we reconsider our approach for this actuator endpoint?

One last bit: it seems that we've let a typo sneak in (and your contribution aligns there, so not your fault!) - it should be ElasticsearchRestHealthContributorAutoConfiguration and not ElasticSearchRestHealthContributorAutoConfiguration, same for ElasticsearchReactiveHealthContributorAutoConfiguration vs. ElasticSearchReactiveHealthContributorAutoConfiguration.

Comment From: mp911de

There's currently no method to issue a request to /_cluster/health/. However, you can interact with the underlying WebClient through

client.execute(webClient -> webClient.get().uri("/_cluster/health/").exchange()).flatMap(clientResponse -> clientResponse.bodyToMono(…))

to query the cluster state.

It would make sense to provide such a method on ReactiveElasticsearchClient to close the feature gap between ReactiveElasticsearchClient and RestHighLevelClient. /cc @sothawo

Comment From: sothawo

@mp911de you mean to to add a healthcheck method to ReactiveElasticsearchClient? Would it even make sense to have something like ClusterOperations that implement (not all of) ES Cluster API?

Comment From: mp911de

I think so. Let’s continue the cluster API discussion in a DATAES ticket.

Comment From: aleksanderlech

@bclozel thanks for the feedback, I did not expect it to be merged straight away and actually had the same concerns regarding the cluster state and response model but wanted to get some feedback from the team first. I would be more very happy if I can implement it following your suggestions.

Comment From: bclozel

See https://jira.spring.io/browse/DATAES-818

Comment From: bclozel

Merged with 86d8366 Thanks @aleksanderlech !