sdk: Improve attributes validation (#460)

4fca8c9 ("Add runtime validation in setAttribute (#348)") added a robust
attribute validation using numbers.Number to validate numeric types.
Although the approach is correct, it presents some complications because
Complex, Fraction and Decimal are accepted because they are Numbers. This
presents a problem to the exporters because they will have to consider all
these different cases when converting attributes to the underlying exporter
representation.

This commit simplifies the logic by accepting only int and float as numeric
values.

Author: mauriciovasquezbernal

opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py

@@ -18,7 +18,6 @@
 import random
 import threading
 from contextlib import contextmanager
-from numbers import Number
 from types import TracebackType
 from typing import Iterator, Optional, Sequence, Tuple, Type
 
@@ -234,12 +233,16 @@ def set_attribute(self, key: str, value: types.AttributeValue) -> None:
             logger.warning("Setting attribute on ended span.")
             return
 
+        if not key:
+            logger.warning("invalid key (empty or null)")
+            return
+
         if isinstance(value, Sequence):
             error_message = self._check_attribute_value_sequence(value)
             if error_message is not None:
                 logger.warning("%s in attribute value sequence", error_message)
                 return
-        elif not isinstance(value, (bool, str, Number, Sequence)):
+        elif not isinstance(value, (bool, str, int, float)):
             logger.warning("invalid type for attribute value")
             return
 
@@ -255,10 +258,7 @@ def _check_attribute_value_sequence(sequence: Sequence) -> Optional[str]:
 
         first_element_type = type(sequence[0])
 
-        if issubclass(first_element_type, Number):
-            first_element_type = Number
-
-        if first_element_type not in (bool, str, Number):
+        if first_element_type not in (bool, str, int, float):
             return "invalid type"
 
         for element in sequence:

opentelemetry-sdk/tests/trace/test_trace.py

@@ -405,7 +405,7 @@ def test_attributes(self):
 
             root.set_attribute("empty-list", [])
             root.set_attribute("list-of-bools", [True, True, False])
-            root.set_attribute("list-of-numerics", [123, 3.14, 0])
+            root.set_attribute("list-of-numerics", [123, 314, 0])
 
             self.assertEqual(len(root.attributes), 10)
             self.assertEqual(root.attributes["component"], "http")
@@ -423,7 +423,7 @@ def test_attributes(self):
                 root.attributes["list-of-bools"], [True, True, False]
             )
             self.assertEqual(
-                root.attributes["list-of-numerics"], [123, 3.14, 0]
+                root.attributes["list-of-numerics"], [123, 314, 0]
             )
 
         attributes = {
@@ -454,6 +454,9 @@ def test_invalid_attribute_values(self):
                 "list-with-non-primitive-data-type", [dict(), 123]
             )
 
+            root.set_attribute("", 123)
+            root.set_attribute(None, 123)
+
             self.assertEqual(len(root.attributes), 0)
 
     def test_check_sequence_helper(self):
@@ -472,8 +475,18 @@ def test_check_sequence_helper(self):
             ),
             "different type",
         )
+        self.assertEqual(
+            trace.Span._check_attribute_value_sequence([1, 2, 3.4, 5]),
+            "different type",
+        )
+        self.assertIsNone(
+            trace.Span._check_attribute_value_sequence([1, 2, 3, 5])
+        )
+        self.assertIsNone(
+            trace.Span._check_attribute_value_sequence([1.2, 2.3, 3.4, 4.5])
+        )
         self.assertIsNone(
-            trace.Span._check_attribute_value_sequence([1, 2, 3.4, 5])
+            trace.Span._check_attribute_value_sequence([True, False])
         )
         self.assertIsNone(
             trace.Span._check_attribute_value_sequence(["ss", "dw", "fw"])