This is an automated email from the ASF dual-hosted git repository. ramanathan1504 pushed a commit to branch fix-trace-metadata-1976 in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit f48769ea0b1b9dbb7f2e57b9ad9836c8291c76ea Author: Ramanathan <[email protected]> AuthorDate: Sat Jul 4 11:06:01 2026 +0530 Enhance TraceContextProviderService for exception safety and simplify trace ID retrieval --- .../core/impl/TraceContextIntegrationTest.java | 54 +++++++++++++++------- .../core/async/RingBufferLogEventTranslator.java | 9 ++-- .../logging/log4j/core/impl/Log4jLogEvent.java | 21 ++++----- .../logging/log4j/core/impl/MutableLogEvent.java | 8 ++-- .../core/util/TraceContextProviderService.java | 45 ++++++++++++++++-- 5 files changed, 94 insertions(+), 43 deletions(-) diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/impl/TraceContextIntegrationTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/impl/TraceContextIntegrationTest.java index e63da79666..3f966b877f 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/impl/TraceContextIntegrationTest.java +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/impl/TraceContextIntegrationTest.java @@ -17,6 +17,7 @@ package org.apache.logging.log4j.core.impl; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; import org.apache.logging.log4j.Level; import org.apache.logging.log4j.ThreadContext; @@ -25,40 +26,31 @@ import org.apache.logging.log4j.core.async.RingBufferLogEvent; import org.apache.logging.log4j.core.async.RingBufferLogEventTranslator; import org.apache.logging.log4j.core.util.ClockFactory; import org.apache.logging.log4j.core.util.DummyNanoClock; +import org.apache.logging.log4j.core.util.TraceContextProviderService; import org.apache.logging.log4j.message.SimpleMessage; +import org.apache.logging.log4j.spi.TraceContextProvider; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; public class TraceContextIntegrationTest { + private final TestTraceContextProvider testProvider = new TestTraceContextProvider(); + @BeforeEach void setUp() { - // No programmatic bypass! Relying entirely on META-INF/services/ auto-discovery + TraceContextProviderService.setActiveProvider(testProvider); TestTraceContextProvider.setContext("4bf92f3577b34da6a3ce929d0e0e4736", "00f067aa0ba902b7", "01"); ThreadContext.clearMap(); } @AfterEach void tearDown() { + TraceContextProviderService.setActiveProvider(null); TestTraceContextProvider.clearContext(); ThreadContext.clearMap(); } - @Test - public void printCompiledServiceFiles() { - // Look directly at the compiled test resources folder - java.io.File servicesDir = new java.io.File("target/test-classes/META-INF/services"); - if (servicesDir.exists() && servicesDir.isDirectory()) { - System.out.println(">>> FILES IN target/test-classes/META-INF/services/:"); - for (String file : servicesDir.list()) { - System.out.println(">>> - " + file); - } - } else { - System.out.println(">>> ERROR: target/test-classes/META-INF/services directory does not exist!"); - } - } - @Test public void testLog4jLogEventNativeCaptureWithEmptyThreadContext() { assertThat(ThreadContext.getContext()).isEmpty(); @@ -69,7 +61,6 @@ public class TraceContextIntegrationTest { .setMessage(new SimpleMessage("Standard event testing")) .build(); - // Must successfully resolve directly via ServiceLoader auto-discovery assertThat(event.getTraceId()).isEqualTo("4bf92f3577b34da6a3ce929d0e0e4736"); assertThat(event.getSpanId()).isEqualTo("00f067aa0ba902b7"); assertThat(event.getTraceFlags()).isEqualTo("01"); @@ -124,4 +115,35 @@ public class TraceContextIntegrationTest { assertThat(event.getTraceFlags()).isEqualTo("01"); assertThat(event.getContextData().isEmpty()).isTrue(); } + + @Test + public void testProviderExceptionSafety() { + TraceContextProviderService.setActiveProvider(new TraceContextProvider() { + @Override + public String getTraceId() { + throw new RuntimeException("Simulated provider failure"); + } + + @Override + public String getSpanId() { + return null; + } + + @Override + public String getTraceFlags() { + return null; + } + }); + + assertThatCode(() -> { + final LogEvent event = Log4jLogEvent.newBuilder() + .setLoggerName("BuggyLogger") + .setLevel(Level.ERROR) + .setMessage(new SimpleMessage("Exception safety test")) + .build(); + + assertThat(event.getTraceId()).isEmpty(); + }) + .doesNotThrowAnyException(); + } } diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/async/RingBufferLogEventTranslator.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/async/RingBufferLogEventTranslator.java index b694c9e013..e7bffd9084 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/async/RingBufferLogEventTranslator.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/async/RingBufferLogEventTranslator.java @@ -26,7 +26,6 @@ import org.apache.logging.log4j.core.util.Clock; import org.apache.logging.log4j.core.util.NanoClock; import org.apache.logging.log4j.core.util.TraceContextProviderService; import org.apache.logging.log4j.message.Message; -import org.apache.logging.log4j.spi.TraceContextProvider; import org.apache.logging.log4j.util.ReadOnlyStringMap; import org.apache.logging.log4j.util.StringMap; @@ -159,8 +158,6 @@ public class RingBufferLogEventTranslator implements EventTranslator<RingBufferL final Clock aClock, final NanoClock aNanoClock) { - final TraceContextProvider provider = TraceContextProviderService.getActiveProvider(); - setBasicValues( anAsyncLogger, aLoggerName, @@ -173,9 +170,9 @@ public class RingBufferLogEventTranslator implements EventTranslator<RingBufferL aLocation, aClock, aNanoClock, - provider.getTraceId(), - provider.getSpanId(), - provider.getTraceFlags()); + TraceContextProviderService.getTraceId(), + TraceContextProviderService.getSpanId(), + TraceContextProviderService.getTraceFlags()); } /** * @deprecated since 2.25.0. {@link RingBufferLogEventTranslator} instances should only be used on the thread that diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/Log4jLogEvent.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/Log4jLogEvent.java index 02ea4e374d..d2693e69d1 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/Log4jLogEvent.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/Log4jLogEvent.java @@ -44,7 +44,6 @@ import org.apache.logging.log4j.message.Message; import org.apache.logging.log4j.message.ReusableMessage; import org.apache.logging.log4j.message.SimpleMessage; import org.apache.logging.log4j.message.TimestampMessage; -import org.apache.logging.log4j.spi.TraceContextProvider; import org.apache.logging.log4j.status.StatusLogger; import org.apache.logging.log4j.util.ReadOnlyStringMap; import org.apache.logging.log4j.util.StackLocatorUtil; @@ -137,17 +136,15 @@ public class Log4jLogEvent implements LogEvent { this.contextData = createContextData((List<Property>) null); this.contextStack = ThreadContext.getImmutableStack(); - final TraceContextProvider provider = TraceContextProviderService.getActiveProvider(); - - final String tId = provider.getTraceId(); + final String tId = TraceContextProviderService.getTraceId(); if (tId != null) { this.traceId = tId; } - final String sId = provider.getSpanId(); + final String sId = TraceContextProviderService.getSpanId(); if (sId != null) { this.spanId = sId; } - final String flags = provider.getTraceFlags(); + final String flags = TraceContextProviderService.getTraceFlags(); if (flags != null) { this.traceFlags = flags; } @@ -474,9 +471,9 @@ public class Log4jLogEvent implements LogEvent { null, // StackTraceElement source CLOCK, // nanoClock.nanoTime(), - TraceContextProviderService.getActiveProvider().getTraceId(), - TraceContextProviderService.getActiveProvider().getSpanId(), - TraceContextProviderService.getActiveProvider().getTraceFlags()); + TraceContextProviderService.getTraceId(), + TraceContextProviderService.getSpanId(), + TraceContextProviderService.getTraceFlags()); } /** @@ -514,9 +511,9 @@ public class Log4jLogEvent implements LogEvent { source, // StackTraceElement source CLOCK, // nanoClock.nanoTime(), - TraceContextProviderService.getActiveProvider().getTraceId(), - TraceContextProviderService.getActiveProvider().getSpanId(), - TraceContextProviderService.getActiveProvider().getTraceFlags()); + TraceContextProviderService.getTraceId(), + TraceContextProviderService.getSpanId(), + TraceContextProviderService.getTraceFlags()); } /** diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/MutableLogEvent.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/MutableLogEvent.java index 83736894be..6733d48a31 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/MutableLogEvent.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/MutableLogEvent.java @@ -37,7 +37,6 @@ import org.apache.logging.log4j.message.ParameterVisitable; import org.apache.logging.log4j.message.ReusableMessage; import org.apache.logging.log4j.message.SimpleMessage; import org.apache.logging.log4j.message.TimestampMessage; -import org.apache.logging.log4j.spi.TraceContextProvider; import org.apache.logging.log4j.util.ReadOnlyStringMap; import org.apache.logging.log4j.util.StackLocatorUtil; import org.apache.logging.log4j.util.StringBuilders; @@ -371,10 +370,9 @@ public class MutableLogEvent implements LogEvent, ReusableMessage, ParameterVisi instant.initFrom(clock); } nanoTime = nanoClock.nanoTime(); - final TraceContextProvider provider = TraceContextProviderService.getActiveProvider(); - this.traceId = provider.getTraceId(); - this.spanId = provider.getSpanId(); - this.traceFlags = provider.getTraceFlags(); + this.traceId = TraceContextProviderService.getTraceId(); + this.spanId = TraceContextProviderService.getSpanId(); + this.traceFlags = TraceContextProviderService.getTraceFlags(); } @Override diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/TraceContextProviderService.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/TraceContextProviderService.java index 82c06bc27a..94ad76b431 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/TraceContextProviderService.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/TraceContextProviderService.java @@ -22,17 +22,16 @@ import org.apache.logging.log4j.spi.TraceContextProvider; import org.apache.logging.log4j.status.StatusLogger; /** - * Service registry designed to discover and cache the active {@link TraceContextProvider} - * natively using the standard Java ServiceLoader. + * Service registry designed to discover, cache, and safely query the active {@link TraceContextProvider}. */ public final class TraceContextProviderService { - private static final TraceContextProvider ACTIVE_PROVIDER; + private static final String GLOBAL_KEY = "log4j2.activeTraceContextProvider"; + private static volatile TraceContextProvider ACTIVE_PROVIDER; static { TraceContextProvider found = null; try { - // Standard ServiceLoader lookup using the Thread Context ClassLoader (TCCL) ClassLoader cl = Thread.currentThread().getContextClassLoader(); if (cl == null) { cl = TraceContextProviderService.class.getClassLoader(); @@ -59,9 +58,47 @@ public final class TraceContextProviderService { } public static TraceContextProvider getActiveProvider() { + final Object globalProvider = System.getProperties().get(GLOBAL_KEY); + if (globalProvider instanceof TraceContextProvider) { + return (TraceContextProvider) globalProvider; + } return ACTIVE_PROVIDER; } + public static void setActiveProvider(final TraceContextProvider provider) { + if (provider == null) { + System.getProperties().remove(GLOBAL_KEY); + ACTIVE_PROVIDER = NoOpTraceContextProvider.INSTANCE; + } else { + System.getProperties().put(GLOBAL_KEY, provider); + ACTIVE_PROVIDER = provider; + } + } + + public static String getTraceId() { + try { + return getActiveProvider().getTraceId(); + } catch (final Throwable t) { + return null; + } + } + + public static String getSpanId() { + try { + return getActiveProvider().getSpanId(); + } catch (final Throwable t) { + return null; + } + } + + public static String getTraceFlags() { + try { + return getActiveProvider().getTraceFlags(); + } catch (final Throwable t) { + return null; + } + } + private static final class NoOpTraceContextProvider implements TraceContextProvider { static final NoOpTraceContextProvider INSTANCE = new NoOpTraceContextProvider();
