Fix status logger message at startup #22687
Conversation
…It basically means that no settings should be used before Log4j's status logger has been configured without configuration by LogConfigurator.configureWithoutConfig() method.
tlrx
added
:Core/Infra/Logging
There was a problem hiding this comment.
This is unfortunate, and the situation feels very fragile to me now. I looked at the call-sites for Settings#getAsBooleanLenientForPreEs6Indices and it looks like in all places we have a deprecation logger that we can push down into this method and just use that logger rather than a static logger on the Settings class. This means that the logging message would also contain the class name where the setting is coming from, rather than a generic o.e.d.c.s.Settings as the logger name. I think that is more helpful for this situation than just saying "only_longest_match" is deprecated as it doesn't tell you where it's coming from. What do you think @tlrx?
| @@ -430,7 +430,7 @@ You can also use Gradle to prepare the test environment and then starts a single | |||
| gradle vagrantFedora24#up | |||
| ------------------------------------------------- | |||
|
|
|||
| Or any of vagrantCentos6#up, vagrantDebian8#up, vagrantFedora24#up, vagrantOel6#up, | |||
| Or any of vagrantCentos6#up, vagrantDebian8#up, vagrantCentos7#up, vagrantOel6#up, | |||
| */ | ||
| public static void configureWithoutConfig(final Settings settings) { | ||
| Objects.requireNonNull(settings); | ||
| public static void configureWithoutConfig(final String loggerLevel) { |
There was a problem hiding this comment.
This situation feels very fragile to me. It means in the future if we do want a settings instance here because we want to control fine-grained logging settings for a CLI tool, we can not without accidentally reintroducing the problem.
|
@jasontedor I fully agree with your concerns. I was happy enough to have found the cause of this regression but this PR is not the long term solution we must have. So I'm +1 on closing this and maybe @danielmitterdorfer who worked on #22200 can implement your suggestion? |
@danielmitterdorfer Will you fix this along the lines I described? |
|
I discussed this with @danielmitterdorfer via another channel and have opened #22696 as a result of that conversation. |
|
Thanks for tracking down the cause of the issue @tlrx. |
|
Superseded by #22696 |
Since #22200 the
Settingsclass now uses a staticDeprecationLogger. This makes Log4j2 status logger yells when Elasticsearch starts with the messageERROR StatusLogger No log4j2 configuration file found. Using default configurationbecause this deprecation logger is instantiated before the status logger has been configured.This commit changes the
LogConfigurator.configureWithoutConfig()method so that the status logger is initialized without using anySettings, making the message disappear.