Thanks Dongjin! I'll take a look soon. In the meantime, you may want to bump the VOTE thread too.
Best, Mickael On Sat, Dec 18, 2021 at 10:00 AM Dongjin Lee <dong...@apache.org> wrote: > > Hi Mickael, > > Finally, I did it! As you can see at the PR > <https://github.com/apache/kafka/pull/10244>, KIP-719 now uses log4j2's > Kafka appender, and log4j-appender is not used by the other modules > anymore. You can see how it will work with KIP-653 at this preview > <http://home.apache.org/~dongjin/post/apache-kafka-log4j2-support/>, based > on Apache Kafka 3.0.0. The proposal document > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-719%3A+Deprecate+Log4J+Appender> > is also updated accordingly, with its title. > > There is a minor issue on log4j2 > <https://issues.apache.org/jira/browse/LOG4J2-3256>, but it seems like it > will be resolved soon. > > Best, > Dongjin > > On Wed, Dec 15, 2021 at 9:28 PM Dongjin Lee <dong...@apache.org> wrote: > > > Hi Mickael, > > > > > Can we do step 3 without breaking any compatibility? If so then that > > sounds like a good idea. > > > > As far as I know, the answer is yes; I am now updating my PR, so I will > > notify you as soon as I complete the work. > > > > Best, > > Dongjin > > > > On Wed, Dec 15, 2021 at 2:00 AM Mickael Maison <mickael.mai...@gmail.com> > > wrote: > > > >> Hi Dongjin, > >> > >> Sorry for the late reply. Can we do step 3 without breaking any > >> compatibility? If so then that sounds like a good idea. > >> > >> Thanks, > >> Mickael > >> > >> > >> > >> On Tue, Nov 23, 2021 at 2:08 PM Dongjin Lee <dong...@apache.org> wrote: > >> > > >> > Hi Mickael, > >> > > >> > I also thought over the issue thoroughly and would like to propose a > >> minor > >> > change to your proposal: > >> > > >> > 1. Deprecate log4j-appender now > >> > 2. Document how to migrate into logging-log4j2 > >> > 3. (Changed) Replace the log4j-appender (in turn log4j 1.x) > >> dependencies in > >> > tools, trogdor, and shell and upgrade to log4j2 in 3.x, removing log4j > >> 1.x > >> > dependencies. > >> > 4. (Changed) Remove log4j-appender in Kafka 4.0 > >> > > >> > What we need to do for the log4j2 upgrade is just removing the log4j > >> > dependencies only, for they can cause a classpath error. And actually, > >> we > >> > can do it without discontinuing publishing the log4j-appender artifact. > >> So, > >> > I suggest separating the upgrade to log4j2 and removing the > >> log4j-appender > >> > module. > >> > > >> > How do you think? If you agree, I will update the KIP and the PR > >> > accordingly ASAP. > >> > > >> > Thanks, > >> > Dongjin > >> > > >> > On Mon, Nov 15, 2021 at 8:06 PM Mickael Maison < > >> mickael.mai...@gmail.com> > >> > wrote: > >> > > >> > > Hi Dongjin, > >> > > > >> > > Thanks for the clarifications. > >> > > > >> > > I wonder if a simpler course of action could be: > >> > > - Deprecate log4j-appender now > >> > > - Document how to use logging-log4j2 > >> > > - Remove log4j-appender and all the log4j dependencies in Kafka 4.0 > >> > > > >> > > This delays KIP-653 till Kafka 4.0 but (so far) Kafka is not directly > >> > > affected by the log4j CVEs. At least this gives us a clear and simple > >> > > roadmap to follow. > >> > > > >> > > What do you think? > >> > > > >> > > On Tue, Nov 9, 2021 at 12:12 PM Dongjin Lee <dong...@apache.org> > >> wrote: > >> > > > > >> > > > Hi Mickael, > >> > > > > >> > > > I greatly appreciate you for reading the proposal so carefully! I > >> wrote > >> > > it > >> > > > quite a while ago and rechecked it today. > >> > > > > >> > > > > 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. > >> > > > > >> > > > Oh, After re-reading it, I realized that this is not clear. Let me > >> > > clarify; > >> > > > > >> > > > 1. Provide a lo4j2 equivalent of traditional log4j-appender, > >> > > > log4j2-appender. > >> > > > 2. Migrate the modules depending on log4j-appender (i.e., tools, > >> trogdor, > >> > > > shell) into log4j2-appender, removing log4j-appender from > >> dependencies. > >> > > > 3. Entirely remove log4j-appender from the project dependencies, > >> along > >> > > with > >> > > > log4j. > >> > > > > >> > > > I think log4j-appender may be published for every new release like > >> > > before, > >> > > > but the committee should make a decision on the policy. > >> > > > > >> > > > > 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? > >> > > > > >> > > > It's totally my fault; I confused it with another appender. The > >> > > > compatibility problem in the logging-log4j2 Kafka appender is not > >> the > >> > > > format but the configuration. logging-log4j2 Kafka appender supports > >> > > > `properties` configuration, which will be directly used to > >> instantiate a > >> > > > Kafka producer. However, log4j-appender has been using non-producer > >> > > config > >> > > > names like brokerList (=bootstrap.servers), requiredNumAcks (=acks). > >> > > > Instead, logging-log4j2 Kafka appender supports retryCount, > >> > > > sendEventTimestamp. > >> > > > > >> > > > On second thought, using logging-log4j2 Kafka appender internally > >> and > >> > > > making log4j2-appender to focus on compatibility facade only would > >> be a > >> > > > better approach; As I described above, the goal of this module is > >> just > >> > > > keeping the backward-compatibility, and (as you pointed out) the > >> current > >> > > > implementation has little value. Since > >> > > org.apache.logging.log4j:log4j-core > >> > > > already includes Kafka appender, we can make use of the 'proven > >> wheel' > >> > > > without adding more dependencies. I have not tried it yet, but I > >> think it > >> > > > is well worth it. (One additional advantage of this approach is > >> > > providing a > >> > > > bridge to the users who hope to move from/into logging-log4j2 Kafka > >> > > > appender.) > >> > > > > >> > > > > 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? > >> > > > > >> > > > In fact, this is what I am doing nowadays. While working with > >> > > > log4j-appender, I found that despite a lack of documentation, > >> > > considerable > >> > > > users are already using it[^1][^2][^3][^4][^5]. So, I think > >> providing a > >> > > > documentation to those who are already using log4j-appender is > >> > > > indispensable. It should include: > >> > > > > >> > > > - What is the difference between log4j-appender vs. log4j2-appender. > >> > > > - Which options are supported and deprecated. > >> > > > - Exemplar configurations that show how to migrate. > >> > > > > >> > > > Here is the summary: > >> > > > > >> > > > 1. The goal of this proposal is to replace the traditional > >> log4j-appender > >> > > > for compatibility concerns. But log4j-appender may be published > >> after the > >> > > > deprecation. > >> > > > 2. As of present, the description about logging-log4j2 Kafka > >> appender is > >> > > > entirely wrong. The problem is interface compatibility, not record > >> > > format. > >> > > > Focusing on the compatibility facade is a good approach. > >> > > > 3. A documentation focus on migration should be provided. > >> > > > > >> > > > If you have any questions or suggestions, don't hesitate to tell me. > >> > > Thanks > >> > > > again for your comments! > >> > > > > >> > > > Best, > >> > > > Dongjin > >> > > > > >> > > > [^1]: > >> > > > > >> > > > >> https://docs.cloudera.com/csa/1.2.0/monitoring/topics/csa-kafka-logging.html > >> > > > [^2]: > >> > > > > >> > > > >> https://stackoverflow.com/questions/22034895/how-to-use-kafka-0-8-log4j-appender > >> > > > [^3]: > >> > > > > >> > > > >> https://stackoverflow.com/questions/32402405/delay-in-kafka-log4j-appender > >> > > > [^4]: > >> > > > > >> > > > >> https://stackoverflow.com/questions/32301129/kafka-log4j-appender-not-sending-messages > >> > > > [^5]: > >> > > > > >> > > > >> https://stackoverflow.com/questions/35628706/kafka-log4j-appender-0-9-does-not-work > >> > > > > >> > > > On Mon, Nov 8, 2021 at 9:04 PM Mickael Maison < > >> mickael.mai...@gmail.com> > >> > > > wrote: > >> > > > > >> > > > > 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>* > >> > > > > > > > >> > > > > > >> > > > > >> > > > > >> > > > -- > >> > > > *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>* > > > > > -- > *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>*
