add check for event attribute values

[AndrewAXue]

This PR addresses issue #352

Validates attribute values and events upon span creation and setting attributes on spans.

Validation logic involves checking if value is one of (int, float, str, bool, list), and if list, each element must be valid and must be all the same primitive, valid type
list values will not contain nested lists
If value is a list, grants immutable property by converting value to a tuple.

[mauriciovasquezbernal]

Given that you are looking at this, could you also consider the case where attributes and events are passed to the span's constructor?

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

Lines 248 to 263 in 0ae01b9

	if attributes is None:
	self.attributes = Span._empty_attributes
	else:
	self.attributes = BoundedDict.from_map(
	MAX_NUM_ATTRIBUTES, attributes
	)
	if events is None:
	self.events = Span._empty_events
	else:
	self.events = BoundedList.from_seq(MAX_NUM_EVENTS, events)
	if links is None:
	self.links = Span._empty_links
	else:
	self.links = BoundedList.from_seq(MAX_NUM_LINKS, links)

I always wanted to open an issue about it but never did.

[sjbrunst]

I know this PR is for #352, but I don't see anything in the PR tying it back to that issue. Should a link be added to the description or something?

[sjbrunst]

A couple minor comments, otherwise LGTM!

[lzchen]

It would be nice if you could add a description of your changes in the PR description. Sometimes linking the issue is ambiguous, especially if the issue has an ongoing discussion (like this one). It's also good practice to include specific implementation details that is not obvious.

For example:
Validates attribute values upon span creation and setting attributes on spans.

1. Validation logic involves checking if value is one of (int, float, str, bool, list), and if list, each element must be valid and must be all the same type
2. list values will not contain nested lists
3. If value is a list, grants immutable property by converting value to a tuple.

[lzchen]

LGTM. Some non-blocking comments.

[c24t]

Logic and tests look great, and nice helpful error messages to boot.

We should consider filtering LazyEvent.attributes too (and caching the attributes after the first call to _event_formatter, which I'm surprised to see we're not doing already), but that's a problem for another PR.

Welcome to the project @AndrewAXue!

[c24t]

Nit: it's a sequence, do you need to cast to slice it?

[AndrewAXue]

Yes, if its a generator its not possible to slice it immediately

[c24t]

Are generators sequences? It looks like they need to implement __getitem__, which is what slice uses under the hood.

[c24t]

@AndrewAXue one more formatting nit: wrapping strings at 80 chars:

diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
index d4054de2..5b74b4a6 100644
--- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
+++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
@@ -192,8 +192,9 @@ class LazyEvent(EventBase):
 def _is_valid_attribute_value(value: types.AttributeValue) -> bool:
     """Checks if attribute value is valid.

-    An attribute value is valid if it is one of the valid types. If the value is a sequence, it is only valid
-    if all items in the sequence are of valid type, not a sequence, and are of the same type.
+    An attribute value is valid if it is one of the valid types. If the value
+    is a sequence, it is only valid if all items in the sequence are of valid
+    type, not a sequence, and are of the same type.
     """

     if isinstance(value, Sequence):
@@ -204,7 +205,8 @@ def _is_valid_attribute_value(value: types.AttributeValue) -> bool:

         if first_element_type not in VALID_ATTR_VALUE_TYPES:
             logger.warning(
-                "Invalid type %s in attribute value sequence. Expected one of %s or a sequence of those types",
+                "Invalid type %s in attribute value sequence. Expected one of "
+                "%s or a sequence of those types",
                 first_element_type.__name__,
                 [valid_type.__name__ for valid_type in VALID_ATTR_VALUE_TYPES],
             )
@@ -220,7 +222,8 @@ def _is_valid_attribute_value(value: types.AttributeValue) -> bool:
                 return False
     elif not isinstance(value, VALID_ATTR_VALUE_TYPES):
         logger.warning(
-            "Invalid type %s for attribute value. Expected one of %s or a sequence of those types",
+            "Invalid type %s for attribute value. Expected one of %s or a "
+            "sequence of those types",
             type(value).__name__,
             [valid_type.__name__ for valid_type in VALID_ATTR_VALUE_TYPES],
         )

It looks like I can't push to your branch directly, did you check "allow edits from maintainers"?

[AndrewAXue]

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

I checked the "allow edits from maintainers" box and it was checked before as well. Maybe it's because I signed the CLA midway through the PR? Either way I've manually added the changes.

[codeboten]

LGTM. Thanks for taking this on