chia7712 commented on code in PR #17373:
URL: https://github.com/apache/kafka/pull/17373#discussion_r1847704612
##########
core/src/main/scala/kafka/utils/Log4jController.scala:
##########
@@ -17,83 +17,90 @@
package kafka.utils
-import java.util
-import java.util.Locale
-
import org.apache.kafka.common.utils.Utils
-import org.apache.log4j.{Level, LogManager, Logger}
+import org.apache.logging.log4j.core.LoggerContext
+import org.apache.logging.log4j.core.config.Configurator
+import org.apache.logging.log4j.{Level, LogManager}
-import scala.collection.mutable
+import java.util
+import java.util.Locale
import scala.jdk.CollectionConverters._
object Log4jController {
- val ROOT_LOGGER = "root"
- private def resolveLevel(logger: Logger): String = {
- var name = logger.getName
- var level = logger.getLevel
- while (level == null) {
- val index = name.lastIndexOf(".")
- if (index > 0) {
- name = name.substring(0, index)
- val ancestor = existingLogger(name)
- if (ancestor != null) {
- level = ancestor.getLevel
- }
- } else {
- level = existingLogger(ROOT_LOGGER).getLevel
- }
- }
- level.toString
- }
+ /**
+ * Note: In log4j, the root logger's name was "root" and Kafka also followed
that name for dynamic logging control feature.
+ *
+ * The root logger's name is changed in log4j2 to empty string (see:
[[LogManager.ROOT_LOGGER_NAME]]) but for backward-
+ * compatibility. Kafka keeps its original root logger name. It is why here
is a dedicated definition for the root logger name.
+ */
+ val ROOT_LOGGER = "root"
/**
- * Returns a map of the log4j loggers and their assigned log level.
- * If a logger does not have a log level assigned, we return the root
logger's log level
- */
- def loggers: mutable.Map[String, String] = {
- val logs = new mutable.HashMap[String, String]()
- val rootLoggerLvl = existingLogger(ROOT_LOGGER).getLevel.toString
- logs.put(ROOT_LOGGER, rootLoggerLvl)
-
- val loggers = LogManager.getCurrentLoggers
- while (loggers.hasMoreElements) {
- val logger = loggers.nextElement().asInstanceOf[Logger]
- if (logger != null) {
- logs.put(logger.getName, resolveLevel(logger))
- }
- }
- logs
+ * Returns a map of the log4j loggers and their assigned log level.
+ * If a logger does not have a log level assigned, we return the log level
of the first ancestor with a level configured.
+ */
+ def loggers: Map[String, String] = {
+ val logContext = LogManager.getContext(false).asInstanceOf[LoggerContext]
+ val rootLoggerLevel = logContext.getRootLogger.getLevel.toString
+
+ // Loggers defined in the configuration
+ val configured = logContext.getConfiguration.getLoggers.asScala
+ .values
+ .filter(_.getName != LogManager.ROOT_LOGGER_NAME)
Review Comment:
Do they use the same reference? or we should use `!equals` instead of `!=`?
##########
core/src/main/scala/kafka/utils/Log4jController.scala:
##########
@@ -17,83 +17,90 @@
package kafka.utils
-import java.util
-import java.util.Locale
-
import org.apache.kafka.common.utils.Utils
-import org.apache.log4j.{Level, LogManager, Logger}
+import org.apache.logging.log4j.core.LoggerContext
+import org.apache.logging.log4j.core.config.Configurator
+import org.apache.logging.log4j.{Level, LogManager}
-import scala.collection.mutable
+import java.util
+import java.util.Locale
import scala.jdk.CollectionConverters._
object Log4jController {
- val ROOT_LOGGER = "root"
- private def resolveLevel(logger: Logger): String = {
- var name = logger.getName
- var level = logger.getLevel
- while (level == null) {
- val index = name.lastIndexOf(".")
- if (index > 0) {
- name = name.substring(0, index)
- val ancestor = existingLogger(name)
- if (ancestor != null) {
- level = ancestor.getLevel
- }
- } else {
- level = existingLogger(ROOT_LOGGER).getLevel
- }
- }
- level.toString
- }
+ /**
+ * Note: In log4j, the root logger's name was "root" and Kafka also followed
that name for dynamic logging control feature.
+ *
+ * The root logger's name is changed in log4j2 to empty string (see:
[[LogManager.ROOT_LOGGER_NAME]]) but for backward-
+ * compatibility. Kafka keeps its original root logger name. It is why here
is a dedicated definition for the root logger name.
+ */
+ val ROOT_LOGGER = "root"
/**
- * Returns a map of the log4j loggers and their assigned log level.
- * If a logger does not have a log level assigned, we return the root
logger's log level
- */
- def loggers: mutable.Map[String, String] = {
- val logs = new mutable.HashMap[String, String]()
- val rootLoggerLvl = existingLogger(ROOT_LOGGER).getLevel.toString
- logs.put(ROOT_LOGGER, rootLoggerLvl)
-
- val loggers = LogManager.getCurrentLoggers
- while (loggers.hasMoreElements) {
- val logger = loggers.nextElement().asInstanceOf[Logger]
- if (logger != null) {
- logs.put(logger.getName, resolveLevel(logger))
- }
- }
- logs
+ * Returns a map of the log4j loggers and their assigned log level.
+ * If a logger does not have a log level assigned, we return the log level
of the first ancestor with a level configured.
+ */
+ def loggers: Map[String, String] = {
+ val logContext = LogManager.getContext(false).asInstanceOf[LoggerContext]
+ val rootLoggerLevel = logContext.getRootLogger.getLevel.toString
+
+ // Loggers defined in the configuration
+ val configured = logContext.getConfiguration.getLoggers.asScala
+ .values
+ .filter(_.getName != LogManager.ROOT_LOGGER_NAME)
+ .map { logger =>
+ logger.getName -> logger.getLevel.toString
+ }.toMap
+
+ // Loggers actually running
+ val actual = logContext.getLoggers.asScala
+ .filter(_.getName != LogManager.ROOT_LOGGER_NAME)
Review Comment:
ditto
##########
config/connect-log4j2.properties:
##########
@@ -0,0 +1,39 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
Review Comment:
Please fix the `connect.py`
https://github.com/apache/kafka/blob/trunk/tests/kafkatest/services/connect.py#L367
https://github.com/apache/kafka/blob/trunk/tests/kafkatest/services/connect.py#L424
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -89,18 +90,11 @@ class PlaintextAdminIntegrationTest extends
BaseAdminIntegrationTest {
@BeforeEach
override def setUp(testInfo: TestInfo): Unit = {
super.setUp(testInfo)
+ Configurator.reconfigure();
brokerLoggerConfigResource = new ConfigResource(
ConfigResource.Type.BROKER_LOGGER, brokers.head.config.brokerId.toString)
}
- @AfterEach
- override def tearDown(): Unit = {
- // Due to the fact that log4j is not re-initialized across tests, changing
a logger's log level persists
- // across test classes. We need to clean up the changes done after testing.
- resetLogging()
Review Comment:
Could you please remove this method also?
##########
build.gradle:
##########
@@ -3487,7 +3515,10 @@ project(':connect:runtime') {
api project(':connect:transforms')
implementation libs.slf4jApi
- implementation libs.reload4j
+ implementation libs.log4j2Api
+ implementation libs.log4j2Core
+ implementation libs.log4j1Bridge2Api
+ implementation libs.spotbugs
Review Comment:
`compileOnly` is good enough I think as we don't use the annotation in
runtime
--
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
