HBASE-24310 Use Slf4jRequestLog for hbase-http (#1634)

Signed-off-by: stack <stack@apache.org>

conf/log4j.properties

@@ -122,3 +122,7 @@ log4j.logger.org.apache.hadoop.hbase.zookeeper.ZKWatcher=${hbase.log.level}
 log4j.logger.org.apache.hadoop.metrics2.impl.MetricsConfig=WARN
 log4j.logger.org.apache.hadoop.metrics2.impl.MetricsSinkAdapter=WARN
 log4j.logger.org.apache.hadoop.metrics2.impl.MetricsSystemImpl=WARN
+
+# Disable request log by default, you can enable this by changing the appender
+log4j.category.http.requests=INFO,NullAppender
+log4j.additivity.http.requests=false

hbase-http/pom.xml

@@ -197,10 +197,6 @@
       <groupId>org.slf4j</groupId>
       <artifactId>slf4j-api</artifactId>
     </dependency>
-    <dependency>
-      <groupId>log4j</groupId>
-      <artifactId>log4j</artifactId>
-    </dependency>
     <dependency>
       <groupId>javax.servlet</groupId>
       <artifactId>javax.servlet-api</artifactId>
@@ -256,6 +252,16 @@
       <artifactId>hadoop-minikdc</artifactId>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.slf4j</groupId>
+      <artifactId>slf4j-log4j12</artifactId>
+      <scope>provided</scope>
+    </dependency>
+    <dependency>
+      <groupId>log4j</groupId>
+      <artifactId>log4j</artifactId>
+      <scope>provided</scope>
+    </dependency>
   </dependencies>
   <profiles>
     <!-- Needs to make the profile in apache parent pom -->

hbase-http/src/main/java/org/apache/hadoop/hbase/http/HttpRequestLog.java

@@ -17,17 +17,11 @@
  */
 package org.apache.hadoop.hbase.http;
 
-import java.util.HashMap;
-import org.apache.commons.logging.LogConfigurationException;
-import org.apache.commons.logging.impl.Log4JLogger;
-import org.apache.log4j.Appender;
-import org.apache.log4j.LogManager;
 import org.apache.yetus.audience.InterfaceAudience;
-import org.eclipse.jetty.server.NCSARequestLog;
 import org.eclipse.jetty.server.RequestLog;
-import org.slf4j.Logger;
+import org.eclipse.jetty.server.Slf4jRequestLog;
-import org.slf4j.LoggerFactory;
+
-import org.slf4j.impl.Log4jLoggerAdapter;
+import org.apache.hbase.thirdparty.com.google.common.collect.ImmutableMap;
 
 /**
  * RequestLog object for use with Http
@@ -35,73 +29,20 @@ import org.slf4j.impl.Log4jLoggerAdapter;
 @InterfaceAudience.Private
 public final class HttpRequestLog {
 
-  private static final Logger LOG = LoggerFactory.getLogger(HttpRequestLog.class);
+  private static final ImmutableMap<String, String> SERVER_TO_COMPONENT =
-  private static final HashMap<String, String> serverToComponent;
+    ImmutableMap.of("master", "master", "region", "regionserver");
-
-  static {
-    serverToComponent = new HashMap<>();
-    serverToComponent.put("master", "master");
-    serverToComponent.put("region", "regionserver");
-  }
-
-  private static org.apache.log4j.Logger getLog4jLogger(String loggerName) {
-    Logger logger = LoggerFactory.getLogger(loggerName);
-
-    if (logger instanceof Log4JLogger) {
-      Log4JLogger httpLog4JLog = (Log4JLogger)logger;
-      return httpLog4JLog.getLogger();
-    } else if (logger instanceof Log4jLoggerAdapter) {
-      return LogManager.getLogger(loggerName);
-    } else {
-      return null;
-    }
-  }
 
   public static RequestLog getRequestLog(String name) {
-
+    String lookup = SERVER_TO_COMPONENT.get(name);
-    String lookup = serverToComponent.get(name);
     if (lookup != null) {
       name = lookup;
     }
     String loggerName = "http.requests." + name;
-    String appenderName = name + "requestlog";
+    Slf4jRequestLog logger = new Slf4jRequestLog();
-
+    logger.setLoggerName(loggerName);
-    org.apache.log4j.Logger httpLogger = getLog4jLogger(loggerName);
+    return logger;
-
-    if (httpLogger == null) {
-      LOG.warn("Jetty request log can only be enabled using Log4j");
-      return null;
-    }
-
-    Appender appender = null;
-
-    try {
-      appender = httpLogger.getAppender(appenderName);
-    } catch (LogConfigurationException e) {
-      LOG.warn("Http request log for " + loggerName
-          + " could not be created");
-      throw e;
-    }
-
-    if (appender == null) {
-      LOG.info("Http request log for " + loggerName
-          + " is not defined");
-      return null;
-    }
-
-    if (appender instanceof HttpRequestLogAppender) {
-      HttpRequestLogAppender requestLogAppender
-        = (HttpRequestLogAppender)appender;
-      NCSARequestLog requestLog = new NCSARequestLog();
-      requestLog.setFilename(requestLogAppender.getFilename());
-      requestLog.setRetainDays(requestLogAppender.getRetainDays());
-      return requestLog;
-    } else {
-      LOG.warn("Jetty request log for " + loggerName
-          + " was of the wrong class");
-      return null;
-    }
   }
 
-  private HttpRequestLog() {}
+  private HttpRequestLog() {
+  }
 }

hbase-http/src/main/java/org/apache/hadoop/hbase/http/HttpRequestLogAppender.java

@@ -1,65 +0,0 @@
-/**
- * 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 org.apache.hadoop.hbase.http;
-
-import org.apache.log4j.AppenderSkeleton;
-import org.apache.log4j.spi.LoggingEvent;
-import org.apache.yetus.audience.InterfaceAudience;
-
-/**
- * Log4j Appender adapter for HttpRequestLog
- */
-@InterfaceAudience.Private
-public class HttpRequestLogAppender extends AppenderSkeleton {
-
-  private String filename;
-  private int retainDays;
-
-  public HttpRequestLogAppender() {
-  }
-
-  public void setRetainDays(int retainDays) {
-    this.retainDays = retainDays;
-  }
-
-  public int getRetainDays() {
-    return retainDays;
-  }
-
-  public void setFilename(String filename) {
-    this.filename = filename;
-  }
-
-  public String getFilename() {
-    return filename;
-  }
-
-  @Override
-  public void append(LoggingEvent event) {
-  }
-
-  @Override
-  public void close() {
-      // Do nothing, we don't have close() on AppenderSkeleton.
-  }
-
-  @Override
-  public boolean requiresLayout() {
-    return false;
-  }
-}

hbase-http/src/main/java/org/apache/hadoop/hbase/http/log/LogLevel.java

@@ -26,14 +26,12 @@ import java.net.URL;
 import java.net.URLConnection;
 import java.util.Objects;
 import java.util.regex.Pattern;
-
 import javax.net.ssl.HttpsURLConnection;
 import javax.net.ssl.SSLSocketFactory;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
-
 import org.apache.commons.logging.impl.Jdk14Logger;
 import org.apache.commons.logging.impl.Log4JLogger;
 import org.apache.hadoop.HadoopIllegalArgumentException;
@@ -48,7 +46,6 @@ import org.apache.hadoop.util.Tool;
 import org.apache.log4j.LogManager;
 import org.apache.yetus.audience.InterfaceAudience;
 import org.apache.yetus.audience.InterfaceStability;
-
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.slf4j.impl.Log4jLoggerAdapter;

hbase-http/src/test/java/org/apache/hadoop/hbase/http/TestHttpRequestLog.java

@@ -19,39 +19,27 @@ package org.apache.hadoop.hbase.http;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
 
 import org.apache.hadoop.hbase.HBaseClassTestRule;
 import org.apache.hadoop.hbase.testclassification.MiscTests;
 import org.apache.hadoop.hbase.testclassification.SmallTests;
-import org.apache.log4j.Logger;
-import org.eclipse.jetty.server.NCSARequestLog;
 import org.eclipse.jetty.server.RequestLog;
+import org.eclipse.jetty.server.Slf4jRequestLog;
 import org.junit.ClassRule;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
-@Category({MiscTests.class, SmallTests.class})
+@Category({ MiscTests.class, SmallTests.class })
 public class TestHttpRequestLog {
 
   @ClassRule
   public static final HBaseClassTestRule CLASS_RULE =
-      HBaseClassTestRule.forClass(TestHttpRequestLog.class);
+    HBaseClassTestRule.forClass(TestHttpRequestLog.class);
-
-  @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", NCSARequestLog.class, requestLog.getClass());
+    assertEquals("Class mismatch", Slf4jRequestLog.class, requestLog.getClass());
   }
 }

hbase-http/src/test/java/org/apache/hadoop/hbase/http/TestHttpRequestLogAppender.java

@@ -1,47 +0,0 @@
-/**
- * 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 org.apache.hadoop.hbase.http;
-
-import static org.junit.Assert.assertEquals;
-
-import org.apache.hadoop.hbase.HBaseClassTestRule;
-import org.apache.hadoop.hbase.testclassification.MiscTests;
-import org.apache.hadoop.hbase.testclassification.SmallTests;
-import org.junit.ClassRule;
-import org.junit.Test;
-import org.junit.experimental.categories.Category;
-
-@Category({MiscTests.class, SmallTests.class})
-public class TestHttpRequestLogAppender {
-
-  @ClassRule
-  public static final HBaseClassTestRule CLASS_RULE =
-      HBaseClassTestRule.forClass(TestHttpRequestLogAppender.class);
-
-  @Test
-  public void testParameterPropagation() {
-
-    HttpRequestLogAppender requestLogAppender = new HttpRequestLogAppender();
-    requestLogAppender.setFilename("jetty-namenode-yyyy_mm_dd.log");
-    requestLogAppender.setRetainDays(17);
-    assertEquals("Filename mismatch", "jetty-namenode-yyyy_mm_dd.log",
-        requestLogAppender.getFilename());
-    assertEquals("Retain days mismatch", 17,
-        requestLogAppender.getRetainDays());
-  }
-}