Allow None in sequence attributes values (open-telemetry#998)

LetzNico · Oct 17, 2020 · fe04503 · fe04503
1 parent 937863f
commit fe04503
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 16 deletions.
diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
@@ -308,34 +308,41 @@ def attributes(self) -> types.Attributes:
 def _is_valid_attribute_value(value: types.AttributeValue) -> bool:
     """Checks if attribute value is valid.
 
-    An attribute value is valid if it is one of the valid types. If the value
-    is a sequence, it is only valid if all items in the sequence are of valid
-    type, not a sequence, and are of the same type.
+    An attribute value is valid if it is one of the valid types.
+    If the value is a sequence, it is only valid if:
+      - All items in the sequence are of valid type or None
+      - All items in the sequence are not a sequence
+      - All items in the sequence are of the same type or None
     """
 
     if isinstance(value, Sequence):
         if len(value) == 0:
             return True
 
-        first_element_type = type(value[0])
-
-        if first_element_type not in VALID_ATTR_VALUE_TYPES:
-            logger.warning(
-                "Invalid type %s in attribute value sequence. Expected one of "
-                "%s or a sequence of those types",
-                first_element_type.__name__,
-                [valid_type.__name__ for valid_type in VALID_ATTR_VALUE_TYPES],
-            )
-            return False
+        sequence_first_valid_type = type(None)
+        for element in value:
+            if element is None:
+                continue
+            element_type = type(element)
+            if element_type not in VALID_ATTR_VALUE_TYPES:
+                logger.warning(
+                    "Invalid type %s in attribute value sequence. Expected one of "
+                    "%s or None",
+                    element_type.__name__,
+                    [valid_type.__name__ for valid_type in VALID_ATTR_VALUE_TYPES],
+                )
+                return False
+            if sequence_first_valid_type is type(None):
+                sequence_first_valid_type = element_type
 
-        for element in list(value)[1:]:
-            if not isinstance(element, first_element_type):
+            if not isinstance(element, sequence_first_valid_type):
                 logger.warning(
                     "Mixed types %s and %s in attribute value sequence",
-                    first_element_type.__name__,
+                    sequence_first_valid_type.__name__,
                     type(element).__name__,
                 )
                 return False
+
     elif not isinstance(value, VALID_ATTR_VALUE_TYPES):
         logger.warning(
             "Invalid type %s for attribute value. Expected one of %s or a "

diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py
@@ -564,6 +564,11 @@ def test_check_attribute_helper(self):
         self.assertTrue(trace._is_valid_attribute_value("hi"))
         self.assertTrue(trace._is_valid_attribute_value(3.4))
         self.assertTrue(trace._is_valid_attribute_value(15))
+        # None in sequences are valid
+        self.assertTrue(trace._is_valid_attribute_value(["A", None, None]))
+        self.assertTrue(trace._is_valid_attribute_value([None, None]))
+        self.assertFalse(trace._is_valid_attribute_value(["A", None, 1]))
+        self.assertFalse(trace._is_valid_attribute_value([None, "A", None, 1]))
 
     def test_sampling_attributes(self):
         sampling_attributes = {