From af0d500df6d57d845ac1a9745dafde8718632038 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Sun, 2 Mar 2025 09:55:09 -0800 Subject: [PATCH] Set root logger level for CLIs (#123742) All CLIs in elasticsearch support command line flags for controlling the output level. When --silent is used, the expectation is that normal logging is omitted. Yet the log4j logger is still configured to output error level logs. This commit sets the appropriate log level for log4j depending on the Terminal log level. --- libs/cli/build.gradle | 1 + libs/cli/src/main/java/module-info.java | 2 + .../java/org/elasticsearch/cli/Command.java | 6 +++ libs/logging/src/main/java/module-info.java | 2 +- .../logging/internal/spi/LoggerFactory.java | 5 +++ .../common/logging/internal/LevelUtil.java | 39 +++++++++++++++++-- .../logging/internal/LoggerFactoryImpl.java | 13 +++++++ .../org/elasticsearch/test/ESTestCase.java | 18 +++++++++ .../security/src/main/java/module-info.java | 1 + 9 files changed, 82 insertions(+), 5 deletions(-) diff --git a/libs/cli/build.gradle b/libs/cli/build.gradle index d5842d4a2c59c..d5b1bd6ac648e 100644 --- a/libs/cli/build.gradle +++ b/libs/cli/build.gradle @@ -12,6 +12,7 @@ apply plugin: 'elasticsearch.publish' dependencies { api 'net.sf.jopt-simple:jopt-simple:5.0.2' api project(':libs:core') + api project(':libs:logging') testImplementation(project(":test:framework")) { exclude group: 'org.elasticsearch', module: 'cli' diff --git a/libs/cli/src/main/java/module-info.java b/libs/cli/src/main/java/module-info.java index e3969a1c74375..eeaf2eae06b96 100644 --- a/libs/cli/src/main/java/module-info.java +++ b/libs/cli/src/main/java/module-info.java @@ -11,6 +11,8 @@ module org.elasticsearch.cli { requires jopt.simple; requires org.elasticsearch.base; + requires java.logging; + requires org.elasticsearch.logging; exports org.elasticsearch.cli; } diff --git a/libs/cli/src/main/java/org/elasticsearch/cli/Command.java b/libs/cli/src/main/java/org/elasticsearch/cli/Command.java index 6d38408ed165a..1690515532e7b 100644 --- a/libs/cli/src/main/java/org/elasticsearch/cli/Command.java +++ b/libs/cli/src/main/java/org/elasticsearch/cli/Command.java @@ -15,6 +15,8 @@ import joptsimple.OptionSpec; import org.elasticsearch.core.SuppressForbidden; +import org.elasticsearch.logging.Level; +import org.elasticsearch.logging.internal.spi.LoggerFactory; import java.io.Closeable; import java.io.IOException; @@ -84,12 +86,16 @@ protected void mainWithoutErrorHandling(String[] args, Terminal terminal, Proces return; } + LoggerFactory loggerFactory = LoggerFactory.provider(); if (options.has(silentOption)) { terminal.setVerbosity(Terminal.Verbosity.SILENT); + loggerFactory.setRootLevel(Level.OFF); } else if (options.has(verboseOption)) { terminal.setVerbosity(Terminal.Verbosity.VERBOSE); + loggerFactory.setRootLevel(Level.DEBUG); } else { terminal.setVerbosity(Terminal.Verbosity.NORMAL); + loggerFactory.setRootLevel(Level.INFO); } execute(terminal, options, processInfo); diff --git a/libs/logging/src/main/java/module-info.java b/libs/logging/src/main/java/module-info.java index 3ff21cefd14df..8c9ebbd91e869 100644 --- a/libs/logging/src/main/java/module-info.java +++ b/libs/logging/src/main/java/module-info.java @@ -9,5 +9,5 @@ module org.elasticsearch.logging { exports org.elasticsearch.logging; - exports org.elasticsearch.logging.internal.spi to org.elasticsearch.server; + exports org.elasticsearch.logging.internal.spi to org.elasticsearch.server, org.elasticsearch.cli; } diff --git a/libs/logging/src/main/java/org/elasticsearch/logging/internal/spi/LoggerFactory.java b/libs/logging/src/main/java/org/elasticsearch/logging/internal/spi/LoggerFactory.java index d5db919197307..a485a78a6e63f 100644 --- a/libs/logging/src/main/java/org/elasticsearch/logging/internal/spi/LoggerFactory.java +++ b/libs/logging/src/main/java/org/elasticsearch/logging/internal/spi/LoggerFactory.java @@ -9,6 +9,7 @@ package org.elasticsearch.logging.internal.spi; +import org.elasticsearch.logging.Level; import org.elasticsearch.logging.Logger; /** @@ -26,6 +27,10 @@ public static LoggerFactory provider() { public abstract Logger getLogger(Class clazz); + public abstract void setRootLevel(Level level); + + public abstract Level getRootLevel(); + public static void setInstance(LoggerFactory INSTANCE) { LoggerFactory.INSTANCE = INSTANCE; } diff --git a/server/src/main/java/org/elasticsearch/common/logging/internal/LevelUtil.java b/server/src/main/java/org/elasticsearch/common/logging/internal/LevelUtil.java index df939dd0f2d21..c1d6a80ee4618 100644 --- a/server/src/main/java/org/elasticsearch/common/logging/internal/LevelUtil.java +++ b/server/src/main/java/org/elasticsearch/common/logging/internal/LevelUtil.java @@ -9,20 +9,51 @@ package org.elasticsearch.common.logging.internal; +import static org.apache.logging.log4j.Level.ALL; +import static org.apache.logging.log4j.Level.DEBUG; +import static org.apache.logging.log4j.Level.ERROR; +import static org.apache.logging.log4j.Level.FATAL; +import static org.apache.logging.log4j.Level.INFO; +import static org.apache.logging.log4j.Level.OFF; +import static org.apache.logging.log4j.Level.TRACE; +import static org.apache.logging.log4j.Level.WARN; + public final class LevelUtil { private LevelUtil() {} public static org.apache.logging.log4j.Level log4jLevel(final org.elasticsearch.logging.Level level) { return switch (level) { - case OFF -> org.apache.logging.log4j.Level.OFF; - case FATAL -> org.apache.logging.log4j.Level.FATAL; + case OFF -> OFF; + case FATAL -> FATAL; case ERROR -> org.apache.logging.log4j.Level.ERROR; - case WARN -> org.apache.logging.log4j.Level.WARN; + case WARN -> WARN; case INFO -> org.apache.logging.log4j.Level.INFO; case DEBUG -> org.apache.logging.log4j.Level.DEBUG; - case TRACE -> org.apache.logging.log4j.Level.TRACE; + case TRACE -> TRACE; case ALL -> org.apache.logging.log4j.Level.ALL; }; } + + public static org.elasticsearch.logging.Level elasticsearchLevel(final org.apache.logging.log4j.Level log4jLevel) { + // we can't use a switch because log4j levels are not an enum + if (log4jLevel == OFF) { + return org.elasticsearch.logging.Level.OFF; + } else if (log4jLevel == FATAL) { + return org.elasticsearch.logging.Level.FATAL; + } else if (log4jLevel == ERROR) { + return org.elasticsearch.logging.Level.ERROR; + } else if (log4jLevel == WARN) { + return org.elasticsearch.logging.Level.WARN; + } else if (log4jLevel == INFO) { + return org.elasticsearch.logging.Level.INFO; + } else if (log4jLevel == DEBUG) { + return org.elasticsearch.logging.Level.DEBUG; + } else if (log4jLevel == TRACE) { + return org.elasticsearch.logging.Level.TRACE; + } else if (log4jLevel == ALL) { + return org.elasticsearch.logging.Level.ALL; + } + throw new AssertionError("unknown log4j level [" + log4jLevel + "]"); + } } diff --git a/server/src/main/java/org/elasticsearch/common/logging/internal/LoggerFactoryImpl.java b/server/src/main/java/org/elasticsearch/common/logging/internal/LoggerFactoryImpl.java index e8354be5ea225..393a94125da60 100644 --- a/server/src/main/java/org/elasticsearch/common/logging/internal/LoggerFactoryImpl.java +++ b/server/src/main/java/org/elasticsearch/common/logging/internal/LoggerFactoryImpl.java @@ -10,6 +10,8 @@ package org.elasticsearch.common.logging.internal; import org.apache.logging.log4j.LogManager; +import org.elasticsearch.common.logging.Loggers; +import org.elasticsearch.logging.Level; import org.elasticsearch.logging.Logger; import org.elasticsearch.logging.internal.spi.LoggerFactory; @@ -30,4 +32,15 @@ public Logger getLogger(Class clazz) { // classloader a class comes from, we will use the root logging config. return getLogger(clazz.getName()); } + + @Override + public void setRootLevel(Level level) { + var log4jLevel = LevelUtil.log4jLevel(level); + Loggers.setLevel(LogManager.getRootLogger(), log4jLevel); + } + + @Override + public Level getRootLevel() { + return LevelUtil.elasticsearchLevel(LogManager.getRootLogger().getLevel()); + } } diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java index a85715ba5a016..694dfc9f30305 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java @@ -117,6 +117,7 @@ import org.elasticsearch.indices.IndicesModule; import org.elasticsearch.indices.analysis.AnalysisModule; import org.elasticsearch.jdk.RuntimeVersionFeature; +import org.elasticsearch.logging.internal.spi.LoggerFactory; import org.elasticsearch.plugins.AnalysisPlugin; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.plugins.scanners.StablePluginsRegistry; @@ -576,6 +577,23 @@ public void removeHeaderWarningAppender() { } } + private static org.elasticsearch.logging.Level capturedLogLevel = null; + + // just capture the expected level once before the suite starts + @BeforeClass + public static void captureLoggingLevel() { + capturedLogLevel = LoggerFactory.provider().getRootLevel(); + } + + @AfterClass + public static void restoreLoggingLevel() { + if (capturedLogLevel != null) { + // log level might not have been captured if suite was skipped + LoggerFactory.provider().setRootLevel(capturedLogLevel); + capturedLogLevel = null; + } + } + @Before public final void before() { LeakTracker.setContextHint(getTestName()); diff --git a/x-pack/plugin/security/src/main/java/module-info.java b/x-pack/plugin/security/src/main/java/module-info.java index 947211559b0c2..a2798faefaa38 100644 --- a/x-pack/plugin/security/src/main/java/module-info.java +++ b/x-pack/plugin/security/src/main/java/module-info.java @@ -49,6 +49,7 @@ requires oauth2.oidc.sdk; requires org.slf4j; requires unboundid.ldapsdk; + requires org.elasticsearch.logging; exports org.elasticsearch.xpack.security.action to org.elasticsearch.server; exports org.elasticsearch.xpack.security.action.apikey to org.elasticsearch.server;