From ba3602b8b77e153b52ae8a4af59d4f7958e7a7d1 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Tue, 27 Feb 2024 13:53:26 +0000 Subject: [PATCH] Fix handling of application names containing parenthesis Closes gh-39564 --- .../logback/ApplicationNameConverter.java | 50 ++++++++++++ .../logback/DefaultLogbackConfiguration.java | 7 +- .../boot/logging/logback/defaults.xml | 5 +- .../log4j2/Log4J2LoggingSystemTests.java | 55 +++++++++++-- .../ApplicationNameConverterTests.java | 81 +++++++++++++++++++ .../logback/LogbackLoggingSystemTests.java | 47 ++++++++++- 6 files changed, 229 insertions(+), 16 deletions(-) create mode 100644 spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/ApplicationNameConverter.java create mode 100644 spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/logback/ApplicationNameConverterTests.java diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/ApplicationNameConverter.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/ApplicationNameConverter.java new file mode 100644 index 000000000000..cc893605c710 --- /dev/null +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/ApplicationNameConverter.java @@ -0,0 +1,50 @@ +/* + * Copyright 2012-2024 the original author or authors. + * + * Licensed 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 + * + * https://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.springframework.boot.logging.logback; + +import ch.qos.logback.classic.pattern.ClassicConverter; +import ch.qos.logback.classic.pattern.PropertyConverter; +import ch.qos.logback.classic.spi.ILoggingEvent; + +import org.springframework.boot.logging.LoggingSystemProperty; + +/** + * Logback {@link ClassicConverter} to convert the + * {@link LoggingSystemProperty#APPLICATION_NAME APPLICATION_NAME} into a value suitable + * for logging. Similar to Logback's {@link PropertyConverter} but a non-existent property + * is logged as an empty string rather than {@code null}. + * + * @author Andy Wilkinson + * @since 3.2.4 + */ +public class ApplicationNameConverter extends ClassicConverter { + + @Override + public String convert(ILoggingEvent event) { + String applicationName = event.getLoggerContextVO() + .getPropertyMap() + .get(LoggingSystemProperty.APPLICATION_NAME.getEnvironmentVariableName()); + if (applicationName == null) { + applicationName = System.getProperty(LoggingSystemProperty.APPLICATION_NAME.getEnvironmentVariableName()); + if (applicationName == null) { + applicationName = ""; + } + } + return applicationName; + } + +} diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/DefaultLogbackConfiguration.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/DefaultLogbackConfiguration.java index dfce5601214b..abba6c24c41d 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/DefaultLogbackConfiguration.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/DefaultLogbackConfiguration.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2023 the original author or authors. + * Copyright 2012-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -68,6 +68,7 @@ void apply(LogbackConfigurator config) { } private void defaults(LogbackConfigurator config) { + config.conversionRule("applicationName", ApplicationNameConverter.class); config.conversionRule("clr", ColorConverter.class); config.conversionRule("correlationId", CorrelationIdConverter.class); config.conversionRule("wex", WhitespaceThrowableProxyConverter.class); @@ -75,7 +76,7 @@ private void defaults(LogbackConfigurator config) { config.getContext() .putProperty("CONSOLE_LOG_PATTERN", resolve(config, "${CONSOLE_LOG_PATTERN:-" + "%clr(%d{${LOG_DATEFORMAT_PATTERN:-yyyy-MM-dd'T'HH:mm:ss.SSSXXX}}){faint} %clr(${LOG_LEVEL_PATTERN:-%5p}) " - + "%clr(${PID:- }){magenta} %clr(---){faint} %clr(${LOGGED_APPLICATION_NAME:-}[%15.15t]){faint} " + + "%clr(${PID:- }){magenta} %clr(---){faint} %clr(%applicationName[%15.15t]){faint} " + "%clr(${LOG_CORRELATION_PATTERN:-}){faint}%clr(%-40.40logger{39}){cyan} " + "%clr(:){faint} %m%n${LOG_EXCEPTION_CONVERSION_WORD:-%wEx}}")); String defaultCharset = Charset.defaultCharset().name(); @@ -84,7 +85,7 @@ private void defaults(LogbackConfigurator config) { config.getContext().putProperty("CONSOLE_LOG_THRESHOLD", resolve(config, "${CONSOLE_LOG_THRESHOLD:-TRACE}")); config.getContext() .putProperty("FILE_LOG_PATTERN", resolve(config, "${FILE_LOG_PATTERN:-" - + "%d{${LOG_DATEFORMAT_PATTERN:-yyyy-MM-dd'T'HH:mm:ss.SSSXXX}} ${LOG_LEVEL_PATTERN:-%5p} ${PID:- } --- ${LOGGED_APPLICATION_NAME:-}[%t] " + + "%d{${LOG_DATEFORMAT_PATTERN:-yyyy-MM-dd'T'HH:mm:ss.SSSXXX}} ${LOG_LEVEL_PATTERN:-%5p} ${PID:- } --- %applicationName[%t] " + "${LOG_CORRELATION_PATTERN:-}" + "%-40.40logger{39} : %m%n${LOG_EXCEPTION_CONVERSION_WORD:-%wEx}}")); config.getContext() diff --git a/spring-boot-project/spring-boot/src/main/resources/org/springframework/boot/logging/logback/defaults.xml b/spring-boot-project/spring-boot/src/main/resources/org/springframework/boot/logging/logback/defaults.xml index 9c02f84e4099..a469b4cba9c4 100644 --- a/spring-boot-project/spring-boot/src/main/resources/org/springframework/boot/logging/logback/defaults.xml +++ b/spring-boot-project/spring-boot/src/main/resources/org/springframework/boot/logging/logback/defaults.xml @@ -5,15 +5,16 @@ Default logback configuration provided for import --> + - + - + diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystemTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystemTests.java index 75564c7662e7..6778c8a90adb 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystemTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystemTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2023 the original author or authors. + * Copyright 2012-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -565,7 +565,7 @@ void correlationLoggingToConsoleWhenHasCorrelationPattern(CapturedOutput output) } @Test - void applicationNameLoggingWhenHasApplicationName(CapturedOutput output) { + void applicationNameLoggingToConsoleWhenHasApplicationName(CapturedOutput output) { this.environment.setProperty("spring.application.name", "myapp"); this.loggingSystem.setStandardConfigLocations(false); this.loggingSystem.beforeInitialize(); @@ -575,25 +575,66 @@ void applicationNameLoggingWhenHasApplicationName(CapturedOutput output) { } @Test - void applicationNamePlaceHolderNotShowingWhenDisabled(CapturedOutput output) { + void applicationNameLoggingToConsoleWhenHasApplicationNameWithParenthesis(CapturedOutput output) { + this.environment.setProperty("spring.application.name", "myapp (dev)"); + this.loggingSystem.setStandardConfigLocations(false); + this.loggingSystem.beforeInitialize(); + this.loggingSystem.initialize(this.initializationContext, null, null); + this.logger.info("Hello world"); + assertThat(getLineWithText(output, "Hello world")).contains("[myapp (dev)] "); + } + + @Test + void applicationNameLoggingToConsoleWhenDisabled(CapturedOutput output) { this.environment.setProperty("spring.application.name", "application-name"); this.environment.setProperty("logging.include-application-name", "false"); this.loggingSystem.setStandardConfigLocations(false); this.loggingSystem.beforeInitialize(); this.loggingSystem.initialize(this.initializationContext, null, null); this.logger.info("Hello world"); - assertThat(getLineWithText(output, "Hello world")).doesNotContain("${sys:LOGGED_APPLICATION_NAME}"); + assertThat(getLineWithText(output, "Hello world")).doesNotContain("${sys:LOGGED_APPLICATION_NAME}") + .doesNotContain("myapp"); } @Test - void applicationNameLoggingWhenDisabled(CapturedOutput output) { + void applicationNameLoggingToFileWhenHasApplicationName() { this.environment.setProperty("spring.application.name", "myapp"); + new LoggingSystemProperties(this.environment).apply(); + File file = new File(tmpDir(), "log4j2-test.log"); + LogFile logFile = getLogFile(file.getPath(), null); + this.loggingSystem.setStandardConfigLocations(false); + this.loggingSystem.beforeInitialize(); + this.loggingSystem.initialize(this.initializationContext, null, logFile); + this.logger.info("Hello world"); + assertThat(getLineWithText(file, "Hello world")).contains("[myapp] "); + } + + @Test + void applicationNameLoggingToFileWhenHasApplicationNameWithParenthesis() { + this.environment.setProperty("spring.application.name", "myapp (dev)"); + new LoggingSystemProperties(this.environment).apply(); + File file = new File(tmpDir(), "log4j2-test.log"); + LogFile logFile = getLogFile(file.getPath(), null); + this.loggingSystem.setStandardConfigLocations(false); + this.loggingSystem.beforeInitialize(); + this.loggingSystem.initialize(this.initializationContext, null, logFile); + this.logger.info("Hello world"); + assertThat(getLineWithText(file, "Hello world")).contains("[myapp (dev)] "); + } + + @Test + void applicationNameLoggingToFileWhenDisabled() { + this.environment.setProperty("spring.application.name", "application-name"); this.environment.setProperty("logging.include-application-name", "false"); + new LoggingSystemProperties(this.environment).apply(); + File file = new File(tmpDir(), "log4j2-test.log"); + LogFile logFile = getLogFile(file.getPath(), null); this.loggingSystem.setStandardConfigLocations(false); this.loggingSystem.beforeInitialize(); - this.loggingSystem.initialize(this.initializationContext, null, null); + this.loggingSystem.initialize(this.initializationContext, null, logFile); this.logger.info("Hello world"); - assertThat(getLineWithText(output, "Hello world")).doesNotContain("myapp"); + assertThat(getLineWithText(file, "Hello world")).doesNotContain("${sys:LOGGED_APPLICATION_NAME}") + .doesNotContain("myapp"); } private String getRelativeClasspathLocation(String fileName) { diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/logback/ApplicationNameConverterTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/logback/ApplicationNameConverterTests.java new file mode 100644 index 000000000000..57b5a867c07e --- /dev/null +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/logback/ApplicationNameConverterTests.java @@ -0,0 +1,81 @@ +/* + * Copyright 2012-2024 the original author or authors. + * + * Licensed 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 + * + * https://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.springframework.boot.logging.logback; + +import java.util.Collections; + +import ch.qos.logback.classic.LoggerContext; +import ch.qos.logback.classic.spi.LoggerContextVO; +import ch.qos.logback.classic.spi.LoggingEvent; +import org.junit.jupiter.api.Test; + +import org.springframework.boot.logging.LoggingSystemProperty; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link ApplicationNameConverter}. + * + * @author Andy Wilkinson + */ +class ApplicationNameConverterTests { + + private final ApplicationNameConverter converter; + + private final LoggingEvent event = new LoggingEvent(); + + ApplicationNameConverterTests() { + this.converter = new ApplicationNameConverter(); + this.converter.setContext(new LoggerContext()); + this.event.setLoggerContextRemoteView( + new LoggerContextVO("test", Collections.emptyMap(), System.currentTimeMillis())); + } + + @Test + void whenNoLoggedApplicationNameConvertReturnsEmptyString() { + withLoggedApplicationName(null, () -> { + this.converter.start(); + String converted = this.converter.convert(this.event); + assertThat(converted).isEqualTo(""); + }); + } + + @Test + void whenLoggedApplicationNameConvertReturnsIt() { + withLoggedApplicationName("my-application", () -> { + this.converter.start(); + String converted = this.converter.convert(this.event); + assertThat(converted).isEqualTo("my-application"); + }); + } + + private void withLoggedApplicationName(String name, Runnable action) { + if (name == null) { + System.clearProperty(LoggingSystemProperty.APPLICATION_NAME.getEnvironmentVariableName()); + } + else { + System.setProperty(LoggingSystemProperty.APPLICATION_NAME.getEnvironmentVariableName(), name); + } + try { + action.run(); + } + finally { + System.clearProperty(LoggingSystemProperty.APPLICATION_NAME.getEnvironmentVariableName()); + } + } + +} diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemTests.java index 845048e3473f..be7e1a5c15f9 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2023 the original author or authors. + * Copyright 2012-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -761,7 +761,7 @@ void correlationLoggingToFileWhenUsingFileConfiguration() { } @Test - void applicationNameLoggingWhenHasApplicationName(CapturedOutput output) { + void applicationNameLoggingToConsoleWhenHasApplicationName(CapturedOutput output) { this.environment.setProperty("spring.application.name", "myapp"); initialize(this.initializationContext, null, null); this.logger.info("Hello world"); @@ -769,12 +769,51 @@ void applicationNameLoggingWhenHasApplicationName(CapturedOutput output) { } @Test - void applicationNameLoggingWhenDisabled(CapturedOutput output) { + void applicationNameLoggingToConsoleWhenHasApplicationNameWithParenthesis(CapturedOutput output) { + this.environment.setProperty("spring.application.name", "myapp (dev)"); + initialize(this.initializationContext, null, null); + this.logger.info("Hello world"); + assertThat(getLineWithText(output, "Hello world")).contains("[myapp (dev)] "); + } + + @Test + void applicationNameLoggingToConsoleWhenDisabled(CapturedOutput output) { this.environment.setProperty("spring.application.name", "myapp"); this.environment.setProperty("logging.include-application-name", "false"); initialize(this.initializationContext, null, null); this.logger.info("Hello world"); - assertThat(getLineWithText(output, "Hello world")).doesNotContain("myapp"); + assertThat(getLineWithText(output, "Hello world")).doesNotContain("myapp").doesNotContain("null"); + } + + @Test + void applicationNameLoggingToFileWhenHasApplicationName() { + this.environment.setProperty("spring.application.name", "myapp"); + File file = new File(tmpDir(), "logback-test.log"); + LogFile logFile = getLogFile(file.getPath(), null); + initialize(this.initializationContext, null, logFile); + this.logger.info("Hello world"); + assertThat(getLineWithText(file, "Hello world")).contains("[myapp] "); + } + + @Test + void applicationNameLoggingToFileWhenHasApplicationNameWithParenthesis() { + this.environment.setProperty("spring.application.name", "myapp (dev)"); + File file = new File(tmpDir(), "logback-test.log"); + LogFile logFile = getLogFile(file.getPath(), null); + initialize(this.initializationContext, null, logFile); + this.logger.info("Hello world"); + assertThat(getLineWithText(file, "Hello world")).contains("[myapp (dev)] "); + } + + @Test + void applicationNameLoggingToFileWhenDisabled(CapturedOutput output) { + this.environment.setProperty("spring.application.name", "myapp"); + this.environment.setProperty("logging.include-application-name", "false"); + File file = new File(tmpDir(), "logback-test.log"); + LogFile logFile = getLogFile(file.getPath(), null); + initialize(this.initializationContext, null, logFile); + this.logger.info("Hello world"); + assertThat(getLineWithText(file, "Hello world")).doesNotContain("myapp").doesNotContain("null"); } @Test