Filter attributes on lazy event add (#1014)

Author: lzchen

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

@@ -305,7 +305,11 @@ def __init__(
 
     @property
     def attributes(self) -> types.Attributes:
-        return self._event_formatter()
+        attributes = self._event_formatter()
+        _filter_attribute_values(attributes)
+        if not attributes:
+            attributes = Span._new_attributes()
+        return attributes
 
 
 def _is_valid_attribute_value(value: types.AttributeValue) -> bool:
@@ -350,6 +354,16 @@ def _is_valid_attribute_value(value: types.AttributeValue) -> bool:
     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)
+
+
 class Span(trace_api.Span):
     """See `opentelemetry.trace.Span`.
 
@@ -401,7 +415,7 @@ def __init__(
         self.status = None
         self._lock = threading.Lock()
 
-        self._filter_attribute_values(attributes)
+        _filter_attribute_values(attributes)
         if not attributes:
             self.attributes = self._new_attributes()
         else:
@@ -412,7 +426,7 @@ def __init__(
         self.events = self._new_events()
         if events:
             for event in events:
-                self._filter_attribute_values(event.attributes)
+                _filter_attribute_values(event.attributes)
                 self.events.append(event)
 
         if links is None:
@@ -553,18 +567,6 @@ def set_attribute(self, key: str, value: types.AttributeValue) -> None:
             with self._lock:
                 self.attributes[key] = value
 
-    @staticmethod
-    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[attr_key] = attr_value
-                else:
-                    attributes.pop(attr_key)
-
     def _add_event(self, event: EventBase) -> None:
         with self._lock:
             if not self.is_recording_events():
@@ -582,7 +584,7 @@ def add_event(
         attributes: types.Attributes = None,
         timestamp: Optional[int] = None,
     ) -> None:
-        self._filter_attribute_values(attributes)
+        _filter_attribute_values(attributes)
         if not attributes:
             attributes = self._new_attributes()
         self._add_event(

opentelemetry-sdk/tests/trace/test_trace.py

@@ -610,6 +610,7 @@ def event_formatter():
 
     def test_invalid_event_attributes(self):
         self.assertEqual(trace_api.get_current_span(), trace_api.INVALID_SPAN)
+        now = time_ns()
 
         with self.tracer.start_as_current_span("root") as root:
             root.add_event("event0", {"attr1": True, "attr2": ["hi", False]})
@@ -623,6 +624,19 @@ def test_invalid_event_attributes(self):
             self.assertEqual(root.events[2].attributes, {})
             self.assertEqual(root.events[3].attributes, {"attr2": (1, 2)})
 
+            def event_formatter():
+                properties = {}
+                properties["attr1"] = dict()
+                properties["attr2"] = "hello"
+                return properties
+
+            root.add_lazy_event("event4", event_formatter, now)
+
+            self.assertEqual(len(root.events), 5)
+            self.assertEqual(root.events[4].name, "event4")
+            self.assertEqual(root.events[4].attributes, {"attr2": "hello"})
+            self.assertEqual(root.events[4].timestamp, now)
+
     def test_links(self):
         other_context1 = trace_api.SpanContext(
             trace_id=trace.generate_trace_id(),