From c10311a1315884dbc39d09fee9a12995e012a63b Mon Sep 17 00:00:00 2001 From: Rob Knox Date: Mon, 26 Oct 2020 14:50:21 -0700 Subject: [PATCH 01/10] Adding support for array attributes to Zipkin exporter (#1110) --- .../opentelemetry/exporter/zipkin/__init__.py | 38 +++++ .../tests/test_zipkin_exporter.py | 160 +++++++++++++++++- 2 files changed, 192 insertions(+), 6 deletions(-) diff --git a/exporter/opentelemetry-exporter-zipkin/src/opentelemetry/exporter/zipkin/__init__.py b/exporter/opentelemetry-exporter-zipkin/src/opentelemetry/exporter/zipkin/__init__.py index c8578c9649c..a7ee95c64da 100644 --- a/exporter/opentelemetry-exporter-zipkin/src/opentelemetry/exporter/zipkin/__init__.py +++ b/exporter/opentelemetry-exporter-zipkin/src/opentelemetry/exporter/zipkin/__init__.py @@ -222,6 +222,11 @@ def _extract_tags_from_dict(self, tags_dict): value = str(attribute_value) elif isinstance(attribute_value, str): value = attribute_value + elif isinstance(attribute_value, Sequence): + value = self._extract_tag_value_string_from_sequence(attribute_value) + if not value: + logger.warning("Could not serialize tag %s", attribute_key) + continue else: logger.warning("Could not serialize tag %s", attribute_key) continue @@ -231,6 +236,39 @@ def _extract_tags_from_dict(self, tags_dict): tags[attribute_key] = value return tags + def _extract_tag_value_string_from_sequence(self, sequence): + if not sequence: + return None + + if self.max_tag_value_length == 1: + return None + + tag_value_elements = [] + running_string_length = 2 # accounts for array brackets in output string + defined_max_tag_value_length = self.max_tag_value_length > 0 + + if isinstance(sequence, (list, tuple, range)): + for element in sequence: + if isinstance(element, (int, bool, float)): + tag_value_element = str(element) + elif isinstance(element, str): + tag_value_element = element + else: + return None + + if defined_max_tag_value_length: + running_string_length += len(tag_value_element) + 2 # +2 accounts for surrounding quotes + if tag_value_elements: + running_string_length += 2 # accounts for ', ' connector + if running_string_length > self.max_tag_value_length: + break + + tag_value_elements.append(tag_value_element) + else: + return None + + return json.dumps(tag_value_elements) + def _extract_tags_from_span(self, span: Span): tags = self._extract_tags_from_dict(getattr(span, "attributes", None)) if span.resource: diff --git a/exporter/opentelemetry-exporter-zipkin/tests/test_zipkin_exporter.py b/exporter/opentelemetry-exporter-zipkin/tests/test_zipkin_exporter.py index c3098da8847..c19db032d4e 100644 --- a/exporter/opentelemetry-exporter-zipkin/tests/test_zipkin_exporter.py +++ b/exporter/opentelemetry-exporter-zipkin/tests/test_zipkin_exporter.py @@ -398,8 +398,23 @@ def test_max_tag_length(self): span.start() span.resource = Resource({}) # added here to preserve order - span.set_attribute("k1", "v" * 500) - span.set_attribute("k2", "v" * 50) + span.set_attribute("string1", "v" * 500) + span.set_attribute("string2", "v" * 50) + span.set_attribute("list1", ["a"] * 25) + span.set_attribute("list2", ["a"] * 10) + span.set_attribute("list3", [2] * 25) + span.set_attribute("list4", [2] * 10) + span.set_attribute("list5", [True] * 25) + span.set_attribute("list6", [True] * 10) + span.set_attribute("tuple1", ("a",) * 25) + span.set_attribute("tuple2", ("a",) * 10) + span.set_attribute("tuple3", (2,) * 25) + span.set_attribute("tuple4", (2,) * 10) + span.set_attribute("tuple5", (True,) * 25) + span.set_attribute("tuple6", (True,) * 10) + span.set_attribute("range1", range(0, 25)) + span.set_attribute("range2", range(0, 10)) + span.set_status( Status(StatusCanonicalCode.UNKNOWN, "Example description") ) @@ -415,8 +430,23 @@ def test_max_tag_length(self): _, kwargs = mock_post.call_args # pylint: disable=E0633 tags = json.loads(kwargs["data"])[0]["tags"] - self.assertEqual(len(tags["k1"]), 128) - self.assertEqual(len(tags["k2"]), 50) + + self.assertEqual(len(tags["string1"]), 128) + self.assertEqual(len(tags["string2"]), 50) + self.assertEqual(tags["list1"], '["a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a"]') + self.assertEqual(tags["list2"], '["a", "a", "a", "a", "a", "a", "a", "a", "a", "a"]') + self.assertEqual(tags["list3"], '["2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2"]') + self.assertEqual(tags["list4"], '["2", "2", "2", "2", "2", "2", "2", "2", "2", "2"]') + self.assertEqual(tags["list5"], '["True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True"]') + self.assertEqual(tags["list6"], '["True", "True", "True", "True", "True", "True", "True", "True", "True", "True"]') + self.assertEqual(tags["tuple1"], '["a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a"]') + self.assertEqual(tags["tuple2"], '["a", "a", "a", "a", "a", "a", "a", "a", "a", "a"]') + self.assertEqual(tags["tuple3"], '["2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2"]') + self.assertEqual(tags["tuple4"], '["2", "2", "2", "2", "2", "2", "2", "2", "2", "2"]') + self.assertEqual(tags["tuple5"], '["True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True"]') + self.assertEqual(tags["tuple6"], '["True", "True", "True", "True", "True", "True", "True", "True", "True", "True"]') + self.assertEqual(tags["range1"], '["0", "1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "11", "12", "13", "14", "15", "16", "17", "18", "19", "20", "21", "22"]') + self.assertEqual(tags["range2"], '["0", "1", "2", "3", "4", "5", "6", "7", "8", "9"]') exporter = ZipkinSpanExporter(service_name, max_tag_value_length=2) mock_post = MagicMock() @@ -427,5 +457,123 @@ def test_max_tag_length(self): _, kwargs = mock_post.call_args # pylint: disable=E0633 tags = json.loads(kwargs["data"])[0]["tags"] - self.assertEqual(len(tags["k1"]), 2) - self.assertEqual(len(tags["k2"]), 2) + self.assertEqual(len(tags["string1"]), 2) + self.assertEqual(len(tags["string2"]), 2) + self.assertEqual(tags["list1"], '[]') + self.assertEqual(tags["list2"], '[]') + self.assertEqual(tags["list3"], '[]') + self.assertEqual(tags["list4"], '[]') + self.assertEqual(tags["list5"], '[]') + self.assertEqual(tags["list6"], '[]') + self.assertEqual(tags["tuple1"], '[]') + self.assertEqual(tags["tuple2"], '[]') + self.assertEqual(tags["tuple3"], '[]') + self.assertEqual(tags["tuple4"], '[]') + self.assertEqual(tags["tuple5"], '[]') + self.assertEqual(tags["tuple6"], '[]') + self.assertEqual(tags["range1"], '[]') + self.assertEqual(tags["range2"], '[]') + + exporter = ZipkinSpanExporter(service_name, max_tag_value_length=5) + mock_post = MagicMock() + with patch("requests.post", mock_post): + mock_post.return_value = MockResponse(200) + status = exporter.export([span]) + self.assertEqual(SpanExportResult.SUCCESS, status) + + _, kwargs = mock_post.call_args # pylint: disable=E0633 + tags = json.loads(kwargs["data"])[0]["tags"] + self.assertEqual(len(tags["string1"]), 5) + self.assertEqual(len(tags["string2"]), 5) + self.assertEqual(tags["list1"], '["a"]') + self.assertEqual(tags["list2"], '["a"]') + self.assertEqual(tags["list3"], '["2"]') + self.assertEqual(tags["list4"], '["2"]') + self.assertEqual(tags["list5"], '[]') + self.assertEqual(tags["list6"], '[]') + self.assertEqual(tags["tuple1"], '["a"]') + self.assertEqual(tags["tuple2"], '["a"]') + self.assertEqual(tags["tuple3"], '["2"]') + self.assertEqual(tags["tuple4"], '["2"]') + self.assertEqual(tags["tuple5"], '[]') + self.assertEqual(tags["tuple6"], '[]') + self.assertEqual(tags["range1"], '["0"]') + self.assertEqual(tags["range2"], '["0"]') + + exporter = ZipkinSpanExporter(service_name, max_tag_value_length=9) + mock_post = MagicMock() + with patch("requests.post", mock_post): + mock_post.return_value = MockResponse(200) + status = exporter.export([span]) + self.assertEqual(SpanExportResult.SUCCESS, status) + + _, kwargs = mock_post.call_args # pylint: disable=E0633 + tags = json.loads(kwargs["data"])[0]["tags"] + self.assertEqual(len(tags["string1"]), 9) + self.assertEqual(len(tags["string2"]), 9) + self.assertEqual(tags["list1"], '["a"]') + self.assertEqual(tags["list2"], '["a"]') + self.assertEqual(tags["list3"], '["2"]') + self.assertEqual(tags["list4"], '["2"]') + self.assertEqual(tags["list5"], '["True"]') + self.assertEqual(tags["list6"], '["True"]') + self.assertEqual(tags["tuple1"], '["a"]') + self.assertEqual(tags["tuple2"], '["a"]') + self.assertEqual(tags["tuple3"], '["2"]') + self.assertEqual(tags["tuple4"], '["2"]') + self.assertEqual(tags["tuple5"], '["True"]') + self.assertEqual(tags["tuple6"], '["True"]') + self.assertEqual(tags["range1"], '["0"]') + self.assertEqual(tags["range2"], '["0"]') + + exporter = ZipkinSpanExporter(service_name, max_tag_value_length=10) + mock_post = MagicMock() + with patch("requests.post", mock_post): + mock_post.return_value = MockResponse(200) + status = exporter.export([span]) + self.assertEqual(SpanExportResult.SUCCESS, status) + + _, kwargs = mock_post.call_args # pylint: disable=E0633 + tags = json.loads(kwargs["data"])[0]["tags"] + self.assertEqual(len(tags["string1"]), 10) + self.assertEqual(len(tags["string2"]), 10) + self.assertEqual(tags["list1"], '["a", "a"]') + self.assertEqual(tags["list2"], '["a", "a"]') + self.assertEqual(tags["list3"], '["2", "2"]') + self.assertEqual(tags["list4"], '["2", "2"]') + self.assertEqual(tags["list5"], '["True"]') + self.assertEqual(tags["list6"], '["True"]') + self.assertEqual(tags["tuple1"], '["a", "a"]') + self.assertEqual(tags["tuple2"], '["a", "a"]') + self.assertEqual(tags["tuple3"], '["2", "2"]') + self.assertEqual(tags["tuple4"], '["2", "2"]') + self.assertEqual(tags["tuple5"], '["True"]') + self.assertEqual(tags["tuple6"], '["True"]') + self.assertEqual(tags["range1"], '["0", "1"]') + self.assertEqual(tags["range2"], '["0", "1"]') + + exporter = ZipkinSpanExporter(service_name, max_tag_value_length=11) + mock_post = MagicMock() + with patch("requests.post", mock_post): + mock_post.return_value = MockResponse(200) + status = exporter.export([span]) + self.assertEqual(SpanExportResult.SUCCESS, status) + + _, kwargs = mock_post.call_args # pylint: disable=E0633 + tags = json.loads(kwargs["data"])[0]["tags"] + self.assertEqual(len(tags["string1"]), 11) + self.assertEqual(len(tags["string2"]), 11) + self.assertEqual(tags["list1"], '["a", "a"]') + self.assertEqual(tags["list2"], '["a", "a"]') + self.assertEqual(tags["list3"], '["2", "2"]') + self.assertEqual(tags["list4"], '["2", "2"]') + self.assertEqual(tags["list5"], '["True"]') + self.assertEqual(tags["list6"], '["True"]') + self.assertEqual(tags["tuple1"], '["a", "a"]') + self.assertEqual(tags["tuple2"], '["a", "a"]') + self.assertEqual(tags["tuple3"], '["2", "2"]') + self.assertEqual(tags["tuple4"], '["2", "2"]') + self.assertEqual(tags["tuple5"], '["True"]') + self.assertEqual(tags["tuple6"], '["True"]') + self.assertEqual(tags["range1"], '["0", "1"]') + self.assertEqual(tags["range2"], '["0", "1"]') From 83a868cc0f0fce5a7a52ff5c71b041bf76bdd357 Mon Sep 17 00:00:00 2001 From: Rob Knox Date: Mon, 26 Oct 2020 16:47:57 -0700 Subject: [PATCH 02/10] lint changes (#1110) --- .../opentelemetry/exporter/zipkin/__init__.py | 16 ++- .../tests/test_zipkin_exporter.py | 106 ++++++++++++------ 2 files changed, 86 insertions(+), 36 deletions(-) diff --git a/exporter/opentelemetry-exporter-zipkin/src/opentelemetry/exporter/zipkin/__init__.py b/exporter/opentelemetry-exporter-zipkin/src/opentelemetry/exporter/zipkin/__init__.py index a7ee95c64da..8b76affc995 100644 --- a/exporter/opentelemetry-exporter-zipkin/src/opentelemetry/exporter/zipkin/__init__.py +++ b/exporter/opentelemetry-exporter-zipkin/src/opentelemetry/exporter/zipkin/__init__.py @@ -223,7 +223,9 @@ def _extract_tags_from_dict(self, tags_dict): elif isinstance(attribute_value, str): value = attribute_value elif isinstance(attribute_value, Sequence): - value = self._extract_tag_value_string_from_sequence(attribute_value) + value = self._extract_tag_value_string_from_sequence( + attribute_value + ) if not value: logger.warning("Could not serialize tag %s", attribute_key) continue @@ -244,7 +246,9 @@ def _extract_tag_value_string_from_sequence(self, sequence): return None tag_value_elements = [] - running_string_length = 2 # accounts for array brackets in output string + running_string_length = ( + 2 # accounts for array brackets in output string + ) defined_max_tag_value_length = self.max_tag_value_length > 0 if isinstance(sequence, (list, tuple, range)): @@ -257,9 +261,13 @@ def _extract_tag_value_string_from_sequence(self, sequence): return None if defined_max_tag_value_length: - running_string_length += len(tag_value_element) + 2 # +2 accounts for surrounding quotes + running_string_length += ( + len(tag_value_element) + 2 + ) # +2 accounts for surrounding quotes if tag_value_elements: - running_string_length += 2 # accounts for ', ' connector + running_string_length += ( + 2 # accounts for ', ' connector + ) if running_string_length > self.max_tag_value_length: break diff --git a/exporter/opentelemetry-exporter-zipkin/tests/test_zipkin_exporter.py b/exporter/opentelemetry-exporter-zipkin/tests/test_zipkin_exporter.py index c19db032d4e..e7dbc6481c3 100644 --- a/exporter/opentelemetry-exporter-zipkin/tests/test_zipkin_exporter.py +++ b/exporter/opentelemetry-exporter-zipkin/tests/test_zipkin_exporter.py @@ -433,20 +433,62 @@ def test_max_tag_length(self): self.assertEqual(len(tags["string1"]), 128) self.assertEqual(len(tags["string2"]), 50) - self.assertEqual(tags["list1"], '["a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a"]') - self.assertEqual(tags["list2"], '["a", "a", "a", "a", "a", "a", "a", "a", "a", "a"]') - self.assertEqual(tags["list3"], '["2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2"]') - self.assertEqual(tags["list4"], '["2", "2", "2", "2", "2", "2", "2", "2", "2", "2"]') - self.assertEqual(tags["list5"], '["True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True"]') - self.assertEqual(tags["list6"], '["True", "True", "True", "True", "True", "True", "True", "True", "True", "True"]') - self.assertEqual(tags["tuple1"], '["a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a"]') - self.assertEqual(tags["tuple2"], '["a", "a", "a", "a", "a", "a", "a", "a", "a", "a"]') - self.assertEqual(tags["tuple3"], '["2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2"]') - self.assertEqual(tags["tuple4"], '["2", "2", "2", "2", "2", "2", "2", "2", "2", "2"]') - self.assertEqual(tags["tuple5"], '["True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True"]') - self.assertEqual(tags["tuple6"], '["True", "True", "True", "True", "True", "True", "True", "True", "True", "True"]') - self.assertEqual(tags["range1"], '["0", "1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "11", "12", "13", "14", "15", "16", "17", "18", "19", "20", "21", "22"]') - self.assertEqual(tags["range2"], '["0", "1", "2", "3", "4", "5", "6", "7", "8", "9"]') + self.assertEqual( + tags["list1"], + '["a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a"]', + ) + self.assertEqual( + tags["list2"], + '["a", "a", "a", "a", "a", "a", "a", "a", "a", "a"]', + ) + self.assertEqual( + tags["list3"], + '["2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2"]', + ) + self.assertEqual( + tags["list4"], + '["2", "2", "2", "2", "2", "2", "2", "2", "2", "2"]', + ) + self.assertEqual( + tags["list5"], + '["True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True"]', + ) + self.assertEqual( + tags["list6"], + '["True", "True", "True", "True", "True", "True", "True", "True", "True", "True"]', + ) + self.assertEqual( + tags["tuple1"], + '["a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a"]', + ) + self.assertEqual( + tags["tuple2"], + '["a", "a", "a", "a", "a", "a", "a", "a", "a", "a"]', + ) + self.assertEqual( + tags["tuple3"], + '["2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2"]', + ) + self.assertEqual( + tags["tuple4"], + '["2", "2", "2", "2", "2", "2", "2", "2", "2", "2"]', + ) + self.assertEqual( + tags["tuple5"], + '["True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True"]', + ) + self.assertEqual( + tags["tuple6"], + '["True", "True", "True", "True", "True", "True", "True", "True", "True", "True"]', + ) + self.assertEqual( + tags["range1"], + '["0", "1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "11", "12", "13", "14", "15", "16", "17", "18", "19", "20", "21", "22"]', + ) + self.assertEqual( + tags["range2"], + '["0", "1", "2", "3", "4", "5", "6", "7", "8", "9"]', + ) exporter = ZipkinSpanExporter(service_name, max_tag_value_length=2) mock_post = MagicMock() @@ -459,20 +501,20 @@ def test_max_tag_length(self): tags = json.loads(kwargs["data"])[0]["tags"] self.assertEqual(len(tags["string1"]), 2) self.assertEqual(len(tags["string2"]), 2) - self.assertEqual(tags["list1"], '[]') - self.assertEqual(tags["list2"], '[]') - self.assertEqual(tags["list3"], '[]') - self.assertEqual(tags["list4"], '[]') - self.assertEqual(tags["list5"], '[]') - self.assertEqual(tags["list6"], '[]') - self.assertEqual(tags["tuple1"], '[]') - self.assertEqual(tags["tuple2"], '[]') - self.assertEqual(tags["tuple3"], '[]') - self.assertEqual(tags["tuple4"], '[]') - self.assertEqual(tags["tuple5"], '[]') - self.assertEqual(tags["tuple6"], '[]') - self.assertEqual(tags["range1"], '[]') - self.assertEqual(tags["range2"], '[]') + self.assertEqual(tags["list1"], "[]") + self.assertEqual(tags["list2"], "[]") + self.assertEqual(tags["list3"], "[]") + self.assertEqual(tags["list4"], "[]") + self.assertEqual(tags["list5"], "[]") + self.assertEqual(tags["list6"], "[]") + self.assertEqual(tags["tuple1"], "[]") + self.assertEqual(tags["tuple2"], "[]") + self.assertEqual(tags["tuple3"], "[]") + self.assertEqual(tags["tuple4"], "[]") + self.assertEqual(tags["tuple5"], "[]") + self.assertEqual(tags["tuple6"], "[]") + self.assertEqual(tags["range1"], "[]") + self.assertEqual(tags["range2"], "[]") exporter = ZipkinSpanExporter(service_name, max_tag_value_length=5) mock_post = MagicMock() @@ -489,14 +531,14 @@ def test_max_tag_length(self): self.assertEqual(tags["list2"], '["a"]') self.assertEqual(tags["list3"], '["2"]') self.assertEqual(tags["list4"], '["2"]') - self.assertEqual(tags["list5"], '[]') - self.assertEqual(tags["list6"], '[]') + self.assertEqual(tags["list5"], "[]") + self.assertEqual(tags["list6"], "[]") self.assertEqual(tags["tuple1"], '["a"]') self.assertEqual(tags["tuple2"], '["a"]') self.assertEqual(tags["tuple3"], '["2"]') self.assertEqual(tags["tuple4"], '["2"]') - self.assertEqual(tags["tuple5"], '[]') - self.assertEqual(tags["tuple6"], '[]') + self.assertEqual(tags["tuple5"], "[]") + self.assertEqual(tags["tuple6"], "[]") self.assertEqual(tags["range1"], '["0"]') self.assertEqual(tags["range2"], '["0"]') From 779b0903b927d9515d594a04f575b7d0a99eb73f Mon Sep 17 00:00:00 2001 From: Rob Knox Date: Tue, 27 Oct 2020 09:50:59 -0700 Subject: [PATCH 03/10] CHANGELOG entry (#1110) --- exporter/opentelemetry-exporter-zipkin/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/exporter/opentelemetry-exporter-zipkin/CHANGELOG.md b/exporter/opentelemetry-exporter-zipkin/CHANGELOG.md index 60d7fbe2ec0..0f66735cbe5 100644 --- a/exporter/opentelemetry-exporter-zipkin/CHANGELOG.md +++ b/exporter/opentelemetry-exporter-zipkin/CHANGELOG.md @@ -2,6 +2,8 @@ ## Unreleased +- Add support for array attributes in Span and Resource exports ([#1110](https://github.com/open-telemetry/opentelemetry-python/pull/1110)) + ## Version 0.14b0 Released 2020-10-13 From 1033a761a6fb93746ffb9f5be86390b7ef75612c Mon Sep 17 00:00:00 2001 From: Rob Knox Date: Fri, 13 Nov 2020 15:47:35 -0800 Subject: [PATCH 04/10] changelog --- exporter/opentelemetry-exporter-zipkin/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exporter/opentelemetry-exporter-zipkin/CHANGELOG.md b/exporter/opentelemetry-exporter-zipkin/CHANGELOG.md index 0f66735cbe5..140782fcec0 100644 --- a/exporter/opentelemetry-exporter-zipkin/CHANGELOG.md +++ b/exporter/opentelemetry-exporter-zipkin/CHANGELOG.md @@ -2,7 +2,7 @@ ## Unreleased -- Add support for array attributes in Span and Resource exports ([#1110](https://github.com/open-telemetry/opentelemetry-python/pull/1110)) +- Add support for array attributes in Span and Resource exports ([#1285](https://github.com/open-telemetry/opentelemetry-python/pull/1285)) ## Version 0.14b0 From 1d3a63f287360effb2faac33cc36ef6a3bab6f8e Mon Sep 17 00:00:00 2001 From: Rob Knox Date: Fri, 13 Nov 2020 15:50:55 -0800 Subject: [PATCH 05/10] lint --- .../tests/test_zipkin_exporter.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/exporter/opentelemetry-exporter-zipkin/tests/test_zipkin_exporter.py b/exporter/opentelemetry-exporter-zipkin/tests/test_zipkin_exporter.py index a8a9f219d59..43589561119 100644 --- a/exporter/opentelemetry-exporter-zipkin/tests/test_zipkin_exporter.py +++ b/exporter/opentelemetry-exporter-zipkin/tests/test_zipkin_exporter.py @@ -415,9 +415,7 @@ def test_max_tag_length(self): span.set_attribute("range1", range(0, 25)) span.set_attribute("range2", range(0, 10)) - span.set_status( - Status(StatusCode.ERROR, "Example description") - ) + span.set_status(Status(StatusCode.ERROR, "Example description")) span.end() exporter = ZipkinSpanExporter(service_name) From 014f5e4d784d79fb320c893d6f0a39d5e899c292 Mon Sep 17 00:00:00 2001 From: Rob Knox Date: Wed, 2 Dec 2020 15:22:13 -0800 Subject: [PATCH 06/10] review refactor --- .../opentelemetry/exporter/zipkin/__init__.py | 44 ++++++++----------- 1 file changed, 19 insertions(+), 25 deletions(-) diff --git a/exporter/opentelemetry-exporter-zipkin/src/opentelemetry/exporter/zipkin/__init__.py b/exporter/opentelemetry-exporter-zipkin/src/opentelemetry/exporter/zipkin/__init__.py index 995bb3f760a..c42846bf482 100644 --- a/exporter/opentelemetry-exporter-zipkin/src/opentelemetry/exporter/zipkin/__init__.py +++ b/exporter/opentelemetry-exporter-zipkin/src/opentelemetry/exporter/zipkin/__init__.py @@ -371,11 +371,8 @@ def _extract_tags_from_dict(self, tags_dict): tags[attribute_key] = value return tags - def _extract_tag_value_string_from_sequence(self, sequence): - if not sequence: - return None - - if self.max_tag_value_length == 1: + def _extract_tag_value_string_from_sequence(self, sequence: Sequence): + if not sequence or self.max_tag_value_length == 1: return None tag_value_elements = [] @@ -384,29 +381,26 @@ def _extract_tag_value_string_from_sequence(self, sequence): ) defined_max_tag_value_length = self.max_tag_value_length > 0 - if isinstance(sequence, (list, tuple, range)): - for element in sequence: - if isinstance(element, (int, bool, float)): - tag_value_element = str(element) - elif isinstance(element, str): - tag_value_element = element - else: - return None + for element in sequence: + if isinstance(element, (int, bool, float)): + tag_value_element = str(element) + elif isinstance(element, str): + tag_value_element = element + else: + return None - if defined_max_tag_value_length: + if defined_max_tag_value_length: + running_string_length += ( + len(tag_value_element) + 2 + ) # +2 accounts for surrounding quotes + if tag_value_elements: running_string_length += ( - len(tag_value_element) + 2 - ) # +2 accounts for surrounding quotes - if tag_value_elements: - running_string_length += ( - 2 # accounts for ', ' connector - ) - if running_string_length > self.max_tag_value_length: - break + 2 # accounts for ', ' connector + ) + if running_string_length > self.max_tag_value_length: + break - tag_value_elements.append(tag_value_element) - else: - return None + tag_value_elements.append(tag_value_element) return json.dumps(tag_value_elements) From 2a5c4b362ad6c1d9eb1a8ab6b2a933b1fe881ec9 Mon Sep 17 00:00:00 2001 From: Rob Knox Date: Wed, 2 Dec 2020 16:22:49 -0800 Subject: [PATCH 07/10] review refactor --- .../opentelemetry/exporter/zipkin/__init__.py | 21 ++-- .../tests/test_zipkin_exporter.py | 98 ++++++++++--------- 2 files changed, 67 insertions(+), 52 deletions(-) diff --git a/exporter/opentelemetry-exporter-zipkin/src/opentelemetry/exporter/zipkin/__init__.py b/exporter/opentelemetry-exporter-zipkin/src/opentelemetry/exporter/zipkin/__init__.py index c42846bf482..7564b30b276 100644 --- a/exporter/opentelemetry-exporter-zipkin/src/opentelemetry/exporter/zipkin/__init__.py +++ b/exporter/opentelemetry-exporter-zipkin/src/opentelemetry/exporter/zipkin/__init__.py @@ -372,7 +372,7 @@ def _extract_tags_from_dict(self, tags_dict): return tags def _extract_tag_value_string_from_sequence(self, sequence: Sequence): - if not sequence or self.max_tag_value_length == 1: + if self.max_tag_value_length == 1: return None tag_value_elements = [] @@ -386,23 +386,28 @@ def _extract_tag_value_string_from_sequence(self, sequence: Sequence): tag_value_element = str(element) elif isinstance(element, str): tag_value_element = element + elif element is None: + tag_value_element = None else: return None if defined_max_tag_value_length: - running_string_length += ( - len(tag_value_element) + 2 - ) # +2 accounts for surrounding quotes + if tag_value_element is None: + running_string_length += 4 # null with no quotes + else: + # + 2 accounts for string quotation marks + running_string_length += len(tag_value_element) + 2 + if tag_value_elements: - running_string_length += ( - 2 # accounts for ', ' connector - ) + # accounts for ',' item separator + running_string_length += 1 + if running_string_length > self.max_tag_value_length: break tag_value_elements.append(tag_value_element) - return json.dumps(tag_value_elements) + return json.dumps(tag_value_elements, separators=(',', ':')) def _extract_tags_from_span(self, span: Span): tags = self._extract_tags_from_dict(getattr(span, "attributes", None)) diff --git a/exporter/opentelemetry-exporter-zipkin/tests/test_zipkin_exporter.py b/exporter/opentelemetry-exporter-zipkin/tests/test_zipkin_exporter.py index 9f5802502f7..e6f69ec5676 100644 --- a/exporter/opentelemetry-exporter-zipkin/tests/test_zipkin_exporter.py +++ b/exporter/opentelemetry-exporter-zipkin/tests/test_zipkin_exporter.py @@ -441,6 +441,8 @@ def test_export_json_max_tag_length(self): span.set_attribute("tuple6", (True,) * 10) span.set_attribute("range1", range(0, 25)) span.set_attribute("range2", range(0, 10)) + span.set_attribute("empty_list", []) + span.set_attribute("none_list", ["hello", None, "world"]) span.set_status(Status(StatusCode.ERROR, "Example description")) span.end() @@ -460,59 +462,67 @@ def test_export_json_max_tag_length(self): self.assertEqual(len(tags["string2"]), 50) self.assertEqual( tags["list1"], - '["a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a"]', + '["a","a","a","a","a","a","a","a","a","a","a","a","a","a","a","a","a","a","a","a","a","a","a","a","a"]', ) self.assertEqual( tags["list2"], - '["a", "a", "a", "a", "a", "a", "a", "a", "a", "a"]', + '["a","a","a","a","a","a","a","a","a","a"]', ) self.assertEqual( tags["list3"], - '["2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2"]', + '["2","2","2","2","2","2","2","2","2","2","2","2","2","2","2","2","2","2","2","2","2","2","2","2","2"]', ) self.assertEqual( tags["list4"], - '["2", "2", "2", "2", "2", "2", "2", "2", "2", "2"]', + '["2","2","2","2","2","2","2","2","2","2"]', ) self.assertEqual( tags["list5"], - '["True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True"]', + '["True","True","True","True","True","True","True","True","True","True","True","True","True","True","True","True","True","True"]', ) self.assertEqual( tags["list6"], - '["True", "True", "True", "True", "True", "True", "True", "True", "True", "True"]', + '["True","True","True","True","True","True","True","True","True","True"]', ) self.assertEqual( tags["tuple1"], - '["a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a"]', + '["a","a","a","a","a","a","a","a","a","a","a","a","a","a","a","a","a","a","a","a","a","a","a","a","a"]', ) self.assertEqual( tags["tuple2"], - '["a", "a", "a", "a", "a", "a", "a", "a", "a", "a"]', + '["a","a","a","a","a","a","a","a","a","a"]', ) self.assertEqual( tags["tuple3"], - '["2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2", "2"]', + '["2","2","2","2","2","2","2","2","2","2","2","2","2","2","2","2","2","2","2","2","2","2","2","2","2"]', ) self.assertEqual( tags["tuple4"], - '["2", "2", "2", "2", "2", "2", "2", "2", "2", "2"]', + '["2","2","2","2","2","2","2","2","2","2"]', ) self.assertEqual( tags["tuple5"], - '["True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True", "True"]', + '["True","True","True","True","True","True","True","True","True","True","True","True","True","True","True","True","True","True"]', ) self.assertEqual( tags["tuple6"], - '["True", "True", "True", "True", "True", "True", "True", "True", "True", "True"]', + '["True","True","True","True","True","True","True","True","True","True"]', ) self.assertEqual( tags["range1"], - '["0", "1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "11", "12", "13", "14", "15", "16", "17", "18", "19", "20", "21", "22"]', + '["0","1","2","3","4","5","6","7","8","9","10","11","12","13","14","15","16","17","18","19","20","21","22","23","24"]', ) self.assertEqual( tags["range2"], - '["0", "1", "2", "3", "4", "5", "6", "7", "8", "9"]', + '["0","1","2","3","4","5","6","7","8","9"]', + ) + self.assertEqual( + tags["empty_list"], + '[]', + ) + self.assertEqual( + tags["none_list"], + '["hello",null,"world"]', ) exporter = ZipkinSpanExporter(service_name, max_tag_value_length=2) @@ -578,20 +588,20 @@ def test_export_json_max_tag_length(self): tags = json.loads(kwargs["data"])[0]["tags"] self.assertEqual(len(tags["string1"]), 9) self.assertEqual(len(tags["string2"]), 9) - self.assertEqual(tags["list1"], '["a"]') - self.assertEqual(tags["list2"], '["a"]') - self.assertEqual(tags["list3"], '["2"]') - self.assertEqual(tags["list4"], '["2"]') + self.assertEqual(tags["list1"], '["a","a"]') + self.assertEqual(tags["list2"], '["a","a"]') + self.assertEqual(tags["list3"], '["2","2"]') + self.assertEqual(tags["list4"], '["2","2"]') self.assertEqual(tags["list5"], '["True"]') self.assertEqual(tags["list6"], '["True"]') - self.assertEqual(tags["tuple1"], '["a"]') - self.assertEqual(tags["tuple2"], '["a"]') - self.assertEqual(tags["tuple3"], '["2"]') - self.assertEqual(tags["tuple4"], '["2"]') + self.assertEqual(tags["tuple1"], '["a","a"]') + self.assertEqual(tags["tuple2"], '["a","a"]') + self.assertEqual(tags["tuple3"], '["2","2"]') + self.assertEqual(tags["tuple4"], '["2","2"]') self.assertEqual(tags["tuple5"], '["True"]') self.assertEqual(tags["tuple6"], '["True"]') - self.assertEqual(tags["range1"], '["0"]') - self.assertEqual(tags["range2"], '["0"]') + self.assertEqual(tags["range1"], '["0","1"]') + self.assertEqual(tags["range2"], '["0","1"]') exporter = ZipkinSpanExporter(service_name, max_tag_value_length=10) mock_post = MagicMock() @@ -604,20 +614,20 @@ def test_export_json_max_tag_length(self): tags = json.loads(kwargs["data"])[0]["tags"] self.assertEqual(len(tags["string1"]), 10) self.assertEqual(len(tags["string2"]), 10) - self.assertEqual(tags["list1"], '["a", "a"]') - self.assertEqual(tags["list2"], '["a", "a"]') - self.assertEqual(tags["list3"], '["2", "2"]') - self.assertEqual(tags["list4"], '["2", "2"]') + self.assertEqual(tags["list1"], '["a","a"]') + self.assertEqual(tags["list2"], '["a","a"]') + self.assertEqual(tags["list3"], '["2","2"]') + self.assertEqual(tags["list4"], '["2","2"]') self.assertEqual(tags["list5"], '["True"]') self.assertEqual(tags["list6"], '["True"]') - self.assertEqual(tags["tuple1"], '["a", "a"]') - self.assertEqual(tags["tuple2"], '["a", "a"]') - self.assertEqual(tags["tuple3"], '["2", "2"]') - self.assertEqual(tags["tuple4"], '["2", "2"]') + self.assertEqual(tags["tuple1"], '["a","a"]') + self.assertEqual(tags["tuple2"], '["a","a"]') + self.assertEqual(tags["tuple3"], '["2","2"]') + self.assertEqual(tags["tuple4"], '["2","2"]') self.assertEqual(tags["tuple5"], '["True"]') self.assertEqual(tags["tuple6"], '["True"]') - self.assertEqual(tags["range1"], '["0", "1"]') - self.assertEqual(tags["range2"], '["0", "1"]') + self.assertEqual(tags["range1"], '["0","1"]') + self.assertEqual(tags["range2"], '["0","1"]') exporter = ZipkinSpanExporter(service_name, max_tag_value_length=11) mock_post = MagicMock() @@ -630,20 +640,20 @@ def test_export_json_max_tag_length(self): tags = json.loads(kwargs["data"])[0]["tags"] self.assertEqual(len(tags["string1"]), 11) self.assertEqual(len(tags["string2"]), 11) - self.assertEqual(tags["list1"], '["a", "a"]') - self.assertEqual(tags["list2"], '["a", "a"]') - self.assertEqual(tags["list3"], '["2", "2"]') - self.assertEqual(tags["list4"], '["2", "2"]') + self.assertEqual(tags["list1"], '["a","a"]') + self.assertEqual(tags["list2"], '["a","a"]') + self.assertEqual(tags["list3"], '["2","2"]') + self.assertEqual(tags["list4"], '["2","2"]') self.assertEqual(tags["list5"], '["True"]') self.assertEqual(tags["list6"], '["True"]') - self.assertEqual(tags["tuple1"], '["a", "a"]') - self.assertEqual(tags["tuple2"], '["a", "a"]') - self.assertEqual(tags["tuple3"], '["2", "2"]') - self.assertEqual(tags["tuple4"], '["2", "2"]') + self.assertEqual(tags["tuple1"], '["a","a"]') + self.assertEqual(tags["tuple2"], '["a","a"]') + self.assertEqual(tags["tuple3"], '["2","2"]') + self.assertEqual(tags["tuple4"], '["2","2"]') self.assertEqual(tags["tuple5"], '["True"]') self.assertEqual(tags["tuple6"], '["True"]') - self.assertEqual(tags["range1"], '["0", "1"]') - self.assertEqual(tags["range2"], '["0", "1"]') + self.assertEqual(tags["range1"], '["0","1"]') + self.assertEqual(tags["range2"], '["0","1"]') # pylint: disable=too-many-locals,too-many-statements def test_export_protobuf(self): From 51d80249341c4451e6222f835f03791d8d611d44 Mon Sep 17 00:00:00 2001 From: Rob Knox Date: Wed, 2 Dec 2020 16:34:14 -0800 Subject: [PATCH 08/10] black --- .../src/opentelemetry/exporter/zipkin/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exporter/opentelemetry-exporter-zipkin/src/opentelemetry/exporter/zipkin/__init__.py b/exporter/opentelemetry-exporter-zipkin/src/opentelemetry/exporter/zipkin/__init__.py index 7564b30b276..96cbaa26fa5 100644 --- a/exporter/opentelemetry-exporter-zipkin/src/opentelemetry/exporter/zipkin/__init__.py +++ b/exporter/opentelemetry-exporter-zipkin/src/opentelemetry/exporter/zipkin/__init__.py @@ -407,7 +407,7 @@ def _extract_tag_value_string_from_sequence(self, sequence: Sequence): tag_value_elements.append(tag_value_element) - return json.dumps(tag_value_elements, separators=(',', ':')) + return json.dumps(tag_value_elements, separators=(",", ":")) def _extract_tags_from_span(self, span: Span): tags = self._extract_tags_from_dict(getattr(span, "attributes", None)) From a226e19a99f7f12652cdae33e9c0cae4e42c4cc2 Mon Sep 17 00:00:00 2001 From: Rob Knox Date: Wed, 2 Dec 2020 16:34:34 -0800 Subject: [PATCH 09/10] black --- .../tests/test_zipkin_exporter.py | 21 +++++++------------ 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/exporter/opentelemetry-exporter-zipkin/tests/test_zipkin_exporter.py b/exporter/opentelemetry-exporter-zipkin/tests/test_zipkin_exporter.py index e6f69ec5676..a21199659b8 100644 --- a/exporter/opentelemetry-exporter-zipkin/tests/test_zipkin_exporter.py +++ b/exporter/opentelemetry-exporter-zipkin/tests/test_zipkin_exporter.py @@ -465,16 +465,14 @@ def test_export_json_max_tag_length(self): '["a","a","a","a","a","a","a","a","a","a","a","a","a","a","a","a","a","a","a","a","a","a","a","a","a"]', ) self.assertEqual( - tags["list2"], - '["a","a","a","a","a","a","a","a","a","a"]', + tags["list2"], '["a","a","a","a","a","a","a","a","a","a"]', ) self.assertEqual( tags["list3"], '["2","2","2","2","2","2","2","2","2","2","2","2","2","2","2","2","2","2","2","2","2","2","2","2","2"]', ) self.assertEqual( - tags["list4"], - '["2","2","2","2","2","2","2","2","2","2"]', + tags["list4"], '["2","2","2","2","2","2","2","2","2","2"]', ) self.assertEqual( tags["list5"], @@ -489,16 +487,14 @@ def test_export_json_max_tag_length(self): '["a","a","a","a","a","a","a","a","a","a","a","a","a","a","a","a","a","a","a","a","a","a","a","a","a"]', ) self.assertEqual( - tags["tuple2"], - '["a","a","a","a","a","a","a","a","a","a"]', + tags["tuple2"], '["a","a","a","a","a","a","a","a","a","a"]', ) self.assertEqual( tags["tuple3"], '["2","2","2","2","2","2","2","2","2","2","2","2","2","2","2","2","2","2","2","2","2","2","2","2","2"]', ) self.assertEqual( - tags["tuple4"], - '["2","2","2","2","2","2","2","2","2","2"]', + tags["tuple4"], '["2","2","2","2","2","2","2","2","2","2"]', ) self.assertEqual( tags["tuple5"], @@ -513,16 +509,13 @@ def test_export_json_max_tag_length(self): '["0","1","2","3","4","5","6","7","8","9","10","11","12","13","14","15","16","17","18","19","20","21","22","23","24"]', ) self.assertEqual( - tags["range2"], - '["0","1","2","3","4","5","6","7","8","9"]', + tags["range2"], '["0","1","2","3","4","5","6","7","8","9"]', ) self.assertEqual( - tags["empty_list"], - '[]', + tags["empty_list"], "[]", ) self.assertEqual( - tags["none_list"], - '["hello",null,"world"]', + tags["none_list"], '["hello",null,"world"]', ) exporter = ZipkinSpanExporter(service_name, max_tag_value_length=2) From 907ead54b3f4dbf975fec6a9b93d81a44582fa3c Mon Sep 17 00:00:00 2001 From: Rob Knox Date: Wed, 2 Dec 2020 16:41:20 -0800 Subject: [PATCH 10/10] simplify tag value formatting, plus gracefully skip invalid sequence elements versus bailing completely out --- .../src/opentelemetry/exporter/zipkin/__init__.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/exporter/opentelemetry-exporter-zipkin/src/opentelemetry/exporter/zipkin/__init__.py b/exporter/opentelemetry-exporter-zipkin/src/opentelemetry/exporter/zipkin/__init__.py index 96cbaa26fa5..c6d3559b905 100644 --- a/exporter/opentelemetry-exporter-zipkin/src/opentelemetry/exporter/zipkin/__init__.py +++ b/exporter/opentelemetry-exporter-zipkin/src/opentelemetry/exporter/zipkin/__init__.py @@ -351,10 +351,8 @@ def _extract_tags_from_dict(self, tags_dict): if not tags_dict: return tags for attribute_key, attribute_value in tags_dict.items(): - if isinstance(attribute_value, (int, bool, float)): + if isinstance(attribute_value, (int, bool, float, str)): value = str(attribute_value) - elif isinstance(attribute_value, str): - value = attribute_value elif isinstance(attribute_value, Sequence): value = self._extract_tag_value_string_from_sequence( attribute_value @@ -382,14 +380,12 @@ def _extract_tag_value_string_from_sequence(self, sequence: Sequence): defined_max_tag_value_length = self.max_tag_value_length > 0 for element in sequence: - if isinstance(element, (int, bool, float)): + if isinstance(element, (int, bool, float, str)): tag_value_element = str(element) - elif isinstance(element, str): - tag_value_element = element elif element is None: tag_value_element = None else: - return None + continue if defined_max_tag_value_length: if tag_value_element is None: