From db36747e831f7045a3e1a0ca390c944981dd141a Mon Sep 17 00:00:00 2001 From: Duo Zhang Date: Thu, 10 Mar 2022 10:15:09 +0800 Subject: [PATCH] HADOOP-17526 Use Slf4jRequestLog for HttpRequestLog (#4050) Signed-off-by: Wei-Chiu Chuang --- .../src/main/conf/log4j.properties | 55 ++++++++------ .../apache/hadoop/http/HttpRequestLog.java | 72 ++++--------------- .../hadoop/http/HttpRequestLogAppender.java | 62 ---------------- .../hadoop/http/TestHttpRequestLog.java | 29 +++----- .../http/TestHttpRequestLogAppender.java | 37 ---------- .../hadoop/http/TestHttpServerLifecycle.java | 21 ------ 6 files changed, 60 insertions(+), 216 deletions(-) delete mode 100644 hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpRequestLogAppender.java delete mode 100644 hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/TestHttpRequestLogAppender.java diff --git a/hadoop-common-project/hadoop-common/src/main/conf/log4j.properties b/hadoop-common-project/hadoop-common/src/main/conf/log4j.properties index 5a2ca4d9228..54d5c729848 100644 --- a/hadoop-common-project/hadoop-common/src/main/conf/log4j.properties +++ b/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.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.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.namenode=INFO,AccessNNDRFA -#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.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.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.datanode=INFO,AccessDNDRFA -#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.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 diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpRequestLog.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpRequestLog.java index b2f18538b6c..863afdf1f2e 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpRequestLog.java +++ b/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 serverToComponent; + private static final Map serverToComponent; static { - serverToComponent = new HashMap(); - serverToComponent.put("cluster", "resourcemanager"); - serverToComponent.put("hdfs", "namenode"); - serverToComponent.put("node", "nodemanager"); + Map map = new HashMap(); + 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); + Slf4jRequestLogWriter writer = new Slf4jRequestLogWriter(); + writer.setLoggerName(loggerName); + return new CustomRequestLog(writer, CustomRequestLog.EXTENDED_NCSA_FORMAT); + } - 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; - } - - 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() { } } diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpRequestLogAppender.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpRequestLogAppender.java deleted file mode 100644 index eda1d1fee40..00000000000 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpRequestLogAppender.java +++ /dev/null @@ -1,62 +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.http; - -import org.apache.log4j.spi.LoggingEvent; -import org.apache.log4j.AppenderSkeleton; - -/** - * Log4j Appender adapter for HttpRequestLog - */ -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() { - } - - @Override - public boolean requiresLayout() { - return false; - } -} diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/TestHttpRequestLog.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/TestHttpRequestLog.java index d0123e32039..58721c4baa8 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/TestHttpRequestLog.java +++ b/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 org.eclipse.jetty.server.CustomRequestLog; -import org.eclipse.jetty.server.RequestLog; -import org.junit.Test; - +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 static org.junit.Assert.assertNull; + +import org.eclipse.jetty.server.CustomRequestLog; +import org.eclipse.jetty.server.RequestLog; +import org.eclipse.jetty.server.Slf4jRequestLogWriter; +import org.junit.Test; 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()); } } diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/TestHttpRequestLogAppender.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/TestHttpRequestLogAppender.java deleted file mode 100644 index e84bee06e6e..00000000000 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/TestHttpRequestLogAppender.java +++ /dev/null @@ -1,37 +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.http; - -import org.junit.Test; - -import static org.junit.Assert.assertEquals; - -public class TestHttpRequestLogAppender { - - @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()); - } -} diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/TestHttpServerLifecycle.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/TestHttpServerLifecycle.java index 40f1b3df08c..757ea0c05e7 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/TestHttpServerLifecycle.java +++ b/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