This PR changes the implementation of the ElasticsearchReactiveHealthIndicator class.

In version 4.2.0 of Spring Data Elasticsearch we added the ability to issue a cluster health request to the ReactiveElasticsearchClient so it is not necessary anymore to directly do this request by using the underlying WebClient. Using the ReactiveElasticsearchClient now makes sure that all configuration the user has done on it is passed in the request. If for example there was a custom header supplier configured, the previous implementation of the health check did and could not add the so provided headers to the request - which caused failing requests, when these headers contain authorization data.

No new or changed tests were needed, the new implementation behaves exactly like the old one.

Comment From: snicoll

@sothawo I am not sure I understand the rationale for the change, to be honest.

Using the ReactiveElasticsearchClient now makes sure that all configuration the user has done on it is passed in the request. If for example there was a custom header supplier configured, the previous implementation of the health check did and could not add the so provided headers to the request - which caused failing requests, when these headers contain authorization data.

The current code uses this.client.execute that provides a WebClient callback. SD ES has full control on the configuration of that WebClient, I believe. Isn't that inconsistent that the necessary headers aren't added? If they were (as I think they should), then this PR would become less relevant.

Comment From: sothawo

@snicoll The DefaultReactiveElasticsearchClient implementation in SD ES creates and initializes a WebClient for every configured host once and then reuses it. The WebClient is configured as far as it is possible at that time, that includes default headers for example that are sent with each request.

But there may be headers that are not available at this time, for example a x-trace-id being sent in from some client or a authentication token which will change over time as it expires. To handle this, the user can configure a callback to provide headers at the time when the request is built.

The DefaultReactiveElasticsearchClient uses the WebClient to create the request, then sets the headers provided by the callback and sends the request off.

In order to set these headers in the health indicator with the current solution, the code customizing the created request would somehow need to get access to the configured headers callback to set these headers before calling exchangeToMono. But that would mean that the ReactiveElasticsearchClient would have to be changed to provide that callback.

The solution we took instead in SD ES is to provide the ReactiveElasticsearchClient.cluster() method providing a client for cluster operations which has the health() method taking and returning the Elasticsearch request and response classes that are used by the non-reactive RestHighLevelClient as well (Adding cluster related methods was an issue we had on the list anyway).

I hope this could clarify the purpose of this PR.

Comment From: mp911de

The DefaultReactiveElasticsearchClient implementation in SD ES creates and initializes a WebClient for every configured host once and then reuses it. The WebClient is configured as far as it is possible at that time, that includes default headers for example that are sent with each request.

But there may be headers that are not available at this time, for example a x-trace-id being sent in from some client or a authentication token which will change over time as it expires. To handle this, the user can configure a callback to provide headers at the time when the request is built.

Whether headers (header suppliers) are configured at the level of the component that provides WebClient or that invokes WebClient doesn't seem to make a difference. This sounds like working around a design issue.

The solution we took instead in SD ES is to provide the ReactiveElasticsearchClient.cluster() method providing a client for cluster operations which has the health() method taking and returning the Elasticsearch request and response classes that are used by the non-reactive RestHighLevelClient as well (Adding cluster related methods was an issue we had on the list anyway).

The imperative health check uses the lower-level RestClient for the health check. Remaining low-level and therefore consistent across the implementations is actually a good thing.

Comment From: sothawo

I moved the logic to call the header supplier into the WebClient.defaultRequest, so this PR is not needed anymore