Apply validation of attributes to Resource, move attribute related logic to util package

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

@@ -65,6 +65,9 @@
 import pkg_resources
 
 from opentelemetry.sdk.environment_variables import OTEL_RESOURCE_ATTRIBUTES
+from opentelemetry.sdk.util import (
+    _filter_attributes,
+)
 from opentelemetry.semconv.resource import ResourceAttributes
 
 LabelValue = typing.Union[str, bool, int, float]
@@ -138,6 +141,7 @@ class Resource:
     """A Resource is an immutable representation of the entity producing telemetry as Attributes."""
 
     def __init__(self, attributes: Attributes):
+        _filter_attributes(attributes)
         self._attributes = attributes.copy()
 
     @staticmethod

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

@@ -48,7 +48,13 @@
 from opentelemetry.sdk.resources import Resource
 from opentelemetry.sdk.trace import sampling
 from opentelemetry.sdk.trace.id_generator import IdGenerator, RandomIdGenerator
-from opentelemetry.sdk.util import BoundedDict, BoundedList
+from opentelemetry.sdk.util import (
+    BoundedDict,
+    BoundedList,
+    _create_immutable_attributes,
+    _filter_attributes,
+    _is_valid_attribute_value,
+)
 from opentelemetry.sdk.util.instrumentation import InstrumentationInfo
 from opentelemetry.trace import SpanContext
 from opentelemetry.trace.propagation import SPAN_KEY
@@ -64,7 +70,6 @@
 
 _SPAN_EVENT_COUNT_LIMIT = int(environ.get(OTEL_SPAN_EVENT_COUNT_LIMIT, 128))
 _SPAN_LINK_COUNT_LIMIT = int(environ.get(OTEL_SPAN_LINK_COUNT_LIMIT, 128))
-_VALID_ATTR_VALUE_TYPES = (bool, str, int, float)
 # pylint: disable=protected-access
 _TRACE_SAMPLER = sampling._get_from_env_or_default()
 
@@ -315,72 +320,6 @@ def attributes(self) -> types.Attributes:
         return self._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 the same valid type or None
-      - are not a sequence
-    """
-
-    if isinstance(value, Sequence):
-        if len(value) == 0:
-            return True
-
-        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",
-                    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 "
-            "sequence of those types",
-            type(value).__name__,
-            [valid_type.__name__ for valid_type in _VALID_ATTR_VALUE_TYPES],
-        )
-        return False
-    return True
-
-
-def _filter_attribute_values(attributes: types.Attributes):
-    if attributes:
-        for attr_key, attr_value in list(attributes.items()):
-            if _is_valid_attribute_value(attr_value):
-                if isinstance(attr_value, MutableSequence):
-                    attributes[attr_key] = tuple(attr_value)
-            else:
-                attributes.pop(attr_key)
-
-
-def _create_immutable_attributes(attributes):
-    return MappingProxyType(attributes.copy() if attributes else {})
-
-
 def _check_span_ended(func):
     def wrapper(self, *args, **kwargs):
         already_ended = False
@@ -619,7 +558,7 @@ def __init__(
         self._span_processor = span_processor
         self._lock = threading.Lock()
 
-        _filter_attribute_values(attributes)
+        _filter_attributes(attributes)
         if not attributes:
             self._attributes = self._new_attributes()
         else:
@@ -630,7 +569,7 @@ def __init__(
         self._events = self._new_events()
         if events:
             for event in events:
-                _filter_attribute_values(event.attributes)
+                _filter_attributes(event.attributes)
                 # pylint: disable=protected-access
                 event._attributes = _create_immutable_attributes(
                     event.attributes
@@ -702,7 +641,7 @@ def add_event(
         attributes: types.Attributes = None,
         timestamp: Optional[int] = None,
     ) -> None:
-        _filter_attribute_values(attributes)
+        _filter_attributes(attributes)
         attributes = _create_immutable_attributes(attributes)
         self._add_event(
             Event(

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

@@ -13,9 +13,96 @@
 # limitations under the License.
 
 import datetime
+import logging
 import threading
 from collections import OrderedDict, deque
 from collections.abc import MutableMapping, Sequence
+from types import MappingProxyType
+from typing import MutableSequence
+
+from opentelemetry.util import types
+
+_VALID_ATTR_VALUE_TYPES = (bool, str, int, float)
+
+
+_logger = logging.getLogger(__name__)
+
+
+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 the same valid type or None
+      - are not a sequence
+    """
+
+    if isinstance(value, Sequence):
+        if len(value) == 0:
+            return True
+
+        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",
+                    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 "
+            "sequence of those types",
+            type(value).__name__,
+            [valid_type.__name__ for valid_type in _VALID_ATTR_VALUE_TYPES],
+        )
+        return False
+    return True
+
+
+def _filter_attributes(attributes: types.Attributes):
+    if attributes:
+        for attr_key, attr_value in list(attributes.items()):
+            if not attr_key:
+                _logger.warning("invalid key `%s` (empty or null)", attr_key)
+                attributes.pop(attr_key)
+                continue
+
+            if _is_valid_attribute_value(attr_value):
+                if isinstance(attr_value, MutableSequence):
+                    attributes[attr_key] = tuple(attr_value)
+                if isinstance(attr_value, bytes):
+                    try:
+                        attributes[attr_key] = attr_value.decode()
+                    except ValueError:
+                        attributes.pop(attr_key)
+                        _logger.warning("Byte attribute could not be decoded.")
+            else:
+                attributes.pop(attr_key)
+
+
+def _create_immutable_attributes(attributes):
+    return MappingProxyType(attributes.copy() if attributes else {})
 
 
 def ns_to_iso_str(nanoseconds):

opentelemetry-sdk/tests/resources/test_resources.py

@@ -16,6 +16,7 @@
 
 import os
 import unittest
+import uuid
 from unittest import mock
 
 from opentelemetry.sdk import resources
@@ -138,6 +139,25 @@ def test_service_name_using_process_name(self):
             "unknown_service:test",
         )
 
+    def test_invalid_resource_attribute_values(self):
+        resource = resources.Resource(
+            {
+                resources.SERVICE_NAME: "test",
+                "non-primitive-data-type": dict(),
+                "invalid-byte-type-attribute": b"\xd8\xe1\xb7\xeb\xa8\xe5 \xd2\xb7\xe1",
+                "": "empty-key-value",
+                None: "null-key-value",
+                "another-non-primitive": uuid.uuid4(),
+            }
+        )
+        self.assertEqual(
+            resource.attributes,
+            {
+                resources.SERVICE_NAME: "test",
+            },
+        )
+        self.assertEqual(len(resource.attributes), 1)
+
     def test_aggregated_resources_no_detectors(self):
         aggregated_resources = resources.get_aggregated_resources([])
         self.assertEqual(aggregated_resources, resources.Resource.get_empty())

opentelemetry-sdk/tests/trace/test_trace.py

@@ -35,7 +35,7 @@
 )
 from opentelemetry.sdk.trace import Resource, sampling
 from opentelemetry.sdk.trace.id_generator import RandomIdGenerator
-from opentelemetry.sdk.util import ns_to_iso_str
+from opentelemetry.sdk.util import _is_valid_attribute_value, ns_to_iso_str
 from opentelemetry.sdk.util.instrumentation import InstrumentationInfo
 from opentelemetry.trace import StatusCode
 from opentelemetry.util._time import _time_ns
@@ -647,32 +647,26 @@ def test_byte_type_attribute_value(self):
 
     def test_check_attribute_helper(self):
         # pylint: disable=protected-access
-        self.assertFalse(trace._is_valid_attribute_value([1, 2, 3.4, "ss", 4]))
-        self.assertFalse(
-            trace._is_valid_attribute_value([dict(), 1, 2, 3.4, 4])
-        )
-        self.assertFalse(
-            trace._is_valid_attribute_value(["sw", "lf", 3.4, "ss"])
-        )
-        self.assertFalse(trace._is_valid_attribute_value([1, 2, 3.4, 5]))
-        self.assertTrue(trace._is_valid_attribute_value([1, 2, 3, 5]))
-        self.assertTrue(trace._is_valid_attribute_value([1.2, 2.3, 3.4, 4.5]))
-        self.assertTrue(trace._is_valid_attribute_value([True, False]))
-        self.assertTrue(trace._is_valid_attribute_value(["ss", "dw", "fw"]))
-        self.assertTrue(trace._is_valid_attribute_value([]))
-        self.assertFalse(trace._is_valid_attribute_value(dict()))
-        self.assertTrue(trace._is_valid_attribute_value(True))
-        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))
+        self.assertFalse(_is_valid_attribute_value([1, 2, 3.4, "ss", 4]))
+        self.assertFalse(_is_valid_attribute_value([dict(), 1, 2, 3.4, 4]))
+        self.assertFalse(_is_valid_attribute_value(["sw", "lf", 3.4, "ss"]))
+        self.assertFalse(_is_valid_attribute_value([1, 2, 3.4, 5]))
+        self.assertTrue(_is_valid_attribute_value([1, 2, 3, 5]))
+        self.assertTrue(_is_valid_attribute_value([1.2, 2.3, 3.4, 4.5]))
+        self.assertTrue(_is_valid_attribute_value([True, False]))
+        self.assertTrue(_is_valid_attribute_value(["ss", "dw", "fw"]))
+        self.assertTrue(_is_valid_attribute_value([]))
+        self.assertFalse(_is_valid_attribute_value(dict()))
+        self.assertTrue(_is_valid_attribute_value(True))
+        self.assertTrue(_is_valid_attribute_value("hi"))
+        self.assertTrue(_is_valid_attribute_value(3.4))
+        self.assertTrue(_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]))
+        self.assertTrue(_is_valid_attribute_value(["A", None, None]))
+        self.assertTrue(_is_valid_attribute_value(["A", None, None, "B"]))
+        self.assertTrue(_is_valid_attribute_value([None, None]))
+        self.assertFalse(_is_valid_attribute_value(["A", None, 1]))
+        self.assertFalse(_is_valid_attribute_value([None, "A", None, 1]))
 
     def test_sampling_attributes(self):
         sampling_attributes = {