Remove contextvars workaround (open-telemetry#2009)

CHANGELOG.md

@@ -16,6 +16,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Add Trace ID validation to meet [TraceID spec](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/overview.md#spancontext) ([#1992](https://github.com/open-telemetry/opentelemetry-python/pull/1992))
 - Fixed Python 3.10 incompatibility in `opentelemetry-opentracing-shim` tests
   ([#2018](https://github.com/open-telemetry/opentelemetry-python/pull/2018))
+- `opentelemetry-sdk` added support for `OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT`
+  ([#2044](https://github.com/open-telemetry/opentelemetry-python/pull/2044))
+- `opentelemetry-sdk` Fixed bugs (#2041 & #2042) in Span Limits
+  ([#2044](https://github.com/open-telemetry/opentelemetry-python/pull/2044))
 
 ## [0.23.1](https://github.com/open-telemetry/opentelemetry-python/pull/1987) - 2021-07-26
 

opentelemetry-api/src/opentelemetry/attributes/__init__.py

@@ -17,36 +17,55 @@
 import threading
 from collections import OrderedDict
 from collections.abc import MutableMapping
-from typing import MutableSequence, Optional, Sequence
+from typing import MutableSequence, Optional, Sequence, Union
 
 from opentelemetry.util import types
 
-_VALID_ATTR_VALUE_TYPES = (bool, str, int, float)
+# bytes are accepted as a user supplied value for attributes but
+# decoded to strings internally.
+_VALID_ATTR_VALUE_TYPES = (bool, str, bytes, int, float)
 
 
 _logger = logging.getLogger(__name__)
 
 
-def _is_valid_attribute_value(value: types.AttributeValue) -> bool:
-    """Checks if attribute value is valid.
+def _clean_attribute(
+    key: str, value: types.AttributeValue, max_len: Optional[int]
+) -> Optional[types.AttributeValue]:
+    """Checks if attribute value is valid and cleans it if required.
 
     An attribute value is valid if it is either:
         - A primitive type: string, boolean, double precision floating
             point (IEEE 754-1985) or integer.
         - An array of primitive type values. The array MUST be homogeneous,
             i.e. it MUST NOT contain values of different types.
-    """
 
-    if isinstance(value, _VALID_ATTR_VALUE_TYPES):
-        return True
+    An attribute needs cleansing if:
+        - Its length is greater than the maximum allowed length.
+        - It needs to be encoded/decoded e.g, bytes to strings.
+    """
 
-    if isinstance(value, Sequence):
+    if key is None or key is "":
+        _logger.warning("invalid key `%s` (empty or null)", key)
+        return None
 
+    if isinstance(value, _VALID_ATTR_VALUE_TYPES):
+        return _clean_attribute_value(value, max_len)
+    elif isinstance(value, Sequence):
         sequence_first_valid_type = None
+        cleaned_seq = []
+
         for element in value:
+            # None is considered valid in any sequence
+            if element is None:
+                cleaned_seq.append(element)
+
+            element = _clean_attribute_value(element, max_len)
             if element is None:
                 continue
+
             element_type = type(element)
+            # Reject attribute value if sequence contains a value with an incompatible type.
             if element_type not in _VALID_ATTR_VALUE_TYPES:
                 _logger.warning(
                     "Invalid type %s in attribute value sequence. Expected one of "
@@ -57,56 +76,51 @@ def _is_valid_attribute_value(value: types.AttributeValue) -> bool:
                         for valid_type in _VALID_ATTR_VALUE_TYPES
                     ],
                 )
-                return False
+                return None
+
             # The type of the sequence must be homogeneous. The first non-None
             # element determines the type of the sequence
             if sequence_first_valid_type is None:
                 sequence_first_valid_type = element_type
-            elif not isinstance(element, sequence_first_valid_type):
+            # use equality instead of isinstance as isinstance(True, int) evaluates to True
+            elif element_type != sequence_first_valid_type:
                 _logger.warning(
                     "Mixed types %s and %s in attribute value sequence",
                     sequence_first_valid_type.__name__,
                     type(element).__name__,
                 )
-                return False
-        return True
+                return None
+
+            if element is not None:
+                cleaned_seq.append(element)
+
+        # Freeze mutable sequences defensively
+        return tuple(cleaned_seq)
 
     _logger.warning(
         "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],
     )
-    return False
-
 
-def _filter_attributes(attributes: types.Attributes) -> None:
-    """Applies attribute validation rules and drops (key, value) pairs
-    that doesn't adhere to attributes specification.
 
-    https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/common/common.md#attributes.
-    """
-    if attributes:
-        for attr_key, attr_value in list(attributes.items()):
-            if not attr_key:
-                _logger.warning("invalid key `%s` (empty or null)", attr_key)
-                attributes.pop(attr_key)
-                continue
+def _clean_attribute_value(
+    value: types.AttributeValue, limit: Optional[int]
+) -> Union[types.AttributeValue, None]:
+    if value is None:
+        return
 
-            if _is_valid_attribute_value(attr_value):
-                if isinstance(attr_value, MutableSequence):
-                    attributes[attr_key] = tuple(attr_value)
-                if isinstance(attr_value, bytes):
-                    try:
-                        attributes[attr_key] = attr_value.decode()
-                    except ValueError:
-                        attributes.pop(attr_key)
-                        _logger.warning("Byte attribute could not be decoded.")
-            else:
-                attributes.pop(attr_key)
+    if isinstance(value, bytes):
+        try:
+            value = value.decode()
+        except ValueError:
+            _logger.warning("Byte attribute could not be decoded.")
+            return None
 
-
-_DEFAULT_LIMIT = 128
+    if limit is not None and isinstance(value, str):
+        value = value[:limit]
+    return value
 
 
 class BoundedAttributes(MutableMapping):
@@ -118,9 +132,10 @@ class BoundedAttributes(MutableMapping):
 
     def __init__(
         self,
-        maxlen: Optional[int] = _DEFAULT_LIMIT,
+        maxlen: Optional[int] = None,
         attributes: types.Attributes = None,
         immutable: bool = True,
+        max_value_len: Optional[int] = None,
     ):
         if maxlen is not None:
             if not isinstance(maxlen, int) or maxlen < 0:
@@ -129,10 +144,10 @@ def __init__(
                 )
         self.maxlen = maxlen
         self.dropped = 0
+        self.max_value_len = max_value_len
         self._dict = OrderedDict()  # type: OrderedDict
         self._lock = threading.Lock()  # type: threading.Lock
         if attributes:
-            _filter_attributes(attributes)
             for key, value in attributes.items():
                 self[key] = value
         self._immutable = immutable
@@ -158,7 +173,10 @@ def __setitem__(self, key, value):
             elif self.maxlen is not None and len(self._dict) == self.maxlen:
                 del self._dict[next(iter(self._dict.keys()))]
                 self.dropped += 1
-            self._dict[key] = value
+
+            value = _clean_attribute(key, value, self.max_value_len)
+            if value is not None:
+                self._dict[key] = value
 
     def __delitem__(self, key):
         if getattr(self, "_immutable", False):

opentelemetry-api/src/opentelemetry/context/contextvars_context.py

@@ -16,15 +16,10 @@
 
 from opentelemetry.context.context import Context, _RuntimeContext
 
-if (3, 5, 3) <= version_info < (3, 7):
-    import aiocontextvars  # type: ignore # pylint:disable=import-error
+if version_info < (3, 7):
+    import aiocontextvars  # type: ignore # pylint: disable=import-error
 
-    aiocontextvars  # pylint:disable=pointless-statement
-
-elif (3, 4) < version_info <= (3, 5, 2):
-    import opentelemetry.context.aiocontextvarsfix  # pylint:disable=wrong-import-position
-
-    opentelemetry.context.aiocontextvarsfix  # pylint:disable=pointless-statement
+    aiocontextvars  # pylint: disable=pointless-statement
 
 
 class ContextVarsRuntimeContext(_RuntimeContext):