Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding support for array attributes to Zipkin exporter #1285

Merged
merged 20 commits into from
Dec 16, 2020
Merged

Adding support for array attributes to Zipkin exporter #1285

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
c10311a
Adding support for array attributes to Zipkin exporter (#1110)
robwknox Oct 26, 2020
83a868c
lint changes (#1110)
robwknox Oct 26, 2020
8d3f204
Merge branch 'master' into 1110_zipkin_exporter_support_array_attrs
robwknox Oct 26, 2020
779b090
CHANGELOG entry (#1110)
robwknox Oct 27, 2020
b31c881
Merge branch 'master' into 1110_zipkin_exporter_support_array_attrs
robwknox Nov 13, 2020
1033a76
changelog
robwknox Nov 13, 2020
1d3a63f
lint
robwknox Nov 13, 2020
95dfae4
Merge branch 'master' into 1110_zipkin_exporter_support_array_attrs
lzchen Nov 19, 2020
0082859
Merge branch 'master' into 1110_zipkin_exporter_support_array_attrs
lzchen Nov 19, 2020
d82fb30
Merge branch 'master' into 1110_zipkin_exporter_support_array_attrs
robwknox Nov 23, 2020
a2f1c8d
Merge branch 'master' into 1110_zipkin_exporter_support_array_attrs
lzchen Nov 24, 2020
75edd7d
Merge branch 'master' into 1110_zipkin_exporter_support_array_attrs
Nov 30, 2020
014f5e4
review refactor
robwknox Dec 2, 2020
2a5c4b3
review refactor
robwknox Dec 3, 2020
85ef28c
Merge branch '1110_zipkin_exporter_support_array_attrs' of https://gi…
robwknox Dec 3, 2020
51d8024
black
robwknox Dec 3, 2020
a226e19
black
robwknox Dec 3, 2020
907ead5
simplify tag value formatting, plus gracefully skip invalid sequence …
robwknox Dec 3, 2020
32d68e8
Merge remote-tracking branch 'origin/master' into 1110_zipkin_exporte…
Dec 14, 2020
7fc3ac9
Merge branch 'master' into 1110_zipkin_exporter_support_array_attrs
Dec 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
`opentelemetry-sdk` package now registers an entrypoint `opentelemetry_configurator`
to allow `opentelemetry-instrument` to load the configuration for the SDK
([#1420](https://github.com/open-telemetry/opentelemetry-python/pull/1420))
- `opentelemetry-exporter-zipkin` Add support for array attributes in Span and Resource exports
([#1285](https://github.com/open-telemetry/opentelemetry-python/pull/1285))

## [0.16b1](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v0.16b1) - 2020-11-26
### Added
Expand Down
47 changes: 44 additions & 3 deletions exporter/opentelemetry-exporter-zipkin/src/opentelemetry/exporter/zipkin/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -351,10 +351,15 @@ 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
)
if not value:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if not value:
if value is None:

logger.warning("Could not serialize tag %s", attribute_key)
continue
else:
logger.warning("Could not serialize tag %s", attribute_key)
continue
Expand All @@ -364,6 +369,42 @@ def _extract_tags_from_dict(self, tags_dict):
tags[attribute_key] = value
return tags

def _extract_tag_value_string_from_sequence(self, sequence: Sequence):
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

for element in sequence:
if isinstance(element, (int, bool, float, str)):
tag_value_element = str(element)
elif element is None:
tag_value_element = None
else:
continue

if defined_max_tag_value_length:
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:
# 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, separators=(",", ":"))

def _extract_tags_from_span(self, span: Span):
tags = self._extract_tags_from_dict(getattr(span, "attributes", None))
if span.resource:
Expand Down
205 changes: 199 additions & 6 deletions exporter/opentelemetry-exporter-zipkin/tests/test_zipkin_exporter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -425,8 +425,25 @@ def test_export_json_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_attribute("empty_list", [])
span.set_attribute("none_list", ["hello", None, "world"])

robwknox marked this conversation as resolved.
Show resolved Hide resolved
span.set_status(Status(StatusCode.ERROR, "Example description"))
span.end()

Expand All @@ -440,8 +457,66 @@ def test_export_json_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)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid these long testing sequences, may I suggest:

from functools import partial
from json import dumps
...
partial_dumps = partial(dumps, separators=(",", ":"))

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"]',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
'["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"]',
partial_dumps(["a"] * 25),

)
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","True","True"]',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
'["True","True","True","True","True","True","True","True","True","True","True","True","True","True","True","True","True","True"]',
partial_dump([str(True)] * 25)

)
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","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","23","24"]',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
'["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"]',
partial_dumps([str(i) for i in range(25)]),

)
self.assertEqual(
tags["range2"], '["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)
mock_post = MagicMock()
Expand All @@ -452,8 +527,126 @@ def test_export_json_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","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=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"]')

# pylint: disable=too-many-locals,too-many-statements
def test_export_protobuf(self):
Expand Down