Enable log-to-file configuration (#7918)

* Enable log-to-file configuration PR #7825 enabled parallel logging to a file with a much more fine-grained log level by default. However, logging at `TRACE` level on Windows appears to be still problematic. This PR reduced the default log level to file from `DEBUG` to `TRACE` and allows to control it via an environment variable if one wishes to change the verbosity without making code changes. * PR comments
enso-org · Oct 2, 2023 · 6380254 · 6380254
1 parent af050f5
commit 6380254
Show file tree

Hide file tree

Showing 13 changed files with 1,075 additions and 51 deletions.
diff --git a/docs/infrastructure/logging.md b/docs/infrastructure/logging.md
@@ -352,10 +352,17 @@ way it can verify that all logs are being reported within the provided code.
 
 By default Enso will attempt to persist (verbose) logs into a designated log
 file. This means that even though a user might be shown `WARNING` level logs in
-the console, Logs with up to `TRACE` level will be dumped into the log file. A
+the console, logs with up to `DEBUG` level will be dumped into the log file. A
 user can disable this parallel logging to a file by setting the environment
 variable:
 
 ```
 ENSO_LOG_TO_FILE=false project-manager ...
 ```
+
+Users can also modify the default maximal log level, `DEBUG`, used when logging
+to a log file by setting the environment variable:
+
+```
+ENSO_LOG_TO_FILE_LOG_LEVEL=trace project-manager ...
+```
diff --git a/engine/language-server/src/main/resources/application.conf b/engine/language-server/src/main/resources/application.conf
@@ -43,6 +43,10 @@ logging-service {
   ]
   default-appender = socket
   default-appender = ${?ENSO_APPENDER_DEFAULT}
-  always-log-to-file = false
-  always-log-to-file = ${?ENSO_LOG_TO_FILE}
+  log-to-file {
+    enable = false
+    enable = ${?ENSO_LOG_TO_FILE}
+    log-level = debug
+    log-level = ${?ENSO_LOG_TO_FILE_LOG_LEVEL}
+  }
 }
diff --git a/engine/launcher/src/main/resources/application.conf b/engine/launcher/src/main/resources/application.conf
@@ -32,6 +32,10 @@ logging-service {
   ]
   default-appender = console
   default-appender = ${?ENSO_APPENDER_DEFAULT}
-  always-log-to-file = true
-  always-log-to-file = ${?ENSO_LOG_TO_FILE}
+  log-to-file {
+    enable = true
+    enable = ${?ENSO_LOG_TO_FILE}
+    log-level = debug
+    log-level = ${?ENSO_LOG_TO_FILE_LOG_LEVEL}
+  }
 }
diff --git a/lib/scala/logging-config/src/main/java/org/enso/logger/config/BaseConfig.java b/lib/scala/logging-config/src/main/java/org/enso/logger/config/BaseConfig.java
@@ -11,11 +11,8 @@ public interface BaseConfig {
   /** Returns a map of appenders defined in the logger section of the config. */
   Map<String, Appender> getAppenders();
 
-  /**
-   * Returns true, if logging infrastructure should always log in verbose mode, irrespective of the
-   * log target.
-   */
-  boolean logToFile();
+  /** Returns configuration for parallel logging of messages to a log file. */
+  LogToFile logToFile();
 
   /**
    * Returns a list of custom loggers and their levels that need to be taken into account when

diff --git a/lib/scala/logging-config/src/main/java/org/enso/logger/config/ConsoleAppender.java b/lib/scala/logging-config/src/main/java/org/enso/logger/config/ConsoleAppender.java
@@ -28,8 +28,9 @@ public boolean setup(Level logLevel, LoggerSetup appenderSetup) {
   @Override
   public boolean setupForPath(
       Level logLevel, Path logRoot, String logPrefix, LoggerSetup loggerSetup) {
-    if (loggerSetup.getConfig().logToFile()) {
-      loggerSetup.setupFileAppender(Level.TRACE, logRoot, logPrefix);
+    if (loggerSetup.getConfig().logToFile().enabled()) {
+      loggerSetup.setupFileAppender(
+          loggerSetup.getConfig().logToFile().logLevel(), logRoot, logPrefix);
     }
     return loggerSetup.setupConsoleAppender(logLevel);
   }

diff --git a/lib/scala/logging-config/src/main/java/org/enso/logger/config/LogToFile.java b/lib/scala/logging-config/src/main/java/org/enso/logger/config/LogToFile.java
@@ -0,0 +1,44 @@
+package org.enso.logger.config;
+
+import com.typesafe.config.Config;
+import org.slf4j.event.Level;
+
+/**
+ * If `enabled`, all logs up to `logLevel` level will be persisted in the log file in parallel to the default appender.
+ */
+public record LogToFile(boolean enabled, Level logLevel) {
+
+    public static final String logToFileEnabledKey = "enable";
+    public static final String logToFileLogLevelKey = "log-level";
+
+    /**
+     * Parses `log-to-file` configuration section.
+     *
+     * @param config application configuration section that determines logging to a file
+     * @return parsed `log-to-file` configuration
+     */
+    public static LogToFile fromConfig(Config config) {
+        if (config.hasPath(logToFileEnabledKey)) {
+            Level lvl = config.hasPath(logToFileLogLevelKey) ? fromConfigValue(config.getString(logToFileLogLevelKey)) : Level.DEBUG;
+            return new LogToFile(config.getBoolean(logToFileEnabledKey), lvl);
+        } else {
+            return disabled();
+        }
+    }
+
+    /**
+     * Returns configuration that disables parallel logging to a file.
+     */
+    public static LogToFile disabled() {
+        return new LogToFile(false, Level.ERROR);
+    }
+
+    private static Level fromConfigValue(String logLevel) {
+        try {
+            return Level.valueOf(logLevel.toUpperCase());
+        } catch (IllegalArgumentException iae) {
+            return Level.DEBUG;
+        }
+    }
+}
+
diff --git a/lib/scala/logging-config/src/main/java/org/enso/logger/config/LoggingServer.java b/lib/scala/logging-config/src/main/java/org/enso/logger/config/LoggingServer.java
@@ -13,7 +13,7 @@
  * @param appender appender's configuration describing how to transform received log events
  * @param start if true, will be started by the service defining the configuration
  */
-public record LoggingServer(int port, Map<String, Appender> appenders, String appender, boolean start, boolean logToFile) implements BaseConfig {
+public record LoggingServer(int port, Map<String, Appender> appenders, String appender, boolean start, LogToFile logToFile) implements BaseConfig {
 
     public static final String startKey = "start";
 
@@ -33,7 +33,7 @@ public static LoggingServer parse(Config config) throws MissingConfigurationFiel
         }
         String defaultAppender = config.getString(LoggingServiceConfig.defaultAppenderKey);
         boolean start = config.hasPath(startKey) ? config.getBoolean(startKey) : false;
-        boolean logToFile = config.hasPath(LoggingServiceConfig.alwaysLogToFileKey) ? config.getBoolean(LoggingServiceConfig.alwaysLogToFileKey) : false;
+        LogToFile logToFile = config.hasPath(LoggingServiceConfig.logToFileKey) ? LogToFile.fromConfig(config.getConfig(LoggingServiceConfig.logToFileKey)) : LogToFile.disabled();
         return new LoggingServer(port, appendersMap, defaultAppender, start, logToFile);
     }
 

diff --git a/lib/scala/logging-config/src/main/java/org/enso/logger/config/LoggingServiceConfig.java b/lib/scala/logging-config/src/main/java/org/enso/logger/config/LoggingServiceConfig.java
@@ -19,13 +19,13 @@ public class LoggingServiceConfig implements BaseConfig {
   public static final String appendersKey = "appenders";
   public static final String defaultAppenderKey = "default-appender";
   public static final String logLevelKey = "log-level";
-  public static final String alwaysLogToFileKey = "always-log-to-file";
 
+  public static final String logToFileKey = "log-to-file";
   private final LoggersLevels loggers;
   private final Map<String, Appender> appenders;
 
   private final String defaultAppenderName;
-  private final boolean alwaysLogToFile;
+  private final LogToFile logToFile;
   private final Optional<String> logLevel;
   private final LoggingServer server;
 
@@ -34,12 +34,12 @@ private LoggingServiceConfig(
       Optional<String> logLevel,
       Map<String, Appender> appenders,
       String defaultAppender,
-      boolean alwaysLogToFile,
+      LogToFile logTo,
       LoggingServer server) {
     this.loggers = loggers;
     this.appenders = appenders;
     this.defaultAppenderName = defaultAppender;
-    this.alwaysLogToFile = alwaysLogToFile;
+    this.logToFile = logTo;
     this.logLevel = logLevel;
     this.server = server;
   }
@@ -68,8 +68,10 @@ public static LoggingServiceConfig parseConfig() throws MissingConfigurationFiel
     } else {
       loggers = LoggersLevels.parse();
     }
-    boolean logToFile =
-        root.hasPath(alwaysLogToFileKey) ? root.getBoolean(alwaysLogToFileKey) : false;
+    LogToFile logToFile =
+        root.hasPath(logToFileKey)
+            ? LogToFile.fromConfig(root.getConfig(logToFileKey))
+            : LogToFile.disabled();
     return new LoggingServiceConfig(
         loggers,
         getStringOpt(logLevelKey, root),
@@ -140,6 +142,11 @@ public LoggingServer getServer() {
     return server;
   }
 
+  @Override
+  public LogToFile logToFile() {
+    return logToFile;
+  }
+
   @Override
   public String toString() {
     return "Loggers: "
@@ -155,9 +162,4 @@ public String toString() {
         + ", server: "
         + server;
   }
-
-  @Override
-  public boolean logToFile() {
-    return alwaysLogToFile;
-  }
 }
diff --git a/lib/scala/logging-config/src/main/java/org/enso/logger/config/SentryAppender.java b/lib/scala/logging-config/src/main/java/org/enso/logger/config/SentryAppender.java
@@ -50,8 +50,9 @@ public boolean setup(Level logLevel, LoggerSetup loggerSetup) {
   @Override
   public boolean setupForPath(
       Level logLevel, Path logRoot, String logPrefix, LoggerSetup loggerSetup) {
-    if (loggerSetup.getConfig().logToFile()) {
-      loggerSetup.setupFileAppender(Level.TRACE, logRoot, logPrefix);
+    if (loggerSetup.getConfig().logToFile().enabled()) {
+      loggerSetup.setupFileAppender(
+          loggerSetup.getConfig().logToFile().logLevel(), logRoot, logPrefix);
     }
     return loggerSetup.setupSentryAppender(logLevel, logRoot);
   }

diff --git a/lib/scala/logging-service-logback/src/main/java/org/enso/logger/LogbackSetup.java b/lib/scala/logging-service-logback/src/main/java/org/enso/logger/LogbackSetup.java
@@ -55,6 +55,10 @@ public LoggingServiceConfig getConfig() {
         return config;
     }
 
+    private boolean logToFileEnabled() {
+        return config.logToFile().enabled();
+    }
+
     private final LoggingServiceConfig config;
     private final LoggerContext context;
 
@@ -101,12 +105,13 @@ public boolean setupSocketAppender(
         Level targetLogLevel;
         // Modify log level if we were asked to always log to a file.
         // The receiver needs to get all logs (up to `trace`) so as to be able to log all verbose messages.
-        if (config.logToFile()) {
-            targetLogLevel = Level.TRACE;
+        if (logToFileEnabled()) {
+            int min = Math.min(Level.TRACE.toInt(), config.logToFile().logLevel().toInt());
+            targetLogLevel = Level.intToLevel(min);
         } else {
             targetLogLevel = logLevel;
         }
-        LoggerAndContext env = contextInit(targetLogLevel, config, !config.logToFile());
+        LoggerAndContext env = contextInit(targetLogLevel, config, !logToFileEnabled());
 
         org.enso.logger.config.SocketAppender appenderConfig = config.getSocketAppender();
 
@@ -119,7 +124,7 @@ public boolean setupSocketAppender(
             socketAppender.setReconnectionDelay(Duration.buildByMilliseconds(appenderConfig.getReconnectionDelay()));
 
 
-        env.finalizeAppender(socketAppender, config.logToFile());
+        env.finalizeAppender(socketAppender);
         return true;
     }
 
@@ -183,7 +188,7 @@ public boolean setupFileAppender(
             fileAppender.setEncoder(encoder);
 
 
-            env.finalizeAppender(fileAppender, false);
+            env.finalizeAppender(fileAppender);
         } catch (Throwable e) {
             e.printStackTrace();
             return false;
@@ -193,7 +198,7 @@ public boolean setupFileAppender(
 
     @Override
     public boolean setupConsoleAppender(Level logLevel) {
-        LoggerAndContext env = contextInit(logLevel, config, !getConfig().logToFile());
+        LoggerAndContext env = contextInit(logLevel, config, !logToFileEnabled());
         org.enso.logger.config.ConsoleAppender appenderConfig = config.getConsoleAppender();
         final PatternLayoutEncoder encoder = new PatternLayoutEncoder();
         try {
@@ -208,7 +213,7 @@ public boolean setupConsoleAppender(Level logLevel) {
         consoleAppender.setName("enso-console");
         consoleAppender.setEncoder(encoder);
 
-        env.finalizeAppender(consoleAppender, false);
+        env.finalizeAppender(consoleAppender);
         return true;
     }
 
@@ -217,7 +222,7 @@ public boolean setupSentryAppender(Level logLevel, Path logRoot) {
         // TODO: handle proxy
         // TODO: shutdown timeout configuration
         try {
-            LoggerAndContext env = contextInit(logLevel, config, !config.logToFile());
+            LoggerAndContext env = contextInit(logLevel, config, !logToFileEnabled());
 
             org.enso.logger.config.SentryAppender appenderConfig = config.getSentryAppender();
             if (appenderConfig == null) {
@@ -242,7 +247,7 @@ public boolean setupSentryAppender(Level logLevel, Path logRoot) {
             opts.setDsn(appenderConfig.getDsn());
             appender.setOptions(opts);
 
-            env.finalizeAppender(appender, config.logToFile());
+            env.finalizeAppender(appender);
         } catch (Throwable e) {
             e.printStackTrace();
             return false;
@@ -257,7 +262,7 @@ public boolean setupNoOpAppender() {
         NOPAppender<ILoggingEvent> appender = new NOPAppender<>();
         appender.setName("enso-noop");
 
-        env.finalizeAppender(appender, false);
+        env.finalizeAppender(appender);
         return true;
     }
 
@@ -290,17 +295,20 @@ void finalizeEncoder(ch.qos.logback.core.encoder.Encoder<ILoggingEvent> encoder)
             encoder.setContext(ctx);
             encoder.start();
         }
-        void finalizeAppender(ch.qos.logback.core.Appender<ILoggingEvent> appender, boolean isLogToFile) {
+        void finalizeAppender(ch.qos.logback.core.Appender<ILoggingEvent> appender) {
             ThresholdFilter threshold = new ThresholdFilter();
             threshold.setLevel(ch.qos.logback.classic.Level.convertAnSLF4JLevel(level).toString());
             appender.addFilter(threshold);
             threshold.setContext(ctx);
             threshold.start();
 
-            // Root's log level is set to TRACE, meaning we want to log all events.
+            // Root's log level is set to the minimal required log level.
             // Log level is controlled by `ThresholdFilter` instead, allowing is to specify different
             // log levels for different outputs.
-            logger.setLevel(ch.qos.logback.classic.Level.TRACE);
+            var minLevelInt = Math.min(Level.TRACE.toInt(), level.toInt());
+            var minLevel = ch.qos.logback.classic.Level.convertAnSLF4JLevel(Level.intToLevel(minLevelInt));
+
+            logger.setLevel(minLevel);
             if (filter != null) {
                 appender.addFilter(filter);
                 filter.setContext(ctx);

diff --git a/lib/scala/logging-service/src/main/java/org/enso/logging/LoggingServiceManager.java b/lib/scala/logging-service/src/main/java/org/enso/logging/LoggingServiceManager.java
@@ -26,7 +26,7 @@ public static Future<URI> setupServer(
       throw new LoggingServiceAlreadySetup();
     } else {
       if (config.appenders().containsKey(config.appender())) {
-        currentLevel = config.logToFile() ? Level.TRACE : logLevel;
+        currentLevel = config.logToFile().enabled() ? config.logToFile().logLevel() : logLevel;
         return Future.apply(
             () -> {
               var server = LoggingServiceFactory.get().localServerFor(port);

diff --git a/lib/scala/project-manager/src/main/resources/application.conf b/lib/scala/project-manager/src/main/resources/application.conf
@@ -40,15 +40,23 @@ logging-service {
   ]
   default-appender = socket
   default-appender = ${?ENSO_APPENDER_DEFAULT}
-  always-log-to-file = true
-  always-log-to-file = ${?ENSO_LOG_TO_FILE}
+  log-to-file {
+    enable = true
+    enable = ${?ENSO_LOG_TO_FILE}
+    log-level = debug
+    log-level = ${?ENSO_LOG_TO_FILE_LOG_LEVEL}
+  }
   server {
     start = true
     start = ${?ENSO_LOGSERVER_START}
     port = 6000
     port = ${?ENSO_LOGSERVER_PORT}
-    always-log-to-file = true
-    always-log-to-file = ${?ENSO_LOG_TO_FILE}
+    log-to-file {
+      enable = true
+      enable = ${?ENSO_LOG_TO_FILE}
+      log-level = debug
+      log-level = ${?ENSO_LOG_TO_FILE_LOG_LEVEL}
+    }
     appenders = [ # file/console/socket/sentry
         {
           name = "file"