Fixes not setting SpanActions when scope were set in OASTLA

fixes gh-356
micrometer-metrics · Sep 13, 2023 · 89b4e94 · 89b4e94
1 parent 9bef9ce
commit 89b4e94
Show file tree

Hide file tree

Showing 5 changed files with 41 additions and 21 deletions.
diff --git a/micrometer-tracing-bridges/micrometer-tracing-bridge-brave/gradle.lockfile b/micrometer-tracing-bridges/micrometer-tracing-bridge-brave/gradle.lockfile
diff --git a/...r-tracing-bridge-brave/src/test/java/io/micrometer/tracing/brave/bridge/BaggageTests.java b/...r-tracing-bridge-brave/src/test/java/io/micrometer/tracing/brave/bridge/BaggageTests.java
@@ -18,8 +18,12 @@
 import java.time.Duration;
 import java.util.HashMap;
 import java.util.Map;
-import java.util.concurrent.*;
-import java.util.concurrent.atomic.AtomicReference;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
 
 import brave.Tracing;
 import brave.baggage.BaggageField;
@@ -41,6 +45,7 @@
 import io.micrometer.tracing.Tracer;
 import io.micrometer.tracing.contextpropagation.ObservationAwareSpanThreadLocalAccessor;
 import org.assertj.core.api.Assertions;
+import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.Test;
 import reactor.core.observability.micrometer.Micrometer;
@@ -228,4 +233,9 @@ void baggageWithContextPropagationWithLegacyApi() {
         }
     }
 
+    @AfterAll
+    static void clear() {
+        ContextRegistry.getInstance().removeThreadLocalAccessor(ObservationAwareSpanThreadLocalAccessor.KEY);
+    }
+
 }
diff --git a/micrometer-tracing-bridges/micrometer-tracing-bridge-otel/gradle.lockfile b/micrometer-tracing-bridges/micrometer-tracing-bridge-otel/gradle.lockfile
diff --git a/...ter-tracing-bridge-otel/src/test/java/io/micrometer/tracing/otel/bridge/BaggageTests.java b/...ter-tracing-bridge-otel/src/test/java/io/micrometer/tracing/otel/bridge/BaggageTests.java
@@ -15,6 +15,17 @@
  */
 package io.micrometer.tracing.otel.bridge;
 
+import java.time.Duration;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+
 import io.micrometer.common.util.internal.logging.InternalLogger;
 import io.micrometer.common.util.internal.logging.InternalLoggerFactory;
 import io.micrometer.context.ContextRegistry;
@@ -33,19 +44,14 @@
 import io.opentelemetry.sdk.OpenTelemetrySdk;
 import io.opentelemetry.sdk.trace.SdkTracerProvider;
 import org.assertj.core.api.Assertions;
+import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.Test;
 import reactor.core.observability.micrometer.Micrometer;
 import reactor.core.publisher.Hooks;
 import reactor.core.publisher.Mono;
 import reactor.core.scheduler.Scheduler;
 import reactor.core.scheduler.Schedulers;
 
-import java.time.Duration;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.Map;
-import java.util.concurrent.*;
-
 import static org.assertj.core.api.BDDAssertions.then;
 
 class BaggageTests {
@@ -200,4 +206,8 @@ void baggageWithContextPropagationWithLegacyApi() {
         }
     }
 
+    @AfterAll
+    static void clear() {
+        ContextRegistry.getInstance().removeThreadLocalAccessor(ObservationAwareSpanThreadLocalAccessor.KEY);
+    }
 }
diff --git a/...ava/io/micrometer/tracing/contextpropagation/ObservationAwareSpanThreadLocalAccessor.java b/...ava/io/micrometer/tracing/contextpropagation/ObservationAwareSpanThreadLocalAccessor.java
@@ -15,6 +15,12 @@
  */
 package io.micrometer.tracing.contextpropagation;
 
+import java.io.Closeable;
+import java.io.IOException;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
+
 import io.micrometer.common.util.internal.logging.InternalLogger;
 import io.micrometer.common.util.internal.logging.InternalLoggerFactory;
 import io.micrometer.context.ContextRegistry;
@@ -26,12 +32,6 @@
 import io.micrometer.tracing.Tracer;
 import io.micrometer.tracing.handler.TracingObservationHandler;
 
-import java.io.Closeable;
-import java.io.IOException;
-import java.util.Map;
-import java.util.Objects;
-import java.util.concurrent.ConcurrentHashMap;
-
 /**
  * A {@link ThreadLocalAccessor} to put and restore current {@link Span} depending on
  * whether {@link ObservationThreadLocalAccessor} did some work or not (if
@@ -121,11 +121,9 @@ public Span getValue() {
     public void setValue(Span value) {
         SpanAction spanAction = spanActions.get(Thread.currentThread());
         Tracer.SpanInScope scope = this.tracer.withSpan(value);
-        if (spanAction == null) {
-            spanAction = new SpanAction(spanActions);
-            spanActions.put(Thread.currentThread(), spanAction);
-        }
-        spanAction.setScope(scope);
+        SpanAction newSpanAction = new SpanAction(spanActions, spanAction);
+        spanActions.put(Thread.currentThread(), newSpanAction);
+        newSpanAction.setScope(scope);
     }
 
     @Override
@@ -171,8 +169,8 @@ static class SpanAction implements Closeable {
 
         Closeable scope;
 
-        SpanAction(Map<Thread, SpanAction> spanActions) {
-            this.previous = spanActions.get(Thread.currentThread());
+        SpanAction(Map<Thread, SpanAction> spanActions, SpanAction previous) {
+            this.previous = previous;
             this.todo = spanActions;
         }