steveloughran commented on code in PR #5456:
URL: https://github.com/apache/hadoop/pull/5456#discussion_r1129648239


##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/log/LogLevel.java:
##########
@@ -338,14 +341,18 @@ public void doGet(HttpServletRequest request, 
HttpServletResponse response
         out.println(MARKER
             + "Submitted Class Name: <b>" + logName + "</b><br />");
 
-        Logger log = Logger.getLogger(logName);
+        org.slf4j.Logger log = LoggerFactory.getLogger(logName);
         out.println(MARKER
             + "Log Class: <b>" + log.getClass().getName() +"</b><br />");
         if (level != null) {
           out.println(MARKER + "Submitted Level: <b>" + level + "</b><br />");
         }
 
-        process(log, level, out);
+        if (GenericsUtil.isLog4jLogger(logName)) {
+          process(Logger.getLogger(logName), level, out);
+        } else {
+          out.println("Sorry, " + log.getClass() + " not supported.<br />");

Review Comment:
   text to explain "log4j loggers only"



##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/GenericsUtil.java:
##########
@@ -89,10 +89,30 @@ public static boolean isLog4jLogger(Class<?> clazz) {
     }
     Logger log = LoggerFactory.getLogger(clazz);
     try {
-      Class log4jClass = Class.forName("org.slf4j.impl.Log4jLoggerAdapter");
+      Class<?> log4jClass = Class.forName("org.slf4j.impl.Log4jLoggerAdapter");

Review Comment:
   make this a constant string and use everywhere it is needed



##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/GenericsUtil.java:
##########
@@ -89,10 +89,30 @@ public static boolean isLog4jLogger(Class<?> clazz) {
     }
     Logger log = LoggerFactory.getLogger(clazz);
     try {
-      Class log4jClass = Class.forName("org.slf4j.impl.Log4jLoggerAdapter");
+      Class<?> log4jClass = Class.forName("org.slf4j.impl.Log4jLoggerAdapter");
       return log4jClass.isInstance(log);
     } catch (ClassNotFoundException e) {
       return false;
     }
   }
+
+  /**
+   * Determine whether the log of the given logger is of Log4J implementation.
+   *
+   * @param logger the logger name, usually class name as string.
+   * @return true if the logger uses Log4J implementation.
+   */
+  public static boolean isLog4jLogger(String logger) {
+    if (logger == null) {
+      return false;
+    }
+    Logger log = LoggerFactory.getLogger(logger);
+    try {
+      Class<?> log4jClass = Class.forName("org.slf4j.impl.Log4jLoggerAdapter");

Review Comment:
   if the class isn't found, then that fact can be remembered in an atomic 
boolean so future loads/checks skipped. 



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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to