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

Properly return Optional.empty() for current span if there is no current OTel span #8583

Merged
merged 1 commit into from
Mar 28, 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
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,13 @@ public class OpenTelemetryTracerProvider implements TracerProvider {
});
}

/**
* Creates a new provider; reserved for service loading.
*/
@Deprecated
public OpenTelemetryTracerProvider() {
}

/**
* Register global tracer.
*
Expand All @@ -90,10 +97,17 @@ public static Tracer globalTracer() {
* @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();
io.opentelemetry.context.Context otelContext = io.opentelemetry.context.Context.current();
Copy link
Member

Choose a reason for hiding this comment

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

I think this entire method can be just:

return Optional.ofNullable(io.opentelemetry.api.trace.Span.fromContextOrNull(io.opentelemetry.context.Context.current()))
  .map(otelSpan -> HelidonOpenTelemetry.create(otelSpan, io.opentelemetry.api.baggage.Baggage.current());

(Some static imports would make this a little easier, maybe.) What do you think?


// OTel Span.current() returns a no-op span if there is no current one. Use fromContextOrNull instead to distinguish.
io.opentelemetry.api.trace.Span otelSpan =
io.opentelemetry.api.trace.Span.fromContextOrNull(otelContext);

if (otelSpan == null) {
return Optional.empty();
}

// OTel returns empty baggage if there is no current one.
// OTel Baggage.current() returns empty baggage if there is no current one. That's OK for baggage.
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,14 +89,11 @@ void testActiveSpanScopeWithBaggage() {
outerSpan.end(e);
}

// There was no active span before outerSpan was activated, so expect the "default" ad-hoc span ID of all zeroes.
// There was no active span before outerSpan was activated, so expect an empty current span.
Optional<Span> currentSpanAfterTryResourcesBlock = Span.current();
assertThat("Current span just after try-resources block",
currentSpanAfterTryResourcesBlock,
OptionalMatcher.optionalPresent());
assertThat("Current span just after try-resources block",
currentSpanAfterTryResourcesBlock.get().context().spanId(),
containsString("00000000"));
OptionalMatcher.optionalEmpty());
}

@Test
Expand Down