Re: [PR] KAFKA-18243 Loggers breaks the use of root logger after migrating to log4j2 [kafka]

[via GitHub]

TaiJuWu commented on code in PR #18185:
URL: https://github.com/apache/kafka/pull/18185#discussion_r1897089329


##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Loggers.java:
##########
@@ -198,26 +194,37 @@ org.apache.logging.log4j.Logger rootLogger() {
     }
 
     private void setLevel(org.apache.logging.log4j.Logger logger, Level level) 
{
-        String loggerName = logger.getName();
-        LoggerContext context = (LoggerContext) LogManager.getContext(false);
-        LoggerConfig loggerConfig = 
context.getConfiguration().getLoggerConfig(loggerName);
-        Level currentLevel = loggerConfig.getLevel();
+        Level currentLevel = logger.getLevel();
 
         if (level.equals(currentLevel)) {
-            log.debug("Skipping update for logger {} since its level is 
already {}", loggerName, level);
+            log.debug("Skipping update for logger {} since its level is 
already {}", logger.getName(), level);
             return;
         }
 
-        log.debug("Setting level of logger {} (excluding children) to {}", 
loggerName, level);
-        Configurator.setLevel(loggerName, level);
-        lastModifiedTimes.put(loggerName, time.milliseconds());
+        log.debug("Setting level of logger {} (excluding children) to {}", 
logger.getName(), level);
+        Configurator.setLevel(logger.getName(), level);
+        lastModifiedTimes.put(logger.getName(), time.milliseconds());
     }
 
     private LoggerLevel loggerLevel(org.apache.logging.log4j.Logger logger) {
-        LoggerContext context = (LoggerContext) LogManager.getContext(false);
-        LoggerConfig loggerConfig = 
context.getConfiguration().getLoggerConfig(logger.getName());
-        Level level = loggerConfig.getLevel();
         Long lastModified = lastModifiedTimes.get(logger.getName());
-        return new LoggerLevel(Objects.toString(level), lastModified);
+        return new LoggerLevel(Objects.toString(logger.getLevel()), 
lastModified);
+    }
+
+    private boolean isValidRootLoggerName(String namespace) {
+        return VALID_ROOT_LOGGER_NAMES.stream()
+            .anyMatch(rootLoggerNames -> 
rootLoggerNames.equalsIgnoreCase(namespace));
+    }

Review Comment:
   Maybe we need to change this to avoid using stream since this is hot path (I 
guess).
   With this change `OffsetUtilsTest` will pass on my local
   ```
   private boolean isValidRootLoggerName(String namespace) {
           boolean result = false;
           for (String rootLoggerName : VALID_ROOT_LOGGER_NAMES) {
               if (rootLoggerName.equalsIgnoreCase(namespace)) {
                   result = true;
                   break;
               }
           }
           return result;
       }
   ```



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