From 7c77e1bb85b94d15b6e33477711a4f2e1ba14cf1 Mon Sep 17 00:00:00 2001 From: Johnny Lim Date: Sun, 2 Jul 2023 19:07:40 +0900 Subject: [PATCH] Polish 'Log correlation IDs when Micrometer tracing is being used' See gh-36158 --- ...ogCorrelationEnvironmentPostProcessor.java | 2 +- ...relationEnvironmentPostProcessorTests.java | 4 ++-- .../boot/logging/CorrelationIdFormatter.java | 21 ++++++++++--------- .../logging/log4j2/Log4J2LoggingSystem.java | 3 +-- .../logging/CorrelationIdFormatterTests.java | 2 +- .../logback/LogbackLoggingSystemTests.java | 2 +- .../build.gradle | 2 +- 7 files changed, 18 insertions(+), 18 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/LogCorrelationEnvironmentPostProcessor.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/LogCorrelationEnvironmentPostProcessor.java index d7e86849d0..50a92939f4 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/LogCorrelationEnvironmentPostProcessor.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/LogCorrelationEnvironmentPostProcessor.java @@ -26,7 +26,7 @@ import org.springframework.util.ClassUtils; /** * {@link EnvironmentPostProcessor} to add a {@link PropertySource} to support log - * correlation IDs when Micrometer is present. Adds support for the + * correlation IDs when Micrometer Tracing is present. Adds support for the * {@value LoggingSystem#EXPECT_CORRELATION_ID_PROPERTY} property by delegating to * {@code management.tracing.enabled}. * diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/LogCorrelationEnvironmentPostProcessorTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/LogCorrelationEnvironmentPostProcessorTests.java index a75a91a28b..4bbfa4a0e8 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/LogCorrelationEnvironmentPostProcessorTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/LogCorrelationEnvironmentPostProcessorTests.java @@ -42,7 +42,7 @@ class LogCorrelationEnvironmentPostProcessorTests { private final LogCorrelationEnvironmentPostProcessor postProcessor = new LogCorrelationEnvironmentPostProcessor(); @Test - void getExpectCorrelationIdPropertyWhenMicrometerPresentReturnsTrue() { + void getExpectCorrelationIdPropertyWhenMicrometerTracingPresentReturnsTrue() { this.postProcessor.postProcessEnvironment(this.environment, this.application); assertThat(this.environment.getProperty(LoggingSystem.EXPECT_CORRELATION_ID_PROPERTY, Boolean.class, false)) .isTrue(); @@ -50,7 +50,7 @@ class LogCorrelationEnvironmentPostProcessorTests { @Test @ClassPathExclusions("micrometer-tracing-*.jar") - void getExpectCorrelationIdPropertyWhenMicrometerMissingReturnsFalse() { + void getExpectCorrelationIdPropertyWhenMicrometerTracingMissingReturnsFalse() { this.postProcessor.postProcessEnvironment(this.environment, this.application); assertThat(this.environment.getProperty(LoggingSystem.EXPECT_CORRELATION_ID_PROPERTY, Boolean.class, false)) .isFalse(); diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/CorrelationIdFormatter.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/CorrelationIdFormatter.java index 5dcab9f4d5..1388aecca0 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/CorrelationIdFormatter.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/CorrelationIdFormatter.java @@ -18,7 +18,6 @@ package org.springframework.boot.logging; import java.io.IOException; import java.io.UncheckedIOException; -import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.Iterator; @@ -36,7 +35,7 @@ import org.springframework.util.StringUtils; /** * Utility class that can be used to format a correlation identifier for logging based on * w3c + * "https://www.w3.org/TR/trace-context/#examples-of-http-traceparent-headers">W3C * recommendations. *

* The formatter can be configured with a comma-separated list of names and the expected @@ -46,8 +45,8 @@ import org.springframework.util.StringUtils; * {@code 16} respectively. *

* Correlation IDs are formatted as dash separated strings surrounded in square brackets. - * Formatted output is always of a fixed width and with trailing whitespace. Dashes are - * omitted if none of the named items can be resolved. + * Formatted output is always of a fixed width and with trailing space. Dashes are omitted + * if none of the named items can be resolved. *

* The following example would return a formatted result of * {@code "[01234567890123456789012345678901-0123456789012345] "}: 

@@ -101,10 +100,12 @@ public final class CorrelationIdFormatter {
 		Predicate canResolve = (part) -> StringUtils.hasLength(resolver.apply(part.name()));
 		try {
 			if (this.parts.stream().anyMatch(canResolve)) {
-				appendable.append("[");
+				appendable.append('[');
 				for (Iterator iterator = this.parts.iterator(); iterator.hasNext();) {
 					appendable.append(iterator.next().resolve(resolver));
-					appendable.append((!iterator.hasNext()) ? "" : "-");
+					if (iterator.hasNext()) {
+						appendable.append('-');
+					}
 				}
 				appendable.append("] ");
 			}
@@ -124,7 +125,7 @@ public final class CorrelationIdFormatter {
 
 	/**
 	 * Create a new {@link CorrelationIdFormatter} instance from the given specification.
-	 * @param spec a comma separated specification
+	 * @param spec a comma-separated specification
 	 * @return a new {@link CorrelationIdFormatter} instance
 	 */
 	public static CorrelationIdFormatter of(String spec) {
@@ -142,7 +143,7 @@ public final class CorrelationIdFormatter {
 	 * @return a new {@link CorrelationIdFormatter} instance
 	 */
 	public static CorrelationIdFormatter of(String[] spec) {
-		return of((spec != null) ? Arrays.asList(spec) : Collections.emptyList());
+		return of((spec != null) ? List.of(spec) : Collections.emptyList());
 	}
 
 	/**
@@ -166,7 +167,7 @@ public final class CorrelationIdFormatter {
 	 */
 	record Part(String name, int length) {
 
-		private static final Pattern pattern = Pattern.compile("^(.+?)\\((\\d+)\\)?$");
+		private static final Pattern pattern = Pattern.compile("^(.+?)\\((\\d+)\\)$");
 
 		String resolve(Function resolver) {
 			String resolved = resolver.apply(name());
@@ -174,7 +175,7 @@ public final class CorrelationIdFormatter {
 				return blank();
 			}
 			int padding = length() - resolved.length();
-			return resolved + " ".repeat((padding > 0) ? padding : 0);
+			return (padding <= 0) ? resolved : resolved + " ".repeat(padding);
 		}
 
 		String blank() {
diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystem.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystem.java
index da996bfd5c..10152f88c3 100644
--- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystem.java
+++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystem.java
@@ -249,8 +249,7 @@ public class Log4J2LoggingSystem extends AbstractLoggingSystem {
 
 	@Override
 	protected void loadDefaults(LoggingInitializationContext initializationContext, LogFile logFile) {
-		String location = (logFile != null) ? getPackagedConfigFile("log4j2-file.xml")
-				: getPackagedConfigFile("log4j2.xml");
+		String location = getPackagedConfigFile((logFile != null) ? "log4j2-file.xml" : "log4j2.xml");
 		load(initializationContext, location, logFile);
 	}
 
diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/CorrelationIdFormatterTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/CorrelationIdFormatterTests.java
index b34771352a..2e36d0f00f 100644
--- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/CorrelationIdFormatterTests.java
+++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/CorrelationIdFormatterTests.java
@@ -96,7 +96,7 @@ class CorrelationIdFormatterTests {
 		context.put("traceId", "01234567890123456789012345678901");
 		context.put("spanId", "0123456789012345");
 		StringBuilder formatted = new StringBuilder();
-		CorrelationIdFormatter.of("").formatTo(context::get, formatted);
+		CorrelationIdFormatter.DEFAULT.formatTo(context::get, formatted);
 		assertThat(formatted).hasToString("[01234567890123456789012345678901-0123456789012345] ");
 	}
 
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 685923ead6..1baa880012 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
@@ -743,7 +743,7 @@ class LogbackLoggingSystemTests extends AbstractLoggingSystemTests {
 	}
 
 	@Test
-	void correlationLoggingToConsoleWhenUsingFileConfiguration() {
+	void correlationLoggingToFileWhenUsingFileConfiguration() {
 		this.environment.setProperty(LoggingSystem.EXPECT_CORRELATION_ID_PROPERTY, "true");
 		File file = new File(tmpDir(), "logback-test.log");
 		LogFile logFile = getLogFile(file.getPath(), null);
diff --git a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator/build.gradle b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator/build.gradle
index 656ad05a75..c5157df03e 100644
--- a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator/build.gradle
+++ b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator/build.gradle
@@ -11,7 +11,7 @@ dependencies {
 	implementation(project(":spring-boot-project:spring-boot-starters:spring-boot-starter-security"))
 	implementation(project(":spring-boot-project:spring-boot-starters:spring-boot-starter-web"))
 	implementation(project(":spring-boot-project:spring-boot-starters:spring-boot-starter-validation"))
-	implementation 'io.micrometer:micrometer-tracing-bridge-brave'
+	implementation("io.micrometer:micrometer-tracing-bridge-brave")
 
 	runtimeOnly("com.h2database:h2")