ZOOKEEPER-4241: Change log level without restarting zookeeper#1637
ZOOKEEPER-4241: Change log level without restarting zookeeper#1637arshadmohammad wants to merge 2 commits intoapache:masterfrom
Conversation
|
is this feature based on log4j 1 ? |
Yes |
|
I believe we must drop log4j1 soon, because it is EOL and unsupported and it has a few CVS. |
AFAIK, moving to log4j2 is in discussion from long time but for one or other reasons getting stuck. This change I have tried to make it very independent, there is only one line change in main zookeeper code. So whenever migration to log4j2 happens this feature can be easily removed, also these capabilities are already there in log4j2. There will no need to have this feature. Anyway we have to remove it. But until migration to log4j2 happens, can we keep this? What you suggest? |
arshadmohammad
force-pushed
the
ZOOKEEPER-4241-master
branch
from
9fd2af6 to
1794a45
Compare
|
I think it would be more constructive to focus on furthering the migration to log4j2. This is a built-in feature of log4j2 that is easy to maintain. This change, on the other hand, further entrenches the direct use of log4j1 in the code, rather than the preferred slf4j logging facade. |
|
There was a problem hiding this comment.
I don't support adding this either especially if log4j2 has built-in support for it. What's the latest with log4j2 migration? Where are we stand?
Also removing a feature is dangerous and not backward-compatible. If we introduce this here, we'll have to continue supporting it in future versions or - once log4j migration happened - create some mechanism to transforms these settings into log4j2 config somehow.
| BufferedWriter bw = null; | ||
| try { | ||
| // log4j configuration file is modified from classpath, not from the resource folder | ||
| inputStream = getClass().getResourceAsStream(log4jConfigurationFile); |
There was a problem hiding this comment.
I detect that this code is problematic. According to the Bad practice (BAD_PRACTICE), UI: Usage of GetResource may be unsafe if class is extended (UI_INHERITANCE_UNSAFE_GETRESOURCE).
Calling this.getClass().getResource(...) could give results other than expected if this class is extended by a class in another package.
5 participants
Added 3 new configurations
log4j.configuration.watch
log4j.configuration.watch.interval
log4j.configuration
By default feature is disabled, configure log4j.configuration.watch=true in zoo.cfg to enable the feature.
When this feature is enabled, ZooKeeper server log levels can be changed without restarting the servers by just modifying the log4j configuration properties.