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

LetzNico · web-flow · commit ddf7eebba303 · 2020-10-26T10:59:02.000-07:00
diff --git a/opentelemetry-api/src/opentelemetry/util/types.py b/opentelemetry-api/src/opentelemetry/util/types.py
@@ -20,10 +20,10 @@
     bool,
     int,
     float,
-    Sequence[str],
-    Sequence[bool],
-    Sequence[int],
-    Sequence[float],
+    Sequence[Union[None, str]],
+    Sequence[Union[None, bool]],
+    Sequence[Union[None, int]],
+    Sequence[Union[None, float]],
 ]
 Attributes = Optional[Mapping[str, AttributeValue]]
 AttributesFormatter = Callable[[], Attributes]
diff --git a/opentelemetry-sdk/CHANGELOG.md b/opentelemetry-sdk/CHANGELOG.md
@@ -6,6 +6,8 @@
   ([#1251](https://github.com/open-telemetry/opentelemetry-python/pull/1251))
 - Fix b3 propagator entrypoint
   ([#1265](https://github.com/open-telemetry/opentelemetry-python/pull/1265))
+- Allow None in sequence attributes values
+  ([#998](https://github.com/open-telemetry/opentelemetry-python/pull/998))
 
 ## Version 0.14b0
 
diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
@@ -308,34 +308,44 @@ 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 the same valid type or None
+      - are not a sequence
     """
 
     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
-
-        for element in list(value)[1:]:
-            if not isinstance(element, first_element_type):
+        sequence_first_valid_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
+            # The type of the sequence must be homogeneous. The first non-None
+            # element determines the type of the sequence
+            if sequence_first_valid_type is None:
+                sequence_first_valid_type = element_type
+            elif 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,14 @@ 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(["A", None, None, "B"])
+        )
+        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 = {
