Hi Dongjin, Thanks for working on the update to log4j2, it's definitively something we should complete. I have a couple of comments:
1) Is the KIP proposing to replace the existing log4-appender or simply add a new one for log4j2? Reading the KIP and with its current title, it's not entirely explicit. For example I don't see a statement under the proposed changes section. The PR seems to only add a new appender but the KIP mentions we want to fully remove dependencies to log4j. 2) Under Rejected Alternative, the KIP states: "the Kafka appender provided by log4j2 community stores log message in the Record key". Looking at the code, it looks like the log message is stored in the Record value: https://github.com/apache/logging-log4j2/blob/master/log4j-kafka/src/main/java/org/apache/logging/log4j/kafka/appender/KafkaManager.java#L135 Am I missing something? Comparing it with the proposed new appender, apart from their configuration format (hence the backwards compatibility issues), they both work pretty much the same way, so it's not clear it would add a ton a value. At a glance, _I've not extensively looked at it_, it does not look very hard to migrate to the appender from the logging team. I was wondering if we should mention it in our documentation but I was not able to find any references to the log4j-appender in the Kafka docs: https://github.com/apache/kafka-site/search?q=KafkaLog4jAppender As the current log4j-appender is not even deprecated yet, in theory we can't remove it till Kafka 4. If we want to speed up the process, I wonder if the lack of documentation and a migration guide could help us. What do you think? Thanks, Mickael On Fri, Jun 11, 2021 at 4:57 PM Boojapho O <booja...@gmail.com> wrote: > > Continuing to use log4j would leave several known security vulnerabilities in > Apache Kafka, including https://nvd.nist.gov/vuln/detail/CVE-2019-17571. The > Apache log4j team will not fix this vulnerability and is urging an upgrade to > log4j2. See https://logging.apache.org/log4j/1.2/ for further information. > > This is desperately needed in Apache 3.0 to keep the software secure. > > On 2021/05/26 12:31:20, Dongjin Lee <dong...@apache.org> wrote: > > CC'd the +1ers of KIP-653 with detailed context: > > > > When I submitted and got the approval of KIP-653: Upgrade log4j to log4j2 > > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-653%3A+Upgrade+log4j+to+log4j2>, > > I thought the log4j2-appender should not be the scope of the work. But it > > was wrong. > > > > Since the VerifiableLog4jAppender tool is built upon log4j-appender, log4j > > 1.x artifact will co-exist with log4j2 artifact in the classpath within > > this scheme. Since the log4j 1.x code is not called anymore, I thought it > > is not problematic but actually, it was not - when I started to provide a > > preview of KIP-653 > > <http://home.apache.org/~dongjin/post/apache-kafka-log4j2-support/>, some > > users reported that sometimes slf4j fails to find the appropriate binding > > within the classpath, resulting fail to append the log message. > > > > To resolve this problem, I subtly adjusted the scope of the work; I > > excluded Tools and Trogdor from KIP-653 and extended KIP-719 to take care > > of them instead, along with providing log4j2-appender. It is why the > > current WIP implementations include some classpath logic in the shell > > script and *why KIP-653 only can't complete the log4j2 migration*. > > > > I hope you will check this proposal out. > > > > Best, > > Dongjin > > > > On Tue, May 25, 2021 at 10:43 PM Dongjin Lee <dong...@apache.org> wrote: > > > > > Bumping up the discussion thread. > > > > > > Recently, I updated the document of KIP-653: Upgrade log4j to log4j2 > > > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-653%3A+Upgrade+log4j+to+log4j2> > > > (accepted) > > > and KIP-719: Add Log4J2 Appender > > > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-719%3A+Add+Log4J2+Appender> > > > (under > > > discussion) reflecting the recent changes to our codebase. Especially: > > > > > > 1. KIP-653 document > > > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-653%3A+Upgrade+log4j+to+log4j2> > > > now > > > explains which modules will be migrated and why. > > > 2. KIP-719 document > > > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-719%3A+Add+Log4J2+Appender> > > > now > > > explains not only the log4j2-appender plan but also upgrading the omitted > > > modules in KIP-653 into log4j2. > > > > > > As you can see here, those two KIPs are the different parts of the same > > > problem. I believe the community will have a good grasp on why both KIPs > > > are best if released altogether. > > > > > > I will open the voting thread now, and please leave a vote if you are > > > interested in this issue. > > > > > > Best, > > > Dongjin > > > > > > On Tue, Mar 2, 2021 at 5:00 PM Dongjin Lee <dong...@apache.org> wrote: > > > > > >> Hi Kafka dev, > > >> > > >> I would like to start the discussion of KIP-719: Add Log4J2 Appender. > > >> > > >> > > >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-719%3A+Add+Log4J2+Appender > > >> > > >> All kinds of feedbacks are greatly appreciated! > > >> > > >> Best, > > >> Dongjin > > >> > > >> -- > > >> *Dongjin Lee* > > >> > > >> *A hitchhiker in the mathematical world.* > > >> > > >> > > >> > > >> *github: <http://goog_969573159/>github.com/dongjinleekr > > >> <https://github.com/dongjinleekr>keybase: https://keybase.io/dongjinleekr > > >> <https://keybase.io/dongjinleekr>linkedin: > > >> kr.linkedin.com/in/dongjinleekr > > >> <https://kr.linkedin.com/in/dongjinleekr>speakerdeck: > > >> speakerdeck.com/dongjin > > >> <https://speakerdeck.com/dongjin>* > > >> > > > > > > > > > -- > > > *Dongjin Lee* > > > > > > *A hitchhiker in the mathematical world.* > > > > > > > > > > > > *github: <http://goog_969573159/>github.com/dongjinleekr > > > <https://github.com/dongjinleekr>keybase: https://keybase.io/dongjinleekr > > > <https://keybase.io/dongjinleekr>linkedin: kr.linkedin.com/in/dongjinleekr > > > <https://kr.linkedin.com/in/dongjinleekr>speakerdeck: > > > speakerdeck.com/dongjin > > > <https://speakerdeck.com/dongjin>* > > > > > > > > > -- > > *Dongjin Lee* > > > > *A hitchhiker in the mathematical world.* > > > > > > > > *github: <http://goog_969573159/>github.com/dongjinleekr > > <https://github.com/dongjinleekr>keybase: https://keybase.io/dongjinleekr > > <https://keybase.io/dongjinleekr>linkedin: kr.linkedin.com/in/dongjinleekr > > <https://kr.linkedin.com/in/dongjinleekr>speakerdeck: > > speakerdeck.com/dongjin > > <https://speakerdeck.com/dongjin>* > >
