Happy New Year, and thank you for driving this, Andor! I am somewhat hesitant about switching direction to Logback: - First, I agree with points already raised by Christopher. - A major reason that my prior work to migrate to Log4J 2 in ZOOKEEPER-2342 stalled out years ago is backward-incompatibility of the logging configuration files. However, I've just learned that Log4J 2 has added support for compatibility with Log4J 1 style configuration files, so it appears this blocker is now resolved. [1] (I have yet to test the compatibility feature myself.) - I don't think Logback supports compatibility with Log4J style configuration. (Please correct me if I'm wrong.) If we previously considered it important to preserve compatibility of configuration for system administrators, then it seems we are abandoning that goal now. Perhaps the escape hatch of swapping out the SLF4J provider during deployment is sufficient to address this. - It has taken a long time, but it appears that the wider big data ecosystem is coming around to Log4J 2. Discussion has renewed in Hadoop. Recent HBase releases use Log4J 2. Spark recently committed a Log4J 2 migration patch for inclusion in a future release. While integration with the big data ecosystem isn't the sole use case for Zookeeper, it's definitely a major one, and I think it's beneficial for big data deployments to have commonality in the logging infrastructure. - Logback is dual-licensed under LGPL and EPL. [2] LGPL is a category X license that would prevent shipping in Apache releases. [3] EPL is a category B license considered acceptable for inclusion. [4] I've personally not dealt with a dual-licensing situation like this before, but my intuition is that we need careful handling in NOTICES.txt to indicate that we choose to adopt the terms of EPL, not LGPL.
I don't mean to impede progress, and I don't intend to -1 a Logback patch if that's the overall community preference. I've already started code reviewing ZOOKEEPER-4427. I would only ask that we also provide clear documentation for administrators who want to swap to the Log4J 2 provider for compatibility with their existing configuration. Thank you again, Andor. The ZOOKEEPER-4427 patch is looking good so far! [1] https://logging.apache.org/log4j/2.x/manual/migration.html [2] https://github.com/qos-ch/logback/blob/master/LICENSE.txt [3] https://www.apache.org/legal/resolved.html#category-x [4] https://www.apache.org/legal/resolved.html#category-b Chris Nauroth On Wed, Jan 5, 2022 at 7:14 AM Andor Molnar <an...@apache.org> wrote: > Hi folks, > > Happy New Year! > > Logback patch is now ready for review: > https://github.com/apache/zookeeper/pull/1793 > > Thanks, > Andor > > > > > On 2021. Dec 21., at 20:44, Brent <brentwritesc...@gmail.com> wrote: > > > > Thank you for the details Andor. It sounds like you have a good plan in > > place for doing the migration. > > > > I had some open work against ZooInspector that I wanted to do, so it > sounds > > like I'd be best focusing my efforts there and leaving this to you. > > > > Thanks for your time and help! > > > > ~Brent > > > > On Tue, Dec 21, 2021 at 3:27 AM Enrico Olivelli <eolive...@gmail.com> > wrote: > > > >> Andor, > >> > >> Il giorno mar 21 dic 2021 alle ore 12:25 Andor Molnar <an...@apache.org > > > >> ha > >> scritto: > >> > >>> 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. > >>> > >> > >> Thanks for the update > >> > >> My experience in "embedding" ZK jars in other products is the same, we > are > >> using slf4j, so we can switch provider very easily > >> > >> looking forward for the patch > >> > >> Enrico > >> > >> > >>> > >>> 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 > >>>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> > >>>>>>>>> > >>>>>>>>> > >>>>>>> > >>>>>>> > >>>>>> > >>>>>> > >>> > >>> > >> > >
