diff --git a/common/src/java/org/apache/hive/http/Log4j2ConfiguratorServlet.java b/common/src/java/org/apache/hive/http/Log4j2ConfiguratorServlet.java index 9e27069dceb9..509d89fb6e4c 100644 --- a/common/src/java/org/apache/hive/http/Log4j2ConfiguratorServlet.java +++ b/common/src/java/org/apache/hive/http/Log4j2ConfiguratorServlet.java @@ -228,31 +228,39 @@ protected void doPost(final HttpServletRequest request, final HttpServletRespons } private void configureLogger(final ConfLoggers confLoggers) { - if (confLoggers != null) { - for (ConfLogger logger : confLoggers.getLoggers()) { - String loggerName = logger.getLogger(); - Level logLevel = Level.getLevel(logger.getLevel()); - if (logLevel == null) { - LOG.warn("Invalid log level: {} for logger: {}. Ignoring reconfiguration.", loggerName, logger.getLevel()); - continue; - } - - LoggerConfig loggerConfig = conf.getLoggerConfig(loggerName); - // if the logger name is not found, root logger is returned. We don't want to change root logger level - // since user either requested a new logger or specified invalid input. In which, we will add the logger - // that user requested. - if (!loggerName.equals(LogManager.ROOT_LOGGER_NAME) && - loggerConfig.getName().equals(LogManager.ROOT_LOGGER_NAME)) { - LOG.debug("Requested logger ({}) not found. Adding as new logger with {} level", loggerName, logLevel); - // requested logger not found. Add the new logger with the requested level - conf.addLogger(loggerName, new LoggerConfig(loggerName, logLevel, true)); - } else { - LOG.debug("Updating logger ({}) to {} level", loggerName, logLevel); - // update the log level for the specified logger - loggerConfig.setLevel(logLevel); - } + if (confLoggers == null) { + return; + } + for (ConfLogger logger : confLoggers.getLoggers()) { + String loggerName = logger.getLogger(); + Level logLevel = Level.getLevel(logger.getLevel()); + if (logLevel == null) { + LOG.warn("Invalid log level: {} for logger: {}. Ignoring reconfiguration.", logger.getLevel(), loggerName); + continue; } - context.updateLoggers(conf); + setLogLevel(loggerName, logLevel); + } + context.updateLoggers(conf); + } + + /** + * Sets the level for a single logger, adding a new logger when it is not explicitly + * configured yet. + *

+ * {@link Configuration#getLoggerConfig(String)} never returns {@code null}: for an + * unconfigured name it returns the closest configured ancestor (the root logger for a + * brand-new name). Comparing the requested name against the returned config's name is + * therefore required so that setting a level for a not-yet-configured child logger does + * not silently change one of its ancestors instead. + */ + void setLogLevel(final String loggerName, final Level logLevel) { + LoggerConfig loggerConfig = conf.getLoggerConfig(loggerName); + if (loggerName.equals(loggerConfig.getName())) { + LOG.debug("Updating logger ({}) to {} level", loggerName, logLevel); + loggerConfig.setLevel(logLevel); + } else { + LOG.debug("Logger ({}) not configured. Adding as new logger with {} level", loggerName, logLevel); + conf.addLogger(loggerName, new LoggerConfig(loggerName, logLevel, true)); } } diff --git a/common/src/test/org/apache/hive/http/TestLog4j2ConfiguratorServlet.java b/common/src/test/org/apache/hive/http/TestLog4j2ConfiguratorServlet.java new file mode 100644 index 000000000000..8db811a0ef29 --- /dev/null +++ b/common/src/test/org/apache/hive/http/TestLog4j2ConfiguratorServlet.java @@ -0,0 +1,130 @@ +/* + * 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.hive.http; + +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.core.LoggerContext; +import org.apache.logging.log4j.core.config.Configuration; +import org.apache.logging.log4j.core.config.LoggerConfig; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import static org.junit.Assert.assertEquals; + +/** + * Tests for {@link Log4j2ConfiguratorServlet#setLogLevel(String, Level)}, the logic behind + * the HiveServer2 WebUI "Configure logging" page and the {@code /conflog} endpoint. + */ +public class TestLog4j2ConfiguratorServlet { + + private static final String PARENT_LOGGER = "org.apache.hive.test.conflog"; + private static final String CHILD_LOGGER = "org.apache.hive.test.conflog.child"; + + // The levels offered by the WebUI "Configure logging" page dropdown. + private static final Level[] SUPPORTED_LEVELS = new Level[] { + Level.TRACE, Level.DEBUG, Level.INFO, Level.WARN, Level.ERROR, Level.FATAL + }; + + private Log4j2ConfiguratorServlet servlet; + private Configuration configuration; + private Level originalRootLevel; + + @Before + public void setUp() throws Exception { + servlet = new Log4j2ConfiguratorServlet(); + servlet.init(); + configuration = ((LoggerContext) LogManager.getContext(false)).getConfiguration(); + originalRootLevel = configuration.getRootLogger().getLevel(); + } + + @After + public void tearDown() { + // Restore the root level so this test cannot leak into other tests in the same JVM. + configuration.getRootLogger().setLevel(originalRootLevel); + } + + /** + * Setting a level for a not-yet-configured child logger must create a dedicated logger for it + * and must not change the level of an existing ancestor logger. + */ + @Test + public void testSetLevelOnNewLoggerDoesNotAffectAncestor() { + servlet.setLogLevel(PARENT_LOGGER, Level.INFO); + servlet.setLogLevel(CHILD_LOGGER, Level.DEBUG); + + LoggerConfig childConfig = configuration.getLoggerConfig(CHILD_LOGGER); + assertEquals("Child logger should have its own configuration", CHILD_LOGGER, childConfig.getName()); + assertEquals("Child logger level should be the requested one", Level.DEBUG, childConfig.getLevel()); + + LoggerConfig parentConfig = configuration.getLoggerConfig(PARENT_LOGGER); + assertEquals("Ancestor logger level must not change when a child is configured", + Level.INFO, parentConfig.getLevel()); + } + + /** + * Setting a level for an already-configured logger must update that logger in place. + */ + @Test + public void testSetLevelUpdatesExistingLogger() { + servlet.setLogLevel(PARENT_LOGGER, Level.INFO); + servlet.setLogLevel(PARENT_LOGGER, Level.WARN); + + LoggerConfig parentConfig = configuration.getLoggerConfig(PARENT_LOGGER); + assertEquals("Existing logger should be updated in place", PARENT_LOGGER, parentConfig.getName()); + assertEquals("Existing logger level should reflect the last update", Level.WARN, parentConfig.getLevel()); + } + + /** + * The empty logger name is the Log4j2 root logger and must update the root config directly. + */ + @Test + public void testSetLevelUpdatesRootLogger() { + servlet.setLogLevel(LogManager.ROOT_LOGGER_NAME, Level.ERROR); + + LoggerConfig rootConfig = configuration.getLoggerConfig(LogManager.ROOT_LOGGER_NAME); + assertEquals("Root logger name should stay empty", LogManager.ROOT_LOGGER_NAME, rootConfig.getName()); + assertEquals("Root logger level should be the requested one", Level.ERROR, rootConfig.getLevel()); + } + + /** + * Every level offered by the WebUI must be applied to and reflected back by a normal logger. + */ + @Test + public void testEveryLevelIsAppliedToLogger() { + for (Level level : SUPPORTED_LEVELS) { + servlet.setLogLevel(PARENT_LOGGER, level); + assertEquals("Logger level should reflect the requested level " + level, + level, configuration.getLoggerConfig(PARENT_LOGGER).getLevel()); + } + } + + /** + * Every level offered by the WebUI must be applied to and reflected back by the root logger. + */ + @Test + public void testEveryLevelIsAppliedToRootLogger() { + for (Level level : SUPPORTED_LEVELS) { + servlet.setLogLevel(LogManager.ROOT_LOGGER_NAME, level); + assertEquals("Root logger level should reflect the requested level " + level, + level, configuration.getLoggerConfig(LogManager.ROOT_LOGGER_NAME).getLevel()); + } + } +} diff --git a/service/src/resources/hive-webapps/hiveserver2/logconf.jsp b/service/src/resources/hive-webapps/hiveserver2/logconf.jsp index 4bc1ce522f58..60dc85ef6c70 100644 --- a/service/src/resources/hive-webapps/hiveserver2/logconf.jsp +++ b/service/src/resources/hive-webapps/hiveserver2/logconf.jsp @@ -17,32 +17,7 @@ * limitations under the License. */ --%> -<%@ page contentType="text/html;charset=UTF-8" - import="org.apache.hadoop.conf.Configuration" - import="org.apache.hadoop.hive.conf.HiveConf" - import="org.apache.hadoop.hive.conf.HiveConf.ConfVars" - import="org.apache.hive.common.util.HiveVersionInfo" - import="org.apache.hive.http.HttpServer" - import="org.apache.hive.service.cli.operation.Operation" - import="org.apache.hive.service.cli.operation.SQLOperation" - import="org.apache.hadoop.hive.ql.QueryInfo" - import="org.apache.hive.service.cli.session.SessionManager" - import="org.apache.hive.service.cli.session.HiveSession" - import="javax.servlet.ServletContext" - import="java.util.Collection" - import="java.util.Date" - import="java.util.List" - import="jodd.net.HtmlEncoder" -%> - -<% - ServletContext ctx = getServletContext(); - Configuration conf = (Configuration)ctx.getAttribute("hive.conf"); - long startcode = conf.getLong("startcode", System.currentTimeMillis()); - SessionManager sessionManager = - (SessionManager)ctx.getAttribute("hive.sm"); - String remoteUser = request.getRemoteUser(); -%> +<%@ page contentType="text/html;charset=UTF-8" %>