improve attribute filtering logic

Author: Andrew Xue

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

@@ -248,30 +248,16 @@ def __init__(
         if attributes is None:
             self.attributes = Span._empty_attributes
         else:
-            self.attributes = BoundedDict(MAX_NUM_ATTRIBUTES)
-            for attr_key, attr_value in attributes.items():
-                error_message = self._check_attribute_value(attr_value)
-                if error_message:
-                    logger.warning(error_message)
-                else:
-                    self.attributes[attr_key] = attr_value
+            self._filter_attribute_values(attributes)
+            self.attributes = BoundedDict.from_map(MAX_NUM_ATTRIBUTES, attributes)
 
         if events is None:
             self.events = Span._empty_events
         else:
             self.events = BoundedList(MAX_NUM_EVENTS)
             for event in events:
-                good_event = True
-                for attr_key, attr_value in list(event.attributes.items()):
-                    error_message = self._check_attribute_value(attr_value)
-                    if error_message:
-                        logger.warning(error_message)
-                        good_event = False
-                        break
-                    if isinstance(attr_value, MutableSequence):
-                        attributes[attr_key] = tuple(attr_value)
-                if good_event:
-                    self.events.append(event)
+                self._filter_attribute_values(event.attributes)
+                self.events.append(event)
 
         if links is None:
             self.links = Span._empty_links
@@ -428,6 +414,17 @@ def _check_attribute_value(value: types.AttributeValue) -> Optional[str]:
                 [valid_type.__name__ for valid_type in valid_types],
             )
 
+    def _filter_attribute_values(self, attributes: types.Attributes):
+        for attr_key, attr_value in list(attributes.items()):
+            error_message = self._check_attribute_value(attr_value)
+            if error_message:
+                attributes.pop(attr_key)
+                logger.warning(error_message)
+            elif isinstance(attr_value, MutableSequence):
+                attributes[attr_key] = tuple(attr_value)
+            else:
+                attributes[attr_key] = attr_value
+
     def _add_event(self, event: EventBase) -> None:
         with self._lock:
             if not self.is_recording_events():
@@ -450,13 +447,7 @@ def add_event(
         if attributes is None:
             attributes = Span._empty_attributes
         else:
-            for attr_key, attr_value in list(attributes.items()):
-                error_message = self._check_attribute_value(attr_value)
-                if error_message:
-                    logger.warning(error_message)
-                    return
-                if isinstance(attr_value, MutableSequence):
-                    attributes[attr_key] = tuple(attr_value)
+            self._filter_attribute_values(attributes)
         self._add_event(
             Event(
                 name=name,

opentelemetry-sdk/tests/trace/test_trace.py

@@ -619,9 +619,13 @@ def test_invalid_event_attributes(self):
             root.add_event("event0", {"attr1": True, "attr2": ["hi", False]})
             root.add_event("event0", {"attr1": dict()})
             root.add_event("event0", {"attr1": [[True]]})
-            root.add_event("event0", {"attr1": [dict()]})
+            root.add_event("event0", {"attr1": [dict()], "attr2": [1, 2]})
 
-            self.assertEqual(len(root.events), 0)
+            self.assertEqual(len(root.events), 4)
+            self.assertEqual(root.events[0].attributes, {"attr1": True})
+            self.assertEqual(root.events[1].attributes, {})
+            self.assertEqual(root.events[2].attributes, {})
+            self.assertEqual(root.events[3].attributes, {"attr2": (1, 2)})
 
     def test_links(self):
         other_context1 = trace_api.SpanContext(