Prior to this commit, Spring Boot would only auto-configure the RestHighLevelClient and RestClientBuilder if the RestHighLevelClient was present. This was done in 1d73d4eda75ef0d18c747c57aa2b3385674483c7 as part of https://github.com/spring-projects/spring-boot/issues/22358

This commit brings back the exposing of the RestClient bean in Spring Boot when exposing the RestHighLevelClient or when the RestHighLevelClient is not present. It allows for using the Spring Boot auto configuration and its customizers of the RestClientBuilder in a similar way as it is done for the RestTeamplateBuilder and the WebClient.Builder. Now the presence of the org.elasticsearch.client:elasticsearch-rest-high-level-client is optional. This opens the door for potentially adding support to the new Elasticsearch Java Client that is based on the same RestClient

The exposing of the RestClient as a bean is not the most important thing in this PR. I would rather say that the fact that the RestClientBuilder is exposed as a bean is the most important part. This allows users to rely on the Spring Boot provided mechanisms for configuring the RestClientBuilder and not depend on the now deprecated Elasticsearch Rest High Level Client.

Comment From: filiphr

I didn't do any changes to the Gradle Wrapper. Seems like some problem in the Gradle Wrapper action itself

Comment From: bclozel

This PR is reversing the team's decision explained in https://github.com/spring-projects/spring-boot/issues/22358#issuecomment-679937047. I'm not sure I understand the rationale behind this. The presence of elasticsearch-rest-high-level-client has always been optional, but we've found that the low level client was of little use in itself at the application level.

If the main goal is to support the newly released client, maybe we should auto-configure as well and deprecate the high-level variant in 2.6.x?

Comment From: filiphr

The biggest reason why I created this PR was to get the RestClientBuilder as a bean in order to rely on the Spring properties / customizers. With the current setup of things you have to have elasticsearch-rest-high-level-client in order to create your own RestClient.

The reason for that is

https://github.com/spring-projects/spring-boot/blob/83e44305122a9bd45194cd5db649ad3a12b64690/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/elasticsearch/ElasticsearchRestClientAutoConfiguration.java#L41-L42

We have a platform that heavily uses Spring Boot and we are using the RestClient directly and not the RestHighLevelClient. We started removing the dependency entirely, since we want to completely not depend on it due to Licensing issues. The High Level Client is since 7.11 no longer Apache 2.0 licensed. I thought that everything will keep working as expected, but then the RestClientBuilder was not present anymore.

The addition of the RestClient as a bean is an addition to the original requirement that we had. I honestly, thought that it is not that complicated to also expose it as a bean since it can be exposed through the RestHighLevelClient#getLowLevelClient easily and if the RestHighLevelClient is not there then it can easily be created through the RestClientBuilder. That's why I did it.

Even if you only partially integrate this (without the new beans) we can easily do


@Bean
public RestClient elasticsearchRestClient(RestTemplateBuilder builder) {
    return builder.build();
}

and have the RestClient bean in our own application without the need to depend on the RestHighLevelClient and without the need to copy things that are already done in Spring Boot.

In addition to all of this, there is also OpenSearch with its own things. Without going into too much detail, for our own simple purposes it might be possible to use one RestClient to communicate with both types of clusters.

In a nutshell for us having to maintain support for our own things on both OpenSearch and Elasticsearch it makes a lot of sense to only use the Low Level Clients. I hope that you are going to take this into consideration and revise your decision about this. Thanks a lot for the understanding

Comment From: bclozel

Thanks for the background information @filiphr, that's super useful.

With that I think we should: * bring back RestClient or its builder as a bean. This should allow the OpenSearch use case * schedule the support for the new official client and deprecate the current one

Comment From: filiphr

Something else, that I forgot to add to this PR. The ElasticSearchRestHealthContributorAutoConfiguration needs to be adapted as well.

Ideally the health contributor will be adapted to only use RestClient or somehow a combination of things (if the RestHighLevelClient is there). e.g. All RestClient beans plus the RestHighLevelClient#getRestLowLevelClient() that are not exposed as beans. The ElasticsearchRestHealthIndicator needs to be adapted to only take the RestClient as a constructor in order to not have a mandatory dependency on the RestHighLevelClient. If this is done then the same class can be used both for the old RestHighLevelClient and new Elasticsearch Java Client.

I can of course provide PRs and / or adapt this PR if you decide that you are OK with my proposals.

Comment From: filiphr

One, I hope final, argument is also the fact that removing the dependency on the RestHighLevelClient just shaved off ~20MB from our final jar. Not sure if things like this are important from your perspective or not, but I wanted to share this as well

Comment From: bclozel

Thanks a lot @filiphr for all your insights.

We've just discussed this as a team and we've decided to: 1. Bring back RestClient as a bean; we initially decided that it had low value for actual applications, but with the latest changes in the elasticsearch ecosystem, it's better to have it. 2. Adapt the health contributor to only depend on the low level RestClient 3. Deprecate the RestHighLevelClient in 2.7, see #28598 4. Support the new ElasticsearchClient in 2.7 as a replacement, see #28597

Could you adapt this PR to address 1) and 2)? Note that we've scheduled that for 2.7, so there's no rush on this as we're going to focus on getting 2.6 out and critical bug fixes first.

Comment From: filiphr

Thanks a lot for this decision @bclozel.

I am going to adapt this PR to take into consideration for 1) and 2).

What about exposing RestClientBuilder as a bean when the RestHighLevelClient is not present, but the RestClientBuilder is. Will you accept a separate PR for 2.6 that will do this?

I have managed to achieve what I need with 2.5 already, but the way I did it is not the best approach, since I am doing some nasty magic with ImportSelector, and I would rather get rid of that nasty magic we have.

Comment From: bclozel

What about exposing RestClientBuilder as a bean when the RestHighLevelClient is not present, but the RestClientBuilder is. Will you accept a separate PR for 2.6 that will do this?

Exposing the RestClientBuilder might work for your case, but I'm wondering if it's the right choice here: I don't think we usually want to have several instances of RestClient in the same application? That would create several HTTP client instances, right? Even if the application itself only creates one, we'd need to reuse it for the health indicator?

It's a bit late in the 2.6 cycle unfortunately so if we want to introduce a change here, it has to be extra-safe.

Comment From: filiphr

@bclozel I think that I have adapted the PR as you asked. Please have a look at it and let me know if there is something more that needs to be done.

Also adapted the use of ElasticsearchRestHealthIndicator. I deprecated the constructor using RestHighLevelClient. Not sure what your policy for removal there is. Ideally I'd like to get rid of the constructor, since the ElasticsearchRestHealthIndicator in the current state is not usable when the RestHighLevelClient is not there. I think that the class will not load at all.

Exposing the RestClientBuilder might work for your case, but I'm wondering if it's the right choice here: I don't think we usually want to have several instances of RestClient in the same application? That would create several HTTP client instances, right? Even if the application itself only creates one, we'd need to reuse it for the health indicator?

Exposing the RestClientBuilder does not mean that there will be several instances of the RestClient. The build method needs to be invoked for a RestClient to be created. Which means that from a Spring Boot point of view for 2.6 when the RestHighLevelClient is not there then there will be a bean of type RestClientBuilder. The users can then choose and decide what to do with that RestClientBuilder. In order to reduce the amount of changes the health indicator will not use it. People will need to expose their own health indicator in 2.6. And as I mentioned in the previous paragraph the current ElasticsearchRestHealthIndicator is not capable to be used without the RestHighLevelClient.

Comment From: bclozel

@filiphr you're right, I forgot we were already exposing the builder as a bean. I need to take a deeper look.

Comment From: filiphr

@bclozel I'll try to do a small proposal as a separate PR for that. You can then decide what you want to do

Comment From: filiphr

@bclozel I did a proposal in PR #28655. Please have a look at it and let me know what you think. This PR and that one of course have some overlap. However, based on your decision, I can adapt this PR on top of that other one if needed.

Comment From: filiphr

@bclozel I finally found the time and adapted the changed in this PR to address point 1) and 2) from https://github.com/spring-projects/spring-boot/pull/28496#issuecomment-965569351. I've changed the base of the PR to be 2.7. Please have a look at it and let me know what you think about it.

Comment From: snicoll

For the record 2.7.x has been upgraded to Elasticsearch 7.16.2.

Comment From: filiphr

For the record 2.7.x has been upgraded to Elasticsearch 7.16.2.

Damn, I first rebased on main, but then realized that main and 2.7.x are not the same, so I did it on 2.7.x. I'll try to do take the new changes from 2.7.x into consideration and fix the conflicts.

Basically, I need to make sure that there are no deprecation warnings, right?

Comment From: wilkinsona

Thanks very much, @filiphr. If you're interested, I reworked things a bit in https://github.com/spring-projects/spring-boot/commit/a7a71da9ef159e375ff0faa344e888d435a7052c. This was primarily to simplify the changes to the health indicator. You can see the net changes in https://github.com/spring-projects/spring-boot/commit/227c3164f1c0f08410d72666fec64bb989d061c5.

Comment From: filiphr

@wilkinsona I agree with you that the code is simplified with your change. However, your change means that you can only have Elasticsearch health checks if you have the RestHighLevelClient on the classpath, because the loading of ElasticsearchRestHealthIndicator will fail due to the RestHighLevelClient being used in one of its constructors.

I personally see no need to lose Elasticsearch auto configuration if you do not have RestHighLevelClient on the classpath. Due to the changes in ElasticSearchRestHealthContributorAutoConfiguration it also means that the application will not boot up properly when you only have the RestClient available.

If this is something that the Boot team wants to do, then I am all OK with it. However, I would ask you to reconsider and have health checks without the RestHighLevelClient.

Comment From: wilkinsona

That’s an unintentional over-simplification. Thanks for catching it, @filiphr. I’ll take another look. Requiring a RestClient bean for auto-configuration of the health indicator looks like it could be a good middle ground.

Comment From: filiphr

Requiring a RestClient bean for auto-configuration of the health indicator looks like it could be a good middle ground.

It is not about the auto-configuration. The auto configuration works correctly and it auto configures if a RestClient bean is there. The problem is the fact that the loading of the ElasticsearchRestHealthIndicator will fail when the RestHighLevelClient class is not there.

Thanks for taking another look at it.

Comment From: filiphr

Thanks a lot for bringing back the old changes @wilkinsona. It is really appreciated.

Comment From: nightswimmings

@wilkinsona Are we opensearch users expected to keep using opensearch.RestHighLevelClient after 2.7.0?

Comment From: wilkinsona

Spring Boot does not have any auto-configuration for OpenSearch so we don't have any expectations about what you should do.

Comment From: filiphr

@nightswimmings as far as I know, you can keep using the Elasticsearch RestClient with OpenSearch. You can't use the RestHighLevelClient though.

Comment From: nightswimmings

Thanks filiphr, we already have all forked opensearch client code, its just that I did not know whether to expect any impact or not on the new autconfiguration and deprecation if you are on opensearch-flavoured els, but if Boot does not account the later, then its not an issue