Skip to content

Commit 4fca8c9

Browse files
jakemalachowskitoumorokoshi
authored andcommitted
Add runtime validation in setAttribute (#348)
Validate attribute value data types before adding to span Add lists as an accepted data type. By adding validation during the setAttribute phase, this allows allows exporters to avoid redundant code to validate attributes.
1 parent 3883e0a commit 4fca8c9

File tree

2 files changed

+84
-1
lines changed

2 files changed

+84
-1
lines changed

opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py

+32
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import random
1919
import threading
2020
from contextlib import contextmanager
21+
from numbers import Number
2122
from types import TracebackType
2223
from typing import Iterator, Optional, Sequence, Tuple, Type
2324

@@ -216,8 +217,39 @@ def set_attribute(self, key: str, value: types.AttributeValue) -> None:
216217
if has_ended:
217218
logger.warning("Setting attribute on ended span.")
218219
return
220+
221+
if isinstance(value, Sequence):
222+
error_message = self._check_attribute_value_sequence(value)
223+
if error_message is not None:
224+
logger.warning("%s in attribute value sequence", error_message)
225+
return
226+
elif not isinstance(value, (bool, str, Number, Sequence)):
227+
logger.warning("invalid type for attribute value")
228+
return
229+
219230
self.attributes[key] = value
220231

232+
@staticmethod
233+
def _check_attribute_value_sequence(sequence: Sequence) -> Optional[str]:
234+
"""
235+
Checks if sequence items are valid and are of the same type
236+
"""
237+
if len(sequence) == 0:
238+
return None
239+
240+
first_element_type = type(sequence[0])
241+
242+
if issubclass(first_element_type, Number):
243+
first_element_type = Number
244+
245+
if first_element_type not in (bool, str, Number):
246+
return "invalid type"
247+
248+
for element in sequence:
249+
if not isinstance(element, first_element_type):
250+
return "different type"
251+
return None
252+
221253
def add_event(
222254
self,
223255
name: str,

opentelemetry-sdk/tests/trace/test_trace.py

+52-1
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,11 @@ def test_attributes(self):
368368
root.set_attribute("attr-key", "attr-value1")
369369
root.set_attribute("attr-key", "attr-value2")
370370

371-
self.assertEqual(len(root.attributes), 7)
371+
root.set_attribute("empty-list", [])
372+
root.set_attribute("list-of-bools", [True, True, False])
373+
root.set_attribute("list-of-numerics", [123, 3.14, 0])
374+
375+
self.assertEqual(len(root.attributes), 10)
372376
self.assertEqual(root.attributes["component"], "http")
373377
self.assertEqual(root.attributes["http.method"], "GET")
374378
self.assertEqual(
@@ -379,6 +383,13 @@ def test_attributes(self):
379383
self.assertEqual(root.attributes["http.status_text"], "OK")
380384
self.assertEqual(root.attributes["misc.pi"], 3.14)
381385
self.assertEqual(root.attributes["attr-key"], "attr-value2")
386+
self.assertEqual(root.attributes["empty-list"], [])
387+
self.assertEqual(
388+
root.attributes["list-of-bools"], [True, True, False]
389+
)
390+
self.assertEqual(
391+
root.attributes["list-of-numerics"], [123, 3.14, 0]
392+
)
382393

383394
attributes = {
384395
"attr-key": "val",
@@ -393,6 +404,46 @@ def test_attributes(self):
393404
self.assertEqual(root.attributes["attr-key2"], "val2")
394405
self.assertEqual(root.attributes["attr-in-both"], "span-attr")
395406

407+
def test_invalid_attribute_values(self):
408+
with self.tracer.start_as_current_span("root") as root:
409+
root.set_attribute("non-primitive-data-type", dict())
410+
root.set_attribute(
411+
"list-of-mixed-data-types-numeric-first",
412+
[123, False, "string"],
413+
)
414+
root.set_attribute(
415+
"list-of-mixed-data-types-non-numeric-first",
416+
[False, 123, "string"],
417+
)
418+
root.set_attribute(
419+
"list-with-non-primitive-data-type", [dict(), 123]
420+
)
421+
422+
self.assertEqual(len(root.attributes), 0)
423+
424+
def test_check_sequence_helper(self):
425+
# pylint: disable=protected-access
426+
self.assertEqual(
427+
trace.Span._check_attribute_value_sequence([1, 2, 3.4, "ss", 4]),
428+
"different type",
429+
)
430+
self.assertEqual(
431+
trace.Span._check_attribute_value_sequence([dict(), 1, 2, 3.4, 4]),
432+
"invalid type",
433+
)
434+
self.assertEqual(
435+
trace.Span._check_attribute_value_sequence(
436+
["sw", "lf", 3.4, "ss"]
437+
),
438+
"different type",
439+
)
440+
self.assertIsNone(
441+
trace.Span._check_attribute_value_sequence([1, 2, 3.4, 5])
442+
)
443+
self.assertIsNone(
444+
trace.Span._check_attribute_value_sequence(["ss", "dw", "fw"])
445+
)
446+
396447
def test_sampling_attributes(self):
397448
decision_attributes = {
398449
"sampler-attr": "sample-val",

0 commit comments

Comments
 (0)