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

Initialise ES logging in CLI #97353

Merged
merged 3 commits into from
Jul 4, 2023
Merged

Conversation

pgomulka
Copy link
Contributor

@pgomulka pgomulka commented Jul 4, 2023

ES logging has to be explicitly initialised with a call to LoggerFactory.setInstance This is normally done as LogConfigurator.configure(Environment,boolean) is calling this as part of initPhase1

However CLI tools were not using that method. Cli tools are using LogConfigurator.configureWithoutConfig and that method was not setting up ES logging

This commit modifies configureWithoutConfig to also configure esLogging It also adds a LogConfigurator.configureESLogging call explicitly in CliToolLauncher This allows to build a new cli-launcher.jar and replace it in previous 8.7-8.9 clusters

closes #97350

ES logging has to be explicitely initialised with a call to LoggerFactory.setInstance
This is normally done as LogConfigurator.configure(Environment,boolean) is calling this as part of
initPhase1

However CLI tools were not using that method. Cli tools are using LogConfigurator.configureWithoutConfig
and that method was not setting up ES logging

This commit modifies configureWithoutConfig to also configure esLogging
It also addes a LogConfigurator.configureESLogging explicitly in CliToolLauncher
This allows to build a new cli-launcher.jar and replace it in previous 8.7-8.9 clusters

closes elastic#97350
@pgomulka pgomulka self-assigned this Jul 4, 2023
@pgomulka pgomulka added :Core/Infra/Logging Log management and logging utilities :Core/Infra/CLI CLI utilities, scripts, and infrastructure labels Jul 4, 2023
@elasticsearchmachine elasticsearchmachine added Team:Core/Infra Meta label for core/infra team v8.10.0 labels Jul 4, 2023
@elasticsearchmachine
Copy link
Collaborator

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

@pgomulka
Copy link
Contributor Author

pgomulka commented Jul 4, 2023

as an improvement we should also return some empty logging implementation


if the loggerFactor was not set, we should perhaps default to not logging implementation

@thecoop
Copy link
Member

thecoop commented Jul 4, 2023

I think defaulting to an empty implementation is sensible, that way we avoid a nasty exception if we get it wrong in the future

@thecoop thecoop added the >bug label Jul 4, 2023
@elasticsearchmachine
Copy link
Collaborator

Hi @pgomulka, I've created a changelog YAML for you.

@pgomulka
Copy link
Contributor Author

pgomulka commented Jul 4, 2023

@elasticmachine run elasticsearch-ci/part-1

@pgomulka
Copy link
Contributor Author

pgomulka commented Jul 4, 2023

@elasticmachine run elasticsearch-ci/docs-check

@rdnm rdnm added the blocker label Jul 4, 2023
@pgomulka pgomulka added auto-backport-and-merge Automatically create backport pull requests and merge when ready and removed :Core/Infra/Logging Log management and logging utilities labels Jul 4, 2023
@pgomulka pgomulka merged commit 3e804bd into elastic:main Jul 4, 2023
12 checks passed
pgomulka added a commit to pgomulka/elasticsearch that referenced this pull request Jul 4, 2023
ES logging has to be explicitly initialised with a call to LoggerFactory.setInstance This is normally done as LogConfigurator.configure(Environment,boolean) is calling this as part of initPhase1

However CLI tools were not using that method. Cli tools are using LogConfigurator.configureWithoutConfig and that method was not setting up ES logging

This commit modifies configureWithoutConfig to also configure esLogging It also adds a LogConfigurator.configureESLogging call explicitly in CliToolLauncher This allows to build a new cli-launcher.jar and replace it in previous 8.7-8.9 clusters

closes elastic#97350
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.9

elasticsearchmachine pushed a commit that referenced this pull request Jul 4, 2023
ES logging has to be explicitly initialised with a call to LoggerFactory.setInstance This is normally done as LogConfigurator.configure(Environment,boolean) is calling this as part of initPhase1

However CLI tools were not using that method. Cli tools are using LogConfigurator.configureWithoutConfig and that method was not setting up ES logging

This commit modifies configureWithoutConfig to also configure esLogging It also adds a LogConfigurator.configureESLogging call explicitly in CliToolLauncher This allows to build a new cli-launcher.jar and replace it in previous 8.7-8.9 clusters

closes #97350
@rjernst rjernst removed the v8.9.1 label Jul 8, 2023
pgomulka added a commit to pgomulka/elasticsearch that referenced this pull request Jul 10, 2023
ES logging has to be explicitly initialised with a call to LoggerFactory.setInstance This is normally done as LogConfigurator.configure(Environment,boolean) is calling this as part of initPhase1

However CLI tools were not using that method. Cli tools are using LogConfigurator.configureWithoutConfig and that method was not setting up ES logging

This commit modifies configureWithoutConfig to also configure esLogging It also adds a LogConfigurator.configureESLogging call explicitly in CliToolLauncher This allows to build a new cli-launcher.jar and replace it in previous 8.7-8.9 clusters

closes elastic#97350
elasticsearchmachine pushed a commit that referenced this pull request Jul 10, 2023
ES logging has to be explicitly initialised with a call to LoggerFactory.setInstance This is normally done as LogConfigurator.configure(Environment,boolean) is calling this as part of initPhase1

However CLI tools were not using that method. Cli tools are using LogConfigurator.configureWithoutConfig and that method was not setting up ES logging

This commit modifies configureWithoutConfig to also configure esLogging It also adds a LogConfigurator.configureESLogging call explicitly in CliToolLauncher This allows to build a new cli-launcher.jar and replace it in previous 8.7-8.9 clusters

closes #97350
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport-and-merge Automatically create backport pull requests and merge when ready blocker >bug :Core/Infra/CLI CLI utilities, scripts, and infrastructure Team:Core/Infra Meta label for core/infra team v8.8.3 v8.9.0 v8.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ES CLI elasticsearch-shard throws exception on startup
5 participants