Fix OTel extract when there is no current context (helidon-io#8578)

* Fix OTel extract when there is no current context Signed-off-by: Tim Quinn <[email protected]> * Review comments; remove unused variables in test method --------- Signed-off-by: Tim Quinn <[email protected]>
barchetta · Mar 29, 2024 · f94924c · f94924c
1 parent 27903ca
commit f94924c
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 5 deletions.
diff --git a/...ing/opentelemetry/src/main/java/io/helidon/tracing/opentelemetry/OpenTelemetryTracer.java b/...ing/opentelemetry/src/main/java/io/helidon/tracing/opentelemetry/OpenTelemetryTracer.java
@@ -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.
@@ -70,8 +70,10 @@ public Span.Builder<?> spanBuilder(String name) {
     public Optional<SpanContext> extract(HeaderProvider headersProvider) {
         Context context = propagator.extract(Context.current(), headersProvider, GETTER);
 
-        return Optional.ofNullable(context)
-                .map(OpenTelemetrySpanContext::new);
+        // 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(context);
+        return Optional.ofNullable(otelSpan)
+                .map(span -> new OpenTelemetrySpanContext(context));
     }
 
     @Override

diff --git a/tracing/opentelemetry/src/test/java/io/helidon/tracing/opentelemetry/TestSpanAndBaggage.java b/tracing/opentelemetry/src/test/java/io/helidon/tracing/opentelemetry/TestSpanAndBaggage.java
@@ -124,7 +124,10 @@ void testActiveSpanScopeWithBaggage() {
     @Test
     void testIncomingBaggage() {
         Tracer tracer = Tracer.global();
-        HeaderProvider inboundHeaders = new MapHeaderProvider(Map.of("baggage", List.of("bag1=val1,bag2=val2")));
+        // Need to supply both the traceparent and the baggage for OTel to construct a proper span context.
+        HeaderProvider inboundHeaders = new MapHeaderProvider(
+                Map.of("traceparent", List.of("00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-01"),
+                       "baggage", List.of("bag1=val1,bag2=val2")));
         Optional<SpanContext> spanContextOpt = tracer.extract(inboundHeaders);
         assertThat("Span context from inbound headers", spanContextOpt, OptionalMatcher.optionalPresent());
         Span span = tracer.spanBuilder("inbound").parent(spanContextOpt.get()).start();
@@ -172,7 +175,7 @@ void testBaggageAddedAfterActivation() {
         final String BAGGAGE_KEY = "mykey";
         final String BAGGAGE_VALUE = "myvalue";
         final var tracer = io.helidon.tracing.Tracer.global();
-        final var span = tracer.spanBuilder("baggageCanaryMinimal").start();
+        final var span = tracer.spanBuilder("baggageMinimal").start();
         try {
             // Set baggage and confirm that it's known in the span
             try (Scope scope = span.activate()) {
@@ -186,6 +189,17 @@ void testBaggageAddedAfterActivation() {
         }
     }
 
+    @Test
+    void testExtractWithNoCurrentSpan() {
+        final var tracer = io.helidon.tracing.Tracer.global();
+
+        HeaderProvider headers = HeaderProvider.create(Map.of("not-a-trace", List.of("1234567890123456"),
+                                                              "not-a-span", List.of("6543210987654321")));
+        Optional<SpanContext> spanContext = tracer.extract(headers);
+
+        assertThat("Current span reported", spanContext, OptionalMatcher.optionalEmpty());
+    }
+
     private void checkBaggage(Tracer tracer, Span span, Supplier<SpanContext> spanContextSupplier) {
         String value = span.baggage(BAGGAGE_KEY).orElseThrow();
         assertThat("baggage value right after set", value, Matchers.is(Matchers.equalTo(BAGGAGE_VALUE)));