From 6914ff3422970a9335678c5a715b4e0c07d28f3b Mon Sep 17 00:00:00 2001 From: jakemalachowski Date: Thu, 26 Dec 2019 21:50:12 -0600 Subject: [PATCH 01/23] Validate attribute value data types before adding to span Add lists as an accepted data type --- .../src/opentelemetry/util/types.py | 2 +- .../src/opentelemetry/sdk/trace/__init__.py | 21 +++++++++++++++++++ opentelemetry-sdk/tests/trace/test_trace.py | 21 ++++++++++++++++++- 3 files changed, 42 insertions(+), 2 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/util/types.py b/opentelemetry-api/src/opentelemetry/util/types.py index 28fab893890..3d34895be13 100644 --- a/opentelemetry-api/src/opentelemetry/util/types.py +++ b/opentelemetry-api/src/opentelemetry/util/types.py @@ -15,5 +15,5 @@ import typing -AttributeValue = typing.Union[str, bool, float] +AttributeValue = typing.Union[str, bool, int, float, list] Attributes = typing.Optional[typing.Dict[str, AttributeValue]] diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 3035ae7ef9f..5dfad79565b 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -208,6 +208,27 @@ def set_attribute(self, key: str, value: types.AttributeValue) -> None: if has_ended: logger.warning("Setting attribute on ended span.") return + + # If value is of type list, validate all items are of same, + # valid data type + if isinstance(value, list) and len(value) > 0: + first_element_data_type = type(value[0]) + # If first value is numeric, validate all values are numeric + if first_element_data_type in (int, float): + if any(not isinstance(item, (int, float)) for item in value): + logger.warning("All data types of attribute value arrays must be identical") + return + # Else, validate all values are of same type + elif first_element_data_type in (bool, str): + if any(not isinstance(item, first_element_data_type) for item in value): + logger.warning("All data types of attribute value arrays must be identical") + return + else: + logger.warning("Invalid data type in value array") + return + elif not isinstance(value, (int, float, bool, str, list)): + logger.warning("Invalid data type for attribute value") + return self.attributes[key] = value def add_event( diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index 98a7bb100e7..8047ef83fe1 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -368,7 +368,11 @@ def test_attributes(self): root.set_attribute("attr-key", "attr-value1") root.set_attribute("attr-key", "attr-value2") - self.assertEqual(len(root.attributes), 7) + root.set_attribute("empty-list", []) + root.set_attribute("list-of-bools", [True, True, False]) + root.set_attribute("list-of-numerics", [123, 3.14, 0]) + + self.assertEqual(len(root.attributes), 10) self.assertEqual(root.attributes["component"], "http") self.assertEqual(root.attributes["http.method"], "GET") self.assertEqual( @@ -379,6 +383,9 @@ def test_attributes(self): self.assertEqual(root.attributes["http.status_text"], "OK") self.assertEqual(root.attributes["misc.pi"], 3.14) self.assertEqual(root.attributes["attr-key"], "attr-value2") + self.assertEqual(root.attributes["empty-list"], []) + self.assertEqual(root.attributes["list-of-bools"], [True, True, False]) + self.assertEqual(root.attributes["list-of-numerics"], [123, 3.14, 0]) attributes = { "attr-key": "val", @@ -393,6 +400,18 @@ def test_attributes(self): self.assertEqual(root.attributes["attr-key2"], "val2") self.assertEqual(root.attributes["attr-in-both"], "span-attr") + def test_invalid_attribute_values(self): + class NonPrimitive: + pass + + with self.tracer.start_as_current_span("root") as root: + root.set_attribute("non-primitive-data-type", NonPrimitive()) + root.set_attribute("list-of-mixed-data-types-numeric-first", [123, False, "string"]) + root.set_attribute("list-of-mixed-data-types-non-numeric-first", [False, 123, "string"]) + root.set_attribute("list-with-non-primitive-data-type", [NonPrimitive(), 123]) + + self.assertEqual(len(root.attributes), 0) + def test_sampling_attributes(self): decision_attributes = { "sampler-attr": "sample-val", From 59107adce4b73825891b74a76e9f9faea51f6a73 Mon Sep 17 00:00:00 2001 From: jakemalachowski Date: Fri, 27 Dec 2019 07:11:25 -0600 Subject: [PATCH 02/23] Change order of checks to remove an else condition --- opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 5dfad79565b..e21af570c23 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -208,7 +208,10 @@ def set_attribute(self, key: str, value: types.AttributeValue) -> None: if has_ended: logger.warning("Setting attribute on ended span.") return - + + if not isinstance(value, (int, float, bool, str, list)): + logger.warning("Invalid data type for attribute value") + return # If value is of type list, validate all items are of same, # valid data type if isinstance(value, list) and len(value) > 0: @@ -226,9 +229,6 @@ def set_attribute(self, key: str, value: types.AttributeValue) -> None: else: logger.warning("Invalid data type in value array") return - elif not isinstance(value, (int, float, bool, str, list)): - logger.warning("Invalid data type for attribute value") - return self.attributes[key] = value def add_event( From 8152fdb4bf7396bb75ac3c614baa9e712269bbae Mon Sep 17 00:00:00 2001 From: jakemalachowski Date: Sat, 28 Dec 2019 19:38:50 -0600 Subject: [PATCH 03/23] Create separate sequence check method, add tests, fix linting issues --- .../src/opentelemetry/sdk/trace/__init__.py | 48 +++++++++++-------- opentelemetry-sdk/tests/trace/test_trace.py | 42 ++++++++++++---- 2 files changed, 62 insertions(+), 28 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index e21af570c23..7c580dc4d30 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -18,6 +18,7 @@ import random import threading from contextlib import contextmanager +from numbers import Number from types import TracebackType from typing import Iterator, Optional, Sequence, Tuple, Type @@ -208,29 +209,38 @@ def set_attribute(self, key: str, value: types.AttributeValue) -> None: if has_ended: logger.warning("Setting attribute on ended span.") return - - if not isinstance(value, (int, float, bool, str, list)): - logger.warning("Invalid data type for attribute value") + + if not isinstance(value, (int, float, bool, str, list, tuple)): + logger.warning("invalid type for attribute value") return - # If value is of type list, validate all items are of same, - # valid data type - if isinstance(value, list) and len(value) > 0: - first_element_data_type = type(value[0]) - # If first value is numeric, validate all values are numeric - if first_element_data_type in (int, float): - if any(not isinstance(item, (int, float)) for item in value): - logger.warning("All data types of attribute value arrays must be identical") - return - # Else, validate all values are of same type - elif first_element_data_type in (bool, str): - if any(not isinstance(item, first_element_data_type) for item in value): - logger.warning("All data types of attribute value arrays must be identical") - return - else: - logger.warning("Invalid data type in value array") + if isinstance(value, (list, tuple)) and len(value) > 0: + return_code = self._check_sequence(value) + if return_code: + logger.warning("%s in attribute value sequence", return_code) return + self.attributes[key] = value + @staticmethod + def _check_sequence(sequence: (list, tuple)) -> (str, None): + """ + Checks if sequence items are valid and identical + """ + first_element_type = type(sequence[0]) + + if issubclass(type(sequence[0]), Number): + first_element_type = Number + + for element in sequence: + + if not isinstance(element, (bool, str, Number)): + return "invalid type" + + if not isinstance(element, first_element_type): + return "different type" + + return None + def add_event( self, name: str, diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index 8047ef83fe1..1151a976cff 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -384,8 +384,12 @@ def test_attributes(self): self.assertEqual(root.attributes["misc.pi"], 3.14) self.assertEqual(root.attributes["attr-key"], "attr-value2") self.assertEqual(root.attributes["empty-list"], []) - self.assertEqual(root.attributes["list-of-bools"], [True, True, False]) - self.assertEqual(root.attributes["list-of-numerics"], [123, 3.14, 0]) + self.assertEqual( + root.attributes["list-of-bools"], [True, True, False] + ) + self.assertEqual( + root.attributes["list-of-numerics"], [123, 3.14, 0] + ) attributes = { "attr-key": "val", @@ -401,17 +405,37 @@ def test_attributes(self): self.assertEqual(root.attributes["attr-in-both"], "span-attr") def test_invalid_attribute_values(self): - class NonPrimitive: - pass - with self.tracer.start_as_current_span("root") as root: - root.set_attribute("non-primitive-data-type", NonPrimitive()) - root.set_attribute("list-of-mixed-data-types-numeric-first", [123, False, "string"]) - root.set_attribute("list-of-mixed-data-types-non-numeric-first", [False, 123, "string"]) - root.set_attribute("list-with-non-primitive-data-type", [NonPrimitive(), 123]) + root.set_attribute("non-primitive-data-type", dict()) + root.set_attribute( + "list-of-mixed-data-types-numeric-first", + [123, False, "string"], + ) + root.set_attribute( + "list-of-mixed-data-types-non-numeric-first", + [False, 123, "string"], + ) + root.set_attribute( + "list-with-non-primitive-data-type", [dict(), 123] + ) self.assertEqual(len(root.attributes), 0) + def test_check_sequence_helper(self): + # pylint: disable=protected-access + self.assertEqual( + trace.Span._check_sequence([1, 2, 3.4, "ss", 4]), "different type" + ) + self.assertEqual( + trace.Span._check_sequence([1, 2, 3.4, dict(), 4]), "invalid type" + ) + self.assertEqual( + trace.Span._check_sequence(["sw", "lf", 3.4, "ss"]), + "different type", + ) + self.assertIsNone(trace.Span._check_sequence([1, 2, 3.4, 5])) + self.assertIsNone(trace.Span._check_sequence(["ss", "dw", "fw"])) + def test_sampling_attributes(self): decision_attributes = { "sampler-attr": "sample-val", From 1d972e80560389117a9129bae3276594af48959d Mon Sep 17 00:00:00 2001 From: jakemalachowski Date: Sat, 28 Dec 2019 19:59:44 -0600 Subject: [PATCH 04/23] Fix attribute value typing --- opentelemetry-api/src/opentelemetry/util/types.py | 2 +- opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/util/types.py b/opentelemetry-api/src/opentelemetry/util/types.py index 3d34895be13..880e96e0b89 100644 --- a/opentelemetry-api/src/opentelemetry/util/types.py +++ b/opentelemetry-api/src/opentelemetry/util/types.py @@ -15,5 +15,5 @@ import typing -AttributeValue = typing.Union[str, bool, int, float, list] +AttributeValue = typing.Union[str, bool, int, float, typing.Sequence[typing.Union[str, bool, int, float]]] Attributes = typing.Optional[typing.Dict[str, AttributeValue]] diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 7c580dc4d30..c7a8f5c8743 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -20,7 +20,7 @@ from contextlib import contextmanager from numbers import Number from types import TracebackType -from typing import Iterator, Optional, Sequence, Tuple, Type +from typing import Iterator, Optional, Sequence, Tuple, Type, Union from opentelemetry import trace as trace_api from opentelemetry.context import Context @@ -222,7 +222,7 @@ def set_attribute(self, key: str, value: types.AttributeValue) -> None: self.attributes[key] = value @staticmethod - def _check_sequence(sequence: (list, tuple)) -> (str, None): + def _check_sequence(sequence: (list, tuple)) -> Union[str, None]: """ Checks if sequence items are valid and identical """ From c535e0ce760d9769a51a5f446b20ee6191d61767 Mon Sep 17 00:00:00 2001 From: jakemalachowski Date: Sat, 28 Dec 2019 20:03:50 -0600 Subject: [PATCH 05/23] Apply linting changes --- opentelemetry-api/src/opentelemetry/util/types.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/opentelemetry-api/src/opentelemetry/util/types.py b/opentelemetry-api/src/opentelemetry/util/types.py index 880e96e0b89..c52020aa43b 100644 --- a/opentelemetry-api/src/opentelemetry/util/types.py +++ b/opentelemetry-api/src/opentelemetry/util/types.py @@ -15,5 +15,7 @@ import typing -AttributeValue = typing.Union[str, bool, int, float, typing.Sequence[typing.Union[str, bool, int, float]]] +AttributeValue = typing.Union[ + str, bool, int, float, typing.Sequence[typing.Union[str, bool, int, float]] +] Attributes = typing.Optional[typing.Dict[str, AttributeValue]] From 59dd5f58849b5c4c777c0ca7e2a9b413cc755e02 Mon Sep 17 00:00:00 2001 From: jakemalachowski Date: Mon, 30 Dec 2019 18:01:58 -0600 Subject: [PATCH 06/23] Use is not None, use optional type instead of union with None --- opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index c7a8f5c8743..6c1e1095f0e 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -20,7 +20,7 @@ from contextlib import contextmanager from numbers import Number from types import TracebackType -from typing import Iterator, Optional, Sequence, Tuple, Type, Union +from typing import Iterator, Optional, Sequence, Tuple, Type from opentelemetry import trace as trace_api from opentelemetry.context import Context @@ -215,14 +215,14 @@ def set_attribute(self, key: str, value: types.AttributeValue) -> None: return if isinstance(value, (list, tuple)) and len(value) > 0: return_code = self._check_sequence(value) - if return_code: + if return_code is not None: logger.warning("%s in attribute value sequence", return_code) return self.attributes[key] = value @staticmethod - def _check_sequence(sequence: (list, tuple)) -> Union[str, None]: + def _check_sequence(sequence: (list, tuple)) -> Optional[str]: """ Checks if sequence items are valid and identical """ From 2a5df2b4437bd5698cd162f1313e436dd89d3062 Mon Sep 17 00:00:00 2001 From: jakemalachowski Date: Thu, 26 Dec 2019 21:50:12 -0600 Subject: [PATCH 07/23] Validate attribute value data types before adding to span Add lists as an accepted data type --- .../src/opentelemetry/util/types.py | 2 +- .../src/opentelemetry/sdk/trace/__init__.py | 21 +++++++++++++++++++ opentelemetry-sdk/tests/trace/test_trace.py | 21 ++++++++++++++++++- 3 files changed, 42 insertions(+), 2 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/util/types.py b/opentelemetry-api/src/opentelemetry/util/types.py index 28fab893890..3d34895be13 100644 --- a/opentelemetry-api/src/opentelemetry/util/types.py +++ b/opentelemetry-api/src/opentelemetry/util/types.py @@ -15,5 +15,5 @@ import typing -AttributeValue = typing.Union[str, bool, float] +AttributeValue = typing.Union[str, bool, int, float, list] Attributes = typing.Optional[typing.Dict[str, AttributeValue]] diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 3035ae7ef9f..5dfad79565b 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -208,6 +208,27 @@ def set_attribute(self, key: str, value: types.AttributeValue) -> None: if has_ended: logger.warning("Setting attribute on ended span.") return + + # If value is of type list, validate all items are of same, + # valid data type + if isinstance(value, list) and len(value) > 0: + first_element_data_type = type(value[0]) + # If first value is numeric, validate all values are numeric + if first_element_data_type in (int, float): + if any(not isinstance(item, (int, float)) for item in value): + logger.warning("All data types of attribute value arrays must be identical") + return + # Else, validate all values are of same type + elif first_element_data_type in (bool, str): + if any(not isinstance(item, first_element_data_type) for item in value): + logger.warning("All data types of attribute value arrays must be identical") + return + else: + logger.warning("Invalid data type in value array") + return + elif not isinstance(value, (int, float, bool, str, list)): + logger.warning("Invalid data type for attribute value") + return self.attributes[key] = value def add_event( diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index 98a7bb100e7..8047ef83fe1 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -368,7 +368,11 @@ def test_attributes(self): root.set_attribute("attr-key", "attr-value1") root.set_attribute("attr-key", "attr-value2") - self.assertEqual(len(root.attributes), 7) + root.set_attribute("empty-list", []) + root.set_attribute("list-of-bools", [True, True, False]) + root.set_attribute("list-of-numerics", [123, 3.14, 0]) + + self.assertEqual(len(root.attributes), 10) self.assertEqual(root.attributes["component"], "http") self.assertEqual(root.attributes["http.method"], "GET") self.assertEqual( @@ -379,6 +383,9 @@ def test_attributes(self): self.assertEqual(root.attributes["http.status_text"], "OK") self.assertEqual(root.attributes["misc.pi"], 3.14) self.assertEqual(root.attributes["attr-key"], "attr-value2") + self.assertEqual(root.attributes["empty-list"], []) + self.assertEqual(root.attributes["list-of-bools"], [True, True, False]) + self.assertEqual(root.attributes["list-of-numerics"], [123, 3.14, 0]) attributes = { "attr-key": "val", @@ -393,6 +400,18 @@ def test_attributes(self): self.assertEqual(root.attributes["attr-key2"], "val2") self.assertEqual(root.attributes["attr-in-both"], "span-attr") + def test_invalid_attribute_values(self): + class NonPrimitive: + pass + + with self.tracer.start_as_current_span("root") as root: + root.set_attribute("non-primitive-data-type", NonPrimitive()) + root.set_attribute("list-of-mixed-data-types-numeric-first", [123, False, "string"]) + root.set_attribute("list-of-mixed-data-types-non-numeric-first", [False, 123, "string"]) + root.set_attribute("list-with-non-primitive-data-type", [NonPrimitive(), 123]) + + self.assertEqual(len(root.attributes), 0) + def test_sampling_attributes(self): decision_attributes = { "sampler-attr": "sample-val", From bacb09beb93c700a041cd439c1a970a16ea5b50b Mon Sep 17 00:00:00 2001 From: jakemalachowski Date: Fri, 27 Dec 2019 07:11:25 -0600 Subject: [PATCH 08/23] Change order of checks to remove an else condition --- opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 5dfad79565b..e21af570c23 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -208,7 +208,10 @@ def set_attribute(self, key: str, value: types.AttributeValue) -> None: if has_ended: logger.warning("Setting attribute on ended span.") return - + + if not isinstance(value, (int, float, bool, str, list)): + logger.warning("Invalid data type for attribute value") + return # If value is of type list, validate all items are of same, # valid data type if isinstance(value, list) and len(value) > 0: @@ -226,9 +229,6 @@ def set_attribute(self, key: str, value: types.AttributeValue) -> None: else: logger.warning("Invalid data type in value array") return - elif not isinstance(value, (int, float, bool, str, list)): - logger.warning("Invalid data type for attribute value") - return self.attributes[key] = value def add_event( From 3e67a9ec44dc407e161feffb552eec70c1e847a4 Mon Sep 17 00:00:00 2001 From: jakemalachowski Date: Sat, 28 Dec 2019 19:38:50 -0600 Subject: [PATCH 09/23] Create separate sequence check method, add tests, fix linting issues --- .../src/opentelemetry/sdk/trace/__init__.py | 48 +++++++++++-------- opentelemetry-sdk/tests/trace/test_trace.py | 42 ++++++++++++---- 2 files changed, 62 insertions(+), 28 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index e21af570c23..7c580dc4d30 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -18,6 +18,7 @@ import random import threading from contextlib import contextmanager +from numbers import Number from types import TracebackType from typing import Iterator, Optional, Sequence, Tuple, Type @@ -208,29 +209,38 @@ def set_attribute(self, key: str, value: types.AttributeValue) -> None: if has_ended: logger.warning("Setting attribute on ended span.") return - - if not isinstance(value, (int, float, bool, str, list)): - logger.warning("Invalid data type for attribute value") + + if not isinstance(value, (int, float, bool, str, list, tuple)): + logger.warning("invalid type for attribute value") return - # If value is of type list, validate all items are of same, - # valid data type - if isinstance(value, list) and len(value) > 0: - first_element_data_type = type(value[0]) - # If first value is numeric, validate all values are numeric - if first_element_data_type in (int, float): - if any(not isinstance(item, (int, float)) for item in value): - logger.warning("All data types of attribute value arrays must be identical") - return - # Else, validate all values are of same type - elif first_element_data_type in (bool, str): - if any(not isinstance(item, first_element_data_type) for item in value): - logger.warning("All data types of attribute value arrays must be identical") - return - else: - logger.warning("Invalid data type in value array") + if isinstance(value, (list, tuple)) and len(value) > 0: + return_code = self._check_sequence(value) + if return_code: + logger.warning("%s in attribute value sequence", return_code) return + self.attributes[key] = value + @staticmethod + def _check_sequence(sequence: (list, tuple)) -> (str, None): + """ + Checks if sequence items are valid and identical + """ + first_element_type = type(sequence[0]) + + if issubclass(type(sequence[0]), Number): + first_element_type = Number + + for element in sequence: + + if not isinstance(element, (bool, str, Number)): + return "invalid type" + + if not isinstance(element, first_element_type): + return "different type" + + return None + def add_event( self, name: str, diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index 8047ef83fe1..1151a976cff 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -384,8 +384,12 @@ def test_attributes(self): self.assertEqual(root.attributes["misc.pi"], 3.14) self.assertEqual(root.attributes["attr-key"], "attr-value2") self.assertEqual(root.attributes["empty-list"], []) - self.assertEqual(root.attributes["list-of-bools"], [True, True, False]) - self.assertEqual(root.attributes["list-of-numerics"], [123, 3.14, 0]) + self.assertEqual( + root.attributes["list-of-bools"], [True, True, False] + ) + self.assertEqual( + root.attributes["list-of-numerics"], [123, 3.14, 0] + ) attributes = { "attr-key": "val", @@ -401,17 +405,37 @@ def test_attributes(self): self.assertEqual(root.attributes["attr-in-both"], "span-attr") def test_invalid_attribute_values(self): - class NonPrimitive: - pass - with self.tracer.start_as_current_span("root") as root: - root.set_attribute("non-primitive-data-type", NonPrimitive()) - root.set_attribute("list-of-mixed-data-types-numeric-first", [123, False, "string"]) - root.set_attribute("list-of-mixed-data-types-non-numeric-first", [False, 123, "string"]) - root.set_attribute("list-with-non-primitive-data-type", [NonPrimitive(), 123]) + root.set_attribute("non-primitive-data-type", dict()) + root.set_attribute( + "list-of-mixed-data-types-numeric-first", + [123, False, "string"], + ) + root.set_attribute( + "list-of-mixed-data-types-non-numeric-first", + [False, 123, "string"], + ) + root.set_attribute( + "list-with-non-primitive-data-type", [dict(), 123] + ) self.assertEqual(len(root.attributes), 0) + def test_check_sequence_helper(self): + # pylint: disable=protected-access + self.assertEqual( + trace.Span._check_sequence([1, 2, 3.4, "ss", 4]), "different type" + ) + self.assertEqual( + trace.Span._check_sequence([1, 2, 3.4, dict(), 4]), "invalid type" + ) + self.assertEqual( + trace.Span._check_sequence(["sw", "lf", 3.4, "ss"]), + "different type", + ) + self.assertIsNone(trace.Span._check_sequence([1, 2, 3.4, 5])) + self.assertIsNone(trace.Span._check_sequence(["ss", "dw", "fw"])) + def test_sampling_attributes(self): decision_attributes = { "sampler-attr": "sample-val", From ff61b9ebb6d620271bb215319cb23e1b5e42e6d6 Mon Sep 17 00:00:00 2001 From: jakemalachowski Date: Sat, 28 Dec 2019 19:59:44 -0600 Subject: [PATCH 10/23] Fix attribute value typing --- opentelemetry-api/src/opentelemetry/util/types.py | 2 +- opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/util/types.py b/opentelemetry-api/src/opentelemetry/util/types.py index 3d34895be13..880e96e0b89 100644 --- a/opentelemetry-api/src/opentelemetry/util/types.py +++ b/opentelemetry-api/src/opentelemetry/util/types.py @@ -15,5 +15,5 @@ import typing -AttributeValue = typing.Union[str, bool, int, float, list] +AttributeValue = typing.Union[str, bool, int, float, typing.Sequence[typing.Union[str, bool, int, float]]] Attributes = typing.Optional[typing.Dict[str, AttributeValue]] diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 7c580dc4d30..c7a8f5c8743 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -20,7 +20,7 @@ from contextlib import contextmanager from numbers import Number from types import TracebackType -from typing import Iterator, Optional, Sequence, Tuple, Type +from typing import Iterator, Optional, Sequence, Tuple, Type, Union from opentelemetry import trace as trace_api from opentelemetry.context import Context @@ -222,7 +222,7 @@ def set_attribute(self, key: str, value: types.AttributeValue) -> None: self.attributes[key] = value @staticmethod - def _check_sequence(sequence: (list, tuple)) -> (str, None): + def _check_sequence(sequence: (list, tuple)) -> Union[str, None]: """ Checks if sequence items are valid and identical """ From 4d74316505ccbb2fd81abbab84e38333d8e5aa55 Mon Sep 17 00:00:00 2001 From: jakemalachowski Date: Sat, 28 Dec 2019 20:03:50 -0600 Subject: [PATCH 11/23] Apply linting changes --- opentelemetry-api/src/opentelemetry/util/types.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/opentelemetry-api/src/opentelemetry/util/types.py b/opentelemetry-api/src/opentelemetry/util/types.py index 880e96e0b89..c52020aa43b 100644 --- a/opentelemetry-api/src/opentelemetry/util/types.py +++ b/opentelemetry-api/src/opentelemetry/util/types.py @@ -15,5 +15,7 @@ import typing -AttributeValue = typing.Union[str, bool, int, float, typing.Sequence[typing.Union[str, bool, int, float]]] +AttributeValue = typing.Union[ + str, bool, int, float, typing.Sequence[typing.Union[str, bool, int, float]] +] Attributes = typing.Optional[typing.Dict[str, AttributeValue]] From 8a7ec1c3beb249294b7d4bb8191439dc831be9d8 Mon Sep 17 00:00:00 2001 From: jakemalachowski Date: Mon, 30 Dec 2019 18:01:58 -0600 Subject: [PATCH 12/23] Use is not None, use optional type instead of union with None --- opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index c7a8f5c8743..6c1e1095f0e 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -20,7 +20,7 @@ from contextlib import contextmanager from numbers import Number from types import TracebackType -from typing import Iterator, Optional, Sequence, Tuple, Type, Union +from typing import Iterator, Optional, Sequence, Tuple, Type from opentelemetry import trace as trace_api from opentelemetry.context import Context @@ -215,14 +215,14 @@ def set_attribute(self, key: str, value: types.AttributeValue) -> None: return if isinstance(value, (list, tuple)) and len(value) > 0: return_code = self._check_sequence(value) - if return_code: + if return_code is not None: logger.warning("%s in attribute value sequence", return_code) return self.attributes[key] = value @staticmethod - def _check_sequence(sequence: (list, tuple)) -> Union[str, None]: + def _check_sequence(sequence: (list, tuple)) -> Optional[str]: """ Checks if sequence items are valid and identical """ From e7e976a3bb007761b87e03e3c9bc53258d90b565 Mon Sep 17 00:00:00 2001 From: Jake Malachowski <5766239+jakemalachowski@users.noreply.github.com> Date: Fri, 3 Jan 2020 07:40:03 -0600 Subject: [PATCH 13/23] Update opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py Co-Authored-By: Yusuke Tsutsumi --- opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 6c1e1095f0e..bda88ab1040 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -224,7 +224,7 @@ def set_attribute(self, key: str, value: types.AttributeValue) -> None: @staticmethod def _check_sequence(sequence: (list, tuple)) -> Optional[str]: """ - Checks if sequence items are valid and identical + Checks if sequence items are valid and are of the same type """ first_element_type = type(sequence[0]) From 75261d81b8833cf54f364a38413dba20f71ab2cd Mon Sep 17 00:00:00 2001 From: jakemalachowski Date: Fri, 3 Jan 2020 08:12:53 -0600 Subject: [PATCH 14/23] Clarify variable and method names, remove redundant check --- .../src/opentelemetry/sdk/trace/__init__.py | 19 ++++++++----------- opentelemetry-sdk/tests/trace/test_trace.py | 10 +++++----- 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 6c1e1095f0e..3f4687b52b7 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -214,33 +214,30 @@ def set_attribute(self, key: str, value: types.AttributeValue) -> None: logger.warning("invalid type for attribute value") return if isinstance(value, (list, tuple)) and len(value) > 0: - return_code = self._check_sequence(value) - if return_code is not None: - logger.warning("%s in attribute value sequence", return_code) + error_message = self._check_attribute_value_sequence(value) + if error_message is not None: + logger.warning("%s in attribute value sequence", error_message) return self.attributes[key] = value @staticmethod - def _check_sequence(sequence: (list, tuple)) -> Optional[str]: + def _check_attribute_value_sequence(sequence: (list, tuple)) -> Optional[str]: """ Checks if sequence items are valid and identical """ first_element_type = type(sequence[0]) - if issubclass(type(sequence[0]), Number): + if issubclass(first_element_type, Number): first_element_type = Number - for element in sequence: - - if not isinstance(element, (bool, str, Number)): - return "invalid type" + if first_element_type not in (bool, str, Number): + return "invalid type" + for element in sequence: if not isinstance(element, first_element_type): return "different type" - return None - def add_event( self, name: str, diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index 1151a976cff..342b14ae47d 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -424,17 +424,17 @@ def test_invalid_attribute_values(self): def test_check_sequence_helper(self): # pylint: disable=protected-access self.assertEqual( - trace.Span._check_sequence([1, 2, 3.4, "ss", 4]), "different type" + trace.Span._check_attribute_value_sequence([1, 2, 3.4, "ss", 4]), "different type" ) self.assertEqual( - trace.Span._check_sequence([1, 2, 3.4, dict(), 4]), "invalid type" + trace.Span._check_attribute_value_sequence([dict(), 1, 2, 3.4, 4]), "invalid type" ) self.assertEqual( - trace.Span._check_sequence(["sw", "lf", 3.4, "ss"]), + trace.Span._check_attribute_value_sequence(["sw", "lf", 3.4, "ss"]), "different type", ) - self.assertIsNone(trace.Span._check_sequence([1, 2, 3.4, 5])) - self.assertIsNone(trace.Span._check_sequence(["ss", "dw", "fw"])) + self.assertIsNone(trace.Span._check_attribute_value_sequence([1, 2, 3.4, 5])) + self.assertIsNone(trace.Span._check_attribute_value_sequence(["ss", "dw", "fw"])) def test_sampling_attributes(self): decision_attributes = { From 1321134f465d2c93e7eabffe68258684ede82ddb Mon Sep 17 00:00:00 2001 From: jakemalachowski Date: Fri, 3 Jan 2020 08:31:54 -0600 Subject: [PATCH 15/23] Re-add explicit return None --- opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 2b4ffa50ce8..5e09f67f55d 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -222,7 +222,9 @@ def set_attribute(self, key: str, value: types.AttributeValue) -> None: self.attributes[key] = value @staticmethod - def _check_attribute_value_sequence(sequence: (list, tuple)) -> Optional[str]: + def _check_attribute_value_sequence( + sequence: (list, tuple) + ) -> Optional[str]: """ Checks if sequence items are valid and are of the same type """ @@ -237,6 +239,7 @@ def _check_attribute_value_sequence(sequence: (list, tuple)) -> Optional[str]: for element in sequence: if not isinstance(element, first_element_type): return "different type" + return None def add_event( self, From 269b006f850e10626e80529b3437e33b98cb6412 Mon Sep 17 00:00:00 2001 From: jakemalachowski Date: Mon, 6 Jan 2020 19:20:37 -0600 Subject: [PATCH 16/23] Commit lint changes --- .../src/opentelemetry/sdk/trace/__init__.py | 8 +++----- opentelemetry-sdk/tests/trace/test_trace.py | 18 +++++++++++++----- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 5e09f67f55d..ac35102ef1c 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -213,7 +213,7 @@ def set_attribute(self, key: str, value: types.AttributeValue) -> None: if not isinstance(value, (int, float, bool, str, list, tuple)): logger.warning("invalid type for attribute value") return - if isinstance(value, (list, tuple)) and len(value) > 0: + if isinstance(value, Sequence) and len(value) > 0: error_message = self._check_attribute_value_sequence(value) if error_message is not None: logger.warning("%s in attribute value sequence", error_message) @@ -222,9 +222,7 @@ def set_attribute(self, key: str, value: types.AttributeValue) -> None: self.attributes[key] = value @staticmethod - def _check_attribute_value_sequence( - sequence: (list, tuple) - ) -> Optional[str]: + def _check_attribute_value_sequence(sequence: Sequence) -> Optional[str]: """ Checks if sequence items are valid and are of the same type """ @@ -239,7 +237,7 @@ def _check_attribute_value_sequence( for element in sequence: if not isinstance(element, first_element_type): return "different type" - return None + return def add_event( self, diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index 342b14ae47d..5f32f775fe9 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -424,17 +424,25 @@ def test_invalid_attribute_values(self): def test_check_sequence_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_sequence([1, 2, 3.4, "ss", 4]), + "different type", ) self.assertEqual( - trace.Span._check_attribute_value_sequence([dict(), 1, 2, 3.4, 4]), "invalid type" + trace.Span._check_attribute_value_sequence([dict(), 1, 2, 3.4, 4]), + "invalid type", ) self.assertEqual( - trace.Span._check_attribute_value_sequence(["sw", "lf", 3.4, "ss"]), + trace.Span._check_attribute_value_sequence( + ["sw", "lf", 3.4, "ss"] + ), "different type", ) - self.assertIsNone(trace.Span._check_attribute_value_sequence([1, 2, 3.4, 5])) - self.assertIsNone(trace.Span._check_attribute_value_sequence(["ss", "dw", "fw"])) + self.assertIsNone( + trace.Span._check_attribute_value_sequence([1, 2, 3.4, 5]) + ) + self.assertIsNone( + trace.Span._check_attribute_value_sequence(["ss", "dw", "fw"]) + ) def test_sampling_attributes(self): decision_attributes = { From 669b3eb5f43876dbe62297a59631949eb2be94bc Mon Sep 17 00:00:00 2001 From: jakemalachowski Date: Mon, 6 Jan 2020 19:47:32 -0600 Subject: [PATCH 17/23] Lint changes --- .../src/opentelemetry/ext/mysql/__init__.py | 1 - ext/opentelemetry-ext-mysql/tests/test_mysql_integration.py | 1 - opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py | 2 +- 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/ext/opentelemetry-ext-mysql/src/opentelemetry/ext/mysql/__init__.py b/ext/opentelemetry-ext-mysql/src/opentelemetry/ext/mysql/__init__.py index 9c8c3e9da70..e809e76398e 100644 --- a/ext/opentelemetry-ext-mysql/src/opentelemetry/ext/mysql/__init__.py +++ b/ext/opentelemetry-ext-mysql/src/opentelemetry/ext/mysql/__init__.py @@ -18,7 +18,6 @@ """ import mysql.connector - from opentelemetry.ext.dbapi import trace_integration as db_integration from opentelemetry.trace import Tracer diff --git a/ext/opentelemetry-ext-mysql/tests/test_mysql_integration.py b/ext/opentelemetry-ext-mysql/tests/test_mysql_integration.py index 1bcd851750c..1006817ee6b 100644 --- a/ext/opentelemetry-ext-mysql/tests/test_mysql_integration.py +++ b/ext/opentelemetry-ext-mysql/tests/test_mysql_integration.py @@ -16,7 +16,6 @@ from unittest import mock import mysql.connector - from opentelemetry import trace as trace_api from opentelemetry.ext.mysql import trace_integration diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index ac35102ef1c..d4c8c359058 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -237,7 +237,7 @@ def _check_attribute_value_sequence(sequence: Sequence) -> Optional[str]: for element in sequence: if not isinstance(element, first_element_type): return "different type" - return + return None def add_event( self, From 4a5812c162ad2e090ab4757931c266bd9b7aa5c0 Mon Sep 17 00:00:00 2001 From: jakemalachowski Date: Mon, 6 Jan 2020 19:51:11 -0600 Subject: [PATCH 18/23] Lint changes --- .../src/opentelemetry/ext/mysql/__init__.py | 1 + ext/opentelemetry-ext-mysql/tests/test_mysql_integration.py | 1 + 2 files changed, 2 insertions(+) diff --git a/ext/opentelemetry-ext-mysql/src/opentelemetry/ext/mysql/__init__.py b/ext/opentelemetry-ext-mysql/src/opentelemetry/ext/mysql/__init__.py index e809e76398e..9c8c3e9da70 100644 --- a/ext/opentelemetry-ext-mysql/src/opentelemetry/ext/mysql/__init__.py +++ b/ext/opentelemetry-ext-mysql/src/opentelemetry/ext/mysql/__init__.py @@ -18,6 +18,7 @@ """ import mysql.connector + from opentelemetry.ext.dbapi import trace_integration as db_integration from opentelemetry.trace import Tracer diff --git a/ext/opentelemetry-ext-mysql/tests/test_mysql_integration.py b/ext/opentelemetry-ext-mysql/tests/test_mysql_integration.py index 1006817ee6b..1bcd851750c 100644 --- a/ext/opentelemetry-ext-mysql/tests/test_mysql_integration.py +++ b/ext/opentelemetry-ext-mysql/tests/test_mysql_integration.py @@ -16,6 +16,7 @@ from unittest import mock import mysql.connector + from opentelemetry import trace as trace_api from opentelemetry.ext.mysql import trace_integration From 6f1dd5d6fd86ed148470bd47e5712ca00594d356 Mon Sep 17 00:00:00 2001 From: jakemalachowski Date: Sat, 11 Jan 2020 12:02:36 -0500 Subject: [PATCH 19/23] Use number instead of int, float Change typing to prevent heterogeneous types in lists --- opentelemetry-api/src/opentelemetry/util/types.py | 15 +++++++++++---- .../src/opentelemetry/sdk/trace/__init__.py | 2 +- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/util/types.py b/opentelemetry-api/src/opentelemetry/util/types.py index c52020aa43b..5ce93d84b25 100644 --- a/opentelemetry-api/src/opentelemetry/util/types.py +++ b/opentelemetry-api/src/opentelemetry/util/types.py @@ -13,9 +13,16 @@ # limitations under the License. -import typing +from typing import Dict, Optional, Sequence, Union -AttributeValue = typing.Union[ - str, bool, int, float, typing.Sequence[typing.Union[str, bool, int, float]] +AttributeValue = Union[ + str, + bool, + int, + float, + Sequence[str], + Sequence[bool], + Sequence[int], + Sequence[float], ] -Attributes = typing.Optional[typing.Dict[str, AttributeValue]] +Attributes = Optional[Dict[str, AttributeValue]] diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index d4c8c359058..a0b162cb4ac 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -210,7 +210,7 @@ def set_attribute(self, key: str, value: types.AttributeValue) -> None: logger.warning("Setting attribute on ended span.") return - if not isinstance(value, (int, float, bool, str, list, tuple)): + if not isinstance(value, (bool, str, Number, Sequence)): logger.warning("invalid type for attribute value") return if isinstance(value, Sequence) and len(value) > 0: From 5225928aa80e4a46d86f385b5425edbafa4a34ae Mon Sep 17 00:00:00 2001 From: jakemalachowski Date: Thu, 16 Jan 2020 10:39:55 -0500 Subject: [PATCH 20/23] Revert AttributeValue typing change --- opentelemetry-api/src/opentelemetry/util/types.py | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/util/types.py b/opentelemetry-api/src/opentelemetry/util/types.py index 5ce93d84b25..4742a7ec690 100644 --- a/opentelemetry-api/src/opentelemetry/util/types.py +++ b/opentelemetry-api/src/opentelemetry/util/types.py @@ -13,16 +13,7 @@ # limitations under the License. -from typing import Dict, Optional, Sequence, Union +import typing -AttributeValue = Union[ - str, - bool, - int, - float, - Sequence[str], - Sequence[bool], - Sequence[int], - Sequence[float], -] -Attributes = Optional[Dict[str, AttributeValue]] +AttributeValue = typing.Union[str, bool, float] +Attributes = typing.Optional[typing.Dict[str, AttributeValue]] \ No newline at end of file From b83dc7b30a9924e4af04f3fa76ef8f82d755cf75 Mon Sep 17 00:00:00 2001 From: jakemalachowski Date: Fri, 17 Jan 2020 06:51:57 -0500 Subject: [PATCH 21/23] Prevent duplicate isinstance checks, run linter --- opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index a0b162cb4ac..0f7d425bbaa 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -210,14 +210,14 @@ def set_attribute(self, key: str, value: types.AttributeValue) -> None: logger.warning("Setting attribute on ended span.") return - if not isinstance(value, (bool, str, Number, Sequence)): - logger.warning("invalid type for attribute value") - return if isinstance(value, Sequence) and len(value) > 0: error_message = self._check_attribute_value_sequence(value) if error_message is not None: logger.warning("%s in attribute value sequence", error_message) return + elif not isinstance(value, (bool, str, Number, Sequence)): + logger.warning("invalid type for attribute value") + return self.attributes[key] = value From d8e5946750838a415a015821d6d1d688569f439d Mon Sep 17 00:00:00 2001 From: jakemalachowski Date: Fri, 17 Jan 2020 07:01:13 -0500 Subject: [PATCH 22/23] Lint --- opentelemetry-api/src/opentelemetry/util/types.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-api/src/opentelemetry/util/types.py b/opentelemetry-api/src/opentelemetry/util/types.py index 4742a7ec690..28fab893890 100644 --- a/opentelemetry-api/src/opentelemetry/util/types.py +++ b/opentelemetry-api/src/opentelemetry/util/types.py @@ -16,4 +16,4 @@ import typing AttributeValue = typing.Union[str, bool, float] -Attributes = typing.Optional[typing.Dict[str, AttributeValue]] \ No newline at end of file +Attributes = typing.Optional[typing.Dict[str, AttributeValue]] From ad588904c7f2d3fc0289eab9b3ac553ec772b4d2 Mon Sep 17 00:00:00 2001 From: jakemalachowski Date: Sat, 18 Jan 2020 15:14:49 -0500 Subject: [PATCH 23/23] Move length check inside validation method --- opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 0f7d425bbaa..ed54639368c 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -210,7 +210,7 @@ def set_attribute(self, key: str, value: types.AttributeValue) -> None: logger.warning("Setting attribute on ended span.") return - if isinstance(value, Sequence) and len(value) > 0: + 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) @@ -226,6 +226,9 @@ def _check_attribute_value_sequence(sequence: Sequence) -> Optional[str]: """ Checks if sequence items are valid and are of the same type """ + if len(sequence) == 0: + return None + first_element_type = type(sequence[0]) if issubclass(first_element_type, Number):