Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix handling of baggage when retrieving current span #8567

Merged
merged 2 commits into from
Mar 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there maybe a way to switch to mutable baggage here and make it work, or is this a limitation of the open telemetry API?
If we have this check + exception, then an application that works fine with other tracing provider may fail with oTel

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
Loading