Skip to content

Commit 366dcb0

Browse files
Fixed OTel baggage tagging
- currently we're having a wrong equality check, we're converting baggage keys to lowercase but we're comparing it to non lowercase tags. We should do ignore case equality check instead - Added tests for baggage tagging for both OTel and Brave fixes gh-375
1 parent dd8bd7d commit 366dcb0

File tree

3 files changed

+82
-7
lines changed

3 files changed

+82
-7
lines changed

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

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,12 @@
1515
*/
1616
package io.micrometer.tracing.brave.bridge;
1717

18+
import brave.Tags;
1819
import brave.Tracing;
1920
import brave.baggage.BaggageField;
2021
import brave.baggage.BaggagePropagation;
2122
import brave.baggage.BaggagePropagationConfig;
23+
import brave.handler.MutableSpan;
2224
import brave.handler.SpanHandler;
2325
import brave.propagation.B3Propagation;
2426
import brave.propagation.StrictCurrentTraceContext;
@@ -46,7 +48,11 @@ class BaggageTests {
4648

4749
public static final String VALUE_1 = "value1";
4850

49-
SpanHandler spanHandler = new TestSpanHandler();
51+
public static final String TAG_KEY = "tagKey";
52+
53+
public static final String TAG_VALUE = "tagValue";
54+
55+
TestSpanHandler spanHandler = new TestSpanHandler();
5056

5157
StrictCurrentTraceContext braveCurrentTraceContext = StrictCurrentTraceContext.create();
5258

@@ -58,9 +64,11 @@ class BaggageTests {
5864
.traceId128Bit(true)
5965
.propagationFactory(BaggagePropagation.newFactoryBuilder(B3Propagation.FACTORY)
6066
.add(BaggagePropagationConfig.SingleBaggageField.remote(BaggageField.create(KEY_1)))
67+
.add(BaggagePropagationConfig.SingleBaggageField.local(BaggageField.create(TAG_KEY)))
6168
.build())
6269
.sampler(Sampler.ALWAYS_SAMPLE)
6370
.addSpanHandler(this.spanHandler)
71+
.addSpanHandler(new BaggageTagSpanHandler(new BaggageField[] { BaggageField.create(TAG_KEY) }))
6472
.build();
6573

6674
brave.Tracer braveTracer = this.tracing.tracer();
@@ -138,4 +146,43 @@ void baggageWithContextPropagation() {
138146
}
139147
}
140148

149+
@Test
150+
void baggageTagKey() {
151+
ScopedSpan span = this.tracer.startScopedSpan("call1");
152+
try {
153+
try (BaggageInScope scope7 = this.tracer.createBaggage(TAG_KEY, TAG_VALUE).makeCurrent()) {
154+
// span should get tagged with baggage
155+
}
156+
}
157+
catch (RuntimeException | Error ex) {
158+
span.error(ex);
159+
throw ex;
160+
}
161+
finally {
162+
span.end();
163+
}
164+
165+
then(spanHandler.spans()).hasSize(1);
166+
MutableSpan mutableSpan = spanHandler.spans().get(0);
167+
then(mutableSpan.tags().get(TAG_KEY)).isEqualTo(TAG_VALUE);
168+
}
169+
170+
static final class BaggageTagSpanHandler extends SpanHandler {
171+
172+
final BaggageField[] fieldsToTag;
173+
174+
BaggageTagSpanHandler(BaggageField[] fieldsToTag) {
175+
this.fieldsToTag = fieldsToTag;
176+
}
177+
178+
@Override
179+
public boolean end(brave.propagation.TraceContext context, MutableSpan span, Cause cause) {
180+
for (BaggageField field : fieldsToTag) {
181+
Tags.BAGGAGE_FIELD.tag(field, context, span);
182+
}
183+
return true;
184+
}
185+
186+
}
187+
141188
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ private io.micrometer.tracing.Baggage doSet(TraceContext context, String value)
101101
Context withBaggage = current.with(baggage);
102102
ctx.updateContext(withBaggage);
103103
contextWithBaggage.set(withBaggage);
104-
if (this.tagFields.stream().map(String::toLowerCase).anyMatch(s -> s.equals(entry().getKey()))) {
104+
if (this.tagFields.stream().anyMatch(s -> s.equalsIgnoreCase(entry().getKey()))) {
105105
currentSpan.setAttribute(entry().getKey(), value);
106106
}
107107
Entry previous = entry();

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

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,18 +17,18 @@
1717

1818
import io.micrometer.common.util.internal.logging.InternalLogger;
1919
import io.micrometer.common.util.internal.logging.InternalLoggerFactory;
20-
import io.micrometer.tracing.Baggage;
21-
import io.micrometer.tracing.BaggageInScope;
22-
import io.micrometer.tracing.Span;
23-
import io.micrometer.tracing.Tracer;
20+
import io.micrometer.tracing.*;
2421
import io.micrometer.tracing.otel.propagation.BaggageTextMapPropagator;
2522
import io.opentelemetry.api.baggage.propagation.W3CBaggagePropagator;
23+
import io.opentelemetry.api.common.AttributeKey;
2624
import io.opentelemetry.api.trace.propagation.W3CTraceContextPropagator;
2725
import io.opentelemetry.context.propagation.ContextPropagators;
2826
import io.opentelemetry.context.propagation.TextMapPropagator;
2927
import io.opentelemetry.extension.trace.propagation.B3Propagator;
3028
import io.opentelemetry.sdk.OpenTelemetrySdk;
3129
import io.opentelemetry.sdk.trace.SdkTracerProvider;
30+
import io.opentelemetry.sdk.trace.data.SpanData;
31+
import io.opentelemetry.sdk.trace.export.SimpleSpanProcessor;
3232
import org.junit.jupiter.api.Test;
3333
import reactor.core.publisher.Hooks;
3434
import reactor.core.publisher.Mono;
@@ -52,8 +52,15 @@ class BaggageTests {
5252

5353
public static final String VALUE_2 = "value2";
5454

55+
public static final String TAG_KEY = "tagKey";
56+
57+
public static final String TAG_VALUE = "tagValue";
58+
59+
ArrayListSpanProcessor spanExporter = new ArrayListSpanProcessor();
60+
5561
SdkTracerProvider sdkTracerProvider = SdkTracerProvider.builder()
5662
.setSampler(io.opentelemetry.sdk.trace.samplers.Sampler.alwaysOn())
63+
.addSpanProcessor(SimpleSpanProcessor.create(spanExporter))
5764
.build();
5865

5966
OpenTelemetrySdk openTelemetrySdk = OpenTelemetrySdk.builder()
@@ -66,7 +73,7 @@ class BaggageTests {
6673
OtelCurrentTraceContext otelCurrentTraceContext = new OtelCurrentTraceContext();
6774

6875
OtelBaggageManager otelBaggageManager = new OtelBaggageManager(otelCurrentTraceContext,
69-
Collections.singletonList(KEY_1), Collections.emptyList());
76+
Collections.singletonList(KEY_1), Collections.singletonList(TAG_KEY));
7077

7178
ContextPropagators contextPropagators = ContextPropagators
7279
.create(TextMapPropagator.composite(W3CBaggagePropagator.getInstance(), W3CTraceContextPropagator.getInstance(),
@@ -144,4 +151,25 @@ void baggageWithContextPropagation() {
144151
}
145152
}
146153

154+
@Test
155+
void baggageTagKey() {
156+
ScopedSpan span = this.tracer.startScopedSpan("call1");
157+
try {
158+
try (BaggageInScope scope7 = this.tracer.createBaggage(TAG_KEY, TAG_VALUE).makeCurrent()) {
159+
// span should get tagged with baggage
160+
}
161+
}
162+
catch (RuntimeException | Error ex) {
163+
span.error(ex);
164+
throw ex;
165+
}
166+
finally {
167+
span.end();
168+
}
169+
170+
then(spanExporter.spans()).hasSize(1);
171+
SpanData spanData = spanExporter.spans().poll();
172+
then(spanData.getAttributes().get(AttributeKey.stringKey(TAG_KEY))).isEqualTo(TAG_VALUE);
173+
}
174+
147175
}

0 commit comments

Comments
 (0)