Skip to content

Commit

Permalink
Fix handling of baggage when retrieving current span (helidon-io#8567)
Browse files Browse the repository at this point in the history
* Fix current span baggage handling

Signed-off-by: Tim Quinn <[email protected]>

* Don't copy baggage from context into active span; just save the reference so changes to the span are reflected in the context's baggage

---------

Signed-off-by: Tim Quinn <[email protected]>
  • Loading branch information
tjquinno authored Mar 27, 2024
1 parent 5d887ff commit 37a8530
Show file tree
Hide file tree
Showing 8 changed files with 293 additions and 13 deletions.
21 changes: 19 additions & 2 deletions tracing/jaeger/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,11 @@
<artifactId>bedrock-testing-support</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>io.helidon.common.testing</groupId>
<artifactId>helidon-common-testing-junit5</artifactId>
<scope>test</scope>
</dependency>
</dependencies>

<build>
Expand All @@ -166,13 +171,13 @@
<id>default-test</id>
<configuration>
<excludes>
<exclude>**/B3HeadersPropagationCaseTest.java</exclude>
<exclude>**/B3HeadersPropagationCaseTest.java,**/JaegerBaggagePropagationTest.java</exclude>
</excludes>
</configuration>
</execution>
<execution>
<!-- Run this test in a separate JVM so we don't init GlobalOpenTelemetry multiple times. -->
<id>headers-propagation-test</id>
<id>b3-headers-propagation-test</id>
<goals>
<goal>test</goal>
</goals>
Expand All @@ -182,6 +187,18 @@
</includes>
</configuration>
</execution>
<execution>
<!-- Run this test in a separate JVM so we don't init GlobalOpenTelemetry multiple times. -->
<id>all-headers-propagation-test</id>
<goals>
<goal>test</goal>
</goals>
<configuration>
<includes>
<include>**/JaegerBaggagePropagationTest.java</include>
</includes>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019, 2022 Oracle and/or its affiliates.
* Copyright (c) 2019, 2024 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -20,11 +20,9 @@
import io.helidon.common.Prioritized;
import io.helidon.tracing.Span;
import io.helidon.tracing.Tracer;
import io.helidon.tracing.opentelemetry.HelidonOpenTelemetry;
import io.helidon.tracing.opentelemetry.OpenTelemetryTracerProvider;
import io.helidon.tracing.spi.TracerProvider;

import io.opentelemetry.context.Context;
import jakarta.annotation.Priority;

/**
Expand All @@ -44,8 +42,7 @@ public void global(Tracer tracer) {

@Override
public Optional<Span> currentSpan() {
return Optional.ofNullable(io.opentelemetry.api.trace.Span.fromContextOrNull(Context.current()))
.map(HelidonOpenTelemetry::create);
return OpenTelemetryTracerProvider.activeSpan();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
/*
* Copyright (c) 2024 Oracle and/or its affiliates.
*
* 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
*
* 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 io.helidon.tracing.jaeger;

import java.util.Optional;
import java.util.TreeMap;
import java.util.function.Supplier;

import io.helidon.common.testing.junit5.OptionalMatcher;
import io.helidon.config.Config;
import io.helidon.tracing.HeaderConsumer;
import io.helidon.tracing.HeaderProvider;
import io.helidon.tracing.Scope;
import io.helidon.tracing.Span;
import io.helidon.tracing.SpanContext;
import io.helidon.tracing.Tracer;
import io.helidon.tracing.TracerBuilder;

import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;

class JaegerBaggagePropagationTest {

static final String BAGGAGE_KEY = "myKey";
static final String BAGGAGE_VALUE = "myValue";

private static final String OTEL_AUTO_CONFIGURE_PROP = "otel.java.global-autoconfigure.enabled";
private static final String OTEL_SDK_DISABLED_PROP = "otel.sdk.disabled";
private static String originalOtelSdkAutoConfiguredSetting;
private static String originalOtelSdkDisabledSetting;

private static Config config;
private static Tracer tracer;

@BeforeAll
static void init() {
config = Config.create().get("tracing");
tracer = TracerBuilder.create(config.get("jaeger-all-propagators")).build();
originalOtelSdkAutoConfiguredSetting = System.setProperty(OTEL_AUTO_CONFIGURE_PROP, "true");
originalOtelSdkDisabledSetting = System.setProperty(OTEL_SDK_DISABLED_PROP, "false");
}

@AfterAll
static void wrapup() {
if (originalOtelSdkAutoConfiguredSetting != null) {
System.setProperty(OTEL_AUTO_CONFIGURE_PROP, originalOtelSdkAutoConfiguredSetting);
}
if (originalOtelSdkDisabledSetting != null) {
System.setProperty(OTEL_SDK_DISABLED_PROP, originalOtelSdkDisabledSetting);
}
}

@Test
void testBaggageProp() {

var span = tracer.spanBuilder("testSpan").start();
span.baggage(BAGGAGE_KEY, BAGGAGE_VALUE);

checkBaggage(tracer, span, span::context);
}

@Test
void testBaggageBeforeActivatingSpan() {
var span = tracer.spanBuilder("activatedTestSpan").start();
span.baggage(BAGGAGE_KEY, BAGGAGE_VALUE);

try (Scope scope = span.activate()) {
checkBaggage(tracer, span, this::currentSpanContext);
}
}

@Test
void testBaggageAfterActivatingSpan() {
var span = tracer.spanBuilder("activatedTestSpan").start();

try (Scope scope = span.activate()) {
span.baggage(BAGGAGE_KEY, BAGGAGE_VALUE);
checkBaggage(tracer, span, this::currentSpanContext);
}
}

private void checkBaggage(Tracer tracer, Span span, Supplier<SpanContext> spanContextSupplier) {
try {
assertThat("Value after initial storage of baggage",
span.baggage(BAGGAGE_KEY),
OptionalMatcher.optionalValue(is(equalTo(BAGGAGE_VALUE))));

var headerConsumer = HeaderConsumer.create(new TreeMap<>(String.CASE_INSENSITIVE_ORDER));
tracer.inject(spanContextSupplier.get(), HeaderProvider.empty(), headerConsumer);

// Make sure the baggage header was set.
Optional<String> baggageHeader = headerConsumer.get("baggage");
assertThat("Baggage contents in propagated headers",
baggageHeader,
OptionalMatcher.optionalValue(is(equalTo(BAGGAGE_KEY + "=" + BAGGAGE_VALUE))));

// Now make sure the baggage is propagated to a new span context based on the header.
Optional<SpanContext> propagatedSpanContext = tracer.extract(headerConsumer);
assertThat("Propagated span context",
propagatedSpanContext,
OptionalMatcher.optionalPresent());
Span spanFromContext = tracer.spanBuilder("fromContext").parent(propagatedSpanContext.get()).build();
assertThat("Baggage value from propagated span context",
spanFromContext.baggage(BAGGAGE_KEY),
OptionalMatcher.optionalValue(is(BAGGAGE_VALUE)));

span.end();
} catch (Exception ex) {
span.end(ex);
}
}

private SpanContext currentSpanContext() {
return Span.current()
.map(Span::context)
.orElseThrow(() -> new IllegalStateException("No current span"));
}
}
5 changes: 4 additions & 1 deletion tracing/jaeger/src/test/resources/application.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#
# Copyright (c) 2019, 2023 Oracle and/or its affiliates.
# Copyright (c) 2019, 2024 Oracle and/or its affiliates.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -46,3 +46,6 @@ tracing:
int-tags:
tag5: 145 # JAEGER_TAGS
tag6: 741 # JAEGER_TAGS
jaeger-all-propagators:
service: "prop-test"
propagation: ["jaeger", "b3", "w3c"]
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2022, 2023 Oracle and/or its affiliates.
* Copyright (c) 2022, 2024 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -22,6 +22,7 @@
import io.helidon.config.Config;

import io.opentelemetry.api.OpenTelemetry;
import io.opentelemetry.api.baggage.Baggage;
import io.opentelemetry.api.trace.Span;
import io.opentelemetry.api.trace.Tracer;

Expand Down Expand Up @@ -57,6 +58,17 @@ public static io.helidon.tracing.Span create(Span span) {
return new OpenTelemetrySpan(span);
}

/**
* Wrap an open telemetry span.
*
* @param span open telemetry span
* @param baggage open telemetry baggage
* @return Helidon (@link io.helidon.tracing.Span}
*/
public static io.helidon.tracing.Span create(Span span, Baggage baggage) {
return new OpenTelemetrySpan(span, baggage);
}


/**
* Check if OpenTelemetry is present by indirect properties.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,24 @@
import io.helidon.tracing.Span;
import io.helidon.tracing.SpanContext;

import io.opentelemetry.api.baggage.Baggage;
import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.common.AttributesBuilder;
import io.opentelemetry.api.trace.StatusCode;
import io.opentelemetry.context.Context;

class OpenTelemetrySpan implements Span {
private final io.opentelemetry.api.trace.Span delegate;
private final MutableOpenTelemetryBaggage baggage = new MutableOpenTelemetryBaggage();
private final Baggage baggage;

OpenTelemetrySpan(io.opentelemetry.api.trace.Span span) {
this.delegate = span;
baggage = new MutableOpenTelemetryBaggage();
}

OpenTelemetrySpan(io.opentelemetry.api.trace.Span span, Baggage baggage) {
delegate = span;
this.baggage = baggage;
}

@Override
Expand Down Expand Up @@ -100,7 +107,12 @@ public Scope activate() {
public Span baggage(String key, String value) {
Objects.requireNonNull(key, "baggage key cannot be null");
Objects.requireNonNull(value, "baggage value cannot be null");
baggage.baggage(key, value);
if (baggage instanceof MutableOpenTelemetryBaggage mutableBaggage) {
mutableBaggage.baggage(key, value);
} else {
throw new UnsupportedOperationException(
"Attempt to set baggage on a span with read-only baggage (perhaps from context");
}
return this;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2022 Oracle and/or its affiliates.
* Copyright (c) 2022, 2024 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -84,6 +84,24 @@ public static Tracer globalTracer() {
return GLOBAL_TRACER.get();
}

/**
* Returns a {@link io.helidon.tracing.Span} representing the currently-active OpenTelemetry span with any current baggage
* set on the returned span.
*
* @return optional of the current span
*/
public static Optional<Span> activeSpan() {
// OTel returns a no-op span if there is no current one.
io.opentelemetry.api.trace.Span otelSpan = io.opentelemetry.api.trace.Span.current();

// OTel returns empty baggage if there is no current one.
io.opentelemetry.api.baggage.Baggage otelBaggage = io.opentelemetry.api.baggage.Baggage.current();

// Create the span directly with the retrieved baggage. Ideally, it will be our writable baggage because we had put it
// there in the context.
return Optional.of(HelidonOpenTelemetry.create(otelSpan, otelBaggage));
}

@Override
public TracerBuilder<?> createBuilder() {
return OpenTelemetryTracer.builder();
Expand All @@ -105,7 +123,7 @@ public void global(Tracer tracer) {

@Override
public Optional<Span> currentSpan() {
return Optional.ofNullable(io.opentelemetry.api.trace.Span.current()).map(HelidonOpenTelemetry::create);
return activeSpan();
}

@Override
Expand Down
Loading

0 comments on commit 37a8530

Please sign in to comment.