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


##########
build.gradle:
##########
@@ -167,8 +167,10 @@ allprojects {
           // ZooKeeper (potentially older and containing CVEs)
           libs.nettyHandler,
           libs.nettyTransportNativeEpoll,
-          // be explicit about the reload4j version instead of relying on the 
transitive versions
-          libs.reload4j
+          libs.reload4j,

Review Comment:
   We still need reload4j here?



##########
bin/windows/zookeeper-server-start.bat:
##########
@@ -19,8 +19,11 @@ IF [%1] EQU [] (
        EXIT /B 1
 )
 
+echo Running with log4j 2.x - Log4j MBean registration is not supported.
+
 SetLocal
 IF ["%KAFKA_LOG4J_OPTS%"] EQU [""] (
+    echo DEPRECATED: using log4j 1.x configuration. To use log4j 2.x 
configuration, run with: 'set 
KAFKA_LOG4J_OPTS=-Dlog4j.configurationFile=file:%~dp0../../config/log4j2.properties'

Review Comment:
   We should remove this file, right?



##########
bin/zookeeper-server-start.sh:
##########
@@ -22,9 +22,12 @@ fi
 base_dir=$(dirname $0)
 
 if [ "x$KAFKA_LOG4J_OPTS" = "x" ]; then
+    echo "DEPRECATED: using log4j 1.x configuration. To use log4j 2.x 
configuration, run with: 'export 
KAFKA_LOG4J_OPTS=\"-Dlog4j.configurationFile=file:$base_dir/../config/log4j2.properties\"'"
     export 
KAFKA_LOG4J_OPTS="-Dlog4j.configuration=file:$base_dir/../config/log4j.properties"
 fi
 
+echo "Running with log4j 2.x - Log4j MBean registration is not supported."

Review Comment:
   Is this right? It's still possible it runs with log4j 1.x, right?



##########
config/log4j2.properties:
##########
@@ -0,0 +1,163 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# Unspecified loggers and loggers with additivity=true output to server.log 
and stdout
+# Note that INFO only applies to unspecified loggers, the log level of the 
child logger is used otherwise
+name=LogConfig
+appenders=stdout,kafkaAppender,stateChangeAppender,requestAppender,cleanerAppender,controllerAppender,authorizerAppender
+
+# Console appender (stdout)
+appender.stdout.type=Console
+appender.stdout.name=STDOUT
+appender.stdout.layout.type=PatternLayout
+appender.stdout.layout.pattern=[%d] %p %m (%c)%n
+
+appender.kafkaAppender.type=RollingFile
+appender.kafkaAppender.name=KafkaAppender
+appender.kafkaAppender.fileName=${kafka.logs.dir}/server.log
+appender.kafkaAppender.filePattern=${kafka.logs.dir}/server.log.%d{yyyy-MM-dd-HH}
+appender.kafkaAppender.layout.type=PatternLayout
+appender.kafkaAppender.layout.pattern=[%d] %p %m (%c)%n
+appender.kafkaAppender.policies.type=TimeBasedTriggeringPolicy
+appender.kafkaAppender.policies.interval=1
+appender.kafkaAppender.policies.modulate=true
+
+# State Change appender
+appender.stateChangeAppender.type=RollingFile
+appender.stateChangeAppender.name=StateChangeAppender
+appender.stateChangeAppender.fileName=${kafka.logs.dir}/state-change.log
+appender.stateChangeAppender.filePattern=${kafka.logs.dir}/state-change.log.%d{yyyy-MM-dd-HH}
+appender.stateChangeAppender.layout.type=PatternLayout
+appender.stateChangeAppender.layout.pattern=[%d] %p %m (%c)%n
+appender.stateChangeAppender.policies.type=TimeBasedTriggeringPolicy
+appender.stateChangeAppender.policies.interval=1
+appender.stateChangeAppender.policies.modulate=true
+
+# Request appender
+appender.requestAppender.type=RollingFile
+appender.requestAppender.name=RequestAppender
+appender.requestAppender.fileName=${kafka.logs.dir}/kafka-request.log
+appender.requestAppender.filePattern=${kafka.logs.dir}/kafka-request.log.%d{yyyy-MM-dd-HH}
+appender.requestAppender.layout.type=PatternLayout
+appender.requestAppender.layout.pattern=[%d] %p %m (%c)%n
+appender.requestAppender.policies.type=TimeBasedTriggeringPolicy
+appender.requestAppender.policies.interval=1
+appender.requestAppender.policies.modulate=true
+
+# Cleaner appender
+appender.cleanerAppender.type=RollingFile
+appender.cleanerAppender.name=CleanerAppender
+appender.cleanerAppender.fileName=${kafka.logs.dir}/log-cleaner.log
+appender.cleanerAppender.filePattern=${kafka.logs.dir}/log-cleaner.log.%d{yyyy-MM-dd-HH}
+appender.cleanerAppender.layout.type=PatternLayout
+appender.cleanerAppender.layout.pattern=[%d] %p %m (%c)%n
+appender.cleanerAppender.policies.type=TimeBasedTriggeringPolicy
+appender.cleanerAppender.policies.interval=1
+appender.cleanerAppender.policies.modulate=true
+
+# Controller appender
+appender.controllerAppender.type=RollingFile
+appender.controllerAppender.name=ControllerAppender
+appender.controllerAppender.fileName=${kafka.logs.dir}/controller.log
+appender.controllerAppender.filePattern=${kafka.logs.dir}/controller.log.%d{yyyy-MM-dd-HH}
+appender.controllerAppender.layout.type=PatternLayout
+appender.controllerAppender.layout.pattern=[%d] %p %m (%c)%n
+appender.controllerAppender.policies.type=TimeBasedTriggeringPolicy
+appender.controllerAppender.policies.interval=1
+appender.controllerAppender.policies.modulate=true
+
+# Authorizer appender
+appender.authorizerAppender.type=RollingFile
+appender.authorizerAppender.name=AuthorizerAppender
+appender.authorizerAppender.fileName=${kafka.logs.dir}/kafka-authorizer.log
+appender.authorizerAppender.filePattern=${kafka.logs.dir}/kafka-authorizer.log.%d{yyyy-MM-dd-HH}
+appender.authorizerAppender.layout.type=PatternLayout
+appender.authorizerAppender.layout.pattern=[%d] %p %m (%c)%n
+appender.authorizerAppender.policies.type=TimeBasedTriggeringPolicy
+appender.authorizerAppender.policies.interval=1
+appender.authorizerAppender.policies.modulate=true
+
+rootLogger.level=INFO
+rootLogger.appenderRefs=stdout,kafkaAppender
+rootLogger.appenderRef.stdout.ref=STDOUT
+rootLogger.appenderRef.kafkaAppender.ref=KafkaAppender
+
+loggers=zookeeper,kafka,apacheKafka,requestLogger,networkRequestChannel,apacheKafkaController,kafkaController,logCleaner,stateChangeLogger,authorizerLogger
+
+# Zookeeper logger
+logger.zookeeper.name=org.apache.zookeeper
+logger.zookeeper.level=INFO

Review Comment:
   Should we remove them?



##########
config/connect-log4j2.properties:
##########
@@ -0,0 +1,39 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+name=ConfigConnectConfig
+
+appenders=console, file
+
+appender.console.type=Console
+appender.console.name=STDOUT
+appender.console.layout.type=PatternLayout
+appender.console.layout.pattern=[%d] %p %X{connector.context}%m (%c:%L)%n

Review Comment:
   In[ previous 
config](https://github.com/apache/kafka/blob/trunk/config/connect-log4j.properties#L32-L39),
 we also set the pattern to ConnectAppender, right?



##########
connect/mirror/src/test/resources/log4j2.properties:
##########
@@ -14,24 +14,30 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 ##
-log4j.rootLogger=INFO, stdout
+name=ConnectMirrorTestConfig

Review Comment:
   I don't understand the changes in the file. We move a log4j.properties in 
`connect/runtime` to `connect/mirror`? Why?



##########
config/log4j2.properties:
##########
@@ -0,0 +1,163 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# Unspecified loggers and loggers with additivity=true output to server.log 
and stdout
+# Note that INFO only applies to unspecified loggers, the log level of the 
child logger is used otherwise
+name=LogConfig
+appenders=stdout,kafkaAppender,stateChangeAppender,requestAppender,cleanerAppender,controllerAppender,authorizerAppender
+
+# Console appender (stdout)
+appender.stdout.type=Console
+appender.stdout.name=STDOUT
+appender.stdout.layout.type=PatternLayout
+appender.stdout.layout.pattern=[%d] %p %m (%c)%n
+
+appender.kafkaAppender.type=RollingFile
+appender.kafkaAppender.name=KafkaAppender
+appender.kafkaAppender.fileName=${kafka.logs.dir}/server.log
+appender.kafkaAppender.filePattern=${kafka.logs.dir}/server.log.%d{yyyy-MM-dd-HH}
+appender.kafkaAppender.layout.type=PatternLayout
+appender.kafkaAppender.layout.pattern=[%d] %p %m (%c)%n
+appender.kafkaAppender.policies.type=TimeBasedTriggeringPolicy
+appender.kafkaAppender.policies.interval=1
+appender.kafkaAppender.policies.modulate=true
+
+# State Change appender
+appender.stateChangeAppender.type=RollingFile
+appender.stateChangeAppender.name=StateChangeAppender
+appender.stateChangeAppender.fileName=${kafka.logs.dir}/state-change.log
+appender.stateChangeAppender.filePattern=${kafka.logs.dir}/state-change.log.%d{yyyy-MM-dd-HH}
+appender.stateChangeAppender.layout.type=PatternLayout
+appender.stateChangeAppender.layout.pattern=[%d] %p %m (%c)%n
+appender.stateChangeAppender.policies.type=TimeBasedTriggeringPolicy
+appender.stateChangeAppender.policies.interval=1
+appender.stateChangeAppender.policies.modulate=true
+
+# Request appender
+appender.requestAppender.type=RollingFile
+appender.requestAppender.name=RequestAppender
+appender.requestAppender.fileName=${kafka.logs.dir}/kafka-request.log
+appender.requestAppender.filePattern=${kafka.logs.dir}/kafka-request.log.%d{yyyy-MM-dd-HH}
+appender.requestAppender.layout.type=PatternLayout
+appender.requestAppender.layout.pattern=[%d] %p %m (%c)%n
+appender.requestAppender.policies.type=TimeBasedTriggeringPolicy
+appender.requestAppender.policies.interval=1
+appender.requestAppender.policies.modulate=true
+
+# Cleaner appender
+appender.cleanerAppender.type=RollingFile
+appender.cleanerAppender.name=CleanerAppender
+appender.cleanerAppender.fileName=${kafka.logs.dir}/log-cleaner.log
+appender.cleanerAppender.filePattern=${kafka.logs.dir}/log-cleaner.log.%d{yyyy-MM-dd-HH}
+appender.cleanerAppender.layout.type=PatternLayout
+appender.cleanerAppender.layout.pattern=[%d] %p %m (%c)%n
+appender.cleanerAppender.policies.type=TimeBasedTriggeringPolicy
+appender.cleanerAppender.policies.interval=1
+appender.cleanerAppender.policies.modulate=true
+
+# Controller appender
+appender.controllerAppender.type=RollingFile
+appender.controllerAppender.name=ControllerAppender
+appender.controllerAppender.fileName=${kafka.logs.dir}/controller.log
+appender.controllerAppender.filePattern=${kafka.logs.dir}/controller.log.%d{yyyy-MM-dd-HH}
+appender.controllerAppender.layout.type=PatternLayout
+appender.controllerAppender.layout.pattern=[%d] %p %m (%c)%n
+appender.controllerAppender.policies.type=TimeBasedTriggeringPolicy
+appender.controllerAppender.policies.interval=1
+appender.controllerAppender.policies.modulate=true
+
+# Authorizer appender
+appender.authorizerAppender.type=RollingFile
+appender.authorizerAppender.name=AuthorizerAppender
+appender.authorizerAppender.fileName=${kafka.logs.dir}/kafka-authorizer.log
+appender.authorizerAppender.filePattern=${kafka.logs.dir}/kafka-authorizer.log.%d{yyyy-MM-dd-HH}
+appender.authorizerAppender.layout.type=PatternLayout
+appender.authorizerAppender.layout.pattern=[%d] %p %m (%c)%n
+appender.authorizerAppender.policies.type=TimeBasedTriggeringPolicy
+appender.authorizerAppender.policies.interval=1
+appender.authorizerAppender.policies.modulate=true
+
+rootLogger.level=INFO
+rootLogger.appenderRefs=stdout,kafkaAppender
+rootLogger.appenderRef.stdout.ref=STDOUT
+rootLogger.appenderRef.kafkaAppender.ref=KafkaAppender
+
+loggers=zookeeper,kafka,apacheKafka,requestLogger,networkRequestChannel,apacheKafkaController,kafkaController,logCleaner,stateChangeLogger,authorizerLogger
+
+# Zookeeper logger
+logger.zookeeper.name=org.apache.zookeeper
+logger.zookeeper.level=INFO
+
+# Kafka logger
+logger.kafka.name=kafka
+logger.kafka.level=INFO
+
+# Kafka org.apache logger
+logger.apacheKafka.name=org.apache.kafka
+logger.apacheKafka.level=INFO
+
+# Kafka request logger
+logger.requestLogger.name=kafka.request.logger
+logger.requestLogger.level=WARN
+logger.requestLogger.additivity=false
+logger.requestLogger.appenderRef.requestAppender.ref=RequestAppender
+
+
+# Uncomment the lines below and change 
log4j.logger.kafka.network.RequestChannel$ to TRACE for additional output
+# related to the handling of requests
+#loggers=zookeeper,kafka,apacheKafka,requestLogger,networkRequestChannel,apacheKafkaController,kafkaController,logCleaner,stateChangeLogger,authorizerLogger,networkProcessor,serverKafkaApis
+#logger.networkProcessor.name=kafka.network.Processor
+#logger.networkProcessor.level=TRACE
+#logger.networkProcessor.appenderRef.requestAppender.ref=RequestAppender
+#logger.serverKafkaApis.name=kafka.server.KafkaApis
+#logger.serverKafkaApis.level=TRACE
+#logger.serverKafkaApis.additivity=false
+#logger.serverKafkaApis.appenderRef.requestAppender.ref=RequestAppender
+
+# Kafka network RequestChannel$ logger
+logger.networkRequestChannel.name=kafka.network.RequestChannel$
+logger.networkRequestChannel.level=WARN
+logger.networkRequestChannel.additivity=false
+logger.networkRequestChannel.appenderRef.requestAppender.ref=RequestAppender
+
+# KRaft mode controller logger
+logger.apacheKafkaController.name=org.apache.kafka.controller
+logger.apacheKafkaController.level=INFO
+logger.apacheKafkaController.additivity=false
+logger.apacheKafkaController.appenderRef.controllerAppender.ref=ControllerAppender
+
+# ZK mode controller logger
+logger.kafkaController.name=kafka.controller
+logger.kafkaController.level=TRACE
+logger.kafkaController.additivity=false
+logger.kafkaController.appenderRef.controllerAppender.ref=ControllerAppender

Review Comment:
   Should we remove them?



-- 
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]

Reply via email to