Updates OTel to 1.23.0

this upgrades OTel libraries to 1.23.0.

Brave works in such a way that its central part is the Span. Span is in thread local. Span has the extra map that contains baggage. Baggage can't work without the span.

OTel works in such a way that its central part is Context. Context may contain span and / or baggage or whatever you want. You can't merge contexts without putting one in scope.

The problem we're facing is that with our API you can create a baggage object and put some values to it at any point in time. In OTel for that baggage to be present you MUST make the baggage current. However, if you make the baggage current you MUST then close it.

In our tests we were doing sth like tracer.createBaggage("foo", "bar") that would create the baggage. Under the hood we would create the OTel Context that would merge the current Span context with Baggage context and MAKE THAT ONE CURRENT. The problem is that we never closed it. So the required change is to always use createBaggage(...) and then call makeCurrent() . If you don't call the latter we were leaking a scope.

This fixes the Context leakage. What remains is the problem with the Baggage API - I doubt that there is any possibility to call baggage.get().set(...) . That works for Brave but won't work for OTel.

fixes gh-173

dependencies.gradle

@@ -34,8 +34,8 @@ def MICROMETER_PLATFORM_VERSIONS = [
 def PLATFORM_VERSIONS = [
 	'io.zipkin.brave:brave-bom:5.14.+',
 	// opentelemetry-instrumentation-api dependency above with this
-	'io.opentelemetry:opentelemetry-bom:1.21.+',
-	'io.opentelemetry:opentelemetry-bom-alpha:1.21.+',
+	'io.opentelemetry:opentelemetry-bom:1.23.+',
+	'io.opentelemetry:opentelemetry-bom-alpha:1.23.+',
 	'org.junit:junit-bom:5.9.+'
 ]
 

micrometer-tracing-bridges/micrometer-tracing-bridge-otel/src/main/java/io/micrometer/tracing/otel/bridge/OtelBaggageInScope.java

@@ -43,6 +43,8 @@ class OtelBaggageInScope implements io.micrometer.tracing.Baggage, BaggageInScop
 
     private final AtomicReference<Entry> entry = new AtomicReference<>();
 
+    private final AtomicReference<Context> contextWithBaggage = new AtomicReference<>(null);
+
     private final AtomicReference<Scope> scope = new AtomicReference<>();
 
     OtelBaggageInScope(OtelBaggageManager otelBaggageManager, CurrentTraceContext currentTraceContext,
@@ -78,12 +80,12 @@ public io.micrometer.tracing.Baggage set(String value) {
     }
 
     private io.micrometer.tracing.Baggage doSet(TraceContext context, String value) {
-        Context current = Context.current();
-        Span currentSpan = Span.current();
-        io.opentelemetry.api.baggage.Baggage baggage;
         if (context == null) {
             return this;
         }
+        Context current = Context.current();
+        Span currentSpan = Span.current();
+        io.opentelemetry.api.baggage.Baggage baggage;
         OtelTraceContext ctx = (OtelTraceContext) context;
         Context storedCtx = ctx.context();
         Baggage fromContext = Baggage.fromContext(storedCtx);
@@ -96,7 +98,7 @@ private io.micrometer.tracing.Baggage doSet(TraceContext context, String value)
         current = current.with(baggage);
         Context withBaggage = current.with(baggage);
         ctx.updateContext(withBaggage);
-        this.scope.set(withBaggage.makeCurrent());
+        contextWithBaggage.set(withBaggage);
         if (this.tagFields.stream().map(String::toLowerCase).anyMatch(s -> s.equals(entry().getKey()))) {
             currentSpan.setAttribute(entry().getKey(), value);
         }
@@ -116,13 +118,14 @@ public io.micrometer.tracing.Baggage set(TraceContext traceContext, String value
 
     @Override
     public BaggageInScope makeCurrent() {
-        if (this.scope.get() == null) {
-            return this;
-        }
-        close();
         Entry entry = entry();
-        Scope scope = Baggage.builder().put(entry.getKey(), entry.getValue(), entry.getMetadata()).build()
-                .makeCurrent();
+        Context context = contextWithBaggage.get();
+        if (context == null) {
+            context = Context.current();
+        }
+        Baggage baggage = Baggage.builder().put(entry.getKey(), entry.getValue(), entry.getMetadata()).build();
+        Context updated = context.with(baggage);
+        Scope scope = updated.makeCurrent();
         this.scope.set(scope);
         return this;
     }

micrometer-tracing-bridges/micrometer-tracing-bridge-otel/src/test/java/io/micrometer/tracing/otel/bridge/BaggageTests.java

@@ -1,12 +1,12 @@
 /**
  * Copyright 2022 the original author or authors.
- *
+ * <p>
  * 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
- *
+ * <p>
  * https://www.apache.org/licenses/LICENSE-2.0
- *
+ * <p>
  * 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.
@@ -68,10 +68,10 @@ void canSetAndGetBaggage() {
         Span span = tracer.nextSpan().start();
         try (Tracer.SpanInScope spanInScope = tracer.withSpan(span)) {
             // WHEN
-            this.tracer.getBaggage(KEY_1).set(VALUE_1);
-
-            // THEN
-            then(tracer.getBaggage(KEY_1).get()).isEqualTo(VALUE_1);
+            try (BaggageInScope bs = this.tracer.createBaggage(KEY_1, VALUE_1).makeCurrent()) {
+                // THEN
+                then(tracer.getBaggage(KEY_1).get()).isEqualTo(VALUE_1);
+            }
         }
     }
 
@@ -82,13 +82,13 @@ void injectAndExtractKeepsTheBaggage() {
 
         Span span = tracer.nextSpan().start();
         try (Tracer.SpanInScope spanInScope = tracer.withSpan(span)) {
-            this.tracer.createBaggage(KEY_1, VALUE_1);
+            try (BaggageInScope bs = this.tracer.createBaggage(KEY_1, VALUE_1).makeCurrent()) {
+                // WHEN
+                this.propagator.inject(tracer.currentTraceContext().context(), carrier, Map::put);
 
-            // WHEN
-            this.propagator.inject(tracer.currentTraceContext().context(), carrier, Map::put);
-
-            // THEN
-            then(carrier.get(KEY_1)).isEqualTo(VALUE_1);
+                // THEN
+                then(carrier.get(KEY_1)).isEqualTo(VALUE_1);
+            }
         }
 
         // WHEN

micrometer-tracing-bridges/micrometer-tracing-bridge-otel/src/test/java/io/micrometer/tracing/otel/bridge/OtelPropagatorTests.java

@@ -62,13 +62,9 @@ void should_propagate_context_with_trace_and_baggage() {
 
         Span span = extract.start();
 
-        BaggageInScope baggage = otelBaggageManager.getBaggage(span.context(), "foo").makeCurrent();
-        try {
+        try (BaggageInScope baggage = otelBaggageManager.getBaggage(span.context(), "foo").makeCurrent()) {
             BDDAssertions.then(baggage.get(span.context())).isEqualTo("bar");
         }
-        finally {
-            baggage.close();
-        }
     }
 
     @Test
@@ -79,13 +75,9 @@ void should_propagate_context_with_baggage_only() {
 
         Span span = extract.start();
 
-        BaggageInScope baggage = otelBaggageManager.getBaggage(span.context(), "foo").makeCurrent();
-        try {
+        try (BaggageInScope baggage = otelBaggageManager.getBaggage(span.context(), "foo").makeCurrent()) {
             BDDAssertions.then(baggage.get(span.context())).isEqualTo("bar");
         }
-        finally {
-            baggage.close();
-        }
     }
 
     @Test
@@ -96,14 +88,10 @@ void should_propagate_context_with_baggage_only_as_field() {
 
         Span span = extract.start();
 
-        BaggageInScope baggage = new OtelBaggageManager(otelCurrentTraceContext, Collections.emptyList(),
-                Collections.emptyList()).getBaggage(span.context(), "foo").makeCurrent();
-        try {
+        try (BaggageInScope baggage = new OtelBaggageManager(otelCurrentTraceContext, Collections.emptyList(),
+                Collections.emptyList()).getBaggage(span.context(), "foo").makeCurrent()) {
             BDDAssertions.then(baggage.get(span.context())).isEqualTo("bar");
         }
-        finally {
-            baggage.close();
-        }
     }
 
 }

micrometer-tracing-bridges/micrometer-tracing-bridge-otel/src/test/java/io/micrometer/tracing/otel/bridge/OtelTracingApiTests.java

@@ -184,6 +184,8 @@ void should_work_with_baggage() {
             }
         }
 
+        then(tracer.currentSpan()).isNull();
+
         // Assuming that you have a handle to the span
         Baggage baggageForExplicitSpan = tracer.createBaggage("from_span").set(span.context(), "value 3");
         try (BaggageInScope baggage = baggageForExplicitSpan.makeCurrent()) {
@@ -193,14 +195,18 @@ void should_work_with_baggage() {
                     .isEqualTo("value 3");
         }
 
+        then(tracer.currentSpan()).isNull();
+
         // Assuming that there's no span in scope
         Baggage baggageFour = tracer.createBaggage("from_span_in_scope 1", "value 1");
 
-        // When there's no span in scope, there will never be any baggage - even if you
-        // make it current
+        then(tracer.currentSpan()).isNull();
+
+        // When there's no span in scope, baggage can still be there (that's incosistent
+        // with Brave)
         try (BaggageInScope baggage = baggageFour.makeCurrent()) {
-            then(baggageFour.get()).as("[Out of span scope] Baggage 1").isNull();
-            then(tracer.getBaggage("from_span_in_scope 1").get()).as("[Out of span scope] Baggage 1").isNull();
+            then(baggageFour.get()).as("[Out of span scope] Baggage 1").isNotNull();
+            then(tracer.getBaggage("from_span_in_scope 1").get()).as("[Out of span scope] Baggage 1").isNotNull();
         }
         then(tracer.getBaggage("from_span_in_scope 1").get()).as("[Out of scope] Baggage 1").isNull();
         then(tracer.getBaggage("from_span_in_scope 2").get()).as("[Out of scope] Baggage 2").isNull();