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.

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

Fix for byte type attributes crashing spans #775

Merged
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
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 opentelemetry-sdk/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
([#751](https://github.com/open-telemetry/opentelemetry-python/pull/751))
- Rename Measure to ValueRecorder in metrics
([#761](https://github.com/open-telemetry/opentelemetry-python/pull/761))
- bugfix: byte type attributes are decoded before adding to attributes dict
([#775](https://github.com/open-telemetry/opentelemetry-python/pull/775))

## 0.8b0

Expand Down
6 changes: 6 additions & 0 deletions opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,12 @@ def set_attribute(self, key: str, value: types.AttributeValue) -> None:
# Freeze mutable sequences defensively
if isinstance(value, MutableSequence):
value = tuple(value)
if isinstance(value, bytes):
try:
value = value.decode()
except ValueError:
logger.warning("Byte attribute could not be decoded.")
return
with self._lock:
self.attributes[key] = value

Expand Down
9 changes: 9 additions & 0 deletions opentelemetry-sdk/tests/trace/test_trace.py
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,15 @@ def test_invalid_attribute_values(self):

Copy link
Member

Choose a reason for hiding this comment

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

Would be nice to also add an invalid byte sequence here to assert it doesn't get added

Copy link
Contributor

Choose a reason for hiding this comment

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

Would be good to check that it logs as well. Something like this:

with self.assertLogs(level=WARNING):
self.assertIs(instrumentor.uninstrument(), None)

self.assertEqual(len(root.attributes), 0)

def test_byte_type_attribute_value(self):
with self.tracer.start_as_current_span("root") as root:
with self.assertLogs(level=WARNING):
root.set_attribute("invalid-byte-type-attribute", b"\xd8\xe1\xb7\xeb\xa8\xe5 \xd2\xb7\xe1")
self.assertFalse("invalid-byte-type-attribute" in root.attributes)

root.set_attribute("valid-byte-type-attribute", b"valid byte")
self.assertTrue(isinstance(root.attributes["valid-byte-type-attribute"], str))

def test_check_attribute_helper(self):
# pylint: disable=protected-access
self.assertFalse(trace._is_valid_attribute_value([1, 2, 3.4, "ss", 4]))
Expand Down