ppkarwasz commented on code in PR #17373:
URL: https://github.com/apache/kafka/pull/17373#discussion_r1898304621


##########
build.gradle:
##########
@@ -1099,15 +1103,17 @@ project(':core') {
       implementation libs.dropwizardMetrics
       exclude module: 'slf4j-log4j12'
       exclude module: 'log4j'
-      // Both Kafka and Zookeeper use slf4j. ZooKeeper moved from log4j to 
logback in v3.8.0, but Kafka relies on reload4j.
+      // Both Kafka and Zookeeper use slf4j. ZooKeeper moved from log4j to 
logback in v3.8.0.
       // We are removing Zookeeper's dependency on logback so we have a 
singular logging backend.
       exclude module: 'logback-classic'
       exclude module: 'logback-core'
     }
     // ZooKeeperMain depends on commons-cli but declares the dependency as 
`provided`
     implementation libs.commonsCli
-
-    compileOnly libs.reload4j
+    implementation libs.log4j2Core
+    implementation libs.log4j2Api
+    implementation libs.log4j1Bridge2Api
+    implementation libs.jacksonDatabindYaml

Review Comment:
   IMHO the `kafka_<scala_version>` artifact should not have these 
dependencies, these dependencies should be added **only** to the binary Kafka 
distribution. Otherwise Kafka will leak the Log4j Core dependencies to its 
consumers, similarly to what was happening with Zookeeper (see 
[ZOOKEEPER-4820](https://issues.apache.org/jira/browse/ZOOKEEPER-4820)).
   
   To add dependencies only to the binary distribution, you could use something 
similar to apache/eventmesh#4719, i.e. a separate `distOnly` Gradle 
configuration.
   
   `Log4jController` can be rewritten to use Log4j Core if present or a no-op 
implementation otherwise, so the Log4j Core can be declared as an [optional 
Gradle dependency](https://blog.gradle.org/optional-dependencies).
   
   **Note**: I am working on a `org.apache.logging:logging-admin` artifact (see 
[`ppkarwasz/logging-admin` repo](https://github.com/ppkarwasz/logging-admin)) 
that would provide the same functionality as `Log4jController`, but in a 
logging implementation independent way. Unfortunately I have a long TODO list 
before I can publish it, so probably it won't be ready for Kafka 4.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to