diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index dfb1534ba40..91eb4d6325d 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -248,14 +248,30 @@ def __init__( if attributes is None: self.attributes = Span._empty_attributes else: - self.attributes = BoundedDict.from_map( - MAX_NUM_ATTRIBUTES, attributes - ) + 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 if events is None: self.events = Span._empty_events else: - self.events = BoundedList.from_seq(MAX_NUM_EVENTS, events) + 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) if links is None: self.links = Span._empty_links @@ -387,23 +403,30 @@ def _check_attribute_value(value: types.AttributeValue) -> Optional[str]: """ Checks if sequence items are valid and are of the same type """ + valid_types = (bool, str, int, float) if isinstance(value, Sequence): if len(value) == 0: return None first_element_type = type(value[0]) - if first_element_type not in (bool, str, int, float): - return "invalid type in attribute value sequence" + if first_element_type not in valid_types: + return "Invalid type {} in attribute value sequence. Expected one of {}".format( + first_element_type.__name__, + [valid_type.__name__ for valid_type in valid_types], + ) for element in value: if not isinstance(element, first_element_type): - return "different types in attribute value sequence" + return "Mixed types {} and {} in attribute value sequence".format( + first_element_type.__name__, type(element).__name__ + ) return None - elif not isinstance(value, (bool, str, int, float)): - return "invalid type for attribute value" - return None - + elif not isinstance(value, valid_types): + return "Invalid type {} for attribute value. Expected one of {}".format( + type(value).__name__, + [valid_type.__name__ for valid_type in valid_types], + ) def _add_event(self, event: EventBase) -> None: with self._lock: diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index 28de157233e..71e3046c0c7 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -491,56 +491,38 @@ def test_check_attribute_helper(self): # pylint: disable=protected-access self.assertEqual( trace.Span._check_attribute_value([1, 2, 3.4, "ss", 4]), - "different types in attribute value sequence", + "Mixed types int and float in attribute value sequence", ) self.assertEqual( trace.Span._check_attribute_value([dict(), 1, 2, 3.4, 4]), - "invalid type in attribute value sequence", + "Invalid type dict in attribute value sequence. Expected one of ['bool', 'str', 'int', 'float']", ) self.assertEqual( - trace.Span._check_attribute_value( - ["sw", "lf", 3.4, "ss"] - ), - "different types in attribute value sequence", + trace.Span._check_attribute_value(["sw", "lf", 3.4, "ss"]), + "Mixed types str and float in attribute value sequence", ) self.assertEqual( trace.Span._check_attribute_value([1, 2, 3.4, 5]), - "different types in attribute value sequence", - ) - self.assertIsNone( - trace.Span._check_attribute_value([1, 2, 3, 5]) + "Mixed types int and float in attribute value sequence", ) + self.assertIsNone(trace.Span._check_attribute_value([1, 2, 3, 5])) self.assertIsNone( trace.Span._check_attribute_value([1.2, 2.3, 3.4, 4.5]) ) - self.assertIsNone( - trace.Span._check_attribute_value([True, False]) - ) + self.assertIsNone(trace.Span._check_attribute_value([True, False])) self.assertIsNone( trace.Span._check_attribute_value(["ss", "dw", "fw"]) ) - self.assertIsNone( - trace.Span._check_attribute_value([]) - ) - + self.assertIsNone(trace.Span._check_attribute_value([])) self.assertEqual( trace.Span._check_attribute_value(dict()), - "invalid type for attribute value", - ) - self.assertIsNone( - trace.Span._check_attribute_value(True) - ) - self.assertIsNone( - trace.Span._check_attribute_value('hi') - ) - self.assertIsNone( - trace.Span._check_attribute_value(3.4) + "Invalid type dict for attribute value. Expected one of ['bool', 'str', 'int', 'float']", ) - self.assertIsNone( - trace.Span._check_attribute_value(15) - ) - + self.assertIsNone(trace.Span._check_attribute_value(True)) + self.assertIsNone(trace.Span._check_attribute_value("hi")) + self.assertIsNone(trace.Span._check_attribute_value(3.4)) + self.assertIsNone(trace.Span._check_attribute_value(15)) def test_sampling_attributes(self): decision_attributes = { @@ -583,7 +565,9 @@ def test_events(self): # event name and attributes now = time_ns() - root.add_event("event1", {"name": "pluto", "some_bools": [True, False]}) + root.add_event( + "event1", {"name": "pluto", "some_bools": [True, False]} + ) # event name, attributes and timestamp now = time_ns() @@ -604,16 +588,25 @@ def event_formatter(): self.assertEqual(root.events[0].attributes, {}) self.assertEqual(root.events[1].name, "event1") - self.assertEqual(root.events[1].attributes, {"name": "pluto", "some_bools": (True, False)}) + self.assertEqual( + root.events[1].attributes, + {"name": "pluto", "some_bools": (True, False)}, + ) self.assertEqual(root.events[2].name, "event2") - self.assertEqual(root.events[2].attributes, {"name": ("birthday",)}) + self.assertEqual( + root.events[2].attributes, {"name": ("birthday",)} + ) self.assertEqual(root.events[2].timestamp, now) self.assertEqual(root.events[3].name, "event3") - self.assertEqual(root.events[3].attributes, {"name": ("original_contents",)}) + self.assertEqual( + root.events[3].attributes, {"name": ("original_contents",)} + ) mutable_list = ["new_contents"] - self.assertEqual(root.events[3].attributes, {"name": ("original_contents",)}) + self.assertEqual( + root.events[3].attributes, {"name": ("original_contents",)} + ) self.assertEqual(root.events[4].name, "event4") self.assertEqual(root.events[4].attributes, {"name": "hello"}) @@ -623,7 +616,7 @@ def test_invalid_event_attributes(self): self.assertIsNone(self.tracer.get_current_span()) with self.tracer.start_as_current_span("root") as root: - root.add_event("event0", {"attr1": True, "attr2": ['hi', False]}) + 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()]}) @@ -923,4 +916,3 @@ def test_add_span_processor_after_span_creation(self): expected_list.append(span_event_end_fmt("SP1", "foo")) self.assertListEqual(spans_calls_list, expected_list) - \ No newline at end of file