From fddb2dd65c28cb3a98ca21b82dfba07ea5f24750 Mon Sep 17 00:00:00 2001 From: Duo Zhang Date: Fri, 8 May 2020 11:16:18 +0800 Subject: [PATCH] HBASE-24310 Use Slf4jRequestLog for hbase-http (#1634) Signed-off-by: stack --- conf/log4j.properties | 4 + hbase-http/pom.xml | 14 +++- .../hadoop/hbase/http/HttpRequestLog.java | 81 +++---------------- .../hbase/http/HttpRequestLogAppender.java | 65 --------------- .../hadoop/hbase/http/log/LogLevel.java | 3 - .../hadoop/hbase/http/TestHttpRequestLog.java | 20 +---- .../http/TestHttpRequestLogAppender.java | 47 ----------- 7 files changed, 29 insertions(+), 205 deletions(-) delete mode 100644 hbase-http/src/main/java/org/apache/hadoop/hbase/http/HttpRequestLogAppender.java delete mode 100644 hbase-http/src/test/java/org/apache/hadoop/hbase/http/TestHttpRequestLogAppender.java diff --git a/conf/log4j.properties b/conf/log4j.properties index af283191e0b..0af1da71752 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 e5ad251d207..c57fed5da2d 100644 --- a/hbase-http/pom.xml +++ b/hbase-http/pom.xml @@ -197,10 +197,6 @@ org.slf4j slf4j-api - - log4j - log4j - javax.servlet javax.servlet-api @@ -256,6 +252,16 @@ hadoop-minikdc test + + org.slf4j + slf4j-log4j12 + provided + + + log4j + log4j + provided + 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 c83fa4feb8d..0eb66a7c72c 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 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 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 fe90498775e..00000000000 --- 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 8135cbbd5b0..3ab3957a144 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 cbda13c826d..eef5e7b6538 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 fa082c988de..00000000000 --- 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()); - } -}