a bit of cleanup; fix a todo

open-telemetry · Sep 9, 2020 · 78de78f · 78de78f
1 parent a39dd09
commit 78de78f
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 22 deletions.
diff --git a/api/src/main/java/io/opentelemetry/common/Attributes.java b/api/src/main/java/io/opentelemetry/common/Attributes.java
@@ -145,6 +145,13 @@ public static <T, U, V, W, X> Attributes of(
   }
 
   private static Attributes sortAndFilterToAttributes(Object... data) {
+    // null out any empty keys
+    for (int i = 0; i < data.length; i += 2) {
+      AttributeKey<?> key = (AttributeKey<?>) data[i];
+      if (key != null && (key.get() == null || "".equals(key.get()))) {
+        data[i] = null;
+      }
+    }
     return new AutoValue_Attributes_ArrayBackedAttributes(sortAndFilter(data));
   }
 

diff --git a/api/src/main/java/io/opentelemetry/common/ImmutableKeyValuePairs.java b/api/src/main/java/io/opentelemetry/common/ImmutableKeyValuePairs.java
@@ -76,13 +76,13 @@ private static <K extends Comparable<K>> void quickSort(
       return;
     }
 
-    K pivotKey = (data[rightIndex] == null) ? null : (K) data[rightIndex];
+    K pivotKey = (K) data[rightIndex];
     int counter = leftIndex;
 
     for (int i = leftIndex; i <= rightIndex; i += 2) {
-      K value = data[i] == null ? null : (K) data[i];
+      K value = (K) data[i];
 
-      if (compareTo(value, pivotKey) <= 0) {
+      if (compareToNullSafe(value, pivotKey) <= 0) {
         swap(data, counter, i);
         counter += 2;
       }
@@ -92,14 +92,14 @@ private static <K extends Comparable<K>> void quickSort(
     quickSort(data, counter, rightIndex);
   }
 
-  private static <K extends Comparable<K>> int compareTo(K value, K pivotKey) {
-    if (value == null) {
+  private static <K extends Comparable<K>> int compareToNullSafe(K key, K pivotKey) {
+    if (key == null) {
       return pivotKey == null ? 0 : -1;
     }
     if (pivotKey == null) {
       return 1;
     }
-    return value.compareTo(pivotKey);
+    return key.compareTo(pivotKey);
   }
 
   private static List<Object> dedupe(Object[] data) {
@@ -112,18 +112,6 @@ private static List<Object> dedupe(Object[] data) {
       if (key == null) {
         continue;
       }
-      // TODO: this is super ugly. It needs to be fixed.
-      // Obviously, this isn't truly generic if we have to peek into the key types for these empty
-      // checks.
-      if (key instanceof String) {
-        if ("".equals(key)) {
-          continue;
-        }
-      } else if (key instanceof AttributeKey) {
-        if ("".equals(((AttributeKey) key).get())) {
-          continue;
-        }
-      }
       if (key.equals(previousKey)) {
         continue;
       }

diff --git a/api/src/main/java/io/opentelemetry/internal/StringUtils.java b/api/src/main/java/io/opentelemetry/internal/StringUtils.java
@@ -17,7 +17,7 @@
 package io.opentelemetry.internal;
 
 import io.opentelemetry.common.AttributeKey;
-import io.opentelemetry.common.AttributeKeyImpl;
+import io.opentelemetry.common.AttributeType;
 import java.util.ArrayList;
 import java.util.List;
 import javax.annotation.Nullable;
@@ -78,12 +78,12 @@ public static <T> T truncateToSize(AttributeKey<T> key, T value, int limit) {
     Utils.checkArgument(limit > 0, "attribute value limit must be positive, got %d", limit);
 
     if (value == null
-        || (!(key instanceof AttributeKeyImpl.StringKey)
-            && !(key instanceof AttributeKeyImpl.StringArrayKey))) {
+        || ((key.getType() != AttributeType.STRING)
+            && (key.getType() != AttributeType.STRING_ARRAY))) {
       return value;
     }
 
-    if (key instanceof AttributeKeyImpl.StringArrayKey) {
+    if (key.getType() == AttributeType.STRING_ARRAY) {
       List<String> strings = (List<String>) value;
       if (strings.isEmpty()) {
         return value;