Elasticsearch low and high level REST clients

[loicmathieu]

This PR integrates the Elasticsearch low level rest client inside Quarkus.

It creates a new extension that allows to configure a connection to Elasticsearch inside application.properties and inject a RestClient.

The configuration properties has the same semantics as the one from the Hibernate search extension.

This is a draft PR, some works still needs to be done around configuration and documentation.

[loicmathieu]

@gsmet the fact that a user can not easily use the high level rest client bother me a lot, I found a way to allow this without providing support for it: we can add the RestClientBuilder as a CDI bean so if someone still want to use the high level rest client he can do something like this:

@Inject RestClientBuilder restClientBuilder;
private HighLevelRestClient highLevelRestClient;

@PostConstruct
void init() {
    highLevelRestClient = new HighLevelRestClient(restClientBuilder);
}

WDYT

[gsmet]

Cool stuff.

I haven't reviewed in details (expect a thorough review at some point ;)) but I have a few questions:

• wouldn't it be possible to include that in the existing extension without it being an issue for the current usage by the Hibernate Search extension? That would solve the name issue.
• should we allow connecting to multiple clusters from the get go?

As for the high level client, I don't have a problem having a separate extension for it. We would need to check how it behaves with native image though. And maybe we could be a bit aggressive with exclusions (maybe it doesn't need Lucene typically).

I agree exposing the builder as a CDI bean looks neat and would be useful in all cases.

[loicmathieu]

wouldn't it be possible to include that in the existing extension without it being an issue for the current usage by the Hibernate Search extension? That would solve the name issue.

It should be possible, as CDI beans will be removed automatically, if not used we can produce them without any issue. The main issue is that hibernate-search users will start seeing an elasticsearch-rest-client extension and will be able to configure it, and the configuration may seems redondant to them as to be able to inject a RestClient they will need to configure Elasticsearch under quarkus.elasticsearch root configuration.

But we should be able to rename the current hidden extension to elasticsearch-core as it's not a user facing extension right ?

[loicmathieu]

should we allow connecting to multiple clusters from the get go?

As you want, it's a common feature request for database extension but I don't know of often it is for Elasticsearch

[loicmathieu]

As for the high level client, I don't have a problem having a separate extension for it. We would need to check how it behaves with native image though. And maybe we could be a bit aggressive with exclusions (maybe it doesn't need Lucene typically).

I agree exposing the builder as a CDI bean looks neat and would be useful in all cases.

It's either way ;)

An other approach would be to propose a PR to the Elasticsearch driver to be able to create a hig level rest client from a low level one ;)

[loicmathieu]

@gsmet I think again at providing connection to multiple clusters. I would prefere to merge this PR without it and opening an Epic that list what we want to support on Elasticsearch: multi-tenant, metrics, investigation of hibernate-search-standalone or Panache, ... The idea is to build our Elasticsearch support in steps.

We can then decide what will be the minimum level of support before we document it and integrate the extension inside code.quarkus.io so it becomes visible to users.

[loicmathieu]

Added the high level client as a separate extension (didn't update the guide yet).

I move the health check to the common extension to be able to use it in both, maybe we can find a way to use it with hibernate-search-elasticsearch also ;)

I have an issue with a banned dependency : log4j2 is banned but the corresponding bridge from jboss-logmanager is not publushed in maven central: https://github.com/jboss-logging/log4j2-jboss-logmanager. I customize the configuration of the enforcer plugin for now but it's temporary ...

[loicmathieu]

I refactor the code to share code between the two clients.

I implement more configuration options + the sniffer.

I integrate the high level client inside the documentation.

[loicmathieu]

I remove all the un-needed dependencies from lucene, I use Jdeps to know which one are needed, then I add back the one that are needed for native-image to works (see the comment inside the pom).

[loicmathieu]

@gsmet I rebased on master to fix some merge issues.

I would love this PR to be considered for 1.5 if it's still possible ?

[gsmet]

I added some comments, mostly in the documentation. Didn't have the time to finish the review before going on PTO, sorry about that :/.

On a more general note, I would go with:

• quarkus-elasticsearch-rest-client-common for the existing extension
• quarkus-elasticsearch-rest-client for the low level client
• quarkus-elasticsearch-rest-high-level-client for the high level client

That way, we would be consistent with the Elasticsearch clients.

[gsmet]

I wonder if we could add substitutions to remove the need for them. I'm pretty sure they are not really useful.

[loicmathieu]

I'll open a followup issue for this if it's OK.

[gsmet]

Sure.

[loicmathieu]

@gsmet I renamed the three extensions, apply your suggestions and implements most of what you ask.
There is still 3 open points that needs your feedback.

[loicmathieu]

@gsmet due to the change that mandates that an extension runtime module depending on another extension mandate that the extension's deployment module also depands on another extension deployment's module, this PR needs to be re-worked.

I think I must create another common module to share the code between the two clients.

So I propose to rename the just renammed elasticsearch-rest-client-common that is shared with Hibernate Search elasticsearch-common and create another module named elasticsearch-rest-client-common with the code shared between the two clients.

[gsmet]

As for the split, let me some time to think about it and check the code. I'm a bit puzzled why we cannot have both the deployment of the low level client and the high level client running at the same time as one is based on the other.

I will have a closer look before the end of the week.

[loicmathieu]

@gsmet I try to move all the common part to the common module and it works and it didn't crash hibernate-search that also use the common module so it appears to be safe.

The only caveat is that I also move the config to the common module as both client uses the same config (but I can duplicate it). I don't know if it's an issue as hibernate-search didn't use the same configuration options, and those options are not used inside the common module.

[gsmet]

I don't think that's a good idea.

For me, here's what it's supposed to do:

• the low level REST client creates a shared low level REST client from the config that is in this very module
• the high level REST client reuses the instance above

This way you don't have to do all sorts of weird things: the low level and high level extensions fully co-exist as the latter is based on the former.

One interesting thing is that the javadoc RestHighLevelClient recommends to subclass it if you want to inject an already built low level client and I think that's what we should do.

Note that even if we subclass it, we should just expose it as a RestHighLevelClient so that users just use this class for injection.

Does it make sense?

[loicmathieu]

@gsmet I restructure the code to reuse the rest client inside the high level one.
I also fix the issue with log4j2.

[gsmet]

Aren't you missing high-level here and below? Not sure as GH UI is doing weird things with those files but better check.

[loicmathieu]

Sorry, bad copy/paste. Fixed.

[gsmet]

I extracted part of the log4j2 fix here: #10560 as I want to backport it and I need it for a Platform related thingy.

[gsmet]

Superseded by #10745 .