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

Fix for byte type attributes crashing spans #775

Merged
merged 18 commits into from
Jun 5, 2020
Merged

Fix for byte type attributes crashing spans #775

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
96c5cef
Add fix for byte type attributes
sethmaxwl Jun 3, 2020
bde2404
Merge branch 'master' into sdk-exporter-bytes-attribute
sethmaxwl Jun 3, 2020
7c0c928
Add try except around byte attribute decoding
sethmaxwl Jun 4, 2020
f8199b3
Merge branch 'sdk-exporter-bytes-attribute' of https://github.com/set…
sethmaxwl Jun 4, 2020
91ef41b
Add fix for byte type attributes
sethmaxwl Jun 3, 2020
e6cecc3
Add try except around byte attribute decoding
sethmaxwl Jun 4, 2020
5fcb426
docs: Fix broken link (#763)
hectorhdzg Jun 1, 2020
7018a78
Rename Measure to ValueRecorder (#761)
lzchen Jun 2, 2020
9825e90
ext/boto: Add boto instrumentation (#665)
ocelotl Jun 2, 2020
20e4ff4
opentracing-shim: add testbed for otshim (#727)
toumorokoshi Jun 3, 2020
f0ead93
chore: removing Oberon00 from approvers (#770)
Jun 3, 2020
efacb60
Add test for byte type attributes
sethmaxwl Jun 5, 2020
0dff5a4
Merge branch 'sdk-exporter-bytes-attribute' of https://github.com/set…
sethmaxwl Jun 5, 2020
c553dc3
Merge branch 'master' into sdk-exporter-bytes-attribute
sethmaxwl Jun 5, 2020
2086515
Add invalid byte sequence test
sethmaxwl Jun 5, 2020
991366b
Merge branch 'sdk-exporter-bytes-attribute' of https://github.com/set…
sethmaxwl Jun 5, 2020
fbde320
Fix linting issues
sethmaxwl Jun 5, 2020
6350de5
Merge branch 'master' into sdk-exporter-bytes-attribute
sethmaxwl Jun 5, 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 opentelemetry-sdk/CHANGELOG.md
View file
Open in desktop
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
View file
Open in desktop
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
16 changes: 16 additions & 0 deletions opentelemetry-sdk/tests/trace/test_trace.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,22 @@ 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:

opentelemetry-python/opentelemetry-auto-instrumentation/tests/test_instrumentor.py

Lines 33 to 34 in c074853

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