GUACAMOLE-374: Allow log level to be configured easily with "log-leve…

…l" property.

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

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"

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;
 

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 =
+              "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
+            + "<configuration>\n"
+            + "\n"
+            + "    <!-- Default appender -->\n"
+            + "    <appender name=\"GUAC-DEFAULT\" class=\"ch.qos.logback.core.ConsoleAppender\">\n"
+            + "        <encoder>\n"
+            + "            <pattern>%%d{HH:mm:ss.SSS} [%%thread] %%-5level %%logger{36} - %%msg%%n</pattern>\n"
+            + "        </encoder>\n"
+            + "    </appender>\n"
+            + "\n"
+            + "    <!-- Log at level defined with \"log-level\" property -->\n"
+            + "    <root level=\"%s\">\n"
+            + "        <appender-ref ref=\"GUAC-DEFAULT\" />\n"
+            + "    </root>\n"
+            + "\n"
+            + "</configuration>\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);
+    }
+
+}

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<LogLevel> LOG_LEVEL = new EnumGuacamoleProperty<LogLevel>(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);
         }
 
     }