Thanks for the feedback Brent.

I currently work on the logback patch and identified the following steps for 
migration:
- Replace log4j references with logback counterparts in pom.xml,
- Refactor unit tests which depend on log4j: they create a custom 
ByteArrayOutputStream for capturing log messages. I need to dig into logback 
implementation for this, but not the end of the world.
- Convert log4j.properties files to logback.xml. The online translator 
(https://logback.qos.ch/translator/) is handy, but not perfect, so this step 
also needs some manual work.

I’ll probably skip the migration of zookeeper-contrib projects to save some 
time. If the community accepts the change, I’ll create further patches to 
polish off everything.

Notice that there’s literally no code change is needed in ZK main codebase 
which I think is awesome. The bottleneck is the holiday season for me.

Can’t say for log4j2, I don’t have experience with that. ZK community was 
always reluctant to take that step, perhaps for a reason.

Andor




> On 2021. Dec 20., at 18:02, Brent <brentwritesc...@gmail.com> wrote:
> 
> In case it helps, I did a quick run over the weekend of all the places I
> see "Log4j" mentioned in code and documentation.  This is a naive search so
> not all of these references are necessarily of equal impact, but I thought
> it might give some context to the scope of the change.  It also seems like
> maybe some pieces of the project could be migrated independently of others
> rather than a "big bang" change to everything.
> 
> ~Brent
> 
> zookeeper/bin/zkCleanup.sh
> zookeeper/bin/zkCli.cmd
> zookeeper/bin/zkCli.sh
> zookeeper/bin/zkEnv.cmd
> zookeeper/bin/zkEnv.sh
> zookeeper/bin/zkServer.cmd
> zookeeper/bin/zkServer.sh
> 
> zookeeper/conf/log4j.properties
> 
> zookeeper/zookeeper-contrib/zookeeper-contrib-fatjar/pom.xml
> 
> zookeeper/zookeeper-contrib/zookeeper-contrib-loggraph/pom.xml
> zookeeper/zookeeper-contrib/zookeeper-contrib-loggraph/src/main/java/org/apache/zookeeper/graph/JsonGenerator.java
> zookeeper/zookeeper-contrib/zookeeper-contrib-loggraph/src/main/java/org/apache/zookeeper/graph/Log4JEntry.java
> zookeeper/zookeeper-contrib/zookeeper-contrib-loggraph/src/main/java/org/apache/zookeeper/graph/LogEntry.java
> zookeeper/zookeeper-contrib/zookeeper-contrib-loggraph/src/main/java/org/apache/zookeeper/graph/Log4JSource.java
> zookeeper/zookeeper-contrib/zookeeper-contrib-loggraph/src/main/java/org/apache/zookeeper/graph/MergedLogSource.java
> zookeeper/zookeeper-contrib/zookeeper-contrib-loggraph/src/main/resources/loggraph-dev.sh
> zookeeper/zookeeper-contrib/zookeeper-contrib-loggraph/src/main/resources/webapp/org/apache/zookeeper/graph/log4j.properties
> zookeeper/zookeeper-contrib/zookeeper-contrib-loggraph/src/test/java/org/apache/zookeeper/graph/servlets/ThroughputTest.java
> 
> zookeeper/zookeeper-contrib/zookeeper-contrib-rest/build.xml
> zookeeper/zookeeper-contrib/zookeeper-contrib-rest/ivy.xml
> zookeeper/zookeeper-contrib/zookeeper-contrib-rest/conf/log4j.properties
> zookeeper/zookeeper-contrib/zookeeper-contrib-rest/pom.xml
> 
> zookeeper/zookeeper-contrib/zookeeper-contrib-zkfuse/src/log4cxx.properties
> 
> zookeeper/zookeeper-contrib/zookeeper-contrib-zooinspector/build.xml
> zookeeper/zookeeper-contrib/zookeeper-contrib-zooinspector/ivy.xml
> zookeeper/zookeeper-contrib/zookeeper-contrib-zooinspector/src/main/resources/log4j.properties
> zookeeper/zookeeper-contrib/zookeeper-contrib-zooinspector/pom.xml
> zookeeper/zookeeper-contrib/zookeeper-contrib-zooinspector/TODO
> 
> zookeeper/zookeeper-docs/src/main/resources/markdown/releasenotes.md
> zookeeper/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md
> zookeeper/zookeeper-docs/src/main/resources/markdown/zookeeperAuditLogs.md
> zookeeper/zookeeper-docs/src/main/resources/markdown/zookeeperInternals.md
> zookeeper/zookeeper-docs/src/main/resources/markdown/zookeeperJMX.md
> zookeeper/zookeeper-docs/src/main/resources/markdown/zookeeperStarted.md
> zookeeper/zookeeper-docs/src/main/resources/markdown/zookeeperTools.md
> 
> zookeeper/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/resources/log4j.properties
> 
> zookeeper/zookeeper-server/pom.xml
> zookeeper/zookeeper-server/src/main/java/org/apache/zookeeper/audit/Log4jAuditLogger.java
> zookeeper/zookeeper-server/src/main/java/org/apache/zookeeper/audit/ZKAuditProvider.java
> zookeeper/zookeeper-server/src/main/java/org/apache/zookeeper/jmx/ManagedUtil.java
> zookeeper/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeerMain.java
> zookeeper/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServerMain.java
> zookeeper/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooTrace.java
> zookeeper/zookeeper-server/src/main/resources/NOTICE.txt
> zookeeper/zookeeper-server/src/test/java/org/apache/zookeeper/audit/Log4jAuditLoggerTest.java
> zookeeper/zookeeper-server/src/test/java/org/apache/zookeeper/audit/StandaloneServerAuditTest.java
> zookeeper/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerMainMultiAddressTest.java
> zookeeper/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java
> zookeeper/zookeeper-server/src/test/java/org/apache/zookeeper/test/ReadOnlyModeTest.java
> zookeeper/zookeeper-server/src/test/java/org/apache/zookeeper/test/ReconfigExceptionTest.java
> zookeeper/zookeeper-server/src/test/resources/log4j.properties
> 
> zookeeper/zookeeper-recipes/zookeeper-recipes-election/build.xml
> 
> zookeeper/zookeeper-recipes/zookeeper-recipes-lock/build.xml
> 
> zookeeper/zookeeper-recipes/zookeeper-recipes-queue/build.xml
> 
> zookeeper/owaspSuppressions.xml
> zookeeper/pom.xml
> 
> On Sat, Dec 18, 2021 at 9:33 PM Brent <brentwritesc...@gmail.com> wrote:
> 
>> Apologies if this is repeated information (I sent some of this to the user@
>> mailing list).
>> 
>> I understand the arguments for/against Log4j 1.x and won't repeat them all
>> here.  It seems like there's still some debate between Log4j2 vs. Logback
>> too.  Does anyone have a feel for how much effort either of these
>> conversions/upgrades/patches would be (hours? days? weeks?)?  Would you all
>> be open to some pull requests to help move the conversation forward?
>> 
>> I'm asking because I know some more cautious organizations are currently
>> taking action to attempt to mitigate existing ZK installations on their own
>> (opinions on 1.x aside, it's happening).  Some of those organizations are
>> also on much older versions of ZK too so there's also the question of which
>> versions are worth updating in addition to 3.8 (3.4? 3.5? 3.6? 3.7?).
>> 
>> I know everyone is pressed for time and I'm looking for ways to help.  I'd
>> be happy to try to pitch in if it would be useful at all.  I just want to
>> make sure I'd be focusing my effort in the right direction.
>> 
>> Regardless, thanks for all the time & effort you all put in on the
>> project, it's very much appreciated.
>> 
>> ~Brent
>> 
>> On Wed, Dec 15, 2021 at 1:50 PM Andor Molnar <an...@apache.org> wrote:
>> 
>>> Gosh, we have a few unit tests with log4j specific code.
>>> I need some free cycles to refactor them properly.
>>> 
>>> Andor
>>> 
>>> 
>>> 
>>> 
>>>> On 2021. Dec 15., at 14:11, Andor Molnar <an...@apache.org> wrote:
>>>> 
>>>> Agreed. My choice is not based on the recent vulnerabilities. There
>>>> probably more to come by the way, so this is not the best timing for
>>>> log4j2.
>>>> 
>>>> Anyway, the main advantage I see for logback is that it's closer to
>>>> log4j1, hence probably easier to migrate to.
>>>> 
>>>> ZooKeeper already uses SLF4j so, as you suggested, we should follow the
>>>> facade / default logging backend approach. Though I believe logback is
>>>> better for the default. Sometimes less is more and in terms of
>>>> vulnerabilities less code has less chance for bugs. If logback has all
>>>> the features which ZooKeeper needs, I think we should choose that.
>>>> 
>>>> Andor
>>>> 
>>>> 
>>>> 
>>>> On Wed, 2021-12-15 at 07:41 -0500, Christopher wrote:
>>>>> I think it would be a mistake to use the recently reported
>>>>> vulnerability as a basis for migrating to logback. Any dependency can
>>>>> have a vulnerability, and logback is not substantially different. No
>>>>> dependency is going to be guaranteed vulnerability-free. Switching on
>>>>> that basis is a wild goose chase. What is important is that people
>>>>> respond to vulnerabilities by updating/patching in a timely manner.
>>>>> 
>>>>> Also, it is my understanding that log4j2 is the evolution of logback
>>>>> and slf4j, incorporating the original enhancements that logback had
>>>>> made as a standard slf4j implementation and incorporating them back
>>>>> into log4j code, as well as providing a lot of additional very useful
>>>>> features and a huge amount of configuration flexibility. Although
>>>>> logback is probably still suitable, log4j2 seems to be much more
>>>>> active, and where the mainline development for Java logging is
>>>>> happening. Moving to logback from log4j2 seems like a step backwards.
>>>>> 
>>>>> Most importantly, though, the actual runtime logging implementation
>>>>> should be independent from ZooKeeper project development. This
>>>>> project
>>>>> should use slf4j as a logging facade exclusively, and users should be
>>>>> able to use whatever slf4j runtime implementation they want. If
>>>>> ZooKeeper wants to choose a simple implementation, it shouldn't use
>>>>> logback, but should use slf4j-simple instead. However, I think it
>>>>> makes more sense to keep log4j2 at runtime for the slf4j
>>>>> implementation. Users can still change it out for whatever they want.
>>>>> There's no need to take action to replace the runtime implementation
>>>>> for slf4j, because users can do that if they want... as long as the
>>>>> project itself limits its logging to using the slf4j API.
>>>>> 
>>>>> On Wed, Dec 15, 2021 at 6:46 AM Andor Molnar <an...@apache.org>
>>>>> wrote:
>>>>>> 
>>>>>> https://issues.apache.org/jira/browse/ZOOKEEPER-4427
>>>>>> 
>>>>>> 
>>>>>> On Wed, 2021-12-15 at 12:35 +0100, Andor Molnar wrote:
>>>>>>> Sure. I'll take care of that, but first things first. Look what
>>>>>>> I've
>>>>>>> found when checking the history of the issue.
>>>>>>> 
>>>>>>> Thumbs-up from Ceki back from 2016:
>>>>>>> 
>>>>>>> 
>>> https://issues.apache.org/jira/browse/ZOOKEEPER-2342?focusedCommentId=15207288&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-15207288
>>>>>>> 
>>>>>>> What else do we need? :)
>>>>>>> 
>>>>>>> Andor
>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>> On Wed, 2021-12-15 at 12:07 +0100, Enrico Olivelli wrote:
>>>>>>>> +1
>>>>>>>> 
>>>>>>>> Would you like to submit a PR ?
>>>>>>>> Then we can release 3.8.0
>>>>>>>> 
>>>>>>>> Enrico
>>>>>>>> 
>>>>>>>> Il giorno mer 15 dic 2021 alle ore 12:04 Flavio Junqueira
>>>>>>>> <f...@apache.org>
>>>>>>>> ha scritto:
>>>>>>>> 
>>>>>>>>> We use logback in Pravega, it works fine for us. I'd be ok
>>>>>>>>> with the
>>>>>>>>> change.
>>>>>>>>> 
>>>>>>>>> -Flavio
>>>>>>>>> 
>>>>>>>>>> On 15 Dec 2021, at 12:02, Andor Molnar <an...@apache.org>
>>>>>>>>>> wrote:
>>>>>>>>>> 
>>>>>>>>>> Hi ZK folks,
>>>>>>>>>> 
>>>>>>>>>> What do you think about migrating ZK to logback?
>>>>>>>>>> The idea just crossed my mind due to the recent turbulence
>>>>>>>>>> with
>>>>>>>>>> log4j.
>>>>>>>>>> 
>>>>>>>>>> Checking some migrating guides, it doesn’t seem the end of
>>>>>>>>>> the
>>>>>>>>>> world.
>>>>>>>>>> 
>>>>>>>>>> Andor
>>>>>>>>>> 
>>>>>>>>> 
>>>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>> 
>>>>>> 
>>>> 
>>>> 
>>> 
>>> 

Reply via email to