You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by zh...@apache.org on 2020/05/08 03:27:48 UTC
[hbase] branch branch-2.3 updated: HBASE-24310 Use Slf4jRequestLog
for hbase-http (#1634)
This is an automated email from the ASF dual-hosted git repository.
zhangduo pushed a commit to branch branch-2.3
in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/branch-2.3 by this push:
new 0f1b9d0 HBASE-24310 Use Slf4jRequestLog for hbase-http (#1634)
0f1b9d0 is described below
commit 0f1b9d0811822b364b56ae23665ca9e8117e1048
Author: Duo Zhang <zh...@apache.org>
AuthorDate: Fri May 8 11:16:18 2020 +0800
HBASE-24310 Use Slf4jRequestLog for hbase-http (#1634)
Signed-off-by: stack <st...@apache.org>
---
conf/log4j.properties | 4 ++
hbase-http/pom.xml | 14 ++--
.../apache/hadoop/hbase/http/HttpRequestLog.java | 81 +++-------------------
.../hadoop/hbase/http/HttpRequestLogAppender.java | 65 -----------------
.../org/apache/hadoop/hbase/http/log/LogLevel.java | 3 -
.../hadoop/hbase/http/TestHttpRequestLog.java | 20 ++----
.../hbase/http/TestHttpRequestLogAppender.java | 47 -------------
7 files changed, 29 insertions(+), 205 deletions(-)
diff --git a/conf/log4j.properties b/conf/log4j.properties
index af28319..0af1da7 100644
--- a/conf/log4j.properties
+++ b/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
diff --git a/hbase-http/pom.xml b/hbase-http/pom.xml
index 7c2237c..c62b12a 100644
--- a/hbase-http/pom.xml
+++ b/hbase-http/pom.xml
@@ -198,10 +198,6 @@
<artifactId>slf4j-api</artifactId>
</dependency>
<dependency>
- <groupId>log4j</groupId>
- <artifactId>log4j</artifactId>
- </dependency>
- <dependency>
<groupId>javax.servlet</groupId>
<artifactId>javax.servlet-api</artifactId>
</dependency>
@@ -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 -->
diff --git a/hbase-http/src/main/java/org/apache/hadoop/hbase/http/HttpRequestLog.java b/hbase-http/src/main/java/org/apache/hadoop/hbase/http/HttpRequestLog.java
index c83fa4f..0eb66a7 100644
--- a/hbase-http/src/main/java/org/apache/hadoop/hbase/http/HttpRequestLog.java
+++ b/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.slf4j.LoggerFactory;
-import org.slf4j.impl.Log4jLoggerAdapter;
+import org.eclipse.jetty.server.Slf4jRequestLog;
+
+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 HashMap<String, String> serverToComponent;
-
- 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;
- }
- }
+ private static final ImmutableMap<String, String> SERVER_TO_COMPONENT =
+ ImmutableMap.of("master", "master", "region", "regionserver");
public static RequestLog getRequestLog(String name) {
-
- String lookup = serverToComponent.get(name);
+ String lookup = SERVER_TO_COMPONENT.get(name);
if (lookup != null) {
name = lookup;
}
String loggerName = "http.requests." + name;
- String appenderName = name + "requestlog";
-
- org.apache.log4j.Logger httpLogger = getLog4jLogger(loggerName);
-
- 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;
- }
+ Slf4jRequestLog logger = new Slf4jRequestLog();
+ logger.setLoggerName(loggerName);
+ return logger;
}
- private HttpRequestLog() {}
+ private HttpRequestLog() {
+ }
}
diff --git a/hbase-http/src/main/java/org/apache/hadoop/hbase/http/HttpRequestLogAppender.java b/hbase-http/src/main/java/org/apache/hadoop/hbase/http/HttpRequestLogAppender.java
deleted file mode 100644
index fe90498..0000000
--- a/hbase-http/src/main/java/org/apache/hadoop/hbase/http/HttpRequestLogAppender.java
+++ /dev/null
@@ -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;
- }
-}
diff --git a/hbase-http/src/main/java/org/apache/hadoop/hbase/http/log/LogLevel.java b/hbase-http/src/main/java/org/apache/hadoop/hbase/http/log/LogLevel.java
index 8135cbb..3ab3957 100644
--- a/hbase-http/src/main/java/org/apache/hadoop/hbase/http/log/LogLevel.java
+++ b/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;
diff --git a/hbase-http/src/test/java/org/apache/hadoop/hbase/http/TestHttpRequestLog.java b/hbase-http/src/test/java/org/apache/hadoop/hbase/http/TestHttpRequestLog.java
index cbda13c..eef5e7b 100644
--- a/hbase-http/src/test/java/org/apache/hadoop/hbase/http/TestHttpRequestLog.java
+++ b/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);
-
- @Test
- public void testAppenderUndefined() {
- RequestLog requestLog = HttpRequestLog.getRequestLog("test");
- assertNull("RequestLog should be null", requestLog);
- }
+ HBaseClassTestRule.forClass(TestHttpRequestLog.class);
@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());
}
}
diff --git a/hbase-http/src/test/java/org/apache/hadoop/hbase/http/TestHttpRequestLogAppender.java b/hbase-http/src/test/java/org/apache/hadoop/hbase/http/TestHttpRequestLogAppender.java
deleted file mode 100644
index fa082c9..0000000
--- a/hbase-http/src/test/java/org/apache/hadoop/hbase/http/TestHttpRequestLogAppender.java
+++ /dev/null
@@ -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());
- }
-}