From 649492b412823744be0263b26c2adf6e6e4dfcdb Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Tue, 23 Apr 2024 10:11:16 -0700 Subject: [PATCH] GUACAMOLE-374: Allow log level to be configured easily with "log-level" property. --- Dockerfile | 4 +- .../000-migrate-legacy-variables.sh | 3 + .../110-configure-guacamole-logging.sh | 33 ---- .../GuacamoleServletContextListener.java | 25 ++- .../org/apache/guacamole/log/LogLevel.java | 163 ++++++++++++++++++ .../org/apache/guacamole/log/LogModule.java | 84 +++++++-- guacamole/src/main/resources/logback.xml | 34 ---- 7 files changed, 255 insertions(+), 91 deletions(-) delete mode 100644 guacamole-docker/entrypoint.d/110-configure-guacamole-logging.sh create mode 100644 guacamole/src/main/java/org/apache/guacamole/log/LogLevel.java delete mode 100644 guacamole/src/main/resources/logback.xml diff --git a/Dockerfile b/Dockerfile index 80aa578ec5..8a3226acbc 100644 --- a/Dockerfile +++ b/Dockerfile @@ -76,9 +76,9 @@ RUN rm -rf /opt/guacamole/build.d /opt/guacamole/bin/build-guacamole.sh # For the runtime image, we start with the official Tomcat distribution FROM tomcat:${TOMCAT_VERSION}-${TOMCAT_JRE} -# Install XMLStarlet for server.xml alterations and unzip for LOGBACK_LEVEL case +# Install XMLStarlet for server.xml alterations RUN apt-get update -qq \ - && apt-get install -y xmlstarlet unzip\ + && apt-get install -y xmlstarlet \ && rm -rf /var/lib/apt/lists/* # This is where the build artifacts go in the runtime image diff --git a/guacamole-docker/entrypoint.d/000-migrate-legacy-variables.sh b/guacamole-docker/entrypoint.d/000-migrate-legacy-variables.sh index 490827feeb..cb56c4c9ae 100644 --- a/guacamole-docker/entrypoint.d/000-migrate-legacy-variables.sh +++ b/guacamole-docker/entrypoint.d/000-migrate-legacy-variables.sh @@ -111,3 +111,6 @@ deprecate_variable "PROXY_IP_HEADER" "REMOTE_IP_VALVE_REMOTE_IP_HEADER" deprecate_variable "PROXY_PROTOCOL_HEADER" "REMOTE_IP_VALVE_PROTOCOL_HEADER" # NOTE: PROXY_BY_HEADER never worked as there is no "remoteIpProxiesHeader" attribute for RemoteIpValve +# The old "LOGBACK_LEVEL" environment variable has been replaced with +# "LOG_LEVEL" for consistency with the guacd image +deprecate_variable "LOGBACK_LEVEL" "LOG_LEVEL" diff --git a/guacamole-docker/entrypoint.d/110-configure-guacamole-logging.sh b/guacamole-docker/entrypoint.d/110-configure-guacamole-logging.sh deleted file mode 100644 index 70a099976c..0000000000 --- a/guacamole-docker/entrypoint.d/110-configure-guacamole-logging.sh +++ /dev/null @@ -1,33 +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. -# - -## -## @fn 030-configure-guacamole-logging.sh -## -## Checks the value of the LOGBACK_LEVEL environment variable, producing a -## corresponding logback.xml file within GUACAMOLE_HOME if a log level has been -## explicitly specified. -## - -# Set logback level if specified -if [ -n "$LOGBACK_LEVEL" ]; then - unzip -o -j /opt/guacamole/guacamole.war WEB-INF/classes/logback.xml -d $GUACAMOLE_HOME - sed -i "s/level=\"info\"/level=\"$LOGBACK_LEVEL\"/" $GUACAMOLE_HOME/logback.xml -fi - diff --git a/guacamole/src/main/java/org/apache/guacamole/GuacamoleServletContextListener.java b/guacamole/src/main/java/org/apache/guacamole/GuacamoleServletContextListener.java index df07662389..d8e3f3d581 100644 --- a/guacamole/src/main/java/org/apache/guacamole/GuacamoleServletContextListener.java +++ b/guacamole/src/main/java/org/apache/guacamole/GuacamoleServletContextListener.java @@ -240,13 +240,24 @@ protected Injector getInjector() { return current; // Create new injector if necessary - Injector injector = Guice.createInjector(Stage.PRODUCTION, - new EnvironmentModule(environment), - new LogModule(environment), - new ExtensionModule(environment), - new RESTServiceModule(sessionMap), - new TunnelModule() - ); + Injector injector = + + // Ensure environment and logging are configured FIRST ... + Guice.createInjector(Stage.PRODUCTION, + new EnvironmentModule(environment), + new LogModule(environment) + ) + + // ... before attempting configuration of any other modules + // (logging within the constructors of other modules may + // otherwise default to including messages from the "debug" + // level, regardless of how the application log level is + // actually configured) + .createChildInjector( + new ExtensionModule(environment), + new RESTServiceModule(sessionMap), + new TunnelModule() + ); return injector; diff --git a/guacamole/src/main/java/org/apache/guacamole/log/LogLevel.java b/guacamole/src/main/java/org/apache/guacamole/log/LogLevel.java new file mode 100644 index 0000000000..27bf0fe882 --- /dev/null +++ b/guacamole/src/main/java/org/apache/guacamole/log/LogLevel.java @@ -0,0 +1,163 @@ +/* + * 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.guacamole.log; + +import java.io.ByteArrayInputStream; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import org.apache.guacamole.properties.EnumGuacamoleProperty.PropertyValue; + +/** + * All log levels supported by the Apache Guacamole web application. Each log + * level describes a different level of verbosity for the log messages included + * in web application logs. + */ +public enum LogLevel { + + /** + * Errors that are fatal in the context of the operation being logged. + */ + @PropertyValue("error") + ERROR("error"), + + /** + * Non-fatal conditions that may indicate the presence of a problem. + */ + @PropertyValue("warning") + @PropertyValue("warn") + WARNING("warning"), + + /** + * Informational messages of general interest to users or administrators. + */ + @PropertyValue("info") + INFO("info"), + + /** + * Informational messages that are useful for debugging, but are generally + * not useful to users or administrators. It is expected that debug-level + * messages, while verbose, will not affect performance. + */ + @PropertyValue("debug") + DEBUG("debug"), + + /** + * Informational messages that may be useful for debugging, but which are + * so low-level that they may affect performance. + */ + @PropertyValue("trace") + TRACE("trace"); + + /** + * Format string whose sole format argument is a String containing the + * name of the log level. As this configuration will be fed to Logback, the + * name used must be a name acceptable by Logback. + */ + private static final String LOGBACK_XML_TEMPLATE = + "\n" + + "\n" + + "\n" + + " \n" + + " \n" + + " \n" + + " %%d{HH:mm:ss.SSS} [%%thread] %%-5level %%logger{36} - %%msg%%n\n" + + " \n" + + " \n" + + "\n" + + " \n" + + " \n" + + " \n" + + " \n" + + "\n" + + "\n"; + + /** + * The name that should be used to refer to this log level in the context + * of configuring Guacamole. This name should be both descriptive and + * acceptable as the value of the "log-level" property. + */ + private final String canonicalName; + + /** + * The raw contents of the "logback.xml" that configures Logback to log + * messages at this level, encoded as UTF-8. + */ + private final byte[] logbackConfig; + + /** + * Creates a new LogLevel with the given names. The pair of names provided + * correspond to the name used within Guacamole's configuration and the + * name used within Logback's configuration. + * + * @param canonicalName + * The name that should be used for this log level when configuring + * Guacamole to log at this level using the "log-level" property. + * + * @param logbackLogLevel + * The name that would be provided to Logback to log at this level if + * manually configuring Logback using "logback.xml". + */ + private LogLevel(String canonicalName, String logbackLogLevel) { + this.canonicalName = canonicalName; + this.logbackConfig = String.format(LOGBACK_XML_TEMPLATE, logbackLogLevel).getBytes(StandardCharsets.UTF_8); + } + + /** + * Creates a new LogLevel with the given name. The provided name corresponds + * to both the name used within Guacamole's configuration and the name used + * within Logback's configuration. + * + * @param logLevel + * The name that should be used for this log level when configuring + * Guacamole to log at this level using the "log-level" property AND + * when manually configuring Logback to log at this level using a + * "logback.xml" configuration file. + */ + private LogLevel(String logLevel) { + this(logLevel, logLevel); + } + + /** + * Returns a name that may be used to refer to this log level when + * configuring Guacamole using the "log-level" property. + * + * @return + * A name that may be used to refer to this log level when + * configuring Guacamole using the "log-level" property. + */ + public String getCanonicalName() { + return canonicalName; + } + + /** + * Returns a new InputStream that streams the contents of an XML + * configuration file that can be provided to Logback to configure logging + * at this log level. + * + * @return + * A a new InputStream that streams the contents of an XML + * configuration file that can be provided to Logback to configure + * logging at this log level. + */ + public InputStream getLogbackConfiguration() { + return new ByteArrayInputStream(logbackConfig); + } + +} diff --git a/guacamole/src/main/java/org/apache/guacamole/log/LogModule.java b/guacamole/src/main/java/org/apache/guacamole/log/LogModule.java index accc4a9ddd..a9e4ac3bca 100644 --- a/guacamole/src/main/java/org/apache/guacamole/log/LogModule.java +++ b/guacamole/src/main/java/org/apache/guacamole/log/LogModule.java @@ -25,7 +25,13 @@ import ch.qos.logback.core.util.StatusPrinter; import com.google.inject.AbstractModule; import java.io.File; +import java.io.FileInputStream; +import java.io.FileNotFoundException; +import java.io.IOException; +import java.io.InputStream; +import org.apache.guacamole.GuacamoleException; import org.apache.guacamole.environment.Environment; +import org.apache.guacamole.properties.EnumGuacamoleProperty; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -44,6 +50,19 @@ public class LogModule extends AbstractModule { */ private final Environment environment; + /** + * Property that specifies the highest level of verbosity that Guacamole + * should use for the messages in its logs. + */ + private static final EnumGuacamoleProperty LOG_LEVEL = new EnumGuacamoleProperty(LogLevel.class) { + + @Override + public String getName() { + return "log-level"; + } + + }; + /** * Creates a new LogModule which uses the given environment to determine * the logging configuration. @@ -54,26 +73,57 @@ public class LogModule extends AbstractModule { public LogModule(Environment environment) { this.environment = environment; } - - @Override - protected void configure() { - // Only load logback configuration if GUACAMOLE_HOME exists - File guacamoleHome = environment.getGuacamoleHome(); - if (!guacamoleHome.isDirectory()) - return; + /** + * Returns an InputStream that streams the contents of the "logback.xml" + * file that Logback should read to configure logging to Guacamole. If the + * user provided their own "logback.xml" within GUACAMOLE_HOME, this will + * be an InputStream that reads the contents of that file. The required + * "logback.xml" will otherwise be dynamically generated based on the value + * of the "log-level" property. + * + * @return + * An InputStream that streams the contents of the "logback.xml" file + * that Logback should read to configure logging to Guacamole. + */ + private InputStream getLogbackConfiguration() { // Check for custom logback.xml - File logbackConfiguration = new File(guacamoleHome, "logback.xml"); - if (!logbackConfiguration.exists()) - return; + File logbackFile = new File(environment.getGuacamoleHome(), "logback.xml"); + if (logbackFile.exists()) { + try { + logger.info("Loading logback configuration from \"{}\".", logbackFile); + return new FileInputStream(logbackFile); + } + catch (FileNotFoundException e) { + logger.info("Logback configuration could not be read " + + "from \"{}\": {}", logbackFile, e.getMessage()); + } + } - logger.info("Loading logback configuration from \"{}\".", logbackConfiguration); + // Default to generating an internal logback.xml based on a simple + // "log-level" property + LogLevel level; + try { + level = environment.getProperty(LOG_LEVEL, LogLevel.INFO); + logger.info("Logging will be at the \"{}\" level.", level.getCanonicalName()); + } + catch (GuacamoleException e) { + level = LogLevel.INFO; + logger.error("Falling back to \"{}\" log level: {}", level.getCanonicalName(), e.getMessage()); + } - LoggerContext context = (LoggerContext) LoggerFactory.getILoggerFactory(); - context.reset(); + return level.getLogbackConfiguration(); - try { + } + + @Override + protected void configure() { + + try (InputStream logbackConfiguration = getLogbackConfiguration()) { + + LoggerContext context = (LoggerContext) LoggerFactory.getILoggerFactory(); + context.reset(); // Initialize logback JoranConfigurator configurator = new JoranConfigurator(); @@ -86,7 +136,11 @@ protected void configure() { } catch (JoranException e) { logger.error("Initialization of logback failed: {}", e.getMessage()); - logger.debug("Unable to load logback configuration..", e); + logger.debug("Unable to load logback configuration.", e); + } + catch (IOException e) { + logger.warn("Logback configuration file could not be cleanly closed: {}", e.getMessage()); + logger.debug("Failed to close logback configuration file.", e); } } diff --git a/guacamole/src/main/resources/logback.xml b/guacamole/src/main/resources/logback.xml deleted file mode 100644 index 0b91a429f2..0000000000 --- a/guacamole/src/main/resources/logback.xml +++ /dev/null @@ -1,34 +0,0 @@ - - - - - - - - %d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n - - - - - - - - - \ No newline at end of file