elastic/elasticsearch

Two ways to define root logger level - should we choose one?

Open

#17,320 opened on Mar 24, 2016

View on GitHub
 (9 comments) (0 reactions) (0 assignees)Java (25,882 forks)batch import
:Core/Infra/Logging>enhancementTeam:Core/Infrahelp wantedtriaged

Repository metrics

Stars
 (76,700 stars)
PR merge metrics
 (Avg merge 2d) (1,000 merged PRs in 30d)

Description

Currently the logger level of the root logger can be defined in two ways:

  1. set logger.level:DEBUG
  2. set logger._root: DEBUG

The first works from command line -Des.logger.level=DEBUG. It does not work with dynamic update settings like

PUT /_cluster/settings
{
    "transient" : {
        "logger.level" : "INFO"
    }
}

The latter works with dynamic update settings like

PUT /_cluster/settings
{
    "transient" : {
        "logger._root" : "INFO"
    }
}

and in test annotation for regular tests @TestLogging("_root:DEBUG"). It does not work when passing in cmd line when we start a node with -Des.logger._root=DEBUG. Consequently test annotations for bwc tests also have no effect for old version nods because here we pass the es.logger._root as cmd line arg. As a funny side effect, when we use logging annotation @TestLogging("level:DEBUG") then the old version nodes will log in DEBUG and the new version nodes not and the other way round if we set @TestLogging("_root:DEBUG")

I am unsure what to do with this. If we want to allow both options we have to fix that somewhere here before we call settingsBuilder.replacePropertyPlaceholders() https://github.com/elastic/elasticsearch/blob/2.x/core/src/main/java/org/elasticsearch/common/logging/log4j/LogConfigurator.java#L109 I think.

But it would be better to have only one way to set the logger level which would then be a bigger operation.

Contributor guide