viktorsomogyi commented on a change in pull request #7898:
URL: https://github.com/apache/kafka/pull/7898#discussion_r783871962
##########
File path: core/src/main/scala/kafka/utils/Log4jController.scala
##########
@@ -17,83 +17,89 @@
package kafka.utils
+import org.apache.logging.log4j.core.LoggerContext
+import org.apache.logging.log4j.core.config.Configurator
+import org.apache.logging.log4j.{Level, LogManager}
+
import java.util
import java.util.Locale
-
-import org.apache.kafka.common.utils.Utils
-import org.apache.log4j.{Level, LogManager, Logger}
-
-import scala.collection.mutable
import scala.jdk.CollectionConverters._
object Log4jController {
+
+ /**
+ * 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"
- 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
- }
+ /**
+ * Returns given logger's parent's (or the first ancestor's) name.
+ *
+ * @throws IllegalArgumentException loggerName is null or empty.
+ */
/**
* 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
+ * 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: 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
+ 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
+ .map(_._2)
Review comment:
nit: instead of `.map(_._2)` you could use `.values`
##########
File path:
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/LoggingResource.java
##########
@@ -60,67 +66,62 @@
@GET
@Path("/")
public Response listLoggers() {
- Map<String, Map<String, String>> loggers = new TreeMap<>();
- Enumeration<Logger> enumeration = currentLoggers();
- Collections.list(enumeration)
- .stream()
- .filter(logger -> logger.getLevel() != null)
- .forEach(logger -> loggers.put(logger.getName(),
levelToMap(logger)));
+ // current loggers
+ final Map<String, Map<String, String>> loggers = currentLoggers()
+ .stream()
+ .filter(logger -> logger.getLevel() != Level.OFF)
+ .collect(Collectors.toMap(logger -> logger.getName(), logger ->
levelToMap(logger)));
+ // Replace "" logger to "root" logger
Logger root = rootLogger();
- if (root.getLevel() != null) {
+ if (root.getLevel() != Level.OFF) {
loggers.put(ROOT_LOGGER_NAME, levelToMap(root));
}
- return Response.ok(loggers).build();
+ return Response.ok(new TreeMap<>(loggers)).build();
}
/**
* Get the log level of a named logger.
*
- * @param namedLogger name of a logger
+ * @param loggerName name of a logger
Review comment:
I think renaming in this case makes the code a bit clearer and
consistent.
##########
File path: streams/src/test/java/org/apache/kafka/streams/KafkaStreamsTest.java
##########
@@ -102,6 +103,7 @@
import static org.junit.Assert.fail;
@RunWith(PowerMockRunner.class)
+@PowerMockIgnore("javax.management.*")
Review comment:
Ran the tests and it seems like the PowerMock class loader isn't able to
load these classes (as they have previously been loaded?). It doesn't cause a
test failure but it's ugly and deferring to the system classloader fixes the
issue. Maybe @dongjinleekr has a more specific answer but I think it's fine to
have this annotation here.
```
2022-01-17 11:43:22,581 Test worker ERROR Could not reconfigure JMX
java.lang.LinkageError: loader constraint violation: loader (instance of
org/powermock/core/classloader/javassist/JavassistMockClassLoader) previously
initiated loading for a different type with name "javax/management/MBeanServer"
at java.lang.ClassLoader.defineClass1(Native Method)
at java.lang.ClassLoader.defineClass(ClassLoader.java:756)
at
org.powermock.core.classloader.javassist.JavassistMockClassLoader.loadUnmockedClass(JavassistMockClassLoader.java:90)
at
org.powermock.core.classloader.MockClassLoader.loadClassByThisClassLoader(MockClassLoader.java:104)
at
org.powermock.core.classloader.DeferSupportingClassLoader.loadClass1(DeferSupportingClassLoader.java:147)
at
org.powermock.core.classloader.DeferSupportingClassLoader.loadClass(DeferSupportingClassLoader.java:98)
at java.lang.ClassLoader.loadClass(ClassLoader.java:351)
at
org.apache.logging.log4j.core.jmx.Server.unregisterAllMatching(Server.java:337)
at
org.apache.logging.log4j.core.jmx.Server.unregisterLoggerContext(Server.java:261)
at
org.apache.logging.log4j.core.jmx.Server.reregisterMBeansAfterReconfigure(Server.java:165)
at
org.apache.logging.log4j.core.jmx.Server.reregisterMBeansAfterReconfigure(Server.java:141)
at
org.apache.logging.log4j.core.LoggerContext.setConfiguration(LoggerContext.java:637)
at
org.apache.logging.log4j.core.LoggerContext.reconfigure(LoggerContext.java:699)
at
org.apache.logging.log4j.core.LoggerContext.reconfigure(LoggerContext.java:716)
at
org.apache.logging.log4j.core.LoggerContext.start(LoggerContext.java:270)
at
org.apache.logging.log4j.core.impl.Log4jContextFactory.getContext(Log4jContextFactory.java:155)
at
org.apache.logging.log4j.core.impl.Log4jContextFactory.getContext(Log4jContextFactory.java:47)
at org.apache.logging.log4j.LogManager.getContext(LogManager.java:196)
at
org.apache.logging.log4j.spi.AbstractLoggerAdapter.getContext(AbstractLoggerAdapter.java:137)
at
org.apache.logging.slf4j.Log4jLoggerFactory.getContext(Log4jLoggerFactory.java:55)
at
org.apache.logging.log4j.spi.AbstractLoggerAdapter.getLogger(AbstractLoggerAdapter.java:47)
at
org.apache.logging.slf4j.Log4jLoggerFactory.getLogger(Log4jLoggerFactory.java:33)
at org.slf4j.LoggerFactory.getLogger(LoggerFactory.java:363)
at org.slf4j.LoggerFactory.getLogger(LoggerFactory.java:388)
at
org.apache.kafka.streams.processor.internals.StateDirectory.<clinit>(StateDirectory.java:67)
at java.lang.Class.forName0(Native Method)
at java.lang.Class.forName(Class.java:348)
at
org.easymock.cglib.core.ReflectUtils.defineClass(ReflectUtils.java:467)
at
org.easymock.cglib.core.AbstractClassGenerator.generate(AbstractClassGenerator.java:339)
at org.easymock.cglib.proxy.Enhancer.generate(Enhancer.java:492)
at
org.easymock.cglib.core.AbstractClassGenerator$ClassLoaderData$3.apply(AbstractClassGenerator.java:96)
at
org.easymock.cglib.core.AbstractClassGenerator$ClassLoaderData$3.apply(AbstractClassGenerator.java:94)
at
org.easymock.cglib.core.internal.LoadingCache$2.call(LoadingCache.java:54)
at java.util.concurrent.FutureTask.run(FutureTask.java:266)
at
org.easymock.cglib.core.internal.LoadingCache.createEntry(LoadingCache.java:61)
at
org.easymock.cglib.core.internal.LoadingCache.get(LoadingCache.java:34)
at
org.easymock.cglib.core.AbstractClassGenerator$ClassLoaderData.get(AbstractClassGenerator.java:119)
at
org.easymock.cglib.core.AbstractClassGenerator.create(AbstractClassGenerator.java:294)
at org.easymock.cglib.proxy.Enhancer.createHelper(Enhancer.java:480)
at org.easymock.cglib.proxy.Enhancer.createClass(Enhancer.java:337)
at
org.easymock.internal.ClassProxyFactory.createProxy(ClassProxyFactory.java:175)
at org.easymock.internal.MocksControl.createMock(MocksControl.java:107)
at org.easymock.internal.MocksControl.createMock(MocksControl.java:80)
at
org.powermock.api.easymock.PowerMock.doCreateMock(PowerMock.java:2023)
at org.powermock.api.easymock.PowerMock.doMock(PowerMock.java:1970)
at org.powermock.api.easymock.PowerMock.createMock(PowerMock.java:84)
at
org.powermock.api.extension.listener.AnnotationMockCreatorFactory$1.createMockInstance(AnnotationMockCreatorFactory.java:34)
at
org.powermock.api.extension.listener.EasyMockAnnotationSupport.createMock(EasyMockAnnotationSupport.java:109)
at
org.powermock.api.extension.listener.EasyMockAnnotationSupport.injectMock(EasyMockAnnotationSupport.java:98)
at
org.powermock.api.extension.listener.EasyMockAnnotationSupport.inject(EasyMockAnnotationSupport.java:90)
at
org.powermock.api.extension.listener.EasyMockAnnotationSupport.injectDefaultMocks(EasyMockAnnotationSupport.java:70)
at
org.powermock.api.extension.listener.EasyMockAnnotationSupport.injectMocks(EasyMockAnnotationSupport.java:56)
at
org.powermock.api.extension.listener.AnnotationEnabler.beforeTestMethod(AnnotationEnabler.java:76)
at
org.powermock.tests.utils.impl.PowerMockTestNotifierImpl.notifyBeforeTestMethod(PowerMockTestNotifierImpl.java:82)
at
org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl$PowerMockJUnit44MethodRunner.executeTest(PowerMockJUnit44RunnerDelegateImpl.java:308)
at
org.powermock.modules.junit4.internal.impl.PowerMockJUnit47RunnerDelegateImpl$PowerMockJUnit47MethodRunner.executeTestInSuper(PowerMockJUnit47RunnerDelegateImpl.java:131)
at
org.powermock.modules.junit4.internal.impl.PowerMockJUnit47RunnerDelegateImpl$PowerMockJUnit47MethodRunner.access$100(PowerMockJUnit47RunnerDelegateImpl.java:59)
at
org.powermock.modules.junit4.internal.impl.PowerMockJUnit47RunnerDelegateImpl$PowerMockJUnit47MethodRunner$TestExecutorStatement.evaluate(PowerMockJUnit47RunnerDelegateImpl.java:147)
at org.junit.rules.TestWatcher$1.evaluate(TestWatcher.java:61)
at
org.powermock.modules.junit4.internal.impl.PowerMockJUnit47RunnerDelegateImpl$PowerMockJUnit47MethodRunner.evaluateStatement(PowerMockJUnit47RunnerDelegateImpl.java:107)
at
org.powermock.modules.junit4.internal.impl.PowerMockJUnit47RunnerDelegateImpl$PowerMockJUnit47MethodRunner.executeTest(PowerMockJUnit47RunnerDelegateImpl.java:82)
at
org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl$PowerMockJUnit44MethodRunner.runBeforesThenTestThenAfters(PowerMockJUnit44RunnerDelegateImpl.java:298)
at org.junit.internal.runners.MethodRoadie.runTest(MethodRoadie.java:87)
at org.junit.internal.runners.MethodRoadie.run(MethodRoadie.java:50)
at
org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl.invokeTestMethod(PowerMockJUnit44RunnerDelegateImpl.java:218)
at
org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl.runMethods(PowerMockJUnit44RunnerDelegateImpl.java:160)
at
org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl$1.run(PowerMockJUnit44RunnerDelegateImpl.java:134)
at
org.junit.internal.runners.ClassRoadie.runUnprotected(ClassRoadie.java:34)
at
org.junit.internal.runners.ClassRoadie.runProtected(ClassRoadie.java:44)
at
org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl.run(PowerMockJUnit44RunnerDelegateImpl.java:136)
at
org.powermock.modules.junit4.common.internal.impl.JUnit4TestSuiteChunkerImpl.run(JUnit4TestSuiteChunkerImpl.java:117)
at
org.powermock.modules.junit4.common.internal.impl.AbstractCommonPowerMockRunner.run(AbstractCommonPowerMockRunner.java:57)
at
org.powermock.modules.junit4.PowerMockRunner.run(PowerMockRunner.java:59)
at
org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.runTestClass(JUnitTestClassExecutor.java:110)
at
org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.execute(JUnitTestClassExecutor.java:58)
at
org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.execute(JUnitTestClassExecutor.java:38)
at
org.gradle.api.internal.tasks.testing.junit.AbstractJUnitTestClassProcessor.processTestClass(AbstractJUnitTestClassProcessor.java:62)
at
org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.processTestClass(SuiteTestClassProcessor.java:51)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at
org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36)
at
org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
at
org.gradle.internal.dispatch.ContextClassLoaderDispatch.dispatch(ContextClassLoaderDispatch.java:33)
at
org.gradle.internal.dispatch.ProxyDispatchAdapter$DispatchingInvocationHandler.invoke(ProxyDispatchAdapter.java:94)
at com.sun.proxy.$Proxy2.processTestClass(Unknown Source)
at
org.gradle.api.internal.tasks.testing.worker.TestWorker$2.run(TestWorker.java:176)
at
org.gradle.api.internal.tasks.testing.worker.TestWorker.executeAndMaintainThreadName(TestWorker.java:129)
at
org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:100)
at
org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:60)
at
org.gradle.process.internal.worker.child.ActionExecutionWorker.execute(ActionExecutionWorker.java:56)
at
org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:133)
at
org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:71)
at
worker.org.gradle.process.internal.worker.GradleWorkerMain.run(GradleWorkerMain.java:69)
at
worker.org.gradle.process.internal.worker.GradleWorkerMain.main(GradleWorkerMain.java:74)
```
##########
File path: core/src/test/scala/unit/kafka/utils/LogCaptureContext.scala
##########
@@ -0,0 +1,77 @@
+/*
+ * 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.
+ */
+package unit.kafka.utils
+
+import java.util.concurrent.{CountDownLatch, TimeUnit}
+
+import org.apache.logging.log4j.Level
+import org.apache.logging.log4j.core.{LogEvent, LoggerContext}
+import org.apache.logging.log4j.test.appender.ListAppender
+
+import scala.jdk.CollectionConverters._
+
+class LogCaptureContext(listAppender: ListAppender, prevLevelMap: Map[String,
Level]) extends AutoCloseable {
+
+ def setLatch(size: Int): Unit = {
+ this.listAppender.countDownLatch = new CountDownLatch(size)
+ }
+
+ @throws[InterruptedException]
+ def await(l: Long, timeUnit: TimeUnit): Unit = {
+ this.listAppender.countDownLatch.await(l, timeUnit)
+ }
+
+ def getMessages: Seq[LogEvent] = listAppender.getEvents.asScala.toSeq
+
+ override def close(): Unit = {
+ val loggerContext = LoggerContext.getContext(false)
+ loggerContext.getRootLogger.removeAppender(listAppender)
+ listAppender.stop()
+
+ // Restore previous logger levels
+ prevLevelMap.foreach { e =>
+ val loggerName = e._1
+ val level = e._2
+ loggerContext.getLogger(loggerName).setLevel(level)
+ }
+ }
+}
+
+object LogCaptureContext {
+ def apply(name: String, levelMap: Map[String, String] = Map()):
LogCaptureContext = {
Review comment:
I think I would also prefer generating a name in the `apply()` method.
If someone uses the same name twice it might introduce an error but it's indeed
easier to read without the `name` as Tom proposed.
--
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
