From 84c0eb35c6a1f231301b11fdfa9780f041382ee5 Mon Sep 17 00:00:00 2001
From: Giovanni Liva <giovanni.liva@dynatrace.com>
Date: Fri, 20 Mar 2020 18:40:38 +0100
Subject: [PATCH] Adapt RecordEventsReadableSpan

---
 .../sdk/trace/RecordEventsReadableSpan.java   | 37 +++----------
 .../trace/RecordEventsReadableSpanTest.java   | 52 ++++++++++++++++++-
 2 files changed, 58 insertions(+), 31 deletions(-)

diff --git a/sdk/src/main/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpan.java b/sdk/src/main/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpan.java
index d3e60268a19..c4b075f4904 100644
--- a/sdk/src/main/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpan.java
+++ b/sdk/src/main/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpan.java
@@ -16,10 +16,11 @@
 
 package io.opentelemetry.sdk.trace;
 
+import static io.opentelemetry.common.AttributeValue.Type.STRING;
+
 import com.google.common.base.Preconditions;
 import com.google.common.collect.EvictingQueue;
 import io.opentelemetry.common.AttributeValue;
-import io.opentelemetry.internal.StringUtils;
 import io.opentelemetry.sdk.common.Clock;
 import io.opentelemetry.sdk.common.InstrumentationLibraryInfo;
 import io.opentelemetry.sdk.resources.Resource;
@@ -331,35 +332,11 @@ public void setAttribute(String key, boolean value) {
   @Override
   public void setAttribute(String key, AttributeValue value) {
     Preconditions.checkNotNull(key, "key");
-    Preconditions.checkNotNull(value, "value");
-    switch (value.getType()) {
-      case STRING:
-        if (StringUtils.isNullOrEmpty(value.getStringValue())) {
-          return;
-        }
-        break;
-      case STRING_ARRAY:
-        if (value.getStringArrayValue().size() == 0) {
-          return;
-        }
-        break;
-      case BOOLEAN_ARRAY:
-        if (value.getBooleanArrayValue().size() == 0) {
-          return;
-        }
-        break;
-      case LONG_ARRAY:
-        if (value.getLongArrayValue().size() == 0) {
-          return;
-        }
-        break;
-      case DOUBLE_ARRAY:
-        if (value.getDoubleArrayValue().size() == 0) {
-          return;
-        }
-        break;
-      default:
-        break;
+    if (value == null || (value.getType().equals(STRING) && value.getStringValue() == null)) {
+      synchronized (lock) {
+        attributes.remove(key);
+      }
+      return;
     }
     synchronized (lock) {
       if (hasEnded) {
diff --git a/sdk/src/test/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpanTest.java b/sdk/src/test/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpanTest.java
index a3ddab5c302..30feab4f5e3 100644
--- a/sdk/src/test/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpanTest.java
+++ b/sdk/src/test/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpanTest.java
@@ -311,7 +311,7 @@ public void setAttribute() {
       span.end();
     }
     SpanData spanData = span.toSpanData();
-    assertThat(spanData.getAttributes().size()).isEqualTo(8);
+    assertThat(spanData.getAttributes().size()).isEqualTo(14);
     assertThat(spanData.getAttributes().get("ArrayStringKey").getStringArrayValue().size())
         .isEqualTo(4);
     assertThat(spanData.getAttributes().get("ArrayLongKey").getLongArrayValue().size())
@@ -322,6 +322,56 @@ public void setAttribute() {
         .isEqualTo(4);
   }
 
+  @Test
+  public void setAttribute_emptyArrayAttributeValue() throws Exception {
+    RecordEventsReadableSpan span = createTestRootSpan();
+    span.setAttribute("stringArrayAttribute", AttributeValue.arrayAttributeValue(new String[0]));
+    span.setAttribute("boolArrayAttribute", AttributeValue.arrayAttributeValue(new Boolean[0]));
+    span.setAttribute("longArrayAttribute", AttributeValue.arrayAttributeValue(new Long[0]));
+    span.setAttribute("doubleArrayAttribute", AttributeValue.arrayAttributeValue(new Double[0]));
+    assertThat(span.toSpanData().getAttributes().size()).isEqualTo(4);
+  }
+
+  @Test
+  public void setAttribute_nullStringValue() throws Exception {
+    RecordEventsReadableSpan span = createTestRootSpan();
+    span.setAttribute("emptyString", "");
+    span.setAttribute("nullString", (String) null);
+    span.setAttribute("nullStringAttributeValue", AttributeValue.stringAttributeValue(null));
+    span.setAttribute("emptyStringAttributeValue", AttributeValue.stringAttributeValue(""));
+    assertThat(span.toSpanData().getAttributes().size()).isEqualTo(2);
+    span.setAttribute("emptyString", (String) null);
+    span.setAttribute("emptyStringAttributeValue", (String) null);
+    assertThat(span.toSpanData().getAttributes()).isEmpty();
+  }
+
+  @Test
+  public void setAttribute_nullAttributeValue() throws Exception {
+    RecordEventsReadableSpan span = createTestRootSpan();
+    span.setAttribute("emptyString", "");
+    span.setAttribute("nullString", (AttributeValue) null);
+    span.setAttribute("nullStringAttributeValue", AttributeValue.stringAttributeValue(null));
+    span.setAttribute("emptyStringAttributeValue", AttributeValue.stringAttributeValue(""));
+    span.setAttribute("longAttribute", 0L);
+    span.setAttribute("boolAttribute", false);
+    span.setAttribute("doubleAttribute", 0.12345f);
+    span.setAttribute("stringArrayAttribute", AttributeValue.arrayAttributeValue("", null));
+    span.setAttribute("boolArrayAttribute", AttributeValue.arrayAttributeValue(true, null));
+    span.setAttribute("longArrayAttribute", AttributeValue.arrayAttributeValue(12345L, null));
+    span.setAttribute("doubleArrayAttribute", AttributeValue.arrayAttributeValue(1.2345, null));
+    assertThat(span.toSpanData().getAttributes().size()).isEqualTo(9);
+    span.setAttribute("emptyString", (AttributeValue) null);
+    span.setAttribute("emptyStringAttributeValue", (AttributeValue) null);
+    span.setAttribute("longAttribute", (AttributeValue) null);
+    span.setAttribute("boolAttribute", (AttributeValue) null);
+    span.setAttribute("doubleAttribute", (AttributeValue) null);
+    span.setAttribute("stringArrayAttribute", (AttributeValue) null);
+    span.setAttribute("boolArrayAttribute", (AttributeValue) null);
+    span.setAttribute("longArrayAttribute", (AttributeValue) null);
+    span.setAttribute("doubleArrayAttribute", (AttributeValue) null);
+    assertThat(span.toSpanData().getAttributes()).isEmpty();
+  }
+
   @Test
   public void addEvent() {
     RecordEventsReadableSpan span = createTestRootSpan();