HADOOP-17526 Use Slf4jRequestLog for HttpRequestLog

hadoop-common-project/hadoop-common/src/main/conf/log4j.properties

@@ -251,30 +251,45 @@ log4j.appender.NMAUDIT.MaxBackupIndex=${nm.audit.log.maxbackupindex}
 #log4j.appender.HSAUDIT.DatePattern=.yyyy-MM-dd
 
 # Http Server Request Logs
-#log4j.logger.http.requests.namenode=INFO,namenoderequestlog
-#log4j.appender.namenoderequestlog=org.apache.hadoop.http.HttpRequestLogAppender
-#log4j.appender.namenoderequestlog.Filename=${hadoop.log.dir}/jetty-namenode-yyyy_mm_dd.log
-#log4j.appender.namenoderequestlog.RetainDays=3
-
-#log4j.logger.http.requests.datanode=INFO,datanoderequestlog
-#log4j.appender.datanoderequestlog=org.apache.hadoop.http.HttpRequestLogAppender
-#log4j.appender.datanoderequestlog.Filename=${hadoop.log.dir}/jetty-datanode-yyyy_mm_dd.log
-#log4j.appender.datanoderequestlog.RetainDays=3
-
-#log4j.logger.http.requests.resourcemanager=INFO,resourcemanagerrequestlog
-#log4j.appender.resourcemanagerrequestlog=org.apache.hadoop.http.HttpRequestLogAppender
-#log4j.appender.resourcemanagerrequestlog.Filename=${hadoop.log.dir}/jetty-resourcemanager-yyyy_mm_dd.log
-#log4j.appender.resourcemanagerrequestlog.RetainDays=3
-
-#log4j.logger.http.requests.jobhistory=INFO,jobhistoryrequestlog
-#log4j.appender.jobhistoryrequestlog=org.apache.hadoop.http.HttpRequestLogAppender
-#log4j.appender.jobhistoryrequestlog.Filename=${hadoop.log.dir}/jetty-jobhistory-yyyy_mm_dd.log
-#log4j.appender.jobhistoryrequestlog.RetainDays=3
-
-#log4j.logger.http.requests.nodemanager=INFO,nodemanagerrequestlog
-#log4j.appender.nodemanagerrequestlog=org.apache.hadoop.http.HttpRequestLogAppender
-#log4j.appender.nodemanagerrequestlog.Filename=${hadoop.log.dir}/jetty-nodemanager-yyyy_mm_dd.log
-#log4j.appender.nodemanagerrequestlog.RetainDays=3
+#log4j.appender.AccessNNDRFA=org.apache.log4j.DailyRollingFileAppender
+#log4j.appender.AccessNNDRFA.File=${hadoop.log.dir}/jetty-namenode.log
+#log4j.appender.AccessNNDRFA.DatePattern=.yyyy-MM-dd
+#log4j.appender.AccessNNDRFA.layout=org.apache.log4j.PatternLayout
+#log4j.appender.AccessNNDRFA.layout.ConversionPattern=%m%n
+
+#log4j.logger.http.requests.namenode=INFO,AccessNNDRFA
+
+#log4j.appender.AccessDNDRFA=org.apache.log4j.DailyRollingFileAppender
+#log4j.appender.AccessDNDRFA.File=${hadoop.log.dir}/jetty-datanode.log
+#log4j.appender.AccessDNDRFA.DatePattern=.yyyy-MM-dd
+#log4j.appender.AccessDNDRFA.layout=org.apache.log4j.PatternLayout
+#log4j.appender.AccessDNDRFA.layout.ConversionPattern=%m%n
+
+#log4j.logger.http.requests.datanode=INFO,AccessDNDRFA
+
+#log4j.appender.AccessRMDRFA=org.apache.log4j.DailyRollingFileAppender
+#log4j.appender.AccessRMDRFA.File=${hadoop.log.dir}/jetty-resourcemanager.log
+#log4j.appender.AccessRMDRFA.DatePattern=.yyyy-MM-dd
+#log4j.appender.AccessRMDRFA.layout=org.apache.log4j.PatternLayout
+#log4j.appender.AccessRMDRFA.layout.ConversionPattern=%m%n
+
+#log4j.logger.http.requests.resourcemanager=INFO,AccessRMDRFA
+
+#log4j.appender.AccessJHDRFA=org.apache.log4j.DailyRollingFileAppender
+#log4j.appender.AccessJHDRFA.File=${hadoop.log.dir}/jetty-jobhistory.log
+#log4j.appender.AccessJHDRFA.DatePattern=.yyyy-MM-dd
+#log4j.appender.AccessJHDRFA.layout=org.apache.log4j.PatternLayout
+#log4j.appender.AccessJHDRFA.layout.ConversionPattern=%m%n
+
+#log4j.logger.http.requests.jobhistory=INFO,AccessJHDRFA
+
+#log4j.appender.AccessNMDRFA=org.apache.log4j.DailyRollingFileAppender
+#log4j.appender.AccessNMDRFA.File=${hadoop.log.dir}/jetty-jobhistory.log
+#log4j.appender.AccessNMDRFA.DatePattern=.yyyy-MM-dd
+#log4j.appender.AccessNMDRFA.layout=org.apache.log4j.PatternLayout
+#log4j.appender.AccessNMDRFA.layout.ConversionPattern=%m%n
+
+#log4j.logger.http.requests.nodemanager=INFO,AccessNMDRFA
 
 # WebHdfs request log on datanodes
 # Specify -Ddatanode.webhdfs.logger=INFO,HTTPDRFA on datanode startup to

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpRequestLog.java

@@ -17,16 +17,12 @@
  */
 package org.apache.hadoop.http;
 
+import java.util.Collections;
 import java.util.HashMap;
-
-import org.apache.commons.logging.impl.Log4JLogger;
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogConfigurationException;
-import org.apache.commons.logging.LogFactory;
-import org.apache.log4j.Appender;
-import org.eclipse.jetty.server.AsyncRequestLogWriter;
+import java.util.Map;
 import org.eclipse.jetty.server.CustomRequestLog;
 import org.eclipse.jetty.server.RequestLog;
+import org.eclipse.jetty.server.Slf4jRequestLogWriter;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -37,67 +33,27 @@ public class HttpRequestLog {
 
   public static final Logger LOG =
       LoggerFactory.getLogger(HttpRequestLog.class);
-  private static final HashMap<String, String> serverToComponent;
+  private static final Map<String, String> serverToComponent;
 
   static {
-    serverToComponent = new HashMap<String, String>();
-    serverToComponent.put("cluster", "resourcemanager");
-    serverToComponent.put("hdfs", "namenode");
-    serverToComponent.put("node", "nodemanager");
+    Map<String, String > map = new HashMap<String, String>();
+    map.put("cluster", "resourcemanager");
+    map.put("hdfs", "namenode");
+    map.put("node", "nodemanager");
+    serverToComponent = Collections.unmodifiableMap(map);
   }
 
   public static RequestLog getRequestLog(String name) {
-
     String lookup = serverToComponent.get(name);
     if (lookup != null) {
       name = lookup;
     }
     String loggerName = "http.requests." + name;
-    String appenderName = name + "requestlog";
-    Log logger = LogFactory.getLog(loggerName);
-
-    boolean isLog4JLogger;;
-    try {
-      isLog4JLogger = logger instanceof Log4JLogger;
-    } catch (NoClassDefFoundError err) {
-      // In some dependent projects, log4j may not even be on the classpath at
-      // runtime, in which case the above instanceof check will throw
-      // NoClassDefFoundError.
-      LOG.debug("Could not load Log4JLogger class", err);
-      isLog4JLogger = false;
-    }
-    if (isLog4JLogger) {
-      Log4JLogger httpLog4JLog = (Log4JLogger)logger;
-      org.apache.log4j.Logger httpLogger = httpLog4JLog.getLogger();
-      Appender appender = null;
-
-      try {
-        appender = httpLogger.getAppender(appenderName);
-      } catch (LogConfigurationException e) {
-        LOG.warn("Http request log for {} could not be created", loggerName);
-        throw e;
-      }
-
-      if (appender == null) {
-        LOG.info("Http request log for {} is not defined", loggerName);
-        return null;
-      }
+    Slf4jRequestLogWriter writer = new Slf4jRequestLogWriter();
+    writer.setLoggerName(loggerName);
+    return new CustomRequestLog(writer, CustomRequestLog.EXTENDED_NCSA_FORMAT);
+  }
 
-      if (appender instanceof HttpRequestLogAppender) {
-        HttpRequestLogAppender requestLogAppender
-          = (HttpRequestLogAppender)appender;
-        AsyncRequestLogWriter logWriter = new AsyncRequestLogWriter();
-        logWriter.setFilename(requestLogAppender.getFilename());
-        logWriter.setRetainDays(requestLogAppender.getRetainDays());
-        return new CustomRequestLog(logWriter,
-            CustomRequestLog.EXTENDED_NCSA_FORMAT);
-      } else {
-        LOG.warn("Jetty request log for {} was of the wrong class", loggerName);
-        return null;
-      }
-    } else {
-      LOG.warn("Jetty request log can only be enabled using Log4j");
-      return null;
-    }
+  private HttpRequestLog() {
   }
 }

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/TestHttpRequestLog.java

@@ -17,32 +17,25 @@
  */
 package org.apache.hadoop.http;
 
-import org.apache.log4j.Logger;
+import static org.hamcrest.CoreMatchers.instanceOf;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+
 import org.eclipse.jetty.server.CustomRequestLog;
 import org.eclipse.jetty.server.RequestLog;
+import org.eclipse.jetty.server.Slf4jRequestLogWriter;
 import org.junit.Test;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-
 public class TestHttpRequestLog {
 
-  @Test
-  public void testAppenderUndefined() {
-    RequestLog requestLog = HttpRequestLog.getRequestLog("test");
-    assertNull("RequestLog should be null", requestLog);
-  }
-
   @Test
   public void testAppenderDefined() {
-    HttpRequestLogAppender requestLogAppender = new HttpRequestLogAppender();
-    requestLogAppender.setName("testrequestlog");
-    Logger.getLogger("http.requests.test").addAppender(requestLogAppender);
     RequestLog requestLog = HttpRequestLog.getRequestLog("test");
-    Logger.getLogger("http.requests.test").removeAppender(requestLogAppender);
     assertNotNull("RequestLog should not be null", requestLog);
-    assertEquals("Class mismatch",
-        CustomRequestLog.class, requestLog.getClass());
+    assertThat(requestLog, instanceOf(CustomRequestLog.class));
+    CustomRequestLog crl = (CustomRequestLog) requestLog;
+    assertThat(crl.getWriter(), instanceOf(Slf4jRequestLogWriter.class));
+    assertEquals(CustomRequestLog.EXTENDED_NCSA_FORMAT, crl.getFormatString());
   }
 }

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/TestHttpServerLifecycle.java

@@ -68,27 +68,6 @@ public void testStartedServerIsAlive() throws Throwable {
     stop(server);
   }
 
-  /**
-   * Test that the server with request logging enabled
-   *
-   * @throws Throwable on failure
-   */
-  @Test
-  public void testStartedServerWithRequestLog() throws Throwable {
-    HttpRequestLogAppender requestLogAppender = new HttpRequestLogAppender();
-    requestLogAppender.setName("httprequestlog");
-    requestLogAppender.setFilename(
-        GenericTestUtils.getTempPath("jetty-name-yyyy_mm_dd.log"));
-    Logger.getLogger(HttpServer2.class.getName() + ".test").addAppender(requestLogAppender);
-    HttpServer2 server = null;
-    server = createTestServer();
-    assertNotLive(server);
-    server.start();
-    assertAlive(server);
-    stop(server);
-    Logger.getLogger(HttpServer2.class.getName() + ".test").removeAppender(requestLogAppender);
-  }
-
   /**
    * Assert that the result of {@link HttpServer2#toString()} contains the specific text
    * @param server server to examine