I just noticed that Patrick commented on ZOOKEEPER-2342 expressing doubt about sticking with Log4J, so I want to make sure his perspective gets included in our thread here. Specifically added him to the To: line.
Chris Nauroth On Thu, Jan 6, 2022 at 11:16 PM Enrico Olivelli <eolive...@gmail.com> wrote: > 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 > > > >>>>>>>>>>>>> > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> > > > >>>>>>>>>> > > > >>>>>>>>>> > > > >>>>>>>>> > > > >>>>>>>>> > > > >>>>>>> > > > >>>>>>> > > > >>>>>> > > > >>>>>> > > > >>> > > > >>> > > > >> > > > > > > > > >
