[GitHub] [hadoop] virajjasani commented on a diff in pull request #5503: HADOOP-18207. Introduce hadoop-logging module

["virajjasani (via GitHub)" <gi...@apache.org>]

virajjasani commented on code in PR #5503:
URL: https://github.com/apache/hadoop/pull/5503#discussion_r1199921484


##########
hadoop-common-project/hadoop-common/src/main/conf/log4j.properties:
##########
@@ -299,7 +299,7 @@ log4j.appender.NMAUDIT.MaxBackupIndex=${nm.audit.log.maxbackupindex}
 yarn.ewma.cleanupInterval=300
 yarn.ewma.messageAgeLimitSeconds=86400
 yarn.ewma.maxUniqueMessages=250
-log4j.appender.EWMA=org.apache.hadoop.yarn.util.Log4jWarningErrorMetricsAppender
+log4j.appender.EWMA=org.apache.hadoop.logging.appenders.Log4jWarningErrorMetricsAppender

Review Comment:
   that's because with this PR, all appenders (including `Log4jWarningErrorMetricsAppender`) are moved to hadoop-logging



##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfiguration.java:
##########
@@ -220,27 +219,21 @@ public void testFinalWarnings() throws Exception {
     InputStream in2 = new ByteArrayInputStream(bytes2);
 
     // Attach our own log appender so we can verify output
-    TestAppender appender = new TestAppender();
-    final Logger logger = Logger.getRootLogger();
-    logger.addAppender(appender);
+    LogCapturer logCapturer = LogCapturer.captureLogs(LoggerFactory.getLogger("root"));

Review Comment:
   correct, this is the right way



##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/log/LogLevel.java:
##########
@@ -349,7 +348,7 @@ public void doGet(HttpServletRequest request, HttpServletResponse response
         }
 
         if (GenericsUtil.isLog4jLogger(logName)) {

Review Comment:
   yes unfortunately, we will still have some log4j references outside of hadoop-logging, moving all references is extremely difficult.
   i am planning to get upgraded to log4j2 with this change and then eventually we can try moving all usages to hadoop-logging. With this PR, we have almost 95% of log4j references moved to hadoop-logging.



-- 
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: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org