From 605dee4700ec97fb48ccd72c3530f37957dbd8a0 Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Thu, 1 Jun 2017 14:31:07 +0200 Subject: [PATCH] Allow to reset a log level This commit ensures that `setLogLevel` on the `LoggingSystem` accepts a `null` level. A `null` level means any customization sets on that level should be removed and the default configuration should be used instead. Effectively, the level of the parent logger is going to be used when `setLevel` is called with `null` for a given logger. Most JMX clients do not accept to pass `null` for an argument so an empty String is translated to null in that specific case. Closes gh-8776 --- .../endpoint/jmx/LoggersEndpointMBean.java | 21 +++++++++++++++---- .../endpoint/LoggersEndpointTests.java | 6 ++++++ .../endpoint/mvc/LoggersMvcEndpointTests.java | 17 +++++++++++++++ .../asciidoc/production-ready-features.adoc | 3 +++ .../boot/logging/LoggingSystem.java | 3 ++- .../boot/logging/java/JavaLoggingSystem.java | 1 - .../logging/java/JavaLoggingSystemTests.java | 13 ++++++++++++ .../log4j2/Log4J2LoggingSystemTests.java | 13 ++++++++++++ .../logback/LogbackLoggingSystemTests.java | 13 ++++++++++++ 9 files changed, 84 insertions(+), 6 deletions(-) diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/LoggersEndpointMBean.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/LoggersEndpointMBean.java index e5f324d8d3..9a5fc17082 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/LoggersEndpointMBean.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/LoggersEndpointMBean.java @@ -24,12 +24,15 @@ import org.springframework.boot.actuate.endpoint.mvc.MvcEndpoint; import org.springframework.boot.logging.LogLevel; import org.springframework.jmx.export.annotation.ManagedAttribute; import org.springframework.jmx.export.annotation.ManagedOperation; -import org.springframework.util.Assert; +import org.springframework.jmx.export.annotation.ManagedOperationParameter; +import org.springframework.jmx.export.annotation.ManagedOperationParameters; +import org.springframework.util.StringUtils; /** * Adapter to expose {@link LoggersEndpoint} as an {@link MvcEndpoint}. * * @author Vedran Pavic + * @author Stephane Nicoll * @since 1.5.0 */ public class LoggersEndpointMBean extends EndpointMBean { @@ -45,15 +48,25 @@ public class LoggersEndpointMBean extends EndpointMBean { } @ManagedOperation(description = "Get log level for a given logger") + @ManagedOperationParameters({ + @ManagedOperationParameter(name = "loggerName", description = "Name of the logger") }) public Object getLogger(String loggerName) { return convert(getEndpoint().invoke(loggerName)); } @ManagedOperation(description = "Set log level for a given logger") + @ManagedOperationParameters({ + @ManagedOperationParameter(name = "loggerName", description = "Name of the logger"), + @ManagedOperationParameter(name = "logLevel", description = "New log level (can be null or empty String to remove the custom level)") }) public void setLogLevel(String loggerName, String logLevel) { - Assert.notNull(logLevel, "LogLevel must not be null"); - LogLevel level = LogLevel.valueOf(logLevel.toUpperCase()); - getEndpoint().setLogLevel(loggerName, level); + getEndpoint().setLogLevel(loggerName, determineLogLevel(logLevel)); + } + + private LogLevel determineLogLevel(String logLevel) { + if (StringUtils.hasText(logLevel)) { + return LogLevel.valueOf(logLevel.toUpperCase()); + } + return null; } @Override diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/LoggersEndpointTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/LoggersEndpointTests.java index 819fa14594..c93ae95d48 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/LoggersEndpointTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/LoggersEndpointTests.java @@ -80,6 +80,12 @@ public class LoggersEndpointTests extends AbstractEndpointTests verify(getLoggingSystem()).setLogLevel("ROOT", LogLevel.DEBUG); } + @Test + public void setLogLevelToNull() { + getEndpointBean().setLogLevel("ROOT", null); + verify(getLoggingSystem()).setLogLevel("ROOT", null); + } + private LoggingSystem getLoggingSystem() { return this.context.getBean(LoggingSystem.class); } diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/LoggersMvcEndpointTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/LoggersMvcEndpointTests.java index d849734f9b..aa26d63ae8 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/LoggersMvcEndpointTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/LoggersMvcEndpointTests.java @@ -64,6 +64,7 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers. * @author Ben Hale * @author Phillip Webb * @author Eddú Meléndez + * @author Stephane Nicoll */ @RunWith(SpringRunner.class) @SpringBootTest @@ -181,6 +182,22 @@ public class LoggersMvcEndpointTests { verifyZeroInteractions(this.loggingSystem); } + @Test + public void setLoggerWithNullLogLevel() throws Exception { + this.mvc.perform(post(PATH + "/ROOT").contentType(MediaType.APPLICATION_JSON) + .content("{\"configuredLevel\": null}")) + .andExpect(status().isNoContent()); + verify(this.loggingSystem).setLogLevel("ROOT", null); + } + + @Test + public void setLoggerWithNoLogLevel() throws Exception { + this.mvc.perform(post(PATH + "/ROOT").contentType(MediaType.APPLICATION_JSON) + .content("{}")) + .andExpect(status().isNoContent()); + verify(this.loggingSystem).setLogLevel("ROOT", null); + } + @Test public void logLevelForLoggerWithNameThatCouldBeMistakenForAPathExtension() throws Exception { diff --git a/spring-boot-docs/src/main/asciidoc/production-ready-features.adoc b/spring-boot-docs/src/main/asciidoc/production-ready-features.adoc index 96e6bc50a3..4c1f692613 100644 --- a/spring-boot-docs/src/main/asciidoc/production-ready-features.adoc +++ b/spring-boot-docs/src/main/asciidoc/production-ready-features.adoc @@ -927,6 +927,9 @@ In order to configure a given logger, you `POST` a partial entity to the resourc } ---- +TIP: You can also pass a `null` `configuredLevel` to "reset" the specific level of the +logger (and use the default configuration instead). + [[production-ready-metrics]] diff --git a/spring-boot/src/main/java/org/springframework/boot/logging/LoggingSystem.java b/spring-boot/src/main/java/org/springframework/boot/logging/LoggingSystem.java index c21f8bba14..d2794ab6b6 100644 --- a/spring-boot/src/main/java/org/springframework/boot/logging/LoggingSystem.java +++ b/spring-boot/src/main/java/org/springframework/boot/logging/LoggingSystem.java @@ -116,7 +116,8 @@ public abstract class LoggingSystem { * Sets the logging level for a given logger. * @param loggerName the name of the logger to set ({@code null} can be used for the * root logger). - * @param level the log level + * @param level the log level ({@code null} can be used to remove any custom level + * for the logger and use the default configuration instead) */ public void setLogLevel(String loggerName, LogLevel level) { throw new UnsupportedOperationException("Unable to set log level"); diff --git a/spring-boot/src/main/java/org/springframework/boot/logging/java/JavaLoggingSystem.java b/spring-boot/src/main/java/org/springframework/boot/logging/java/JavaLoggingSystem.java index 16d3941a7e..779e5a3529 100644 --- a/spring-boot/src/main/java/org/springframework/boot/logging/java/JavaLoggingSystem.java +++ b/spring-boot/src/main/java/org/springframework/boot/logging/java/JavaLoggingSystem.java @@ -117,7 +117,6 @@ public class JavaLoggingSystem extends AbstractLoggingSystem { @Override public void setLogLevel(String loggerName, LogLevel level) { - Assert.notNull(level, "Level must not be null"); if (loggerName == null || ROOT_LOGGER_NAME.equals(loggerName)) { loggerName = ""; } diff --git a/spring-boot/src/test/java/org/springframework/boot/logging/java/JavaLoggingSystemTests.java b/spring-boot/src/test/java/org/springframework/boot/logging/java/JavaLoggingSystemTests.java index fa715308b6..d45ba1118a 100644 --- a/spring-boot/src/test/java/org/springframework/boot/logging/java/JavaLoggingSystemTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/logging/java/JavaLoggingSystemTests.java @@ -168,6 +168,19 @@ public class JavaLoggingSystemTests extends AbstractLoggingSystemTests { .isEqualTo(1); } + @Test + public void setLevelToNull() throws Exception { + this.loggingSystem.beforeInitialize(); + this.loggingSystem.initialize(null, null, null); + this.logger.fine("Hello"); + this.loggingSystem.setLogLevel("org.springframework.boot", LogLevel.DEBUG); + this.logger.fine("Hello"); + this.loggingSystem.setLogLevel("org.springframework.boot", null); + this.logger.fine("Hello"); + assertThat(StringUtils.countOccurrencesOf(this.output.toString(), "Hello")) + .isEqualTo(1); + } + @Test public void getLoggingConfigurations() throws Exception { this.loggingSystem.beforeInitialize(); diff --git a/spring-boot/src/test/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystemTests.java b/spring-boot/src/test/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystemTests.java index 2938ce72fe..fa61d4823d 100644 --- a/spring-boot/src/test/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystemTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystemTests.java @@ -141,6 +141,19 @@ public class Log4J2LoggingSystemTests extends AbstractLoggingSystemTests { .isEqualTo(1); } + @Test + public void setLevelToNull() throws Exception { + this.loggingSystem.beforeInitialize(); + this.loggingSystem.initialize(null, null, null); + this.logger.debug("Hello"); + this.loggingSystem.setLogLevel("org.springframework.boot", LogLevel.DEBUG); + this.logger.debug("Hello"); + this.loggingSystem.setLogLevel("org.springframework.boot", null); + this.logger.debug("Hello"); + assertThat(StringUtils.countOccurrencesOf(this.output.toString(), "Hello")) + .isEqualTo(1); + } + @Test public void getLoggingConfigurations() throws Exception { this.loggingSystem.beforeInitialize(); diff --git a/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemTests.java b/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemTests.java index ea0470f611..d408664a88 100644 --- a/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemTests.java @@ -184,6 +184,19 @@ public class LogbackLoggingSystemTests extends AbstractLoggingSystemTests { .isEqualTo(1); } + @Test + public void setLevelToNull() throws Exception { + this.loggingSystem.beforeInitialize(); + this.loggingSystem.initialize(this.initializationContext, null, null); + this.logger.debug("Hello"); + this.loggingSystem.setLogLevel("org.springframework.boot", LogLevel.DEBUG); + this.logger.debug("Hello"); + this.loggingSystem.setLogLevel("org.springframework.boot", null); + this.logger.debug("Hello"); + assertThat(StringUtils.countOccurrencesOf(this.output.toString(), "Hello")) + .isEqualTo(1); + } + @Test public void getLoggingConfigurations() throws Exception { this.loggingSystem.beforeInitialize();