From 1b50786bbd3542a7cf8b99b6314f759fd42eecbd Mon Sep 17 00:00:00 2001
From: Andrew Xue <aaxue@google.com>
Date: Mon, 11 May 2020 17:40:28 -0400
Subject: [PATCH 01/12] add check for event attribute values

---
 .../src/opentelemetry/sdk/trace/__init__.py   | 47 ++++++-----
 opentelemetry-sdk/tests/trace/test_trace.py   | 84 ++++++++++++++-----
 2 files changed, 91 insertions(+), 40 deletions(-)

diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
index 5eff5e61307..dfb1534ba40 100644
--- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
+++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
@@ -372,38 +372,39 @@ def set_attribute(self, key: str, value: types.AttributeValue) -> None:
             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
-            # Freeze mutable sequences defensively
-            if isinstance(value, MutableSequence):
-                value = tuple(value)
-        elif not isinstance(value, (bool, str, int, float)):
-            logger.warning("invalid type for attribute value")
+        error_message = self._check_attribute_value(value)
+        if error_message is not None:
+            logger.warning(error_message)
             return
+        # Freeze mutable sequences defensively
+        if isinstance(value, MutableSequence):
+            value = tuple(value)
 
         self.attributes[key] = value
 
     @staticmethod
-    def _check_attribute_value_sequence(sequence: Sequence) -> Optional[str]:
+    def _check_attribute_value(value: types.AttributeValue) -> Optional[str]:
         """
         Checks if sequence items are valid and are of the same type
         """
-        if len(sequence) == 0:
-            return None
+        if isinstance(value, Sequence):
+            if len(value) == 0:
+                return None
 
-        first_element_type = type(sequence[0])
+            first_element_type = type(value[0])
 
-        if first_element_type not in (bool, str, int, float):
-            return "invalid type"
+            if first_element_type not in (bool, str, int, float):
+                return "invalid type in attribute value sequence"
 
-        for element in sequence:
-            if not isinstance(element, first_element_type):
-                return "different type"
+            for element in value:
+                if not isinstance(element, first_element_type):
+                    return "different types in attribute value sequence"
+            return None
+        elif not isinstance(value, (bool, str, int, float)):
+            return "invalid type for attribute value"
         return None
 
+
     def _add_event(self, event: EventBase) -> None:
         with self._lock:
             if not self.is_recording_events():
@@ -425,6 +426,14 @@ def add_event(
     ) -> None:
         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._add_event(
             Event(
                 name=name,
diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py
index 1094f1afb98..28de157233e 100644
--- a/opentelemetry-sdk/tests/trace/test_trace.py
+++ b/opentelemetry-sdk/tests/trace/test_trace.py
@@ -487,39 +487,61 @@ def test_invalid_attribute_values(self):
 
             self.assertEqual(len(root.attributes), 0)
 
-    def test_check_sequence_helper(self):
+    def test_check_attribute_helper(self):
         # pylint: disable=protected-access
         self.assertEqual(
-            trace.Span._check_attribute_value_sequence([1, 2, 3.4, "ss", 4]),
-            "different type",
+            trace.Span._check_attribute_value([1, 2, 3.4, "ss", 4]),
+            "different types in attribute value sequence",
         )
         self.assertEqual(
-            trace.Span._check_attribute_value_sequence([dict(), 1, 2, 3.4, 4]),
-            "invalid type",
+            trace.Span._check_attribute_value([dict(), 1, 2, 3.4, 4]),
+            "invalid type in attribute value sequence",
         )
         self.assertEqual(
-            trace.Span._check_attribute_value_sequence(
+            trace.Span._check_attribute_value(
                 ["sw", "lf", 3.4, "ss"]
             ),
-            "different type",
+            "different types in attribute value sequence",
         )
         self.assertEqual(
-            trace.Span._check_attribute_value_sequence([1, 2, 3.4, 5]),
-            "different type",
+            trace.Span._check_attribute_value([1, 2, 3.4, 5]),
+            "different types in attribute value sequence",
         )
         self.assertIsNone(
-            trace.Span._check_attribute_value_sequence([1, 2, 3, 5])
+            trace.Span._check_attribute_value([1, 2, 3, 5])
         )
         self.assertIsNone(
-            trace.Span._check_attribute_value_sequence([1.2, 2.3, 3.4, 4.5])
+            trace.Span._check_attribute_value([1.2, 2.3, 3.4, 4.5])
         )
         self.assertIsNone(
-            trace.Span._check_attribute_value_sequence([True, False])
+            trace.Span._check_attribute_value([True, False])
         )
         self.assertIsNone(
-            trace.Span._check_attribute_value_sequence(["ss", "dw", "fw"])
+            trace.Span._check_attribute_value(["ss", "dw", "fw"])
+        )
+        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)
+        )
+        self.assertIsNone(
+            trace.Span._check_attribute_value(15)
         )
 
+
     def test_sampling_attributes(self):
         decision_attributes = {
             "sampler-attr": "sample-val",
@@ -561,33 +583,52 @@ def test_events(self):
 
             # event name and attributes
             now = time_ns()
-            root.add_event("event1", {"name": "pluto"})
+            root.add_event("event1", {"name": "pluto", "some_bools": [True, False]})
 
             # event name, attributes and timestamp
             now = time_ns()
-            root.add_event("event2", {"name": "birthday"}, now)
+            root.add_event("event2", {"name": ["birthday"]}, now)
+
+            mutable_list = ["original_contents"]
+            root.add_event("event3", {"name": mutable_list})
 
             def event_formatter():
                 return {"name": "hello"}
 
             # lazy event
-            root.add_lazy_event("event3", event_formatter, now)
+            root.add_lazy_event("event4", event_formatter, now)
 
-            self.assertEqual(len(root.events), 4)
+            self.assertEqual(len(root.events), 5)
 
             self.assertEqual(root.events[0].name, "event0")
             self.assertEqual(root.events[0].attributes, {})
 
             self.assertEqual(root.events[1].name, "event1")
-            self.assertEqual(root.events[1].attributes, {"name": "pluto"})
+            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": "hello"})
-            self.assertEqual(root.events[3].timestamp, now)
+            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[4].name, "event4")
+            self.assertEqual(root.events[4].attributes, {"name": "hello"})
+            self.assertEqual(root.events[4].timestamp, now)
+
+    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": dict()})
+            root.add_event("event0", {"attr1": [[True]]})
+            root.add_event("event0", {"attr1": [dict()]})
+
+            self.assertEqual(len(root.events), 0)
 
     def test_links(self):
         other_context1 = trace_api.SpanContext(
@@ -882,3 +923,4 @@ 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

From 18e9de7e3f3b026bf255da99b11a1f8fca86566a Mon Sep 17 00:00:00 2001
From: Andrew Xue <aaxue@google.com>
Date: Tue, 12 May 2020 13:49:01 -0400
Subject: [PATCH 02/12] update error message

---
 .../src/opentelemetry/sdk/trace/__init__.py   | 45 +++++++++---
 opentelemetry-sdk/tests/trace/test_trace.py   | 68 ++++++++-----------
 2 files changed, 64 insertions(+), 49 deletions(-)

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

From f38ab13711dddd904483c3db6569bb97446c4744 Mon Sep 17 00:00:00 2001
From: Andrew Xue <aaxue@google.com>
Date: Tue, 12 May 2020 14:03:31 -0400
Subject: [PATCH 03/12] improve attribute filtering logic

---
 .../src/opentelemetry/sdk/trace/__init__.py   | 47 ++++++++-----------
 opentelemetry-sdk/tests/trace/test_trace.py   |  8 +++-
 2 files changed, 25 insertions(+), 30 deletions(-)

diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
index 91eb4d6325d..106a80df739 100644
--- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
+++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
@@ -245,33 +245,19 @@ def __init__(
         self.status = None
         self._lock = threading.Lock()
 
-        if attributes is None:
+        self._filter_attribute_values(attributes)
+        if attributes is None or len(attributes) == 0:
             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.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,18 @@ 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):
+        if 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():
@@ -447,16 +445,9 @@ def add_event(
         attributes: types.Attributes = None,
         timestamp: Optional[int] = None,
     ) -> None:
-        if attributes is None:
+        self._filter_attribute_values(attributes)
+        if attributes is None or len(attributes) == 0:
             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._add_event(
             Event(
                 name=name,
diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py
index 71e3046c0c7..5ff118d5579 100644
--- a/opentelemetry-sdk/tests/trace/test_trace.py
+++ b/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(

From 2fe391b58017d4fd9d537fed435863618b45a67b Mon Sep 17 00:00:00 2001
From: Andrew Xue <aaxue@google.com>
Date: Tue, 12 May 2020 14:39:45 -0400
Subject: [PATCH 04/12] python cool

---
 opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
index 106a80df739..b5a65114ffb 100644
--- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
+++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
@@ -246,7 +246,7 @@ def __init__(
         self._lock = threading.Lock()
 
         self._filter_attribute_values(attributes)
-        if attributes is None or len(attributes) == 0:
+        if not attributes:
             self.attributes = Span._empty_attributes
         else:
             self.attributes = BoundedDict.from_map(MAX_NUM_ATTRIBUTES, attributes)
@@ -446,7 +446,7 @@ def add_event(
         timestamp: Optional[int] = None,
     ) -> None:
         self._filter_attribute_values(attributes)
-        if attributes is None or len(attributes) == 0:
+        if not attributes:
             attributes = Span._empty_attributes
         self._add_event(
             Event(

From e3d1012831304d223b8b459fdf6e864570592782 Mon Sep 17 00:00:00 2001
From: Andrew Xue <aaxue@google.com>
Date: Tue, 12 May 2020 14:43:52 -0400
Subject: [PATCH 05/12] dont check first

---
 opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py | 6 +++---
 opentelemetry-sdk/tests/trace/test_trace.py               | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
index b5a65114ffb..35a4c2e301b 100644
--- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
+++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
@@ -397,19 +397,19 @@ def _check_attribute_value(value: types.AttributeValue) -> Optional[str]:
             first_element_type = type(value[0])
 
             if first_element_type not in valid_types:
-                return "Invalid type {} in attribute value sequence. Expected one of {}".format(
+                return "Invalid type {} in attribute value sequence. Expected one of {} or a sequence of those types".format(
                     first_element_type.__name__,
                     [valid_type.__name__ for valid_type in valid_types],
                 )
 
-            for element in value:
+            for element in list(value)[1:]:
                 if not isinstance(element, first_element_type):
                     return "Mixed types {} and {} in attribute value sequence".format(
                         first_element_type.__name__, type(element).__name__
                     )
             return None
         elif not isinstance(value, valid_types):
-            return "Invalid type {} for attribute value. Expected one of {}".format(
+            return "Invalid type {} for attribute value. Expected one of {} or a sequence of those types".format(
                 type(value).__name__,
                 [valid_type.__name__ for valid_type in valid_types],
             )
diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py
index 5ff118d5579..e3f3c0de1e4 100644
--- a/opentelemetry-sdk/tests/trace/test_trace.py
+++ b/opentelemetry-sdk/tests/trace/test_trace.py
@@ -495,7 +495,7 @@ def test_check_attribute_helper(self):
         )
         self.assertEqual(
             trace.Span._check_attribute_value([dict(), 1, 2, 3.4, 4]),
-            "Invalid type dict in attribute value sequence. Expected one of ['bool', 'str', 'int', 'float']",
+            "Invalid type dict in attribute value sequence. Expected one of ['bool', 'str', 'int', 'float'] or a sequence of those types",
         )
         self.assertEqual(
             trace.Span._check_attribute_value(["sw", "lf", 3.4, "ss"]),
@@ -517,7 +517,7 @@ def test_check_attribute_helper(self):
 
         self.assertEqual(
             trace.Span._check_attribute_value(dict()),
-            "Invalid type dict for attribute value. Expected one of ['bool', 'str', 'int', 'float']",
+            "Invalid type dict for attribute value. Expected one of ['bool', 'str', 'int', 'float'] or a sequence of those types",
         )
         self.assertIsNone(trace.Span._check_attribute_value(True))
         self.assertIsNone(trace.Span._check_attribute_value("hi"))

From 8cfe083228ab49bdf53d5114a23b84c1c3fa660d Mon Sep 17 00:00:00 2001
From: Andrew Xue <aaxue@google.com>
Date: Tue, 12 May 2020 14:46:01 -0400
Subject: [PATCH 06/12] comment

---
 opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
index 35a4c2e301b..572e2158b18 100644
--- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
+++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
@@ -387,7 +387,9 @@ def set_attribute(self, key: str, value: types.AttributeValue) -> None:
     @staticmethod
     def _check_attribute_value(value: types.AttributeValue) -> Optional[str]:
         """
-        Checks if sequence items are valid and are of the same type
+        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 valid type, not a sequence, and are of the same type.
         """
         valid_types = (bool, str, int, float)
         if isinstance(value, Sequence):

From 0495a9f01f145a3d47361ba8f43e9cf43bdf7772 Mon Sep 17 00:00:00 2001
From: Andrew Xue <aaxue@google.com>
Date: Tue, 12 May 2020 16:20:31 -0400
Subject: [PATCH 07/12] address comments

---
 .../src/opentelemetry/sdk/trace/__init__.py   | 96 +++++++++----------
 opentelemetry-sdk/tests/trace/test_trace.py   | 48 +++-------
 2 files changed, 61 insertions(+), 83 deletions(-)

diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
index 572e2158b18..55a80a7489f 100644
--- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
+++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
@@ -17,7 +17,6 @@
 import atexit
 import json
 import logging
-import os
 import random
 import threading
 from collections import OrderedDict
@@ -41,6 +40,7 @@
 MAX_NUM_ATTRIBUTES = 32
 MAX_NUM_EVENTS = 128
 MAX_NUM_LINKS = 32
+VALID_ATTR_VALUE_TYPES = (bool, str, int, float)
 
 
 class SpanProcessor:
@@ -188,6 +188,40 @@ def __init__(
     def attributes(self) -> types.Attributes:
         return self._event_formatter()
 
+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 valid type, not a sequence, and are of the same type.
+    """
+
+    if isinstance(value, Sequence):
+        if len(value) == 0:
+            return True
+
+        first_element_type = type(value[0])
+
+        if first_element_type not in VALID_ATTR_VALUE_TYPES:
+            logger.warning("Invalid type {} in attribute value sequence. Expected one of {} or a sequence of those types".format(
+                first_element_type.__name__,
+                [valid_type.__name__ for valid_type in VALID_ATTR_VALUE_TYPES],
+            ))
+            return False
+
+        for element in list(value)[1:]:
+            if not isinstance(element, first_element_type):
+                logger.warning("Mixed types {} and {} in attribute value sequence".format(
+                    first_element_type.__name__, type(element).__name__
+                ))
+                return False
+    elif not isinstance(value, VALID_ATTR_VALUE_TYPES):
+        logger.warning("Invalid type {} for attribute value. Expected one of {} or a sequence of those types".format(
+            type(value).__name__,
+            [valid_type.__name__ for valid_type in VALID_ATTR_VALUE_TYPES],
+        ))
+        return False
+    return True
+
 
 class Span(trace_api.Span):
     """See `opentelemetry.trace.Span`.
@@ -374,59 +408,23 @@ def set_attribute(self, key: str, value: types.AttributeValue) -> None:
             logger.warning("invalid key (empty or null)")
             return
 
-        error_message = self._check_attribute_value(value)
-        if error_message is not None:
-            logger.warning(error_message)
-            return
-        # Freeze mutable sequences defensively
-        if isinstance(value, MutableSequence):
-            value = tuple(value)
-
-        self.attributes[key] = value
-
-    @staticmethod
-    def _check_attribute_value(value: types.AttributeValue) -> Optional[str]:
-        """
-        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 valid type, not a sequence, 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 valid_types:
-                return "Invalid type {} in attribute value sequence. Expected one of {} or a sequence of those types".format(
-                    first_element_type.__name__,
-                    [valid_type.__name__ for valid_type in valid_types],
-                )
-
-            for element in list(value)[1:]:
-                if not isinstance(element, first_element_type):
-                    return "Mixed types {} and {} in attribute value sequence".format(
-                        first_element_type.__name__, type(element).__name__
-                    )
-            return None
-        elif not isinstance(value, valid_types):
-            return "Invalid type {} for attribute value. Expected one of {} or a sequence of those types".format(
-                type(value).__name__,
-                [valid_type.__name__ for valid_type in valid_types],
-            )
+        if is_valid_attribute_value(value):
+            # Freeze mutable sequences defensively
+            if isinstance(value, MutableSequence):
+                value = tuple(value)
+            with self._lock:
+                self.attributes[key] = value
 
     def _filter_attribute_values(self, attributes: types.Attributes):
         if 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)
+                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[attr_key] = attr_value
+                    attributes.pop(attr_key)
 
     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 e3f3c0de1e4..761c8c998e9 100644
--- a/opentelemetry-sdk/tests/trace/test_trace.py
+++ b/opentelemetry-sdk/tests/trace/test_trace.py
@@ -489,40 +489,20 @@ def test_invalid_attribute_values(self):
 
     def test_check_attribute_helper(self):
         # pylint: disable=protected-access
-        self.assertEqual(
-            trace.Span._check_attribute_value([1, 2, 3.4, "ss", 4]),
-            "Mixed types int and float in attribute value sequence",
-        )
-        self.assertEqual(
-            trace.Span._check_attribute_value([dict(), 1, 2, 3.4, 4]),
-            "Invalid type dict in attribute value sequence. Expected one of ['bool', 'str', 'int', 'float'] or a sequence of those types",
-        )
-        self.assertEqual(
-            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]),
-            "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(["ss", "dw", "fw"])
-        )
-        self.assertIsNone(trace.Span._check_attribute_value([]))
-
-        self.assertEqual(
-            trace.Span._check_attribute_value(dict()),
-            "Invalid type dict for attribute value. Expected one of ['bool', 'str', 'int', 'float'] or a sequence of those types",
-        )
-        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))
+        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))
 
     def test_sampling_attributes(self):
         decision_attributes = {

From 9a005c02bedf91c2ebdb0d6367ceb55732b62820 Mon Sep 17 00:00:00 2001
From: Andrew Xue <aaxue@google.com>
Date: Tue, 12 May 2020 16:49:07 -0400
Subject: [PATCH 08/12] address comments

---
 .../src/opentelemetry/sdk/trace/__init__.py   | 43 ++++++++++++-------
 opentelemetry-sdk/tests/trace/test_trace.py   | 32 ++++++++------
 2 files changed, 45 insertions(+), 30 deletions(-)

diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
index 55a80a7489f..2cbe509ce0b 100644
--- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
+++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
@@ -188,7 +188,8 @@ def __init__(
     def attributes(self) -> types.Attributes:
         return self._event_formatter()
 
-def is_valid_attribute_value(value: types.AttributeValue) -> bool:
+
+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
@@ -202,23 +203,32 @@ def is_valid_attribute_value(value: types.AttributeValue) -> bool:
         first_element_type = type(value[0])
 
         if first_element_type not in VALID_ATTR_VALUE_TYPES:
-            logger.warning("Invalid type {} in attribute value sequence. Expected one of {} or a sequence of those types".format(
-                first_element_type.__name__,
-                [valid_type.__name__ for valid_type in VALID_ATTR_VALUE_TYPES],
-            ))
+            logger.warning(
+                "Invalid type {} in attribute value sequence. Expected one of {} or a sequence of those types".format(
+                    first_element_type.__name__,
+                    [
+                        valid_type.__name__
+                        for valid_type in VALID_ATTR_VALUE_TYPES
+                    ],
+                )
+            )
             return False
 
         for element in list(value)[1:]:
             if not isinstance(element, first_element_type):
-                logger.warning("Mixed types {} and {} in attribute value sequence".format(
-                    first_element_type.__name__, type(element).__name__
-                ))
+                logger.warning(
+                    "Mixed types {} and {} in attribute value sequence".format(
+                        first_element_type.__name__, type(element).__name__
+                    )
+                )
                 return False
     elif not isinstance(value, VALID_ATTR_VALUE_TYPES):
-        logger.warning("Invalid type {} for attribute value. Expected one of {} or a sequence of those types".format(
-            type(value).__name__,
-            [valid_type.__name__ for valid_type in VALID_ATTR_VALUE_TYPES],
-        ))
+        logger.warning(
+            "Invalid type {} for attribute value. Expected one of {} or a sequence of those types".format(
+                type(value).__name__,
+                [valid_type.__name__ for valid_type in VALID_ATTR_VALUE_TYPES],
+            )
+        )
         return False
     return True
 
@@ -283,7 +293,9 @@ def __init__(
         if not attributes:
             self.attributes = Span._empty_attributes
         else:
-            self.attributes = BoundedDict.from_map(MAX_NUM_ATTRIBUTES, attributes)
+            self.attributes = BoundedDict.from_map(
+                MAX_NUM_ATTRIBUTES, attributes
+            )
 
         if events is None:
             self.events = Span._empty_events
@@ -408,7 +420,7 @@ def set_attribute(self, key: str, value: types.AttributeValue) -> None:
             logger.warning("invalid key (empty or null)")
             return
 
-        if is_valid_attribute_value(value):
+        if _is_valid_attribute_value(value):
             # Freeze mutable sequences defensively
             if isinstance(value, MutableSequence):
                 value = tuple(value)
@@ -418,7 +430,7 @@ def set_attribute(self, key: str, value: types.AttributeValue) -> None:
     def _filter_attribute_values(self, attributes: types.Attributes):
         if attributes:
             for attr_key, attr_value in list(attributes.items()):
-                if is_valid_attribute_value(attr_value):
+                if _is_valid_attribute_value(attr_value):
                     if isinstance(attr_value, MutableSequence):
                         attributes[attr_key] = tuple(attr_value)
                     else:
@@ -537,7 +549,6 @@ def __exit__(
             and self._set_status_on_exception
             and exc_val is not None
         ):
-
             self.set_status(
                 Status(
                     canonical_code=StatusCanonicalCode.UNKNOWN,
diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py
index 761c8c998e9..e468652ec00 100644
--- a/opentelemetry-sdk/tests/trace/test_trace.py
+++ b/opentelemetry-sdk/tests/trace/test_trace.py
@@ -489,20 +489,24 @@ def test_invalid_attribute_values(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(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))
 
     def test_sampling_attributes(self):
         decision_attributes = {

From 34f913430b9d4f5fbcb9e94afcf86911f822b3a9 Mon Sep 17 00:00:00 2001
From: Andrew Xue <aaxue@google.com>
Date: Tue, 12 May 2020 17:01:58 -0400
Subject: [PATCH 09/12] update shim test

---
 ext/opentelemetry-ext-opentracing-shim/tests/test_shim.py | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/ext/opentelemetry-ext-opentracing-shim/tests/test_shim.py b/ext/opentelemetry-ext-opentracing-shim/tests/test_shim.py
index 7a0913f973b..abca2e052bc 100644
--- a/ext/opentelemetry-ext-opentracing-shim/tests/test_shim.py
+++ b/ext/opentelemetry-ext-opentracing-shim/tests/test_shim.py
@@ -463,9 +463,6 @@ def test_span_on_error(self):
 
         # Verify exception details have been added to span.
         self.assertEqual(scope.span.unwrap().attributes["error"], True)
-        self.assertEqual(
-            scope.span.unwrap().events[0].attributes["error.kind"], Exception
-        )
 
     def test_inject_http_headers(self):
         """Test `inject()` method for Format.HTTP_HEADERS."""

From f23ebf554c5629e0fe43c644410b4cd784a08736 Mon Sep 17 00:00:00 2001
From: Andrew Xue <aaxue@google.com>
Date: Tue, 12 May 2020 17:28:20 -0400
Subject: [PATCH 10/12] fix lint

---
 .../src/opentelemetry/sdk/trace/__init__.py   | 26 ++++++++-----------
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
index 2cbe509ce0b..d4054de262f 100644
--- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
+++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
@@ -204,30 +204,25 @@ def _is_valid_attribute_value(value: types.AttributeValue) -> bool:
 
         if first_element_type not in VALID_ATTR_VALUE_TYPES:
             logger.warning(
-                "Invalid type {} in attribute value sequence. Expected one of {} or a sequence of those types".format(
-                    first_element_type.__name__,
-                    [
-                        valid_type.__name__
-                        for valid_type in VALID_ATTR_VALUE_TYPES
-                    ],
-                )
+                "Invalid type %s in attribute value sequence. Expected one of %s or a sequence of those types",
+                first_element_type.__name__,
+                [valid_type.__name__ for valid_type in VALID_ATTR_VALUE_TYPES],
             )
             return False
 
         for element in list(value)[1:]:
             if not isinstance(element, first_element_type):
                 logger.warning(
-                    "Mixed types {} and {} in attribute value sequence".format(
-                        first_element_type.__name__, type(element).__name__
-                    )
+                    "Mixed types %s and %s in attribute value sequence",
+                    first_element_type.__name__,
+                    type(element).__name__,
                 )
                 return False
     elif not isinstance(value, VALID_ATTR_VALUE_TYPES):
         logger.warning(
-            "Invalid type {} for attribute value. Expected one of {} or a sequence of those types".format(
-                type(value).__name__,
-                [valid_type.__name__ for valid_type in VALID_ATTR_VALUE_TYPES],
-            )
+            "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
@@ -427,7 +422,8 @@ def set_attribute(self, key: str, value: types.AttributeValue) -> None:
             with self._lock:
                 self.attributes[key] = value
 
-    def _filter_attribute_values(self, attributes: types.Attributes):
+    @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):

From b7b0c7fe42da63359f44c0a0f9f8338bcd46a123 Mon Sep 17 00:00:00 2001
From: Andrew Xue <aaxue@google.com>
Date: Wed, 13 May 2020 18:09:56 -0400
Subject: [PATCH 11/12] address comments

---
 .../src/opentelemetry/sdk/trace/__init__.py           | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
index d4054de262f..5b74b4a6186 100644
--- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
+++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
@@ -192,8 +192,9 @@ def attributes(self) -> types.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 valid type, not a sequence, and are of the same type.
+    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 valid
+    type, not a sequence, and are of the same type.
     """
 
     if isinstance(value, Sequence):
@@ -204,7 +205,8 @@ def _is_valid_attribute_value(value: types.AttributeValue) -> bool:
 
         if first_element_type not in VALID_ATTR_VALUE_TYPES:
             logger.warning(
-                "Invalid type %s in attribute value sequence. Expected one of %s or a sequence of those types",
+                "Invalid type %s in attribute value sequence. Expected one of "
+                "%s or a sequence of those types",
                 first_element_type.__name__,
                 [valid_type.__name__ for valid_type in VALID_ATTR_VALUE_TYPES],
             )
@@ -220,7 +222,8 @@ def _is_valid_attribute_value(value: types.AttributeValue) -> bool:
                 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",
+            "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],
         )

From fa51d63b8c595d3dc31448392eaf4bcb71970991 Mon Sep 17 00:00:00 2001
From: Andrew Xue <aaxue@google.com>
Date: Thu, 14 May 2020 16:41:34 -0400
Subject: [PATCH 12/12] update changelog

---
 opentelemetry-sdk/CHANGELOG.md | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/opentelemetry-sdk/CHANGELOG.md b/opentelemetry-sdk/CHANGELOG.md
index f771a2df346..476908cf91d 100644
--- a/opentelemetry-sdk/CHANGELOG.md
+++ b/opentelemetry-sdk/CHANGELOG.md
@@ -2,6 +2,8 @@
 
 ## Unreleased
 
+- Validate span attribute types in SDK (#678)
+
 ## 0.7b1
 
 Released 2020-05-12