Fix setting OTel status on required situations

This will fix setting the OTel span status when: * the span ends and is not yet set * the error state for a span is set Fixes #890
micrometer-metrics · Nov 24, 2024 · 0d7fe5b · 0d7fe5b
1 parent 9143d7a
commit 0d7fe5b
Show file tree

Hide file tree

Showing 5 changed files with 117 additions and 4 deletions.
diff --git a/...r-tracing-bridge-otel/src/main/java/io/micrometer/tracing/otel/bridge/OtelScopedSpan.java b/...r-tracing-bridge-otel/src/main/java/io/micrometer/tracing/otel/bridge/OtelScopedSpan.java
@@ -18,7 +18,9 @@
 import io.micrometer.tracing.ScopedSpan;
 import io.micrometer.tracing.TraceContext;
 import io.opentelemetry.api.trace.Span;
+import io.opentelemetry.api.trace.StatusCode;
 import io.opentelemetry.context.Scope;
+import io.opentelemetry.sdk.trace.ReadableSpan;
 
 /**
  * OpenTelemetry implementation of a {@link ScopedSpan}.
@@ -68,11 +70,21 @@ public ScopedSpan event(String value) {
     @Override
     public ScopedSpan error(Throwable throwable) {
         this.span.recordException(throwable);
+        this.span.setStatus(StatusCode.ERROR, throwable.getMessage());
         return this;
     }
 
     @Override
     public void end() {
+        if (this.span instanceof ReadableSpan) {
+            boolean isStatusUnset = ((ReadableSpan) this.span).toSpanData()
+                .getStatus()
+                .getStatusCode() == StatusCode.UNSET;
+            if (isStatusUnset) {
+                this.span.setStatus(StatusCode.OK);
+            }
+        }
+
         this.scope.close();
         this.span.end();
     }

diff --git a/...rometer-tracing-bridge-otel/src/main/java/io/micrometer/tracing/otel/bridge/OtelSpan.java b/...rometer-tracing-bridge-otel/src/main/java/io/micrometer/tracing/otel/bridge/OtelSpan.java
@@ -19,6 +19,7 @@
 import io.opentelemetry.api.common.AttributeKey;
 import io.opentelemetry.api.trace.StatusCode;
 import io.opentelemetry.context.Context;
+import io.opentelemetry.sdk.trace.ReadableSpan;
 import io.opentelemetry.semconv.SemanticAttributes;
 
 import java.util.List;
@@ -151,6 +152,9 @@ public Span tagOfBooleans(String key, List<Boolean> values) {
 
     @Override
     public void end(long time, TimeUnit timeUnit) {
+        if (this.isStatusUnset()) {
+            this.delegate.setStatus(StatusCode.OK);
+        }
         this.delegate.end(time, timeUnit);
     }
 
@@ -170,6 +174,9 @@ public Span error(Throwable throwable) {
 
     @Override
     public void end() {
+        if (this.isStatusUnset()) {
+            this.delegate.setStatus(StatusCode.OK);
+        }
         this.delegate.end();
     }
 
@@ -210,4 +217,11 @@ public int hashCode() {
         return Objects.hash(this.delegate);
     }
 
+    private boolean isStatusUnset() {
+        if (this.delegate instanceof ReadableSpan) {
+            return ((ReadableSpan) this.delegate).toSpanData().getStatus().getStatusCode() == StatusCode.UNSET;
+        }
+        return false;
+    }
+
 }
diff --git a/...-tracing-bridge-otel/src/main/java/io/micrometer/tracing/otel/bridge/OtelSpanBuilder.java b/...-tracing-bridge-otel/src/main/java/io/micrometer/tracing/otel/bridge/OtelSpanBuilder.java
@@ -22,10 +22,7 @@
 import io.opentelemetry.api.common.AttributeKey;
 import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.api.common.AttributesBuilder;
-import io.opentelemetry.api.trace.SpanBuilder;
-import io.opentelemetry.api.trace.SpanContext;
-import io.opentelemetry.api.trace.SpanKind;
-import io.opentelemetry.api.trace.Tracer;
+import io.opentelemetry.api.trace.*;
 import io.opentelemetry.semconv.SemanticAttributes;
 
 import java.util.AbstractMap.SimpleEntry;
@@ -243,6 +240,7 @@ public Span start() {
         io.opentelemetry.api.trace.Span span = spanBuilder.startSpan();
         if (this.error != null) {
             span.recordException(this.error);
+            span.setStatus(StatusCode.ERROR, this.error.getMessage());
         }
         this.annotations.forEach(span::addEvent);
         if (this.parentTraceContext != null) {

diff --git a/...ing-bridge-otel/src/test/java/io/micrometer/tracing/otel/bridge/OtelSpanBuilderTests.java b/...ing-bridge-otel/src/test/java/io/micrometer/tracing/otel/bridge/OtelSpanBuilderTests.java
@@ -20,11 +20,15 @@
 import io.micrometer.tracing.TraceContext;
 import io.opentelemetry.api.common.AttributeKey;
 import io.opentelemetry.api.common.Attributes;
+import io.opentelemetry.api.trace.StatusCode;
 import io.opentelemetry.context.propagation.ContextPropagators;
 import io.opentelemetry.extension.trace.propagation.B3Propagator;
 import io.opentelemetry.sdk.OpenTelemetrySdk;
 import io.opentelemetry.sdk.trace.SdkTracerProvider;
+import io.opentelemetry.sdk.trace.data.EventData;
 import io.opentelemetry.sdk.trace.data.SpanData;
+import io.opentelemetry.sdk.trace.data.StatusData;
+import org.assertj.core.api.ObjectAssert;
 import org.junit.jupiter.api.Test;
 
 import java.util.Arrays;
@@ -33,6 +37,7 @@
 import java.util.stream.Collectors;
 
 import static org.assertj.core.api.BDDAssertions.then;
+import static org.assertj.core.api.InstanceOfAssertFactories.COLLECTION;
 
 class OtelSpanBuilderTests {
 
@@ -139,6 +144,21 @@ void should_honor_parent_context_using_tracecontextbuilder() {
         then(span.context().traceId()).isEqualTo(foo.context().traceId());
     }
 
+    @Test
+    void should_add_event_with_exception_and_set_error_status() {
+        Span.Builder builder = new OtelSpanBuilder(otelTracer).name("foo");
+
+        builder.error(new RuntimeException("something went wrong")).start().end();
+
+        SpanData poll = processor.spans().poll();
+        ObjectAssert<EventData> eventAssert = then(poll.getEvents()).hasSize(1).element(0);
+        eventAssert.extracting(EventData::getName).isEqualTo("exception");
+        eventAssert.extracting(data -> data.getAttributes().asMap().values())
+            .asInstanceOf(COLLECTION)
+            .containsAnyOf("something went wrong");
+        then(poll.getStatus()).isEqualTo(StatusData.create(StatusCode.ERROR, "something went wrong"));
+    }
+
     private Map<String, Object> tags() {
         Map<String, Object> map = new HashMap<>();
         map.put("tag1", "value1");

diff --git a/...cing-bridge-otel/src/test/java/io/micrometer/tracing/otel/bridge/OtelTracingApiTests.java b/...cing-bridge-otel/src/test/java/io/micrometer/tracing/otel/bridge/OtelTracingApiTests.java
@@ -17,14 +17,18 @@
 
 import io.micrometer.tracing.*;
 import io.opentelemetry.api.common.AttributeKey;
+import io.opentelemetry.api.trace.StatusCode;
 import io.opentelemetry.context.Context;
 import io.opentelemetry.context.Scope;
 import io.opentelemetry.context.propagation.ContextPropagators;
 import io.opentelemetry.extension.trace.propagation.B3Propagator;
 import io.opentelemetry.sdk.OpenTelemetrySdk;
 import io.opentelemetry.sdk.trace.SdkTracerProvider;
+import io.opentelemetry.sdk.trace.data.EventData;
 import io.opentelemetry.sdk.trace.data.LinkData;
 import io.opentelemetry.sdk.trace.data.SpanData;
+import io.opentelemetry.sdk.trace.data.StatusData;
+import org.assertj.core.api.ObjectAssert;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.Test;
 import org.slf4j.MDC;
@@ -35,9 +39,11 @@
 import java.util.Queue;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
 
 import static io.opentelemetry.sdk.trace.samplers.Sampler.alwaysOn;
 import static org.assertj.core.api.BDDAssertions.then;
+import static org.assertj.core.api.InstanceOfAssertFactories.COLLECTION;
 
 class OtelTracingApiTests {
 
@@ -406,6 +412,69 @@ void should_work_with_with_scope_for_tracer_and_current_trace_context() {
         then(tracer.currentSpan()).isNull();
     }
 
+    @Test
+    void should_set_ok_status_for_span() {
+        this.tracer.nextSpan().end();
+
+        SpanData poll = spanExporter.spans().poll();
+        then(poll.getStatus()).isEqualTo(StatusData.ok());
+    }
+
+    @Test
+    void should_set_error_and_error_status_for_span() {
+        this.tracer.nextSpan().error(new RuntimeException("something went wrong")).end();
+
+        SpanData poll = spanExporter.spans().poll();
+        ObjectAssert<EventData> eventAssert = then(poll.getEvents()).hasSize(1).element(0);
+        eventAssert.extracting(EventData::getName).isEqualTo("exception");
+        eventAssert.extracting(data -> data.getAttributes().asMap().values())
+            .asInstanceOf(COLLECTION)
+            .containsAnyOf("something went wrong");
+        then(poll.getStatus()).isEqualTo(StatusData.create(StatusCode.ERROR, "something went wrong"));
+    }
+
+    @Test
+    void should_set_ok_status_for_timed_span() {
+        this.tracer.nextSpan().end(1, TimeUnit.SECONDS);
+
+        SpanData poll = spanExporter.spans().poll();
+        then(poll.getStatus()).isEqualTo(StatusData.ok());
+    }
+
+    @Test
+    void should_set_error_and_error_status_for_timed_span() {
+        this.tracer.nextSpan().error(new RuntimeException("something went wrong")).end(1, TimeUnit.SECONDS);
+
+        SpanData poll = spanExporter.spans().poll();
+        ObjectAssert<EventData> eventAssert = then(poll.getEvents()).hasSize(1).element(0);
+        eventAssert.extracting(EventData::getName).isEqualTo("exception");
+        eventAssert.extracting(data -> data.getAttributes().asMap().values())
+            .asInstanceOf(COLLECTION)
+            .containsAnyOf("something went wrong");
+        then(poll.getStatus()).isEqualTo(StatusData.create(StatusCode.ERROR, "something went wrong"));
+    }
+
+    @Test
+    void should_set_ok_status_for_scoped_span() {
+        this.tracer.startScopedSpan("scoped").end();
+
+        SpanData poll = spanExporter.spans().poll();
+        then(poll.getStatus()).isEqualTo(StatusData.ok());
+    }
+
+    @Test
+    void should_set_error_and_error_status_for_scoped_span() {
+        this.tracer.startScopedSpan("scoped").error(new RuntimeException("something went wrong")).end();
+
+        SpanData poll = spanExporter.spans().poll();
+        ObjectAssert<EventData> eventAssert = then(poll.getEvents()).hasSize(1).element(0);
+        eventAssert.extracting(EventData::getName).isEqualTo("exception");
+        eventAssert.extracting(data -> data.getAttributes().asMap().values())
+            .asInstanceOf(COLLECTION)
+            .containsAnyOf("something went wrong");
+        then(poll.getStatus()).isEqualTo(StatusData.create(StatusCode.ERROR, "something went wrong"));
+    }
+
     private static OtelCurrentTraceContext.WrappedScope getScopeFromTracerScope(OtelTracer.WrappedSpanInScope ws) {
         return (OtelCurrentTraceContext.WrappedScope) ws.scope;
     }