Do not allow null description in the StatusData (#2896)

Signed-off-by: Bogdan Drutu <[email protected]>
open-telemetry · Feb 22, 2021 · 9e1c76b · 9e1c76b
1 parent 013643c
commit 9e1c76b
Show file tree

Hide file tree

Showing 8 changed files with 19 additions and 22 deletions.
diff --git a/api/all/src/main/java/io/opentelemetry/api/trace/Span.java b/api/all/src/main/java/io/opentelemetry/api/trace/Span.java
@@ -278,7 +278,7 @@ default Span addEvent(String name, Attributes attributes, Instant timestamp) {
    * @return this.
    */
   default Span setStatus(StatusCode statusCode) {
-    return setStatus(statusCode, null);
+    return setStatus(statusCode, "");
   }
 
   /**

diff --git a/exporters/jaeger-thrift/src/main/java/io/opentelemetry/exporter/jaeger/thrift/Adapter.java b/exporters/jaeger-thrift/src/main/java/io/opentelemetry/exporter/jaeger/thrift/Adapter.java
@@ -96,7 +96,7 @@ static Span toJaeger(SpanData span) {
               .setVStr(span.getKind().name().toLowerCase(Locale.ROOT)));
     }
 
-    if (span.getStatus().getDescription() != null) {
+    if (!span.getStatus().getDescription().isEmpty()) {
       tags.add(
           new Tag(KEY_SPAN_STATUS_MESSAGE, TagType.STRING)
               .setVStr(span.getStatus().getDescription()));

diff --git a/exporters/jaeger/src/main/java/io/opentelemetry/exporter/jaeger/Adapter.java b/exporters/jaeger/src/main/java/io/opentelemetry/exporter/jaeger/Adapter.java
@@ -96,7 +96,7 @@ static Model.Span toJaeger(SpanData span) {
               .build());
     }
 
-    if (span.getStatus().getDescription() != null) {
+    if (!span.getStatus().getDescription().isEmpty()) {
       target.addTags(
           Model.KeyValue.newBuilder()
               .setKey(KEY_SPAN_STATUS_MESSAGE)

diff --git a/exporters/otlp/common/src/main/java/io/opentelemetry/exporter/otlp/internal/SpanAdapter.java b/exporters/otlp/common/src/main/java/io/opentelemetry/exporter/otlp/internal/SpanAdapter.java
@@ -168,7 +168,7 @@ static Status toStatusProto(StatusData status) {
     @SuppressWarnings("deprecation") // setDeprecatedCode is deprecated.
     Status.Builder builder =
         Status.newBuilder().setCode(protoStatusCode).setDeprecatedCode(deprecatedStatusCode);
-    if (status.getDescription() != null) {
+    if (!status.getDescription().isEmpty()) {
       builder.setMessage(status.getDescription());
     }
     return builder.build();

diff --git a/exporters/zipkin/src/test/java/io/opentelemetry/exporter/zipkin/ZipkinSpanExporterTest.java b/exporters/zipkin/src/test/java/io/opentelemetry/exporter/zipkin/ZipkinSpanExporterTest.java
@@ -278,12 +278,12 @@ void generateSpan_WithRpcTimeoutErrorStatus_WithTimeoutErrorDescription() {
   }
 
   @Test
-  void generateSpan_WithRpcErrorStatus_WithNullErrorDescription() {
+  void generateSpan_WithRpcErrorStatus_WithEmptyErrorDescription() {
     Attributes attributeMap = Attributes.of(SemanticAttributes.RPC_SERVICE, "my service name");
 
     SpanData data =
         buildStandardSpan()
-            .setStatus(StatusData.create(StatusCode.ERROR, null))
+            .setStatus(StatusData.create(StatusCode.ERROR, ""))
             .setAttributes(attributeMap)
             .build();
 
@@ -302,7 +302,7 @@ void generateSpan_WithRpcUnsetStatus() {
 
     SpanData data =
         buildStandardSpan()
-            .setStatus(StatusData.create(StatusCode.UNSET, null))
+            .setStatus(StatusData.create(StatusCode.UNSET, ""))
             .setAttributes(attributeMap)
             .build();
 

diff --git a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/data/ImmutableStatusData.java b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/data/ImmutableStatusData.java
@@ -9,7 +9,6 @@
 import io.opentelemetry.api.trace.Span;
 import io.opentelemetry.api.trace.StatusCode;
 import java.util.EnumMap;
-import javax.annotation.Nullable;
 import javax.annotation.concurrent.Immutable;
 
 /**
@@ -25,13 +24,13 @@ abstract class ImmutableStatusData implements StatusData {
    * The operation has been validated by an Application developers or Operator to have completed
    * successfully.
    */
-  static final StatusData OK = createInternal(StatusCode.OK, null);
+  static final StatusData OK = createInternal(StatusCode.OK, "");
 
   /** The default status. */
-  static final StatusData UNSET = createInternal(StatusCode.UNSET, null);
+  static final StatusData UNSET = createInternal(StatusCode.UNSET, "");
 
   /** The operation contains an error. */
-  static final StatusData ERROR = createInternal(StatusCode.ERROR, null);
+  static final StatusData ERROR = createInternal(StatusCode.ERROR, "");
 
   // Visible for test
   static final EnumMap<StatusCode, StatusData> codeToStatus = new EnumMap<>(StatusCode.class);
@@ -47,7 +46,7 @@ abstract class ImmutableStatusData implements StatusData {
     for (StatusCode code : codes) {
       StatusData status = codeToStatus.get(code);
       if (status == null) {
-        codeToStatus.put(code, createInternal(code, null));
+        codeToStatus.put(code, createInternal(code, ""));
       }
     }
   }
@@ -58,14 +57,14 @@ abstract class ImmutableStatusData implements StatusData {
    * @param description the new description of the {@code Status}.
    * @return The newly created {@code Status} with the given description.
    */
-  static StatusData create(StatusCode statusCode, @Nullable String description) {
-    if (description == null) {
+  static StatusData create(StatusCode statusCode, String description) {
+    if (description == null || description.isEmpty()) {
       return codeToStatus.get(statusCode);
     }
     return createInternal(statusCode, description);
   }
 
-  private static StatusData createInternal(StatusCode statusCode, @Nullable String description) {
+  private static StatusData createInternal(StatusCode statusCode, String description) {
     return new AutoValue_ImmutableStatusData(statusCode, description);
   }
 }
diff --git a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/data/StatusData.java b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/data/StatusData.java
@@ -7,7 +7,6 @@
 
 import io.opentelemetry.api.trace.Span;
 import io.opentelemetry.api.trace.StatusCode;
-import javax.annotation.Nullable;
 import javax.annotation.concurrent.Immutable;
 
 /**
@@ -39,7 +38,7 @@ static StatusData error() {
    * Returns a {@link StatusData} with the given {@code code} and {@code description}. If {@code
    * description} is {@code null}, the returned {@link StatusData} does not have a description.
    */
-  static StatusData create(StatusCode code, @Nullable String description) {
+  static StatusData create(StatusCode code, String description) {
     return ImmutableStatusData.create(code, description);
   }
 
@@ -51,6 +50,5 @@ static StatusData create(StatusCode code, @Nullable String description) {
    *
    * @return the description of this {@code Status}.
    */
-  @Nullable
   String getDescription();
 }
diff --git a/sdk/trace/src/test/java/io/opentelemetry/sdk/trace/data/ImmutableStatusDataTest.java b/sdk/trace/src/test/java/io/opentelemetry/sdk/trace/data/ImmutableStatusDataTest.java
@@ -17,11 +17,11 @@ void defaultConstants() {
     StatusCode[] codes = StatusCode.values();
     assertThat(codes).hasSize(3);
     assertThat(StatusData.unset().getStatusCode()).isEqualTo(StatusCode.UNSET);
-    assertThat(StatusData.unset().getDescription()).isNull();
+    assertThat(StatusData.unset().getDescription()).isEmpty();
     assertThat(StatusData.ok().getStatusCode()).isEqualTo(StatusCode.OK);
-    assertThat(StatusData.ok().getDescription()).isNull();
+    assertThat(StatusData.ok().getDescription()).isEmpty();
     assertThat(StatusData.error().getStatusCode()).isEqualTo(StatusCode.ERROR);
-    assertThat(StatusData.error().getDescription()).isNull();
+    assertThat(StatusData.error().getDescription()).isEmpty();
   }
 
   @Test
@@ -31,7 +31,7 @@ void generateCodeToStatus() {
     for (StatusCode code : codes) {
       assertThat(ImmutableStatusData.codeToStatus.get(code)).isNotNull();
       assertThat(ImmutableStatusData.codeToStatus.get(code).getStatusCode()).isEqualTo(code);
-      assertThat(ImmutableStatusData.codeToStatus.get(code).getDescription()).isNull();
+      assertThat(ImmutableStatusData.codeToStatus.get(code).getDescription()).isEmpty();
     }
   }
 }