gharris1727 commented on code in PR #16324:
URL: https://github.com/apache/kafka/pull/16324#discussion_r1643357210
##########
bin/kafka-run-class.sh:
##########
@@ -243,6 +243,14 @@ fi
(( WINDOWS_OS_FORMAT )) && LOG_DIR=$(cygpath --path --mixed "${LOG_DIR}")
KAFKA_LOG4J_CMD_OPTS="-Dkafka.logs.dir=$LOG_DIR $KAFKA_LOG4J_OPTS"
+# slf4j provider settings
+if [ -z "$SLF4J_PROVIDER" ]; then
Review Comment:
nit: Should this be KAFKA_SLF4J_PROVIDER? Or is this an idiomatic variable
that we are following by convention?
##########
bin/kafka-run-class.sh:
##########
@@ -243,6 +243,14 @@ fi
(( WINDOWS_OS_FORMAT )) && LOG_DIR=$(cygpath --path --mixed "${LOG_DIR}")
KAFKA_LOG4J_CMD_OPTS="-Dkafka.logs.dir=$LOG_DIR $KAFKA_LOG4J_OPTS"
+# slf4j provider settings
+if [ -z "$SLF4J_PROVIDER" ]; then
+ SLF4J_PROVIDER="org.slf4j.reload4j.Reload4jServiceProvider"
+fi
+
+# Add the slf4j provider to KAFKA_LOG4J_CMD_OPTS
+KAFKA_LOG4J_CMD_OPTS="$KAFKA_LOG4J_CMD_OPTS -Dslf4j.provider=$SLF4J_PROVIDER"
Review Comment:
This is a new system property, so KAFKA_LOG4J_CMD_OPTS shouldn't already
contain -Dslf4j.provider
👍
##########
build.gradle:
##########
@@ -928,14 +928,15 @@ project(':server') {
implementation libs.jacksonDatabind
implementation libs.slf4jApi
+ implementation libs.slf4jReload4j
Review Comment:
This is the only dependency change that isn't a just a version change.
Based on my understanding, we should only include slf4jReload4j in
situations when we are also providing reload4j. Otherwise we provide the
binding but not the actual logger.
So this should be compileOnly, like libs.reload4j? Or not at all, like
`:core`? I'm not exactly sure.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
