From 2030f70988769c916b297dc4afb2b1fa9c1d3d9a Mon Sep 17 00:00:00 2001 From: Rajesh Date: Sun, 18 Oct 2026 11:14:26 +0530 Subject: [PATCH 01/10] Add trace and span IDs to log MDC via OpenTelemetry ContextStorageProvider --- .../tracing/OzoneContextStorageProvider.java | 87 +++++++++++++ ...entelemetry.context.ContextStorageProvider | 16 +++ .../TestOzoneContextStorageProvider.java | 121 ++++++++++++++++++ .../dist/src/shell/conf/log4j.properties | 8 +- 4 files changed, 228 insertions(+), 4 deletions(-) create mode 100644 hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/tracing/OzoneContextStorageProvider.java create mode 100644 hadoop-hdds/common/src/main/resources/META-INF/services/io.opentelemetry.context.ContextStorageProvider create mode 100644 hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/tracing/TestOzoneContextStorageProvider.java diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/tracing/OzoneContextStorageProvider.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/tracing/OzoneContextStorageProvider.java new file mode 100644 index 000000000000..e87898cd8f25 --- /dev/null +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/tracing/OzoneContextStorageProvider.java @@ -0,0 +1,87 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 + * + * http://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.apache.hadoop.hdds.tracing; + +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.api.trace.SpanContext; +import io.opentelemetry.context.Context; +import io.opentelemetry.context.ContextStorage; +import io.opentelemetry.context.ContextStorageProvider; +import io.opentelemetry.context.Scope; +import org.slf4j.MDC; + +/** + * Registered through {@code META-INF/services} so OpenTelemetry uses {@link OzoneContextStorage}, + * which wraps the default thread-local storage. + */ +public final class OzoneContextStorageProvider implements ContextStorageProvider { + public static final String MDC_TRACE_ID = "trace_id"; + public static final String MDC_SPAN_ID = "span_id"; + + @Override + public ContextStorage get() { + return new OzoneContextStorage(ContextStorage.defaultStorage()); + } + + /** + * {@link ContextStorage} that keeps the logging MDC in sync with the current span. + * Every {@code makeCurrent()} / {@code Context.wrap(...)} goes through {@link #attach(Context)}, + * so trace and span IDs are available to log patterns without changes at the call sites. + * The MDC is always derived from the current context (never saved and restored), + * so values inherited by child threads through the MDC are overwritten on the next attach or close. + */ + static final class OzoneContextStorage implements ContextStorage { + private final ContextStorage delegate; + + OzoneContextStorage(ContextStorage delegate) { + this.delegate = delegate; + } + + @Override + public Scope attach(Context toAttach) { + Scope scope = delegate.attach(toAttach); + updateMdc(delegate.current()); + return () -> { + scope.close(); + updateMdc(delegate.current()); + }; + } + + @Override + public Context current() { + return delegate.current(); + } + + @Override + public Context root() { + return delegate.root(); + } + + private static void updateMdc(Context context) { + SpanContext spanContext = context == null ? SpanContext.getInvalid() + : Span.fromContext(context).getSpanContext(); + if (spanContext.isValid()) { + MDC.put(MDC_TRACE_ID, spanContext.getTraceId()); + MDC.put(MDC_SPAN_ID, spanContext.getSpanId()); + } else { + MDC.remove(MDC_TRACE_ID); + MDC.remove(MDC_SPAN_ID); + } + } + } +} diff --git a/hadoop-hdds/common/src/main/resources/META-INF/services/io.opentelemetry.context.ContextStorageProvider b/hadoop-hdds/common/src/main/resources/META-INF/services/io.opentelemetry.context.ContextStorageProvider new file mode 100644 index 000000000000..8da279befb60 --- /dev/null +++ b/hadoop-hdds/common/src/main/resources/META-INF/services/io.opentelemetry.context.ContextStorageProvider @@ -0,0 +1,16 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You 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 +# +# http://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. + +org.apache.hadoop.hdds.tracing.OzoneContextStorageProvider diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/tracing/TestOzoneContextStorageProvider.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/tracing/TestOzoneContextStorageProvider.java new file mode 100644 index 000000000000..f23137bb3839 --- /dev/null +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/tracing/TestOzoneContextStorageProvider.java @@ -0,0 +1,121 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 + * + * http://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.apache.hadoop.hdds.tracing; + +import static org.apache.hadoop.hdds.tracing.OzoneContextStorageProvider.MDC_SPAN_ID; +import static org.apache.hadoop.hdds.tracing.OzoneContextStorageProvider.MDC_TRACE_ID; +import static org.assertj.core.api.Assertions.assertThat; + +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.api.trace.SpanContext; +import io.opentelemetry.api.trace.TraceFlags; +import io.opentelemetry.api.trace.TraceState; +import io.opentelemetry.context.Context; +import io.opentelemetry.context.ContextStorage; +import io.opentelemetry.context.Scope; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import org.apache.hadoop.hdds.conf.InMemoryConfigurationForTesting; +import org.apache.hadoop.hdds.conf.MutableConfigurationSource; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.slf4j.MDC; + +/** + * Test for {@link OzoneContextStorageProvider}. + */ +public class TestOzoneContextStorageProvider { + private static final String TRACE_ID = "0af7651916cd43dd8448eb211c80319c"; + private static final String PARENT_SPAN_ID = "b7ad6b7169203331"; + private static final String CHILD_SPAN_ID = "00f067aa0ba902b7"; + + @AfterEach + public void clearMdc() { + MDC.clear(); + } + + private static Span span(String spanId) { + return Span.wrap(SpanContext.create(TRACE_ID, spanId, TraceFlags.getSampled(), TraceState.getDefault())); + } + + @Test + public void testProviderIsRegistered() { + assertThat(ContextStorage.get()).isInstanceOf(OzoneContextStorageProvider.OzoneContextStorage.class); + } + + @Test + public void testMakeCurrentSetsAndRestoresMdc() { + try (Scope ignored = span(PARENT_SPAN_ID).makeCurrent()) { + assertThat(MDC.get(MDC_TRACE_ID)).isEqualTo(TRACE_ID); + assertThat(MDC.get(MDC_SPAN_ID)).isEqualTo(PARENT_SPAN_ID); + try (Scope ignoredInner = span(CHILD_SPAN_ID).makeCurrent()) { + assertThat(MDC.get(MDC_SPAN_ID)).isEqualTo(CHILD_SPAN_ID); + } + assertThat(MDC.get(MDC_SPAN_ID)).isEqualTo(PARENT_SPAN_ID); + try (Scope ignoredInner = Span.getInvalid().makeCurrent()) { + assertThat(MDC.get(MDC_TRACE_ID)).isNull(); + assertThat(MDC.get(MDC_SPAN_ID)).isNull(); + } + assertThat(MDC.get(MDC_TRACE_ID)).isEqualTo(TRACE_ID); + } + assertThat(MDC.get(MDC_TRACE_ID)).isNull(); + assertThat(MDC.get(MDC_SPAN_ID)).isNull(); + } + + @Test + public void testStaleMdcIsClearedAfterClose() { + // e.g. values inherited from the parent thread through log4j's InheritableThreadLocal MDC + MDC.put(MDC_TRACE_ID, "stale-trace"); + MDC.put(MDC_SPAN_ID, "stale-span"); + try (Scope ignored = span(PARENT_SPAN_ID).makeCurrent()) { + assertThat(MDC.get(MDC_SPAN_ID)).isEqualTo(PARENT_SPAN_ID); + } + assertThat(MDC.get(MDC_TRACE_ID)).isNull(); + assertThat(MDC.get(MDC_SPAN_ID)).isNull(); + } + + @Test + public void testWrappedTaskOnOtherThread() throws Exception { + ExecutorService executor = Executors.newSingleThreadExecutor(); + try { + Future inSpan; + try (Scope ignored = span(PARENT_SPAN_ID).makeCurrent()) { + inSpan = executor.submit(Context.current().wrap(() -> MDC.get(MDC_SPAN_ID))); + } + assertThat(inSpan.get()).isEqualTo(PARENT_SPAN_ID); + // after the wrapped task, the worker thread must not keep the span IDs + assertThat(executor.submit(() -> MDC.get(MDC_SPAN_ID)).get()).isNull(); + } finally { + executor.shutdownNow(); + } + } + + @Test + public void testCreateActivatedSpanPutsIdsInMdc() { + MutableConfigurationSource conf = new InMemoryConfigurationForTesting(); + conf.setBoolean("ozone.tracing.enabled", true); + TracingUtil.initTracing("mdc", conf.getObject(TracingConfig.class)); + try (TracingUtil.TraceCloseable ignored = TracingUtil.createActivatedSpan("mdc-span")) { + SpanContext ctx = Span.current().getSpanContext(); + assertThat(MDC.get(MDC_TRACE_ID)).isEqualTo(ctx.getTraceId()); + assertThat(MDC.get(MDC_SPAN_ID)).isEqualTo(ctx.getSpanId()); + } + assertThat(MDC.get(MDC_TRACE_ID)).isNull(); + } +} diff --git a/hadoop-ozone/dist/src/shell/conf/log4j.properties b/hadoop-ozone/dist/src/shell/conf/log4j.properties index 5ed63ffd1ee5..c42b9954b99b 100644 --- a/hadoop-ozone/dist/src/shell/conf/log4j.properties +++ b/hadoop-ozone/dist/src/shell/conf/log4j.properties @@ -64,7 +64,7 @@ log4j.appender.RFA.rollingPolicy.maxIndex=10 log4j.appender.RFA.layout=org.apache.log4j.PatternLayout # Pattern format: Date LogLevel LoggerName LogMessage -log4j.appender.RFA.layout.ConversionPattern=%d{ISO8601} [%t] %p %c: %m%n +log4j.appender.RFA.layout.ConversionPattern=%d{ISO8601} [%t] %p [trace=%X{trace_id} span=%X{span_id}] %c: %m%n # # Async Rolling File Appender @@ -73,7 +73,7 @@ log4j.appender.ASYNCRFA=org.apache.hadoop.ozone.utils.AsyncRollingFileAppender log4j.appender.ASYNCRFA.fileName=${hadoop.log.dir}/${hadoop.log.file} log4j.appender.ASYNCRFA.maxFileSize=${hadoop.log.maxfilesize} log4j.appender.ASYNCRFA.maxBackupIndex=${hadoop.log.maxbackupindex} -log4j.appender.ASYNCRFA.conversionPattern=%d{ISO8601} [%t] %p %c: %m%n +log4j.appender.ASYNCRFA.conversionPattern=%d{ISO8601} [%t] %p [trace=%X{trace_id} span=%X{span_id}] %c: %m%n # @@ -89,7 +89,7 @@ log4j.appender.DRFA.DatePattern=.yyyy-MM-dd log4j.appender.DRFA.layout=org.apache.log4j.PatternLayout # Pattern format: Date LogLevel LoggerName LogMessage -log4j.appender.DRFA.layout.ConversionPattern=%d{ISO8601} [%t] %p %c: %m%n +log4j.appender.DRFA.layout.ConversionPattern=%d{ISO8601} [%t] %p [trace=%X{trace_id} span=%X{span_id}] %c: %m%n # Debugging Pattern format #log4j.appender.DRFA.layout.ConversionPattern=%d{ISO8601} [%t] %-5p %c{2} (%F:%M(%L)) - %m%n @@ -102,7 +102,7 @@ log4j.appender.DRFA.layout.ConversionPattern=%d{ISO8601} [%t] %p %c: %m%n log4j.appender.console=org.apache.log4j.ConsoleAppender log4j.appender.console.target=System.err log4j.appender.console.layout=org.apache.log4j.PatternLayout -log4j.appender.console.layout.ConversionPattern=%d{ISO8601} [%t] %p %c{2}: %m%n +log4j.appender.console.layout.ConversionPattern=%d{ISO8601} [%t] %p [trace=%X{trace_id} span=%X{span_id}] %c{2}: %m%n # # TaskLog Appender From 8318652bcbc6cd029e436474abbd603a116119ce Mon Sep 17 00:00:00 2001 From: Rajesh Date: Mon, 19 Oct 2026 11:45:08 +0530 Subject: [PATCH 02/10] Revert log4j pattern change; trace IDs remain available via %X{trace_id} and %X{span_id} --- hadoop-ozone/dist/src/shell/conf/log4j.properties | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hadoop-ozone/dist/src/shell/conf/log4j.properties b/hadoop-ozone/dist/src/shell/conf/log4j.properties index c42b9954b99b..5ed63ffd1ee5 100644 --- a/hadoop-ozone/dist/src/shell/conf/log4j.properties +++ b/hadoop-ozone/dist/src/shell/conf/log4j.properties @@ -64,7 +64,7 @@ log4j.appender.RFA.rollingPolicy.maxIndex=10 log4j.appender.RFA.layout=org.apache.log4j.PatternLayout # Pattern format: Date LogLevel LoggerName LogMessage -log4j.appender.RFA.layout.ConversionPattern=%d{ISO8601} [%t] %p [trace=%X{trace_id} span=%X{span_id}] %c: %m%n +log4j.appender.RFA.layout.ConversionPattern=%d{ISO8601} [%t] %p %c: %m%n # # Async Rolling File Appender @@ -73,7 +73,7 @@ log4j.appender.ASYNCRFA=org.apache.hadoop.ozone.utils.AsyncRollingFileAppender log4j.appender.ASYNCRFA.fileName=${hadoop.log.dir}/${hadoop.log.file} log4j.appender.ASYNCRFA.maxFileSize=${hadoop.log.maxfilesize} log4j.appender.ASYNCRFA.maxBackupIndex=${hadoop.log.maxbackupindex} -log4j.appender.ASYNCRFA.conversionPattern=%d{ISO8601} [%t] %p [trace=%X{trace_id} span=%X{span_id}] %c: %m%n +log4j.appender.ASYNCRFA.conversionPattern=%d{ISO8601} [%t] %p %c: %m%n # @@ -89,7 +89,7 @@ log4j.appender.DRFA.DatePattern=.yyyy-MM-dd log4j.appender.DRFA.layout=org.apache.log4j.PatternLayout # Pattern format: Date LogLevel LoggerName LogMessage -log4j.appender.DRFA.layout.ConversionPattern=%d{ISO8601} [%t] %p [trace=%X{trace_id} span=%X{span_id}] %c: %m%n +log4j.appender.DRFA.layout.ConversionPattern=%d{ISO8601} [%t] %p %c: %m%n # Debugging Pattern format #log4j.appender.DRFA.layout.ConversionPattern=%d{ISO8601} [%t] %-5p %c{2} (%F:%M(%L)) - %m%n @@ -102,7 +102,7 @@ log4j.appender.DRFA.layout.ConversionPattern=%d{ISO8601} [%t] %p [trace=%X{trace log4j.appender.console=org.apache.log4j.ConsoleAppender log4j.appender.console.target=System.err log4j.appender.console.layout=org.apache.log4j.PatternLayout -log4j.appender.console.layout.ConversionPattern=%d{ISO8601} [%t] %p [trace=%X{trace_id} span=%X{span_id}] %c{2}: %m%n +log4j.appender.console.layout.ConversionPattern=%d{ISO8601} [%t] %p %c{2}: %m%n # # TaskLog Appender From 84c8640bb9eb420e50661e3206f5d6e729358ef8 Mon Sep 17 00:00:00 2001 From: Rajesh Date: Mon, 19 Oct 2026 11:48:52 +0530 Subject: [PATCH 03/10] changing the test-cases as per the comments --- .../tracing/TestOzoneContextStorageProvider.java | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/tracing/TestOzoneContextStorageProvider.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/tracing/TestOzoneContextStorageProvider.java index f23137bb3839..4adbd2e8a3ab 100644 --- a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/tracing/TestOzoneContextStorageProvider.java +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/tracing/TestOzoneContextStorageProvider.java @@ -73,6 +73,7 @@ public void testMakeCurrentSetsAndRestoresMdc() { assertThat(MDC.get(MDC_SPAN_ID)).isNull(); } assertThat(MDC.get(MDC_TRACE_ID)).isEqualTo(TRACE_ID); + assertThat(MDC.get(MDC_SPAN_ID)).isEqualTo(PARENT_SPAN_ID); } assertThat(MDC.get(MDC_TRACE_ID)).isNull(); assertThat(MDC.get(MDC_SPAN_ID)).isNull(); @@ -93,16 +94,26 @@ public void testStaleMdcIsClearedAfterClose() { @Test public void testWrappedTaskOnOtherThread() throws Exception { ExecutorService executor = Executors.newSingleThreadExecutor(); + ExecutorService clearedExecutor = Executors.newSingleThreadExecutor(); try { + // a thread that explicitly cleared its MDC still gets the IDs from the next wrapped task + clearedExecutor.submit(MDC::clear).get(); Future inSpan; + Future inSpanCleared; try (Scope ignored = span(PARENT_SPAN_ID).makeCurrent()) { inSpan = executor.submit(Context.current().wrap(() -> MDC.get(MDC_SPAN_ID))); + inSpanCleared = clearedExecutor.submit(Context.current().wrap( + () -> new String[] {MDC.get(MDC_TRACE_ID), MDC.get(MDC_SPAN_ID)})); } assertThat(inSpan.get()).isEqualTo(PARENT_SPAN_ID); - // after the wrapped task, the worker thread must not keep the span IDs + assertThat(inSpanCleared.get()).containsExactly(TRACE_ID, PARENT_SPAN_ID); + // after the wrapped task, the worker threads must not keep the span IDs assertThat(executor.submit(() -> MDC.get(MDC_SPAN_ID)).get()).isNull(); + assertThat(clearedExecutor.submit(() -> MDC.get(MDC_TRACE_ID)).get()).isNull(); + assertThat(clearedExecutor.submit(() -> MDC.get(MDC_SPAN_ID)).get()).isNull(); } finally { executor.shutdownNow(); + clearedExecutor.shutdownNow(); } } From 8479ce8ff20eba9490f22cb15ae567e676c961fc Mon Sep 17 00:00:00 2001 From: Rajesh Date: Sun, 25 Oct 2026 11:14:12 +0530 Subject: [PATCH 04/10] Install MDC trace context only in Ozone services and CLIs, not in client applications Co-Authored-By: Claude Code --- .../tracing/OzoneContextStorageProvider.java | 87 ------------ .../hadoop/hdds/tracing/TracingUtil.java | 75 ++++++++++ ...entelemetry.context.ContextStorageProvider | 16 --- .../TestOzoneContextStorageProvider.java | 132 ------------------ .../hadoop/hdds/tracing/TestTracingUtil.java | 112 +++++++++++++++ .../conf/TracingReconfigurationCallback.java | 1 + .../org/apache/hadoop/ozone/s3/Gateway.java | 1 + 7 files changed, 189 insertions(+), 235 deletions(-) delete mode 100644 hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/tracing/OzoneContextStorageProvider.java delete mode 100644 hadoop-hdds/common/src/main/resources/META-INF/services/io.opentelemetry.context.ContextStorageProvider delete mode 100644 hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/tracing/TestOzoneContextStorageProvider.java diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/tracing/OzoneContextStorageProvider.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/tracing/OzoneContextStorageProvider.java deleted file mode 100644 index e87898cd8f25..000000000000 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/tracing/OzoneContextStorageProvider.java +++ /dev/null @@ -1,87 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You 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 - * - * http://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.apache.hadoop.hdds.tracing; - -import io.opentelemetry.api.trace.Span; -import io.opentelemetry.api.trace.SpanContext; -import io.opentelemetry.context.Context; -import io.opentelemetry.context.ContextStorage; -import io.opentelemetry.context.ContextStorageProvider; -import io.opentelemetry.context.Scope; -import org.slf4j.MDC; - -/** - * Registered through {@code META-INF/services} so OpenTelemetry uses {@link OzoneContextStorage}, - * which wraps the default thread-local storage. - */ -public final class OzoneContextStorageProvider implements ContextStorageProvider { - public static final String MDC_TRACE_ID = "trace_id"; - public static final String MDC_SPAN_ID = "span_id"; - - @Override - public ContextStorage get() { - return new OzoneContextStorage(ContextStorage.defaultStorage()); - } - - /** - * {@link ContextStorage} that keeps the logging MDC in sync with the current span. - * Every {@code makeCurrent()} / {@code Context.wrap(...)} goes through {@link #attach(Context)}, - * so trace and span IDs are available to log patterns without changes at the call sites. - * The MDC is always derived from the current context (never saved and restored), - * so values inherited by child threads through the MDC are overwritten on the next attach or close. - */ - static final class OzoneContextStorage implements ContextStorage { - private final ContextStorage delegate; - - OzoneContextStorage(ContextStorage delegate) { - this.delegate = delegate; - } - - @Override - public Scope attach(Context toAttach) { - Scope scope = delegate.attach(toAttach); - updateMdc(delegate.current()); - return () -> { - scope.close(); - updateMdc(delegate.current()); - }; - } - - @Override - public Context current() { - return delegate.current(); - } - - @Override - public Context root() { - return delegate.root(); - } - - private static void updateMdc(Context context) { - SpanContext spanContext = context == null ? SpanContext.getInvalid() - : Span.fromContext(context).getSpanContext(); - if (spanContext.isValid()) { - MDC.put(MDC_TRACE_ID, spanContext.getTraceId()); - MDC.put(MDC_SPAN_ID, spanContext.getSpanId()); - } else { - MDC.remove(MDC_TRACE_ID); - MDC.remove(MDC_SPAN_ID); - } - } - } -} diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/tracing/TracingUtil.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/tracing/TracingUtil.java index 810555ca3e45..b845c828996b 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/tracing/TracingUtil.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/tracing/TracingUtil.java @@ -22,11 +22,13 @@ import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.trace.Span; +import io.opentelemetry.api.trace.SpanContext; import io.opentelemetry.api.trace.SpanKind; import io.opentelemetry.api.trace.StatusCode; import io.opentelemetry.api.trace.Tracer; import io.opentelemetry.api.trace.propagation.W3CTraceContextPropagator; import io.opentelemetry.context.Context; +import io.opentelemetry.context.ContextStorage; import io.opentelemetry.context.Scope; import io.opentelemetry.context.propagation.ContextPropagators; import io.opentelemetry.context.propagation.TextMapGetter; @@ -48,6 +50,7 @@ import org.apache.ratis.util.function.CheckedSupplier; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.slf4j.MDC; /** * Utility class to collect all the tracing helper methods. @@ -62,11 +65,29 @@ public final class TracingUtil { private static Tracer tracer = OpenTelemetry.noop().getTracer("noop"); private static volatile SdkTracerProvider sdkTracerProvider; private static BatchSpanProcessor batchSpanProcessor; + private static boolean mdcContextStorageInstalled; public static final String GLOBAL_TRACER_NAME = "ozone"; private TracingUtil() { } + /** + * Adds the current trace and span IDs to the logging MDC. + * Only for Ozone services and CLIs, not the client library, so applications keep their own MDC. + * Must be called before OpenTelemetry {@link Context} is first used in the JVM. + */ + public static synchronized void installMdcContextStorage() { + if (mdcContextStorageInstalled) { + return; + } + mdcContextStorageInstalled = true; + ContextStorage.addWrapper(MdcContextStorage::new); + if (!(ContextStorage.get() instanceof MdcContextStorage)) { + LOG.warn("OpenTelemetry context is already initialized, {} and {} will not be added to the logging MDC", + MdcContextStorage.MDC_TRACE_ID, MdcContextStorage.MDC_SPAN_ID); + } + } + /** * Initialize the tracing with the given service name. */ @@ -128,6 +149,7 @@ public static synchronized void flushTracing() { /** * This function initializes tracing, runs the command in a span, and exports spans before returning for CLI spans. + * Only for Ozone CLIs: it also installs the MDC context storage, see {@link #installMdcContextStorage()}. */ public static R execute( String serviceName, @@ -143,6 +165,7 @@ public static R execute( SpanKind spanKind, ConfigurationSource conf, CheckedSupplier supplier) throws E { + installMdcContextStorage(); initTracing(serviceName, conf); try { return executeInNewSpan(spanName, spanKind, supplier); @@ -648,4 +671,56 @@ public static TraceCloseable createActivatedSpanFromW3cHttpHeaders( span.end(); }; } + + /** + * {@link ContextStorage} that keeps the logging MDC in sync with the current span. + * Installed through {@link #installMdcContextStorage()} by Ozone services and CLIs only, + * so applications using the Ozone client keep their own MDC and context storage. + * Every {@code makeCurrent()} / {@code Context.wrap(...)} goes through {@link #attach(Context)}, + * so trace and span IDs are available to log patterns without changes at the call sites. + * The MDC is always derived from the current context (never saved and restored), + * so values inherited by child threads through the MDC are overwritten on the next attach or close. + */ + static final class MdcContextStorage implements ContextStorage { + static final String MDC_TRACE_ID = "trace_id"; + static final String MDC_SPAN_ID = "span_id"; + + private final ContextStorage delegate; + + MdcContextStorage(ContextStorage delegate) { + this.delegate = delegate; + } + + @Override + public Scope attach(Context toAttach) { + Scope scope = delegate.attach(toAttach); + updateMdc(delegate.current()); + return () -> { + scope.close(); + updateMdc(delegate.current()); + }; + } + + @Override + public Context current() { + return delegate.current(); + } + + @Override + public Context root() { + return delegate.root(); + } + + private static void updateMdc(Context context) { + SpanContext spanContext = context == null ? SpanContext.getInvalid() + : Span.fromContext(context).getSpanContext(); + if (spanContext.isValid()) { + MDC.put(MDC_TRACE_ID, spanContext.getTraceId()); + MDC.put(MDC_SPAN_ID, spanContext.getSpanId()); + } else { + MDC.remove(MDC_TRACE_ID); + MDC.remove(MDC_SPAN_ID); + } + } + } } diff --git a/hadoop-hdds/common/src/main/resources/META-INF/services/io.opentelemetry.context.ContextStorageProvider b/hadoop-hdds/common/src/main/resources/META-INF/services/io.opentelemetry.context.ContextStorageProvider deleted file mode 100644 index 8da279befb60..000000000000 --- a/hadoop-hdds/common/src/main/resources/META-INF/services/io.opentelemetry.context.ContextStorageProvider +++ /dev/null @@ -1,16 +0,0 @@ -# Licensed to the Apache Software Foundation (ASF) under one or more -# contributor license agreements. See the NOTICE file distributed with -# this work for additional information regarding copyright ownership. -# The ASF licenses this file to You 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 -# -# http://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. - -org.apache.hadoop.hdds.tracing.OzoneContextStorageProvider diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/tracing/TestOzoneContextStorageProvider.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/tracing/TestOzoneContextStorageProvider.java deleted file mode 100644 index 4adbd2e8a3ab..000000000000 --- a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/tracing/TestOzoneContextStorageProvider.java +++ /dev/null @@ -1,132 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You 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 - * - * http://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.apache.hadoop.hdds.tracing; - -import static org.apache.hadoop.hdds.tracing.OzoneContextStorageProvider.MDC_SPAN_ID; -import static org.apache.hadoop.hdds.tracing.OzoneContextStorageProvider.MDC_TRACE_ID; -import static org.assertj.core.api.Assertions.assertThat; - -import io.opentelemetry.api.trace.Span; -import io.opentelemetry.api.trace.SpanContext; -import io.opentelemetry.api.trace.TraceFlags; -import io.opentelemetry.api.trace.TraceState; -import io.opentelemetry.context.Context; -import io.opentelemetry.context.ContextStorage; -import io.opentelemetry.context.Scope; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; -import java.util.concurrent.Future; -import org.apache.hadoop.hdds.conf.InMemoryConfigurationForTesting; -import org.apache.hadoop.hdds.conf.MutableConfigurationSource; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.Test; -import org.slf4j.MDC; - -/** - * Test for {@link OzoneContextStorageProvider}. - */ -public class TestOzoneContextStorageProvider { - private static final String TRACE_ID = "0af7651916cd43dd8448eb211c80319c"; - private static final String PARENT_SPAN_ID = "b7ad6b7169203331"; - private static final String CHILD_SPAN_ID = "00f067aa0ba902b7"; - - @AfterEach - public void clearMdc() { - MDC.clear(); - } - - private static Span span(String spanId) { - return Span.wrap(SpanContext.create(TRACE_ID, spanId, TraceFlags.getSampled(), TraceState.getDefault())); - } - - @Test - public void testProviderIsRegistered() { - assertThat(ContextStorage.get()).isInstanceOf(OzoneContextStorageProvider.OzoneContextStorage.class); - } - - @Test - public void testMakeCurrentSetsAndRestoresMdc() { - try (Scope ignored = span(PARENT_SPAN_ID).makeCurrent()) { - assertThat(MDC.get(MDC_TRACE_ID)).isEqualTo(TRACE_ID); - assertThat(MDC.get(MDC_SPAN_ID)).isEqualTo(PARENT_SPAN_ID); - try (Scope ignoredInner = span(CHILD_SPAN_ID).makeCurrent()) { - assertThat(MDC.get(MDC_SPAN_ID)).isEqualTo(CHILD_SPAN_ID); - } - assertThat(MDC.get(MDC_SPAN_ID)).isEqualTo(PARENT_SPAN_ID); - try (Scope ignoredInner = Span.getInvalid().makeCurrent()) { - assertThat(MDC.get(MDC_TRACE_ID)).isNull(); - assertThat(MDC.get(MDC_SPAN_ID)).isNull(); - } - assertThat(MDC.get(MDC_TRACE_ID)).isEqualTo(TRACE_ID); - assertThat(MDC.get(MDC_SPAN_ID)).isEqualTo(PARENT_SPAN_ID); - } - assertThat(MDC.get(MDC_TRACE_ID)).isNull(); - assertThat(MDC.get(MDC_SPAN_ID)).isNull(); - } - - @Test - public void testStaleMdcIsClearedAfterClose() { - // e.g. values inherited from the parent thread through log4j's InheritableThreadLocal MDC - MDC.put(MDC_TRACE_ID, "stale-trace"); - MDC.put(MDC_SPAN_ID, "stale-span"); - try (Scope ignored = span(PARENT_SPAN_ID).makeCurrent()) { - assertThat(MDC.get(MDC_SPAN_ID)).isEqualTo(PARENT_SPAN_ID); - } - assertThat(MDC.get(MDC_TRACE_ID)).isNull(); - assertThat(MDC.get(MDC_SPAN_ID)).isNull(); - } - - @Test - public void testWrappedTaskOnOtherThread() throws Exception { - ExecutorService executor = Executors.newSingleThreadExecutor(); - ExecutorService clearedExecutor = Executors.newSingleThreadExecutor(); - try { - // a thread that explicitly cleared its MDC still gets the IDs from the next wrapped task - clearedExecutor.submit(MDC::clear).get(); - Future inSpan; - Future inSpanCleared; - try (Scope ignored = span(PARENT_SPAN_ID).makeCurrent()) { - inSpan = executor.submit(Context.current().wrap(() -> MDC.get(MDC_SPAN_ID))); - inSpanCleared = clearedExecutor.submit(Context.current().wrap( - () -> new String[] {MDC.get(MDC_TRACE_ID), MDC.get(MDC_SPAN_ID)})); - } - assertThat(inSpan.get()).isEqualTo(PARENT_SPAN_ID); - assertThat(inSpanCleared.get()).containsExactly(TRACE_ID, PARENT_SPAN_ID); - // after the wrapped task, the worker threads must not keep the span IDs - assertThat(executor.submit(() -> MDC.get(MDC_SPAN_ID)).get()).isNull(); - assertThat(clearedExecutor.submit(() -> MDC.get(MDC_TRACE_ID)).get()).isNull(); - assertThat(clearedExecutor.submit(() -> MDC.get(MDC_SPAN_ID)).get()).isNull(); - } finally { - executor.shutdownNow(); - clearedExecutor.shutdownNow(); - } - } - - @Test - public void testCreateActivatedSpanPutsIdsInMdc() { - MutableConfigurationSource conf = new InMemoryConfigurationForTesting(); - conf.setBoolean("ozone.tracing.enabled", true); - TracingUtil.initTracing("mdc", conf.getObject(TracingConfig.class)); - try (TracingUtil.TraceCloseable ignored = TracingUtil.createActivatedSpan("mdc-span")) { - SpanContext ctx = Span.current().getSpanContext(); - assertThat(MDC.get(MDC_TRACE_ID)).isEqualTo(ctx.getTraceId()); - assertThat(MDC.get(MDC_SPAN_ID)).isEqualTo(ctx.getSpanId()); - } - assertThat(MDC.get(MDC_TRACE_ID)).isNull(); - } -} diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/tracing/TestTracingUtil.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/tracing/TestTracingUtil.java index 49bb7a451783..ba72f16d3e29 100644 --- a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/tracing/TestTracingUtil.java +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/tracing/TestTracingUtil.java @@ -17,12 +17,16 @@ package org.apache.hadoop.hdds.tracing; +import static org.apache.hadoop.hdds.tracing.TracingUtil.MdcContextStorage.MDC_SPAN_ID; +import static org.apache.hadoop.hdds.tracing.TracingUtil.MdcContextStorage.MDC_TRACE_ID; import static org.apache.hadoop.hdds.tracing.TracingUtil.createProxy; import static org.apache.hadoop.hdds.tracing.TracingUtil.exportCurrentSpan; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -30,19 +34,47 @@ import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.SpanContext; import io.opentelemetry.api.trace.SpanKind; +import io.opentelemetry.api.trace.TraceFlags; +import io.opentelemetry.api.trace.TraceState; +import io.opentelemetry.context.Context; +import io.opentelemetry.context.ContextStorage; import io.opentelemetry.context.Scope; import io.opentelemetry.sdk.trace.ReadableSpan; import java.io.IOException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; import org.apache.hadoop.hdds.conf.InMemoryConfigurationForTesting; import org.apache.hadoop.hdds.conf.MutableConfigurationSource; import org.apache.hadoop.hdds.tracing.TestTraceAllMethod.Service; import org.apache.hadoop.hdds.tracing.TestTraceAllMethod.ServiceImpl; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; +import org.slf4j.MDC; /** * Test for {@link TracingUtil}. + * OpenTelemetry fixes the context storage once per JVM and this class never installs + * {@link TracingUtil.MdcContextStorage} before using {@link Context}, so the global storage here is + * the "not installed" case (as in client applications). The MDC behaviour is tested on a local instance. */ public class TestTracingUtil { + private static final String TRACE_ID = "0af7651916cd43dd8448eb211c80319c"; + private static final String PARENT_SPAN_ID = "b7ad6b7169203331"; + private static final String CHILD_SPAN_ID = "00f067aa0ba902b7"; + + @AfterEach + public void clearMdc() { + MDC.clear(); + } + + private static Context spanContext(String spanId) { + return Context.root().with(Span.wrap( + SpanContext.create(TRACE_ID, spanId, TraceFlags.getSampled(), TraceState.getDefault()))); + } + + private static TracingUtil.MdcContextStorage mdcContextStorage() { + return new TracingUtil.MdcContextStorage(ContextStorage.defaultStorage()); + } private static MutableConfigurationSource tracingEnabled() { MutableConfigurationSource config = new InMemoryConfigurationForTesting(); @@ -227,4 +259,84 @@ public void testImportAndCreateSpanUsesExplicitKind() { server.end(); } } + + @Test + public void testMdcNotChangedWithoutInstall() { + assertFalse(ContextStorage.get() instanceof TracingUtil.MdcContextStorage); + TracingUtil.initTracing("mdc-not-installed", tracingEnabled().getObject(TracingConfig.class)); + MDC.put(MDC_TRACE_ID, "app-trace"); + try (TracingUtil.TraceCloseable ignored = TracingUtil.createActivatedSpan("span")) { + assertTrue(Span.current().getSpanContext().isValid()); + assertEquals("app-trace", MDC.get(MDC_TRACE_ID)); + assertNull(MDC.get(MDC_SPAN_ID)); + } + try (Scope ignored = Span.getInvalid().makeCurrent()) { + assertEquals("app-trace", MDC.get(MDC_TRACE_ID)); + } + assertEquals("app-trace", MDC.get(MDC_TRACE_ID)); + } + + @Test + public void testInstallAfterContextInitializedHasNoEffect() { + ContextStorage storage = ContextStorage.get(); + Context.current(); + TracingUtil.installMdcContextStorage(); + assertSame(storage, ContextStorage.get()); + try (Scope ignored = spanContext(PARENT_SPAN_ID).makeCurrent()) { + assertNull(MDC.get(MDC_TRACE_ID)); + } + } + + @Test + public void testMdcContextStorageSetsAndRestoresMdc() { + TracingUtil.MdcContextStorage storage = mdcContextStorage(); + try (Scope ignored = storage.attach(spanContext(PARENT_SPAN_ID))) { + assertEquals(TRACE_ID, MDC.get(MDC_TRACE_ID)); + assertEquals(PARENT_SPAN_ID, MDC.get(MDC_SPAN_ID)); + try (Scope ignoredInner = storage.attach(spanContext(CHILD_SPAN_ID))) { + assertEquals(CHILD_SPAN_ID, MDC.get(MDC_SPAN_ID)); + } + assertEquals(PARENT_SPAN_ID, MDC.get(MDC_SPAN_ID)); + try (Scope ignoredInner = storage.attach(Context.root().with(Span.getInvalid()))) { + assertNull(MDC.get(MDC_TRACE_ID)); + assertNull(MDC.get(MDC_SPAN_ID)); + } + assertEquals(TRACE_ID, MDC.get(MDC_TRACE_ID)); + assertEquals(PARENT_SPAN_ID, MDC.get(MDC_SPAN_ID)); + } + assertNull(MDC.get(MDC_TRACE_ID)); + assertNull(MDC.get(MDC_SPAN_ID)); + } + + @Test + public void testMdcContextStorageClearsStaleMdc() { + // e.g. values inherited from the parent thread through log4j's InheritableThreadLocal MDC + MDC.put(MDC_TRACE_ID, "stale-trace"); + MDC.put(MDC_SPAN_ID, "stale-span"); + try (Scope ignored = mdcContextStorage().attach(spanContext(PARENT_SPAN_ID))) { + assertEquals(PARENT_SPAN_ID, MDC.get(MDC_SPAN_ID)); + } + assertNull(MDC.get(MDC_TRACE_ID)); + assertNull(MDC.get(MDC_SPAN_ID)); + } + + @Test + public void testMdcContextStorageOnOtherThread() throws Exception { + TracingUtil.MdcContextStorage storage = mdcContextStorage(); + ExecutorService executor = Executors.newSingleThreadExecutor(); + try { + String[] inSpan = executor.submit(() -> { + try (Scope ignored = storage.attach(spanContext(PARENT_SPAN_ID))) { + return new String[] {MDC.get(MDC_TRACE_ID), MDC.get(MDC_SPAN_ID)}; + } + }).get(); + assertEquals(TRACE_ID, inSpan[0]); + assertEquals(PARENT_SPAN_ID, inSpan[1]); + // after the task, the worker thread must not keep the span IDs + assertNull(executor.submit(() -> MDC.get(MDC_TRACE_ID)).get()); + assertNull(executor.submit(() -> MDC.get(MDC_SPAN_ID)).get()); + } finally { + executor.shutdownNow(); + } + } } diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/conf/TracingReconfigurationCallback.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/conf/TracingReconfigurationCallback.java index 4255371bfb66..dbdc90954804 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/conf/TracingReconfigurationCallback.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/conf/TracingReconfigurationCallback.java @@ -45,6 +45,7 @@ private TracingReconfigurationCallback(String serviceName, TracingConfig tracing */ public static TracingReconfigurationCallback init( String serviceName, TracingConfig tracingConfig) { + TracingUtil.installMdcContextStorage(); TracingUtil.initTracing(serviceName, tracingConfig); return new TracingReconfigurationCallback(serviceName, tracingConfig); } diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/Gateway.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/Gateway.java index 0b738d01eb87..6b8ef8f22afc 100644 --- a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/Gateway.java +++ b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/Gateway.java @@ -88,6 +88,7 @@ static void redirectJulToSlf4j() { public Void call() throws Exception { OzoneConfiguration ozoneConfiguration = getOzoneConf(); OzoneConfigurationHolder.setConfiguration(ozoneConfiguration); + TracingUtil.installMdcContextStorage(); TracingUtil.initTracing("S3gateway", OzoneConfigurationHolder.configuration()); UserGroupInformation.setConfiguration(OzoneConfigurationHolder.configuration()); loginS3GUser(OzoneConfigurationHolder.configuration()); From 7192157df3beec8bf3182d78f3777c39965f4794 Mon Sep 17 00:00:00 2001 From: Rajesh Date: Mon, 26 Oct 2026 13:52:16 +0530 Subject: [PATCH 05/10] Add tests for MDC context storage installed through TracingUtil Co-Authored-By: Claude --- .../tracing/TestMdcContextStorageInstall.java | 153 ++++++++++++++++++ .../hadoop/hdds/tracing/TestTracingUtil.java | 5 - 2 files changed, 153 insertions(+), 5 deletions(-) create mode 100644 hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/tracing/TestMdcContextStorageInstall.java diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/tracing/TestMdcContextStorageInstall.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/tracing/TestMdcContextStorageInstall.java new file mode 100644 index 000000000000..64440226768c --- /dev/null +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/tracing/TestMdcContextStorageInstall.java @@ -0,0 +1,153 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 + * + * http://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.apache.hadoop.hdds.tracing; + +import static org.apache.hadoop.hdds.tracing.TracingUtil.MdcContextStorage.MDC_SPAN_ID; +import static org.apache.hadoop.hdds.tracing.TracingUtil.MdcContextStorage.MDC_TRACE_ID; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import io.opentelemetry.api.GlobalOpenTelemetry; +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.api.trace.SpanContext; +import io.opentelemetry.context.Context; +import io.opentelemetry.context.ContextStorage; +import io.opentelemetry.context.Scope; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import org.apache.hadoop.hdds.conf.InMemoryConfigurationForTesting; +import org.apache.hadoop.hdds.conf.MutableConfigurationSource; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.slf4j.MDC; + +/** + * Tests {@link TracingUtil#installMdcContextStorage()}. + * When running together with other test classes, use fork mode per class. + */ +public class TestMdcContextStorageInstall { + + @BeforeAll + public static void install() { + TracingUtil.installMdcContextStorage(); + assertInstanceOf(TracingUtil.MdcContextStorage.class, ContextStorage.get(), "use fork mode per class"); + } + + @BeforeEach + public void setUp() { + TracingUtil.shutdownTracing(); + GlobalOpenTelemetry.resetForTest(); + MDC.clear(); + } + + @AfterEach + public void tearDown() { + TracingUtil.shutdownTracing(); + GlobalOpenTelemetry.resetForTest(); + MDC.clear(); + } + + private static void initTracing(boolean enabled) { + MutableConfigurationSource conf = new InMemoryConfigurationForTesting(); + conf.setBoolean("ozone.tracing.enabled", enabled); + TracingUtil.initTracing("mdc-install", conf); + } + + private static void assertMdcMatchesCurrentSpan() { + SpanContext current = Span.current().getSpanContext(); + assertTrue(current.isValid()); + assertEquals(current.getTraceId(), MDC.get(MDC_TRACE_ID)); + assertEquals(current.getSpanId(), MDC.get(MDC_SPAN_ID)); + } + + private static void assertMdcEmpty() { + assertNull(MDC.get(MDC_TRACE_ID)); + assertNull(MDC.get(MDC_SPAN_ID)); + } + + @Test + public void testActivatedSpanSetsMdc() { + initTracing(true); + try (TracingUtil.TraceCloseable ignored = TracingUtil.createActivatedSpan("outer")) { + assertMdcMatchesCurrentSpan(); + String traceId = MDC.get(MDC_TRACE_ID); + String outerSpanId = MDC.get(MDC_SPAN_ID); + try (TracingUtil.TraceCloseable ignoredInner = TracingUtil.createActivatedSpan("inner")) { + assertMdcMatchesCurrentSpan(); + assertEquals(traceId, MDC.get(MDC_TRACE_ID)); + assertNotEquals(outerSpanId, MDC.get(MDC_SPAN_ID)); + } + assertEquals(traceId, MDC.get(MDC_TRACE_ID)); + assertEquals(outerSpanId, MDC.get(MDC_SPAN_ID)); + } + assertMdcEmpty(); + } + + @Test + public void testImportedParentSpanSetsMdc() { + initTracing(true); + String carrier; + String parentTraceId; + try (TracingUtil.TraceCloseable ignored = TracingUtil.createActivatedSpan("client")) { + carrier = TracingUtil.exportCurrentSpan(); + parentTraceId = Span.current().getSpanContext().getTraceId(); + } + assertMdcEmpty(); + + Span server = TracingUtil.importAndCreateSpan("server", carrier); + try (Scope ignored = server.makeCurrent()) { + assertMdcMatchesCurrentSpan(); + assertEquals(parentTraceId, MDC.get(MDC_TRACE_ID)); + } finally { + server.end(); + } + assertMdcEmpty(); + } + + @Test + public void testContextWrapPropagatesMdcToOtherThread() throws Exception { + initTracing(true); + ExecutorService executor = Executors.newSingleThreadExecutor(); + try { + try (TracingUtil.TraceCloseable ignored = TracingUtil.createActivatedSpan("span")) { + String[] inTask = executor.submit(Context.current().wrap( + () -> new String[] {MDC.get(MDC_TRACE_ID), MDC.get(MDC_SPAN_ID)})).get(); + assertEquals(MDC.get(MDC_TRACE_ID), inTask[0]); + assertEquals(MDC.get(MDC_SPAN_ID), inTask[1]); + } + assertNull(executor.submit(() -> MDC.get(MDC_TRACE_ID)).get()); + assertNull(executor.submit(() -> MDC.get(MDC_SPAN_ID)).get()); + } finally { + executor.shutdownNow(); + } + } + + @Test + public void testTracingDisabledLeavesMdcEmpty() { + initTracing(false); + try (TracingUtil.TraceCloseable ignored = TracingUtil.createActivatedSpan("span")) { + assertMdcEmpty(); + } + assertMdcEmpty(); + } +} diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/tracing/TestTracingUtil.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/tracing/TestTracingUtil.java index ba72f16d3e29..5b3e805700a0 100644 --- a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/tracing/TestTracingUtil.java +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/tracing/TestTracingUtil.java @@ -53,9 +53,6 @@ /** * Test for {@link TracingUtil}. - * OpenTelemetry fixes the context storage once per JVM and this class never installs - * {@link TracingUtil.MdcContextStorage} before using {@link Context}, so the global storage here is - * the "not installed" case (as in client applications). The MDC behaviour is tested on a local instance. */ public class TestTracingUtil { private static final String TRACE_ID = "0af7651916cd43dd8448eb211c80319c"; @@ -310,7 +307,6 @@ public void testMdcContextStorageSetsAndRestoresMdc() { @Test public void testMdcContextStorageClearsStaleMdc() { - // e.g. values inherited from the parent thread through log4j's InheritableThreadLocal MDC MDC.put(MDC_TRACE_ID, "stale-trace"); MDC.put(MDC_SPAN_ID, "stale-span"); try (Scope ignored = mdcContextStorage().attach(spanContext(PARENT_SPAN_ID))) { @@ -332,7 +328,6 @@ public void testMdcContextStorageOnOtherThread() throws Exception { }).get(); assertEquals(TRACE_ID, inSpan[0]); assertEquals(PARENT_SPAN_ID, inSpan[1]); - // after the task, the worker thread must not keep the span IDs assertNull(executor.submit(() -> MDC.get(MDC_TRACE_ID)).get()); assertNull(executor.submit(() -> MDC.get(MDC_SPAN_ID)).get()); } finally { From 23140e57bba605fe2674e7e2aa3e73afb74663a9 Mon Sep 17 00:00:00 2001 From: Rajesh Date: Mon, 26 Oct 2026 14:26:58 +0530 Subject: [PATCH 06/10] Install MDC context storage before OM and SCM init and bootstrap Co-Authored-By: Claude --- .../hadoop/hdds/scm/server/StorageContainerManagerStarter.java | 2 ++ .../java/org/apache/hadoop/ozone/om/OzoneManagerStarter.java | 2 ++ 2 files changed, 4 insertions(+) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManagerStarter.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManagerStarter.java index 737c7a024c0e..f4138756a98d 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManagerStarter.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManagerStarter.java @@ -24,6 +24,7 @@ import org.apache.hadoop.hdds.cli.GenericCli; import org.apache.hadoop.hdds.cli.HddsVersionProvider; import org.apache.hadoop.hdds.conf.OzoneConfiguration; +import org.apache.hadoop.hdds.tracing.TracingUtil; import org.apache.hadoop.hdds.utils.HddsServerUtil; import org.apache.hadoop.hdds.utils.HddsVersionInfo; import org.apache.hadoop.ozone.common.StorageInfo; @@ -145,6 +146,7 @@ private void startScm() throws Exception { * is set and print the startup banner message. */ private void commonInit() { + TracingUtil.installMdcContextStorage(); conf = getOzoneConf(); String[] originalArgs = getCmd().getParseResult().originalArgs() diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManagerStarter.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManagerStarter.java index f24c584c9af6..85420cfe9314 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManagerStarter.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManagerStarter.java @@ -24,6 +24,7 @@ import org.apache.hadoop.hdds.cli.GenericCli; import org.apache.hadoop.hdds.cli.HddsVersionProvider; import org.apache.hadoop.hdds.conf.OzoneConfiguration; +import org.apache.hadoop.hdds.tracing.TracingUtil; import org.apache.hadoop.hdds.utils.HddsServerUtil; import org.apache.hadoop.ozone.util.OzoneNetUtils; import org.apache.hadoop.ozone.util.OzoneVersionInfo; @@ -168,6 +169,7 @@ public void bootstrapOM(@CommandLine.Option(names = {"--force"}, * is set and print the startup banner message. */ private void commonInit() { + TracingUtil.installMdcContextStorage(); conf = getOzoneConf(); String[] originalArgs = getCmd().getParseResult().originalArgs() From 591553eac040d0677323fa07529a676b9bdf68bb Mon Sep 17 00:00:00 2001 From: Rajesh Date: Mon, 26 Oct 2026 14:45:31 +0530 Subject: [PATCH 07/10] Move MDC context storage install from TracingReconfigurationCallback to the datanode entry point Co-Authored-By: Claude --- .../main/java/org/apache/hadoop/ozone/HddsDatanodeService.java | 2 ++ .../apache/hadoop/hdds/conf/TracingReconfigurationCallback.java | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/HddsDatanodeService.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/HddsDatanodeService.java index 493c341d4028..47840ab85bf3 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/HddsDatanodeService.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/HddsDatanodeService.java @@ -78,6 +78,7 @@ import org.apache.hadoop.hdds.server.http.HttpConfig; import org.apache.hadoop.hdds.server.http.RatisDropwizardExports; import org.apache.hadoop.hdds.tracing.TracingConfig; +import org.apache.hadoop.hdds.tracing.TracingUtil; import org.apache.hadoop.hdds.utils.HddsServerUtil; import org.apache.hadoop.hdds.utils.HddsVersionInfo; import org.apache.hadoop.hdds.utils.IOUtils; @@ -189,6 +190,7 @@ public static void main(String[] args) { @Override public Void call() throws Exception { + TracingUtil.installMdcContextStorage(); OzoneConfiguration configuration = getOzoneConf(); if (printBanner) { HddsServerUtil.startupShutdownMessage(HddsVersionInfo.HDDS_VERSION_INFO, diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/conf/TracingReconfigurationCallback.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/conf/TracingReconfigurationCallback.java index dbdc90954804..4255371bfb66 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/conf/TracingReconfigurationCallback.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/conf/TracingReconfigurationCallback.java @@ -45,7 +45,6 @@ private TracingReconfigurationCallback(String serviceName, TracingConfig tracing */ public static TracingReconfigurationCallback init( String serviceName, TracingConfig tracingConfig) { - TracingUtil.installMdcContextStorage(); TracingUtil.initTracing(serviceName, tracingConfig); return new TracingReconfigurationCallback(serviceName, tracingConfig); } From ddb442fdd4394593c47d3c3f299b522095b36935 Mon Sep 17 00:00:00 2001 From: Rajesh Date: Wed, 28 Oct 2026 16:21:26 +0530 Subject: [PATCH 08/10] Avoid OTLP export retries in TestMdcContextStorageInstall when no collector is running --- .../hadoop/hdds/tracing/TestMdcContextStorageInstall.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/tracing/TestMdcContextStorageInstall.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/tracing/TestMdcContextStorageInstall.java index 64440226768c..073704b4d7b6 100644 --- a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/tracing/TestMdcContextStorageInstall.java +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/tracing/TestMdcContextStorageInstall.java @@ -31,6 +31,8 @@ import io.opentelemetry.context.Context; import io.opentelemetry.context.ContextStorage; import io.opentelemetry.context.Scope; +import io.opentelemetry.sdk.OpenTelemetrySdk; +import io.opentelemetry.sdk.trace.SdkTracerProvider; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import org.apache.hadoop.hdds.conf.InMemoryConfigurationForTesting; @@ -68,6 +70,9 @@ public void tearDown() { } private static void initTracing(boolean enabled) { + SdkTracerProvider provider = SdkTracerProvider.builder().build(); + OpenTelemetrySdk sdk = OpenTelemetrySdk.builder().setTracerProvider(provider).build(); + GlobalOpenTelemetry.set(sdk); MutableConfigurationSource conf = new InMemoryConfigurationForTesting(); conf.setBoolean("ozone.tracing.enabled", enabled); TracingUtil.initTracing("mdc-install", conf); From d075f11dd6fc3a988955df0839ded803167c98e6 Mon Sep 17 00:00:00 2001 From: Rajesh Date: Thu, 29 Oct 2026 11:51:38 +0530 Subject: [PATCH 09/10] fix for the test failure --- .../hadoop/hdds/tracing/TestMdcContextStorageInstall.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/tracing/TestMdcContextStorageInstall.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/tracing/TestMdcContextStorageInstall.java index 073704b4d7b6..8f3817d11f0a 100644 --- a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/tracing/TestMdcContextStorageInstall.java +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/tracing/TestMdcContextStorageInstall.java @@ -35,6 +35,8 @@ import io.opentelemetry.sdk.trace.SdkTracerProvider; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import io.opentelemetry.sdk.trace.export.SimpleSpanProcessor; +import io.opentelemetry.sdk.trace.export.SpanExporter; import org.apache.hadoop.hdds.conf.InMemoryConfigurationForTesting; import org.apache.hadoop.hdds.conf.MutableConfigurationSource; import org.junit.jupiter.api.AfterEach; @@ -70,7 +72,8 @@ public void tearDown() { } private static void initTracing(boolean enabled) { - SdkTracerProvider provider = SdkTracerProvider.builder().build(); + SdkTracerProvider provider = SdkTracerProvider.builder() + .addSpanProcessor(SimpleSpanProcessor.create(SpanExporter.noop())).build(); OpenTelemetrySdk sdk = OpenTelemetrySdk.builder().setTracerProvider(provider).build(); GlobalOpenTelemetry.set(sdk); MutableConfigurationSource conf = new InMemoryConfigurationForTesting(); @@ -136,7 +139,7 @@ public void testContextWrapPropagatesMdcToOtherThread() throws Exception { try { try (TracingUtil.TraceCloseable ignored = TracingUtil.createActivatedSpan("span")) { String[] inTask = executor.submit(Context.current().wrap( - () -> new String[] {MDC.get(MDC_TRACE_ID), MDC.get(MDC_SPAN_ID)})).get(); + () -> new String[]{MDC.get(MDC_TRACE_ID), MDC.get(MDC_SPAN_ID)})).get(); assertEquals(MDC.get(MDC_TRACE_ID), inTask[0]); assertEquals(MDC.get(MDC_SPAN_ID), inTask[1]); } From 6e58d54ff450b14083fa14edae62f35b49a19b3d Mon Sep 17 00:00:00 2001 From: Rajesh Date: Thu, 29 Oct 2026 12:48:31 +0530 Subject: [PATCH 10/10] checkstyle fix --- .../hadoop/hdds/tracing/TestMdcContextStorageInstall.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/tracing/TestMdcContextStorageInstall.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/tracing/TestMdcContextStorageInstall.java index 8f3817d11f0a..3dace96c5387 100644 --- a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/tracing/TestMdcContextStorageInstall.java +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/tracing/TestMdcContextStorageInstall.java @@ -33,10 +33,10 @@ import io.opentelemetry.context.Scope; import io.opentelemetry.sdk.OpenTelemetrySdk; import io.opentelemetry.sdk.trace.SdkTracerProvider; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; import io.opentelemetry.sdk.trace.export.SimpleSpanProcessor; import io.opentelemetry.sdk.trace.export.SpanExporter; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; import org.apache.hadoop.hdds.conf.InMemoryConfigurationForTesting; import org.apache.hadoop.hdds.conf.MutableConfigurationSource; import org.junit.jupiter.api.AfterEach;