Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Isolate console appender startup logic #86243

Merged
merged 2 commits into from
Apr 28, 2022

Conversation

rjernst
Copy link
Member

@rjernst rjernst commented Apr 28, 2022

When bootstrapping Elasticsearch, the console appender is automatically
added by our log4j config. However, there are some cases where we want
to remove that appender. First, if --quiet is passed, we do not want to
log anything to it. Second, if we are daemonizing, then we want to close
the streams, so we need to remove it if it exists. Third, when huge
guice/startup exceptions occur, we log these only to the ES log file, so
we need to remove the appender temporarily.

This commit moves the logic for mucking with the console appender into
LogConfigurator. In the future this can be better isolated within
logging, perhaps even avoiding creating the console appender to begin
with when using --quiet, but for now this at least gets some log specific
logic out of bootstrap.

relates #85758

When bootstrapping Elasticsearch, the console appender is automatically
added by our log4j config. However, there are some cases where we want
to remove that appender. First, if --quiet is passed, we do not want to
log anything to it. Second, if we are daemonizing, then we want to close
the streams, so we need to remove it if it exists. Third, when huge
guice/startup exceptions occur, we log these only to the ES log file, so
we need to remove the appender temporarily.

This commit moves the logic for mucking with the console appender into
LogConfigurator. In the future this can be better isolated within
logging, perhaps even avoiding creating the console appender to begin
with when using --quiet, but for now this at least gets some log specific
logic out of bootstrap.

relates elastic#85758
@rjernst rjernst added :Core/Infra/Logging Log management and logging utilities >refactoring v8.3.0 labels Apr 28, 2022
@rjernst rjernst requested a review from pgomulka April 28, 2022 00:31
@elasticmachine elasticmachine added the Team:Core/Infra Meta label for core/infra team label Apr 28, 2022
@elasticmachine
Copy link
Collaborator

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

Copy link
Contributor

@pgomulka pgomulka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@rjernst rjernst merged commit d742be8 into elastic:master Apr 28, 2022
@rjernst rjernst deleted the cli/logging_startup branch April 28, 2022 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Logging Log management and logging utilities >refactoring Team:Core/Infra Meta label for core/infra team v8.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants