I believe that Chris points are valid. My understanding, especially after seeing Andor's patch is that we are depending on an implementation in two cases: - in some tests (we grab the logs) - for the binary tarbal (for real users if the server)
If we could rely on slf4j-simple for the tests (or write a little mock binding) then switching will be only a matter of changing the slf4j implementation and deal with the configuration files. I appreciate a lot Andor's work, and the fact that we are finally addressing this long standing issue. Andor, What do you think about moving to log4j2? Personally I don't have much time next week to help, so I am +1 to commit Andor's patch and get rid of log4j1. Enrico Il Gio 6 Gen 2022, 20:38 Chris Nauroth <cnaur...@apache.org> ha scritto: > 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 > > >>>>>>>>>>>>> > > >>>>>>>>>>>> > > >>>>>>>>>>>> > > >>>>>>>>>> > > >>>>>>>>>> > > >>>>>>>>> > > >>>>>>>>> > > >>>>>>> > > >>>>>>> > > >>>>>> > > >>>>>> > > >>> > > >>> > > >> > > > > >
