Fix incorrect variable placeholder for Strings.format calls

[ywangd]

The curly bracket placeholder works for LoggerMessageFormat.format and ParameterizedMessage.format, but Not for Strings.format which requires Java's string format syntax. This PR fixes the incorrect usages.

Relates: #86549, #87924

PS: I labelled this >non-issue because the changes are all for debug/trace level loggings.

[elasticsearchmachine]

Pinging @elastic/es-core-infra (Team:Core/Infra)

[ywangd]

@pgomulka We have choice to use either of the following four static format methods

1. LoggerMessageFormat.format
2. ParameterizedMessage.format
3. Strings.format
4. String.format

The first two support {} as variable placeholder while the last two do not. It is rather confusing at times, especially when the methods are statically imported and used without their class qualifier. Do we need to do something so that misuse of placeholders is less likely in future?

[pgomulka]

@ywangd thank you for fixing this. Ideally we should use org.elasticsearch.core.Strings.format. Do you think you could also change the imports?
That method already populates Locale.ROOT and handles exceptions when format is incorrect.

Agree we need some static analysis tool to fix the incorrect usages of logger formatting. Unfortunately ESLoggerUsageChecker does not check this at the moment. I have this on a list as a possible refactoring during the tech debt week.

[n1v0lg]

Thank you for fixing this!

I think replacing static imports i.e., format() with Strings.format(...) would be nice (I think this is what @pgomulka is also suggesting).

[ywangd]

@pgomulka All usages of the format method in this PR is org.elasticsearch.core.Strings.format. Could you please clarify what you want me to do? Are you suggesting another sweep across the code base to change any usage of java.lang.String.format to org.elasticsearch.core.Strings.format? Or are you suggesting we should avoid static imports and the format method should be called like Strings.format(...)?

Agree we need some static analysis tool to fix the incorrect usages of logger formatting. Unfortunately ESLoggerUsageChecker does not check this at the moment. I have this on a list as a possible refactoring during the tech debt week.

It's great to know this is on your (team)'s radar. If we don't want use java.lang.String.format, maybe we can also add it to the list of forbiddenApis.

[pgomulka]

@ywangd appologies, I noticed the String.format usage in JavaModulePrecommitTask and I thought that this PR has a mixture of different .format methods. All is good. thanks