Remove type casts in logging in server component

[dnhatn]

This commit removes type-casts in logging in the server component (other components will be done later). This also adds a parameterized message test which would be able to catch breaking-changes related to lambdas in Log4J.

[jasontedor]

What do you mean incorrect, there is not any method on Logger that accepts a java.util.Supplier, that actually can not be used here? Using a java.util.function.Supplier would actually be wrong because it would invoke

Logger#void info(Object message, Throwable t);

Please note that this change cause these method invocations to change from invoking:

Logger#void info(Supplier<?> msgSupplier, Throwable t);

to invoking

Logger#void info(MessageSupplier msgSupplier, Throwable t);

(for example). This is probably fine. Both of these will be removed in Log4j 3 where they will use JDK interfaces instead.

Historically, the reason that we use Supplier<?> is because MessageSupplier was initially deprecated in 2.6 (we started with Log4j 2.6.2). It has since been un-deprecated in 2.8.1 and is now deprecated again (for removal in Log4j 3). The Javadocs for Logger say this:

 * Note that although {@link MessageSupplier} is provided, using {@link Supplier Supplier<Message>} works just the
 * same. MessageSupplier was deprecated in 2.6 and un-deprecated in 2.8.1. Anonymous class usage of these APIs
 * should prefer using Supplier instead.

Functionally, the two are equivalent because we supply Messages everywhere.

My preference is that we leave this as-is. Why? Because in Log4j 3 when these are removed we will break at compile time. That is a good thing, it means we can check that when we switch to using java.util.function.Supplier we are invoking the desired method (rather than this silently switching on us from Logger#void info(MessageSupplier msgSupplier, Throwable t); to something we do not check. The explicitness is advantageous here.

[dnhatn]

@jasontedor Thank you for your clear explanation.

What do you mean incorrect, there is not any method on Logger that accepts a java.util.Supplier, that actually can not be used here? Using a java.util.function.Supplier would actually be wrong.

I meant we should not use org.apache.logging.log4j.util.Supplier in places which are not related to logging. In some classes, we already imported org.apache.logging.log4j.util.Supplier and happily use the log4j's Supplier for other purposes. For example:

elasticsearch/server/src/main/java/org/elasticsearch/cluster/service/ClusterApplierService.java

Line 24 in 99f88f1

import org.apache.logging.log4j.util.Supplier;

elasticsearch/server/src/main/java/org/elasticsearch/cluster/service/ClusterApplierService.java

Line 100 in 99f88f1

private Supplier<ClusterState.Builder> stateBuilderSupplier;

Actually @ywelsch suggested me to clean up these (#28534 (comment))

[jasontedor]

Well, I think what we have here is confusion that arises when two distinct changes are attempted in the same pull request. Changing the usage in ClusterApplierService to java.util.function.Supplier is a change that we should make, it will feel cleaner. We could consider a static checker that ensures that if a org.apache.logging.log4j.util.Supplier is a method parameter that that method is a method on the Logger interface. However, I am not sure if this is worth the trouble, and I certainly do not think it should crowd out other more important efforts. The usage here is not incorrect nor broken nor wrong, the code still does what we want it to do, obviously, it just feels dirty because instead of using the built-in Java interface for the same we ended up accidentally using one from a logging dependency. Oh well. Let's fix it in a small change and decide what to do with the rest of this PR separately.

[dnhatn]

@jasontedor I opened #28812 to replace log4j.Supplier by jdk’s in non-logging usage.

My preference is that we leave this as-is.

I am fine to close this. I don't have any objection here.

[dnhatn]

As discussed with Yannick, this PR can be proceeded (after #28969).
@jasontedor, Would you please give it another go. Thank you!

[jasontedor]

LGTM.

[dnhatn]

@jasontedor Thanks for reviewing!