Fix span status for EH and SB (#33836)

Azure · Mar 2, 2023 · f1d88c7 · f1d88c7
1 parent 6d92f51
commit f1d88c7
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 15 deletions.
diff --git a/...-opentelemetry/src/main/java/com/azure/core/tracing/opentelemetry/OpenTelemetryUtils.java b/...-opentelemetry/src/main/java/com/azure/core/tracing/opentelemetry/OpenTelemetryUtils.java
@@ -130,26 +130,25 @@ static void addAttribute(Span span, String key, Object value, OpenTelemetrySchem
     }
 
     /**
-     * Parses an OpenTelemetry Status from AMQP Error Condition.
+     * Parses an OpenTelemetry status from error description.
      *
      * @param span the span to set the status for.
-     * @param statusMessage description for this error condition.
+     * @param statusMessage description for this error condition. Any non-null {@code statusMessage} indicates an error.
+     *                      Pass empty string to create error status without description.
      * @param throwable the error occurred during response transmission (optional).
      * @return the corresponding OpenTelemetry {@link Span}.
      */
-    public static Span setError(Span span, String statusMessage, Throwable throwable) {
+    static Span setError(Span span, String statusMessage, Throwable throwable) {
         if (throwable != null) {
             span.recordException(throwable);
             return span.setStatus(StatusCode.ERROR, statusMessage);
         }
 
-        if (statusMessage != null) {
-            if ("error".equals(statusMessage)) {
-                return span.setStatus(StatusCode.ERROR);
-            }
-            return span.setStatus(StatusCode.ERROR, statusMessage);
+        // "success" is needed for back compat with older Event Hubs and Service Bus, don't use it.
+        if (statusMessage == null || "success".equals(statusMessage)) {
+            return span;
         }
 
-        return span;
+        return span.setStatus(StatusCode.ERROR, statusMessage);
     }
 }
diff --git a/...telemetry/src/test/java/com/azure/core/tracing/opentelemetry/OpenTelemetryTracerTest.java b/...telemetry/src/test/java/com/azure/core/tracing/opentelemetry/OpenTelemetryTracerTest.java
@@ -1170,14 +1170,13 @@ public void setStatusErrorMessage() {
     @Test
     public void setStatusErrorMessageNoDescription() {
         final Context span = openTelemetryTracer.start(METHOD_NAME, tracingContext);
-        openTelemetryTracer.end("error", null, span);
+        openTelemetryTracer.end("", null, span);
 
         SpanData spanData = getSpan(span).toSpanData();
         assertEquals(ERROR, spanData.getStatus().getStatusCode());
         assertEquals("", spanData.getStatus().getDescription());
     }
 
-
     @Test
     public void setStatusThrowable() {
         final Context span = openTelemetryTracer.start(METHOD_NAME, tracingContext);

diff --git a/sdk/core/azure-core/src/main/java/com/azure/core/implementation/http/rest/RestProxyBase.java b/sdk/core/azure-core/src/main/java/com/azure/core/implementation/http/rest/RestProxyBase.java
@@ -208,7 +208,7 @@ void endTracingSpan(HttpResponseDecoder.HttpDecodedResponse httpDecodedResponse,
         if (httpDecodedResponse != null) {
             //noinspection ConstantConditions
             int statusCode = httpDecodedResponse.getSourceResponse().getStatusCode();
-            tracer.end(statusCode >= 400 ? "error" : null, null, span);
+            tracer.end(statusCode >= 400 ? "" : null, null, span);
         }
     }
 

diff --git a/sdk/core/azure-core/src/main/java/com/azure/core/util/tracing/Tracer.java b/sdk/core/azure-core/src/main/java/com/azure/core/util/tracing/Tracer.java
@@ -297,7 +297,7 @@ default void end(int responseCode, Throwable error, Context context) {
     }
 
     /**
-     * Completes the current tracing span for AMQP calls.
+     * Completes span on the context.
      *
      * <p><strong>Code samples</strong></p>
      *
@@ -334,8 +334,9 @@ default void end(int responseCode, Throwable error, Context context) {
      * </pre>
      * <!-- end com.azure.core.util.tracing.end#exception -->
      *
-     * @param errorMessage The error message that occurred during the call, or {@code null} if no error
-     * occurred.
+     * @param errorMessage The error message that occurred during the call, or {@code null} if no error.
+     *   occurred. Any other non-null string indicates an error with description provided in {@code errorMessage}.
+     *
      * @param throwable {@link Throwable} that happened during the span or {@code null} if no exception occurred.
      * @param context Additional metadata that is passed through the call stack.
      * @throws NullPointerException if {@code context} is {@code null}.