Skip to content

Commit

Permalink
add check for event attribute values
Browse files Browse the repository at this point in the history
  • Loading branch information
Andrew Xue committed May 12, 2020
1 parent 82359d5 commit 3ab4182
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 40 deletions.
47 changes: 28 additions & 19 deletions opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -372,38 +372,39 @@ def set_attribute(self, key: str, value: types.AttributeValue) -> None:
logger.warning("invalid key (empty or null)")
return

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)
return
# Freeze mutable sequences defensively
if isinstance(value, MutableSequence):
value = tuple(value)
elif not isinstance(value, (bool, str, int, float)):
logger.warning("invalid type for attribute value")
error_message = self._check_attribute_value(value)
if error_message is not None:
logger.warning(error_message)
return
# Freeze mutable sequences defensively
if isinstance(value, MutableSequence):
value = tuple(value)

self.attributes[key] = value

@staticmethod
def _check_attribute_value_sequence(sequence: Sequence) -> Optional[str]:
def _check_attribute_value(value: types.AttributeValue) -> Optional[str]:
"""
Checks if sequence items are valid and are of the same type
"""
if len(sequence) == 0:
return None
if isinstance(value, Sequence):
if len(value) == 0:
return None

first_element_type = type(sequence[0])
first_element_type = type(value[0])

if first_element_type not in (bool, str, int, float):
return "invalid type"
if first_element_type not in (bool, str, int, float):
return "invalid type in attribute value sequence"

for element in sequence:
if not isinstance(element, first_element_type):
return "different type"
for element in value:
if not isinstance(element, first_element_type):
return "different types in attribute value sequence"
return None
elif not isinstance(value, (bool, str, int, float)):
return "invalid type for attribute value"
return None


def _add_event(self, event: EventBase) -> None:
with self._lock:
if not self.is_recording_events():
Expand All @@ -425,6 +426,14 @@ def add_event(
) -> None:
if attributes is None:
attributes = Span._empty_attributes
else:
for attr_key, attr_value in list(attributes.items()):
error_message = self._check_attribute_value(attr_value)
if error_message:
logger.warning(error_message)
return
if isinstance(attr_value, MutableSequence):
attributes[attr_key] = tuple(attr_value)
self._add_event(
Event(
name=name,
Expand Down
84 changes: 63 additions & 21 deletions opentelemetry-sdk/tests/trace/test_trace.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -487,39 +487,61 @@ def test_invalid_attribute_values(self):

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

def test_check_sequence_helper(self):
def test_check_attribute_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([1, 2, 3.4, "ss", 4]),
"different types in attribute value sequence",
)
self.assertEqual(
trace.Span._check_attribute_value_sequence([dict(), 1, 2, 3.4, 4]),
"invalid type",
trace.Span._check_attribute_value([dict(), 1, 2, 3.4, 4]),
"invalid type in attribute value sequence",
)
self.assertEqual(
trace.Span._check_attribute_value_sequence(
trace.Span._check_attribute_value(
["sw", "lf", 3.4, "ss"]
),
"different type",
"different types in attribute value sequence",
)
self.assertEqual(
trace.Span._check_attribute_value_sequence([1, 2, 3.4, 5]),
"different type",
trace.Span._check_attribute_value([1, 2, 3.4, 5]),
"different types in attribute value sequence",
)
self.assertIsNone(
trace.Span._check_attribute_value_sequence([1, 2, 3, 5])
trace.Span._check_attribute_value([1, 2, 3, 5])
)
self.assertIsNone(
trace.Span._check_attribute_value_sequence([1.2, 2.3, 3.4, 4.5])
trace.Span._check_attribute_value([1.2, 2.3, 3.4, 4.5])
)
self.assertIsNone(
trace.Span._check_attribute_value_sequence([True, False])
trace.Span._check_attribute_value([True, False])
)
self.assertIsNone(
trace.Span._check_attribute_value_sequence(["ss", "dw", "fw"])
trace.Span._check_attribute_value(["ss", "dw", "fw"])
)
self.assertIsNone(
trace.Span._check_attribute_value([])
)


self.assertEqual(
trace.Span._check_attribute_value(dict()),
"invalid type for attribute value",
)
self.assertIsNone(
trace.Span._check_attribute_value(True)
)
self.assertIsNone(
trace.Span._check_attribute_value('hi')
)
self.assertIsNone(
trace.Span._check_attribute_value(3.4)
)
self.assertIsNone(
trace.Span._check_attribute_value(15)
)


def test_sampling_attributes(self):
decision_attributes = {
"sampler-attr": "sample-val",
Expand Down Expand Up @@ -561,33 +583,52 @@ def test_events(self):

# event name and attributes
now = time_ns()
root.add_event("event1", {"name": "pluto"})
root.add_event("event1", {"name": "pluto", "some_bools": [True, False]})

# event name, attributes and timestamp
now = time_ns()
root.add_event("event2", {"name": "birthday"}, now)
root.add_event("event2", {"name": ["birthday"]}, now)

mutable_list = ["original_contents"]
root.add_event("event3", {"name": mutable_list})

def event_formatter():
return {"name": "hello"}

# lazy event
root.add_lazy_event("event3", event_formatter, now)
root.add_lazy_event("event4", event_formatter, now)

self.assertEqual(len(root.events), 4)
self.assertEqual(len(root.events), 5)

self.assertEqual(root.events[0].name, "event0")
self.assertEqual(root.events[0].attributes, {})

self.assertEqual(root.events[1].name, "event1")
self.assertEqual(root.events[1].attributes, {"name": "pluto"})
self.assertEqual(root.events[1].attributes, {"name": "pluto", "some_bools": (True, False)})

self.assertEqual(root.events[2].name, "event2")
self.assertEqual(root.events[2].attributes, {"name": "birthday"})
self.assertEqual(root.events[2].attributes, {"name": ("birthday",)})
self.assertEqual(root.events[2].timestamp, now)

self.assertEqual(root.events[3].name, "event3")
self.assertEqual(root.events[3].attributes, {"name": "hello"})
self.assertEqual(root.events[3].timestamp, now)
self.assertEqual(root.events[3].attributes, {"name": ("original_contents",)})
mutable_list = ["new_contents"]
self.assertEqual(root.events[3].attributes, {"name": ("original_contents",)})

self.assertEqual(root.events[4].name, "event4")
self.assertEqual(root.events[4].attributes, {"name": "hello"})
self.assertEqual(root.events[4].timestamp, now)

def test_invalid_event_attributes(self):
self.assertIsNone(self.tracer.get_current_span())

with self.tracer.start_as_current_span("root") as root:
root.add_event("event0", {"attr1": True, "attr2": ['hi', False]})
root.add_event("event0", {"attr1": dict()})
root.add_event("event0", {"attr1": [[True]]})
root.add_event("event0", {"attr1": [dict()]})

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

def test_links(self):
other_context1 = trace_api.SpanContext(
Expand Down Expand Up @@ -882,3 +923,4 @@ def test_add_span_processor_after_span_creation(self):
expected_list.append(span_event_end_fmt("SP1", "foo"))

self.assertListEqual(spans_calls_list, expected_list)

0 comments on commit 3ab4182

Please sign in to comment.