chia7712 commented on code in PR #17373:
URL: https://github.com/apache/kafka/pull/17373#discussion_r1898365578
##########
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:
> These dependencies should probably be handled in a similar way.
Yes, we can declare them as compileOnly and then add them to the
distribution. However, the flexibility of replacing the SLF4J provider at
runtime may break the functionality of Log4jController (similar to [Juma's
comment](https://github.com/apache/kafka/pull/17373#discussion_r1894553658)).
KIP-1064 is attempting to use SLF4J2's system variable to choose the provider
more effectively.
> No modern library uses Log4j 1 in code (they use JCL, SLF4J or Log4j API),
so my guess is that libs.log4j1Bridge2Api could be dropped entirely.
Pardon me, in the #18290 we decide to allow users to use log4j.properties -
so we still need `log4j-1.2-api`, right?
--
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
