diff --git a/exporter/opentelemetry-exporter-otlp-proto-common/src/opentelemetry/exporter/otlp/proto/common/_internal/__init__.py b/exporter/opentelemetry-exporter-otlp-proto-common/src/opentelemetry/exporter/otlp/proto/common/_internal/__init__.py index 2f49502cf1d..4ba90f61f1a 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-common/src/opentelemetry/exporter/otlp/proto/common/_internal/__init__.py +++ b/exporter/opentelemetry-exporter-otlp-proto-common/src/opentelemetry/exporter/otlp/proto/common/_internal/__init__.py @@ -45,7 +45,7 @@ ) from opentelemetry.sdk.trace import Resource from opentelemetry.sdk.util.instrumentation import InstrumentationScope -from opentelemetry.util.types import _ExtendedAttributes +from opentelemetry.util.types import Attributes _logger = logging.getLogger(__name__) @@ -136,7 +136,7 @@ def _encode_trace_id(trace_id: int) -> bytes: def _encode_attributes( - attributes: _ExtendedAttributes, + attributes: Attributes, allow_null: bool = False, ) -> Optional[List[PB2KeyValue]]: if attributes: diff --git a/opentelemetry-api/src/opentelemetry/_events/__init__.py b/opentelemetry-api/src/opentelemetry/_events/__init__.py index f073b223345..3772489fa77 100644 --- a/opentelemetry-api/src/opentelemetry/_events/__init__.py +++ b/opentelemetry-api/src/opentelemetry/_events/__init__.py @@ -25,7 +25,7 @@ from opentelemetry.trace.span import TraceFlags from opentelemetry.util._once import Once from opentelemetry.util._providers import _load_provider -from opentelemetry.util.types import AnyValue, _ExtendedAttributes +from opentelemetry.util.types import AnyValue, Attributes _logger = getLogger(__name__) @@ -40,7 +40,7 @@ def __init__( trace_flags: Optional["TraceFlags"] = None, body: Optional[AnyValue] = None, severity_number: Optional[SeverityNumber] = None, - attributes: Optional[_ExtendedAttributes] = None, + attributes: Optional[Attributes] = None, ): attributes = attributes or {} event_attributes = { @@ -65,7 +65,7 @@ def __init__( name: str, version: Optional[str] = None, schema_url: Optional[str] = None, - attributes: Optional[_ExtendedAttributes] = None, + attributes: Optional[Attributes] = None, ): self._name = name self._version = version @@ -88,7 +88,7 @@ def __init__( name: str, version: Optional[str] = None, schema_url: Optional[str] = None, - attributes: Optional[_ExtendedAttributes] = None, + attributes: Optional[Attributes] = None, ): super().__init__( name=name, @@ -125,7 +125,7 @@ def get_event_logger( name: str, version: Optional[str] = None, schema_url: Optional[str] = None, - attributes: Optional[_ExtendedAttributes] = None, + attributes: Optional[Attributes] = None, ) -> EventLogger: """Returns an EventLoggerProvider for use.""" @@ -136,7 +136,7 @@ def get_event_logger( name: str, version: Optional[str] = None, schema_url: Optional[str] = None, - attributes: Optional[_ExtendedAttributes] = None, + attributes: Optional[Attributes] = None, ) -> EventLogger: return NoOpEventLogger( name, version=version, schema_url=schema_url, attributes=attributes @@ -149,7 +149,7 @@ def get_event_logger( name: str, version: Optional[str] = None, schema_url: Optional[str] = None, - attributes: Optional[_ExtendedAttributes] = None, + attributes: Optional[Attributes] = None, ) -> EventLogger: if _EVENT_LOGGER_PROVIDER: return _EVENT_LOGGER_PROVIDER.get_event_logger( @@ -211,7 +211,7 @@ def get_event_logger( name: str, version: Optional[str] = None, schema_url: Optional[str] = None, - attributes: Optional[_ExtendedAttributes] = None, + attributes: Optional[Attributes] = None, event_logger_provider: Optional[EventLoggerProvider] = None, ) -> "EventLogger": if event_logger_provider is None: diff --git a/opentelemetry-api/src/opentelemetry/_logs/_internal/__init__.py b/opentelemetry-api/src/opentelemetry/_logs/_internal/__init__.py index 71fc97b0aaa..17f42f1f302 100644 --- a/opentelemetry-api/src/opentelemetry/_logs/_internal/__init__.py +++ b/opentelemetry-api/src/opentelemetry/_logs/_internal/__init__.py @@ -44,7 +44,7 @@ from opentelemetry.trace.span import TraceFlags from opentelemetry.util._once import Once from opentelemetry.util._providers import _load_provider -from opentelemetry.util.types import AnyValue, _ExtendedAttributes +from opentelemetry.util.types import AnyValue, Attributes _logger = getLogger(__name__) @@ -67,7 +67,7 @@ def __init__( severity_text: Optional[str] = None, severity_number: Optional[SeverityNumber] = None, body: AnyValue = None, - attributes: Optional[_ExtendedAttributes] = None, + attributes: Optional[Attributes] = None, ): self.timestamp = timestamp if observed_timestamp is None: @@ -90,7 +90,7 @@ def __init__( name: str, version: Optional[str] = None, schema_url: Optional[str] = None, - attributes: Optional[_ExtendedAttributes] = None, + attributes: Optional[Attributes] = None, ) -> None: super().__init__() self._name = name @@ -119,7 +119,7 @@ def __init__( # pylint: disable=super-init-not-called name: str, version: Optional[str] = None, schema_url: Optional[str] = None, - attributes: Optional[_ExtendedAttributes] = None, + attributes: Optional[Attributes] = None, ): self._name = name self._version = version @@ -158,7 +158,7 @@ def get_logger( name: str, version: Optional[str] = None, schema_url: Optional[str] = None, - attributes: Optional[_ExtendedAttributes] = None, + attributes: Optional[Attributes] = None, ) -> Logger: """Returns a `Logger` for use by the given instrumentation library. @@ -196,7 +196,7 @@ def get_logger( name: str, version: Optional[str] = None, schema_url: Optional[str] = None, - attributes: Optional[_ExtendedAttributes] = None, + attributes: Optional[Attributes] = None, ) -> Logger: """Returns a NoOpLogger.""" return NoOpLogger( @@ -210,7 +210,7 @@ def get_logger( name: str, version: Optional[str] = None, schema_url: Optional[str] = None, - attributes: Optional[_ExtendedAttributes] = None, + attributes: Optional[Attributes] = None, ) -> Logger: if _LOGGER_PROVIDER: return _LOGGER_PROVIDER.get_logger( @@ -273,7 +273,7 @@ def get_logger( instrumenting_library_version: str = "", logger_provider: Optional[LoggerProvider] = None, schema_url: Optional[str] = None, - attributes: Optional[_ExtendedAttributes] = None, + attributes: Optional[Attributes] = None, ) -> "Logger": """Returns a `Logger` for use within a python process. diff --git a/opentelemetry-api/src/opentelemetry/attributes/__init__.py b/opentelemetry-api/src/opentelemetry/attributes/__init__.py index fc3d494631a..f1b8d6e6b3a 100644 --- a/opentelemetry-api/src/opentelemetry/attributes/__init__.py +++ b/opentelemetry-api/src/opentelemetry/attributes/__init__.py @@ -16,13 +16,13 @@ import threading from collections import OrderedDict from collections.abc import MutableMapping -from typing import Mapping, Optional, Sequence, Tuple, Union +from typing import Any, Mapping, Optional, Sequence, Union from opentelemetry.util import types # bytes are accepted as a user supplied value for attributes but # decoded to strings internally. -_VALID_ATTR_VALUE_TYPES = (bool, str, bytes, int, float) +_PRIMITIVE_ATTRIBUTES = (bool, str, bytes, int, float) # AnyValue possible values _VALID_ANY_VALUE_TYPES = ( type(None), @@ -35,197 +35,147 @@ Mapping, ) - _logger = logging.getLogger(__name__) -def _clean_attribute( - key: str, value: types.AttributeValue, max_len: Optional[int] -) -> Optional[Union[types.AttributeValue, Tuple[Union[str, int, float], ...]]]: +def _is_homogeneous_list_of_primitives(value: Sequence) -> tuple[bool, type]: + if len(value) == 0: + return (True, None) + + first_type = None # type: Optional[type] + for v in value: + if not v: + continue + if first_type is None: + first_type = type(v) + else: + if not isinstance(v, first_type): + return (False, None) + if not isinstance(v, _PRIMITIVE_ATTRIBUTES): + return (False, None) + return (True, first_type) + + +def _clean_extended_attribute( + key: str, value: types.AnyValue, max_value_len: Optional[int] +) -> tuple[types.AnyValue, bool]: """Checks if attribute value is valid and cleans it if required. The function returns the cleaned value or None if the value is not valid. - 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. - + An attribute value is valid if it is an AnyValue. 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 not (key and isinstance(key, str)): _logger.warning("invalid key `%s`. must be non-empty string.", key) - return None - - if isinstance(value, _VALID_ATTR_VALUE_TYPES): - return _clean_attribute_value(value, max_len) - - if isinstance(value, Sequence): - sequence_first_valid_type = None - cleaned_seq = [] - - for element in value: - element = _clean_attribute_value(element, max_len) # type: ignore - if element is None: - cleaned_seq.append(element) - 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 '%s' value sequence. Expected one of " - "%s or None", - element_type.__name__, - key, - [ - valid_type.__name__ - for valid_type in _VALID_ATTR_VALUE_TYPES - ], - ) - 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 - # use equality instead of isinstance as isinstance(True, int) evaluates to True - elif element_type != sequence_first_valid_type: - _logger.warning( - "Attribute %r mixes types %s and %s in attribute value sequence", - key, - sequence_first_valid_type.__name__, - type(element).__name__, - ) - return None + return (None, False) - cleaned_seq.append(element) - - # Freeze mutable sequences defensively - return tuple(cleaned_seq) - - _logger.warning( - "Invalid type %s for attribute '%s' value. Expected one of %s or a " - "sequence of those types", - type(value).__name__, - key, - [valid_type.__name__ for valid_type in _VALID_ATTR_VALUE_TYPES], - ) - return None + try: + return _clean_extended_attribute_value( + value, max_value_len=max_value_len + ) + except TypeError as exception: + _logger.warning( + f"Error processing attribute {key}", exc_info=exception + ) + return (None, False) def _clean_extended_attribute_value( - value: types.AnyValue, max_len: Optional[int] -) -> types.AnyValue: - # for primitive types just return the value and eventually shorten the string length - if value is None or isinstance(value, _VALID_ATTR_VALUE_TYPES): - if max_len is not None and isinstance(value, str): - value = value[:max_len] - return value - - if isinstance(value, Mapping): - cleaned_dict: dict[str, types.AnyValue] = {} - for key, element in value.items(): - # skip invalid keys - if not (key and isinstance(key, str)): - _logger.warning( - "invalid key `%s`. must be non-empty string.", key + value: types.AnyValue, max_value_len: Optional[int] +) -> tuple[types.AnyValue, bool]: + if value is None or isinstance(value, _PRIMITIVE_ATTRIBUTES): + # keeping b'string' backwards compatible with old standard + # attribute behavior. It also keeps it consistent + # across different attribute flavors. + if isinstance(value, bytes): + try: + return _clean_extended_attribute_value( + value.decode(), max_value_len=max_value_len ) - continue + except UnicodeDecodeError: + pass - cleaned_dict[key] = _clean_extended_attribute( - key=key, value=element, max_len=max_len - ) + if max_value_len is not None and isinstance(value, str): + return (value[:max_value_len], True) - return cleaned_dict + return (value, True) if isinstance(value, Sequence): - sequence_first_valid_type = None - cleaned_seq: list[types.AnyValue] = [] + if max_value_len is None: + (is_homogeneous, value_type) = _is_homogeneous_list_of_primitives( + value + ) + if ( + is_homogeneous + and value_type is not str + and value_type is not bytes + ): + return (tuple(value), True) + + return ( + ( + tuple( + [ + _clean_extended_attribute_value( + v, max_value_len=max_value_len + )[0] + for v in value + ] + ) + ), + True, + ) - for element in value: - if element is None: - cleaned_seq.append(element) + if isinstance(value, Mapping): + cleaned_list = [] + for k, v in value.items(): + if not (k and isinstance(k, str)): continue - if max_len is not None and isinstance(element, str): - element = element[:max_len] - - element_type = type(element) - if element_type not in _VALID_ATTR_VALUE_TYPES: - element = _clean_extended_attribute_value( - element, max_len=max_len - ) - element_type = type(element) # type: ignore - - # 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 - # 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__, + cleaned_list.append( + ( + k, + _clean_extended_attribute_value( + v, max_value_len=max_value_len + )[0], ) - return None - - cleaned_seq.append(element) + ) - # Freeze mutable sequences defensively - return tuple(cleaned_seq) + return (OrderedDict(sorted(cleaned_list)), True) - raise TypeError( - f"Invalid type {type(value).__name__} for attribute value. " - f"Expected one of {[valid_type.__name__ for valid_type in _VALID_ANY_VALUE_TYPES]} or a " - "sequence of those types", + _logger.warning( + "Attribute value %s is not valid. Must be one of %s", + value, + _VALID_ANY_VALUE_TYPES, ) + return (None, False) + + +def _count_leaves(value: types.AnyValue) -> int: + # it should be possible to optimize if we have a dedicated type for + # AnyValue and have leaf_count as a property of the type. + # but we don't expect attributes with huge number of leaves + # and recommend to use flat attributes whenever possible, + # so performance in case of the complex value is not a priority. + if ( + value is None + or isinstance(value, _PRIMITIVE_ATTRIBUTES) + or isinstance(value, Sequence) + ): + return 1 + if isinstance(value, Mapping): + leaf_count = 0 -def _clean_extended_attribute( - key: str, value: types.AnyValue, max_len: Optional[int] -) -> types.AnyValue: - """Checks if attribute value is valid and cleans it if required. - - The function returns the cleaned value or None if the value is not valid. - - An attribute value is valid if it is an AnyValue. - An attribute needs cleansing if: - - Its length is greater than the maximum allowed length. - """ - - if not (key and isinstance(key, str)): - _logger.warning("invalid key `%s`. must be non-empty string.", key) - return None - - try: - return _clean_extended_attribute_value(value, max_len=max_len) - except TypeError as exception: - _logger.warning("Attribute %s: %s", key, exception) - return None - - -def _clean_attribute_value( - value: types.AttributeValue, limit: Optional[int] -) -> Optional[types.AttributeValue]: - if value is None: - return None + for v in value.values(): + leaf_count += _count_leaves(v) - if isinstance(value, bytes): - try: - value = value.decode() - except UnicodeDecodeError: - _logger.warning("Byte attribute could not be decoded.") - return None + return max(1, leaf_count) - if limit is not None and isinstance(value, str): - value = value[:limit] - return value + return 0 class BoundedAttributes(MutableMapping): # type: ignore @@ -238,10 +188,10 @@ class BoundedAttributes(MutableMapping): # type: ignore def __init__( self, maxlen: Optional[int] = None, - attributes: Optional[types._ExtendedAttributes] = None, + attributes: Optional[types.Attributes] = None, immutable: bool = True, max_value_len: Optional[int] = None, - extended_attributes: bool = False, + extended_attributes: bool = True, ): if maxlen is not None: if not isinstance(maxlen, int) or maxlen < 0: @@ -250,8 +200,8 @@ def __init__( ) self.maxlen = maxlen self.dropped = 0 + self.leaf_count = 0 self.max_value_len = max_value_len - self._extended_attributes = extended_attributes # OrderedDict is not used until the maxlen is reached for efficiency. self._dict: Union[ @@ -278,30 +228,46 @@ def __setitem__(self, key: str, value: types.AnyValue) -> None: self.dropped += 1 return - if self._extended_attributes: - value = _clean_extended_attribute( - key, value, self.max_value_len - ) - else: - value = _clean_attribute(key, value, self.max_value_len) # type: ignore - if value is None: - return + (cleaned_value, is_valid) = _clean_extended_attribute( + key, value, self.max_value_len + ) + + if not is_valid: + return + + leafs = _count_leaves(cleaned_value) + + if ( + self.maxlen + and isinstance(cleaned_value, Mapping) + and (self.leaf_count + leafs > self.maxlen) + ): + self.dropped += 1 + return if key in self._dict: - del self._dict[key] - elif self.maxlen is not None and len(self._dict) == self.maxlen: + self._update_count_drop(self._dict.pop(key)) + elif ( + self.maxlen is not None + and (self.leaf_count + leafs) > self.maxlen + ): if not isinstance(self._dict, OrderedDict): self._dict = OrderedDict(self._dict) - self._dict.popitem(last=False) # type: ignore + item = self._dict.popitem(last=False) # type: ignore + self._update_count_drop(item[1]) self.dropped += 1 - self._dict[key] = value # type: ignore + self._dict[key] = cleaned_value # type: ignore + self.leaf_count += leafs + + def _update_count_drop(self, val: Any) -> None: + self.leaf_count -= _count_leaves(val) def __delitem__(self, key: str) -> None: if getattr(self, "_immutable", False): # type: ignore raise TypeError with self._lock: - del self._dict[key] + self._update_count_drop(self._dict.pop(key)) def __iter__(self): # type: ignore with self._lock: diff --git a/opentelemetry-api/src/opentelemetry/metrics/_internal/instrument.py b/opentelemetry-api/src/opentelemetry/metrics/_internal/instrument.py index 0d5ec951074..f05bdf1825c 100644 --- a/opentelemetry-api/src/opentelemetry/metrics/_internal/instrument.py +++ b/opentelemetry-api/src/opentelemetry/metrics/_internal/instrument.py @@ -25,6 +25,7 @@ Generator, Generic, Iterable, + Mapping, Optional, Sequence, TypeVar, @@ -35,9 +36,6 @@ from opentelemetry import metrics from opentelemetry.context import Context from opentelemetry.metrics._internal.observation import Observation -from opentelemetry.util.types import ( - Attributes, -) _logger = getLogger(__name__) @@ -69,6 +67,23 @@ class CallbackOptions: Generator[Iterable[Observation], CallbackOptions, None], ] +_MetricAttributes = Optional[ + Mapping[ + str, + Union[ + str, + bool, + int, + float, + bytes, + Sequence[str], + Sequence[bool], + Sequence[int], + Sequence[float], + ], + ] +] + class Instrument(ABC): """Abstract class that serves as base for all instruments.""" @@ -180,7 +195,7 @@ class Counter(Synchronous): def add( self, amount: Union[int, float], - attributes: Optional[Attributes] = None, + attributes: Optional[_MetricAttributes] = None, context: Optional[Context] = None, ) -> None: pass @@ -200,7 +215,7 @@ def __init__( def add( self, amount: Union[int, float], - attributes: Optional[Attributes] = None, + attributes: Optional[_MetricAttributes] = None, context: Optional[Context] = None, ) -> None: return super().add(amount, attributes=attributes, context=context) @@ -210,7 +225,7 @@ class _ProxyCounter(_ProxyInstrument[Counter], Counter): def add( self, amount: Union[int, float], - attributes: Optional[Attributes] = None, + attributes: Optional[_MetricAttributes] = None, context: Optional[Context] = None, ) -> None: if self._real_instrument: @@ -231,7 +246,7 @@ class UpDownCounter(Synchronous): def add( self, amount: Union[int, float], - attributes: Optional[Attributes] = None, + attributes: Optional[_MetricAttributes] = None, context: Optional[Context] = None, ) -> None: pass @@ -251,7 +266,7 @@ def __init__( def add( self, amount: Union[int, float], - attributes: Optional[Attributes] = None, + attributes: Optional[_MetricAttributes] = None, context: Optional[Context] = None, ) -> None: return super().add(amount, attributes=attributes, context=context) @@ -261,7 +276,7 @@ class _ProxyUpDownCounter(_ProxyInstrument[UpDownCounter], UpDownCounter): def add( self, amount: Union[int, float], - attributes: Optional[Attributes] = None, + attributes: Optional[_MetricAttributes] = None, context: Optional[Context] = None, ) -> None: if self._real_instrument: @@ -373,7 +388,7 @@ def __init__( def record( self, amount: Union[int, float], - attributes: Optional[Attributes] = None, + attributes: Optional[_MetricAttributes] = None, context: Optional[Context] = None, ) -> None: pass @@ -399,7 +414,7 @@ def __init__( def record( self, amount: Union[int, float], - attributes: Optional[Attributes] = None, + attributes: Optional[_MetricAttributes] = None, context: Optional[Context] = None, ) -> None: return super().record(amount, attributes=attributes, context=context) @@ -421,7 +436,7 @@ def __init__( def record( self, amount: Union[int, float], - attributes: Optional[Attributes] = None, + attributes: Optional[_MetricAttributes] = None, context: Optional[Context] = None, ) -> None: if self._real_instrument: @@ -483,7 +498,7 @@ class Gauge(Synchronous): def set( self, amount: Union[int, float], - attributes: Optional[Attributes] = None, + attributes: Optional[_MetricAttributes] = None, context: Optional[Context] = None, ) -> None: pass @@ -503,7 +518,7 @@ def __init__( def set( self, amount: Union[int, float], - attributes: Optional[Attributes] = None, + attributes: Optional[_MetricAttributes] = None, context: Optional[Context] = None, ) -> None: return super().set(amount, attributes=attributes, context=context) @@ -516,7 +531,7 @@ class _ProxyGauge( def set( self, amount: Union[int, float], - attributes: Optional[Attributes] = None, + attributes: Optional[_MetricAttributes] = None, context: Optional[Context] = None, ) -> None: if self._real_instrument: diff --git a/opentelemetry-api/src/opentelemetry/metrics/_internal/observation.py b/opentelemetry-api/src/opentelemetry/metrics/_internal/observation.py index ffc254b20a4..a25d05e5bce 100644 --- a/opentelemetry-api/src/opentelemetry/metrics/_internal/observation.py +++ b/opentelemetry-api/src/opentelemetry/metrics/_internal/observation.py @@ -12,10 +12,26 @@ # See the License for the specific language governing permissions and # limitations under the License. -from typing import Optional, Union +from typing import Mapping, Optional, Sequence, Union from opentelemetry.context import Context -from opentelemetry.util.types import Attributes + +_MetricAttributes = Optional[ + Mapping[ + str, + Union[ + str, + bool, + int, + float, + bytes, + Sequence[str], + Sequence[bool], + Sequence[int], + Sequence[float], + ], + ] +] class Observation: @@ -32,7 +48,7 @@ class Observation: def __init__( self, value: Union[int, float], - attributes: Attributes = None, + attributes: _MetricAttributes = None, context: Optional[Context] = None, ) -> None: self._value = value @@ -44,7 +60,7 @@ def value(self) -> Union[float, int]: return self._value @property - def attributes(self) -> Attributes: + def attributes(self) -> _MetricAttributes: return self._attributes @property diff --git a/opentelemetry-api/src/opentelemetry/trace/span.py b/opentelemetry-api/src/opentelemetry/trace/span.py index 6e54dfc7212..83ca221b851 100644 --- a/opentelemetry-api/src/opentelemetry/trace/span.py +++ b/opentelemetry-api/src/opentelemetry/trace/span.py @@ -79,7 +79,7 @@ def get_span_context(self) -> "SpanContext": @abc.abstractmethod def set_attributes( - self, attributes: typing.Mapping[str, types.AttributeValue] + self, attributes: typing.Mapping[str, types.AnyValue] ) -> None: """Sets Attributes. @@ -92,7 +92,7 @@ def set_attributes( """ @abc.abstractmethod - def set_attribute(self, key: str, value: types.AttributeValue) -> None: + def set_attribute(self, key: str, value: types.AnyValue) -> None: """Sets an Attribute. Sets a single Attribute with the key and value passed as arguments. @@ -529,11 +529,11 @@ def end(self, end_time: typing.Optional[int] = None) -> None: pass def set_attributes( - self, attributes: typing.Mapping[str, types.AttributeValue] + self, attributes: typing.Mapping[str, types.AnyValue] ) -> None: pass - def set_attribute(self, key: str, value: types.AttributeValue) -> None: + def set_attribute(self, key: str, value: types.AnyValue) -> None: pass def add_event( diff --git a/opentelemetry-api/src/opentelemetry/util/types.py b/opentelemetry-api/src/opentelemetry/util/types.py index 7455c741c93..a6a8a4ec4e2 100644 --- a/opentelemetry-api/src/opentelemetry/util/types.py +++ b/opentelemetry-api/src/opentelemetry/util/types.py @@ -28,17 +28,9 @@ None, ] -AttributeValue = Union[ - str, - bool, - int, - float, - Sequence[str], - Sequence[bool], - Sequence[int], - Sequence[float], -] -Attributes = Optional[Mapping[str, AttributeValue]] +AttributeValue = AnyValue +Attributes = Optional[Mapping[str, AnyValue]] + AttributesAsKey = Tuple[ Tuple[ str, @@ -55,5 +47,3 @@ ], ..., ] - -_ExtendedAttributes = Mapping[str, "AnyValue"] diff --git a/opentelemetry-api/tests/attributes/test_attributes.py b/opentelemetry-api/tests/attributes/test_attributes.py index 8a653387254..549250afca4 100644 --- a/opentelemetry-api/tests/attributes/test_attributes.py +++ b/opentelemetry-api/tests/attributes/test_attributes.py @@ -19,7 +19,6 @@ from opentelemetry.attributes import ( BoundedAttributes, - _clean_attribute, _clean_extended_attribute, ) @@ -30,10 +29,14 @@ def assertValid(self, value, key="k"): expected = value if isinstance(value, MutableSequence): expected = tuple(value) - self.assertEqual(_clean_attribute(key, value, None), expected) + (cleaned, is_valid) = _clean_extended_attribute(key, value, None) + self.assertEqual(cleaned, expected) + self.assertTrue(is_valid) def assertInvalid(self, value, key="k"): - self.assertIsNone(_clean_attribute(key, value, None)) + (cleaned, is_valid) = _clean_extended_attribute(key, value, None) + self.assertIsNone(cleaned) + self.assertFalse(is_valid) def test_attribute_key_validation(self): # only non-empty strings are valid keys @@ -46,12 +49,12 @@ def test_attribute_key_validation(self): self.assertValid(1, "1") def test_clean_attribute(self): - self.assertInvalid([1, 2, 3.4, "ss", 4]) - self.assertInvalid([{}, 1, 2, 3.4, 4]) - self.assertInvalid(["sw", "lf", 3.4, "ss"]) - self.assertInvalid([1, 2, 3.4, 5]) - self.assertInvalid({}) - self.assertInvalid([1, True]) + self.assertValid([1, 2, 3.4, "ss", 4]) + self.assertValid([{}, 1, 2, 3.4, 4]) + self.assertValid(["sw", "lf", 3.4, "ss"]) + self.assertValid([1, 2, 3.4, 5]) + self.assertValid({}) + self.assertValid([1, True]) self.assertValid(True) self.assertValid("hi") self.assertValid(3.4) @@ -65,8 +68,8 @@ def test_clean_attribute(self): self.assertValid(["A", None, None]) self.assertValid(["A", None, None, "B"]) self.assertValid([None, None]) - self.assertInvalid(["A", None, 1]) - self.assertInvalid([None, "A", None, 1]) + self.assertValid(["A", None, 1]) + self.assertValid([None, "A", None, 1]) # test keys self.assertValid("value", "key") @@ -85,12 +88,12 @@ def test_sequence_attr_decode(self): None, "Content-Disposition", "Content-Type", - None, + b"\x81", "Keep-Alive", ] - self.assertEqual( - _clean_attribute("headers", seq, None), tuple(expected) - ) + (cleaned, is_valid) = _clean_extended_attribute("headers", seq, None) + self.assertEqual(cleaned, tuple(expected)) + self.assertTrue(is_valid) class TestExtendedAttributes(unittest.TestCase): @@ -99,10 +102,15 @@ def assertValid(self, value, key="k"): expected = value if isinstance(value, MutableSequence): expected = tuple(value) - self.assertEqual(_clean_extended_attribute(key, value, None), expected) + + (cleaned, is_valid) = _clean_extended_attribute(key, value, None) + self.assertEqual(cleaned, expected) + self.assertTrue(is_valid) def assertInvalid(self, value, key="k"): - self.assertIsNone(_clean_extended_attribute(key, value, None)) + (cleaned, is_valid) = _clean_extended_attribute(key, value, None) + self.assertIsNone(cleaned) + self.assertFalse(is_valid) def test_attribute_key_validation(self): # only non-empty strings are valid keys @@ -115,11 +123,11 @@ def test_attribute_key_validation(self): self.assertValid(1, "1") def test_clean_extended_attribute(self): - self.assertInvalid([1, 2, 3.4, "ss", 4]) - self.assertInvalid([{}, 1, 2, 3.4, 4]) - self.assertInvalid(["sw", "lf", 3.4, "ss"]) - self.assertInvalid([1, 2, 3.4, 5]) - self.assertInvalid([1, True]) + self.assertValid([1, 2, 3.4, "ss", 4]) + self.assertValid([{}, 1, 2, 3.4, 4]) + self.assertValid(["sw", "lf", 3.4, "ss"]) + self.assertValid([1, 2, 3.4, 5]) + self.assertValid([1, True]) self.assertValid(None) self.assertValid(True) self.assertValid("hi") @@ -134,8 +142,8 @@ def test_clean_extended_attribute(self): self.assertValid(["A", None, None]) self.assertValid(["A", None, None, "B"]) self.assertValid([None, None]) - self.assertInvalid(["A", None, 1]) - self.assertInvalid([None, "A", None, 1]) + self.assertValid(["A", None, 1]) + self.assertValid([None, "A", None, 1]) # mappings self.assertValid({}) self.assertValid({"k": "v"}) @@ -155,9 +163,17 @@ def test_sequence_attr_decode(self): b"\x81", b"Keep-Alive", ] - self.assertEqual( - _clean_extended_attribute("headers", seq, None), tuple(seq) - ) + expected = [ + None, + "Content-Disposition", + "Content-Type", + b"\x81", + "Keep-Alive", + ] + + (cleaned, is_valid) = _clean_extended_attribute("headers", seq, None) + self.assertEqual(cleaned, tuple(expected)) + self.assertTrue(is_valid) def test_mapping(self): mapping = { @@ -166,7 +182,7 @@ def test_mapping(self): "none": {"": "invalid"}, "valid_primitive": "str", "valid_sequence": ["str"], - "invalid_sequence": ["str", 1], + "heterogeneous_sequence": ["str", 1], "valid_mapping": {"str": 1}, "invalid_mapping": {"": 1}, } @@ -174,13 +190,14 @@ def test_mapping(self): "none": {}, "valid_primitive": "str", "valid_sequence": ("str",), - "invalid_sequence": None, + "heterogeneous_sequence": ("str", 1), "valid_mapping": {"str": 1}, "invalid_mapping": {}, } - self.assertEqual( - _clean_extended_attribute("headers", mapping, None), expected - ) + + cleaned, is_valid = _clean_extended_attribute("headers", mapping, None) + self.assertEqual(cleaned, expected) + self.assertTrue(is_valid) class TestBoundedAttributes(unittest.TestCase): @@ -200,7 +217,6 @@ def test_from_map(self): dic_len = len(self.base) base_copy = self.base.copy() bdict = BoundedAttributes(dic_len, base_copy) - self.assertEqual(len(bdict), dic_len) # modify base_copy and test that bdict is not changed @@ -250,7 +266,9 @@ def test_bounded_dict(self): self.assertEqual(len(bdict), dic_len) self.assertEqual(bdict.dropped, dic_len) # Invalid values shouldn't be considered for `dropped` - bdict["invalid-seq"] = [None, 1, "2"] + bdict[""] = [None, 1, "2"] + bdict["foo"] = self + self.assertEqual(len(bdict), dic_len) self.assertEqual(bdict.dropped, dic_len) # test that elements in the dict are the new ones @@ -290,14 +308,3 @@ def test_locking(self): for num in range(100): self.assertEqual(bdict[str(num)], num) - - # pylint: disable=no-self-use - def test_extended_attributes(self): - bdict = BoundedAttributes(extended_attributes=True, immutable=False) - with unittest.mock.patch( - "opentelemetry.attributes._clean_extended_attribute", - return_value="mock_value", - ) as clean_extended_attribute_mock: - bdict["key"] = "value" - - clean_extended_attribute_mock.assert_called_once() diff --git a/opentelemetry-api/tests/events/test_proxy_event.py b/opentelemetry-api/tests/events/test_proxy_event.py index 44121a97d46..736dcf35d60 100644 --- a/opentelemetry-api/tests/events/test_proxy_event.py +++ b/opentelemetry-api/tests/events/test_proxy_event.py @@ -4,7 +4,7 @@ import opentelemetry._events as events from opentelemetry.test.globals_test import EventsGlobalsTest -from opentelemetry.util.types import _ExtendedAttributes +from opentelemetry.util.types import Attributes class TestProvider(events.NoOpEventLoggerProvider): @@ -13,7 +13,7 @@ def get_event_logger( name: str, version: typing.Optional[str] = None, schema_url: typing.Optional[str] = None, - attributes: typing.Optional[_ExtendedAttributes] = None, + attributes: typing.Optional[Attributes] = None, ) -> events.EventLogger: return LoggerTest(name) diff --git a/opentelemetry-api/tests/logs/test_proxy.py b/opentelemetry-api/tests/logs/test_proxy.py index 64c024c3fa1..8e87ceb96ea 100644 --- a/opentelemetry-api/tests/logs/test_proxy.py +++ b/opentelemetry-api/tests/logs/test_proxy.py @@ -19,7 +19,7 @@ import opentelemetry._logs._internal as _logs_internal from opentelemetry import _logs from opentelemetry.test.globals_test import LoggingGlobalsTest -from opentelemetry.util.types import _ExtendedAttributes +from opentelemetry.util.types import Attributes class TestProvider(_logs.NoOpLoggerProvider): @@ -28,7 +28,7 @@ def get_logger( name: str, version: typing.Optional[str] = None, schema_url: typing.Optional[str] = None, - attributes: typing.Optional[_ExtendedAttributes] = None, + attributes: typing.Optional[Attributes] = None, ) -> _logs.Logger: return LoggerTest(name) diff --git a/opentelemetry-sdk/benchmarks/metrics/test_benchmark_metrics_histogram.py b/opentelemetry-sdk/benchmarks/metrics/test_benchmark_metrics_histogram.py index 1c7cdf2cb5a..f4d3c518925 100644 --- a/opentelemetry-sdk/benchmarks/metrics/test_benchmark_metrics_histogram.py +++ b/opentelemetry-sdk/benchmarks/metrics/test_benchmark_metrics_histogram.py @@ -13,6 +13,7 @@ # limitations under the License. # pylint: disable=invalid-name +import json import random import pytest @@ -68,38 +69,77 @@ def _generate_bounds(bound_count): hist1000 = meter.create_histogram("test_histogram_1000_bound") -@pytest.mark.parametrize("num_labels", [0, 1, 3, 5, 7]) -def test_histogram_record(benchmark, num_labels): +@pytest.mark.parametrize("num_labels", [1, 3]) +def test_histogram_record_10(benchmark, num_labels): labels = {} for i in range(num_labels): labels[f"Key{i}"] = "Value{i}" - def benchmark_histogram_record(): - hist.record(random.random() * MAX_BOUND_VALUE) + def benchmark_histogram_record_10(): + hist10.record(random.random() * MAX_BOUND_VALUE, attributes=labels) - benchmark(benchmark_histogram_record) + benchmark(benchmark_histogram_record_10) -@pytest.mark.parametrize("num_labels", [0, 1, 3, 5, 7]) -def test_histogram_record_10(benchmark, num_labels): +@pytest.mark.parametrize("num_labels", [1, 3]) +def test_histogram_record_10_small_mapping_attrs(benchmark, num_labels): labels = {} for i in range(num_labels): - labels[f"Key{i}"] = "Value{i}" + labels[f"Key{i}"] = {"k1": "v1"} - def benchmark_histogram_record_10(): - hist10.record(random.random() * MAX_BOUND_VALUE) + def test_histogram_record_10_complex_attrs(): + hist10.record(random.random() * MAX_BOUND_VALUE, attributes=labels) - benchmark(benchmark_histogram_record_10) + benchmark(test_histogram_record_10_complex_attrs) -@pytest.mark.parametrize("num_labels", [0, 1, 3, 5, 7]) +@pytest.mark.parametrize("num_labels", [1, 3]) +def test_histogram_record_10_complex_attrs(benchmark, num_labels): + labels = {} + for i in range(num_labels): + labels[f"Key{i}"] = {"k1": "v1", "k2": {"k3": "v3", "k4": [1, 2, 3]}} + + def test_histogram_record_10_complex_attrs(): + hist10.record(random.random() * MAX_BOUND_VALUE, attributes=labels) + + benchmark(test_histogram_record_10_complex_attrs) + + +@pytest.mark.parametrize("num_labels", [1, 3]) +def test_histogram_record_10_array_attrs(benchmark, num_labels): + labels = {} + for i in range(num_labels): + labels[f"Key{i}"] = ["k1", "v1", "k2", "k3", "v3", "k4", "1", "2", "3"] + + def test_histogram_record_10_array_attrs(): + hist10.record(random.random() * MAX_BOUND_VALUE, attributes=labels) + + benchmark(test_histogram_record_10_array_attrs) + + +@pytest.mark.parametrize("num_labels", [1, 3]) +def test_histogram_record_10_json_string_attrs(benchmark, num_labels): + labels = {} + for i in range(num_labels): + labels[f"Key{i}"] = {"k1": "v1", "k2": {"k3": "v3", "k4": [1, 2, 3]}} + + def test_histogram_record_10_json_string_attrs(): + hist10.record( + random.random() * MAX_BOUND_VALUE, + attributes={k: json.dumps(v) for k, v in labels.items()}, + ) + + benchmark(test_histogram_record_10_json_string_attrs) + + +@pytest.mark.parametrize("num_labels", [1, 3]) def test_histogram_record_49(benchmark, num_labels): labels = {} for i in range(num_labels): labels[f"Key{i}"] = "Value{i}" def benchmark_histogram_record_49(): - hist49.record(random.random() * MAX_BOUND_VALUE) + hist49.record(random.random() * MAX_BOUND_VALUE, attributes=labels) benchmark(benchmark_histogram_record_49) @@ -111,7 +151,7 @@ def test_histogram_record_50(benchmark, num_labels): labels[f"Key{i}"] = "Value{i}" def benchmark_histogram_record_50(): - hist50.record(random.random() * MAX_BOUND_VALUE) + hist50.record(random.random() * MAX_BOUND_VALUE, attributes=labels) benchmark(benchmark_histogram_record_50) @@ -123,6 +163,6 @@ def test_histogram_record_1000(benchmark, num_labels): labels[f"Key{i}"] = "Value{i}" def benchmark_histogram_record_1000(): - hist1000.record(random.random() * MAX_BOUND_VALUE) + hist1000.record(random.random() * MAX_BOUND_VALUE, attributes=labels) benchmark(benchmark_histogram_record_1000) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_events/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/_events/__init__.py index c427a48e2f8..ae16302546d 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_events/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_events/__init__.py @@ -21,7 +21,7 @@ from opentelemetry._events import EventLoggerProvider as APIEventLoggerProvider from opentelemetry._logs import NoOpLogger, SeverityNumber, get_logger_provider from opentelemetry.sdk._logs import Logger, LoggerProvider, LogRecord -from opentelemetry.util.types import _ExtendedAttributes +from opentelemetry.util.types import Attributes _logger = logging.getLogger(__name__) @@ -33,7 +33,7 @@ def __init__( name: str, version: Optional[str] = None, schema_url: Optional[str] = None, - attributes: Optional[_ExtendedAttributes] = None, + attributes: Optional[Attributes] = None, ): super().__init__( name=name, @@ -74,7 +74,7 @@ def get_event_logger( name: str, version: Optional[str] = None, schema_url: Optional[str] = None, - attributes: Optional[_ExtendedAttributes] = None, + attributes: Optional[Attributes] = None, ) -> EventLogger: if not name: _logger.warning("EventLogger created with invalid name: %s", name) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py index 9060e49aac4..8f4d9a641a6 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py @@ -53,7 +53,7 @@ get_current_span, ) from opentelemetry.trace.span import TraceFlags -from opentelemetry.util.types import AnyValue, _ExtendedAttributes +from opentelemetry.util.types import AnyValue, Attributes _logger = logging.getLogger(__name__) @@ -183,7 +183,7 @@ def __init__( severity_number: SeverityNumber | None = None, body: AnyValue | None = None, resource: Resource | None = None, - attributes: _ExtendedAttributes | None = None, + attributes: Attributes | None = None, limits: LogLimits | None = _UnsetLogLimits, ): super().__init__( @@ -482,7 +482,7 @@ def __init__( self._logger_provider = logger_provider or get_logger_provider() @staticmethod - def _get_attributes(record: logging.LogRecord) -> _ExtendedAttributes: + def _get_attributes(record: logging.LogRecord) -> Attributes: attributes = { k: v for k, v in vars(record).items() if k not in _RESERVED_ATTRS } @@ -643,7 +643,7 @@ def _get_logger_no_cache( name: str, version: str | None = None, schema_url: str | None = None, - attributes: _ExtendedAttributes | None = None, + attributes: Attributes | None = None, ) -> Logger: return Logger( self._resource, @@ -677,7 +677,7 @@ def get_logger( name: str, version: str | None = None, schema_url: str | None = None, - attributes: _ExtendedAttributes | None = None, + attributes: Attributes | None = None, ) -> Logger: if self._disabled: return NoOpLogger( diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/_view_instrument_match.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/_view_instrument_match.py index be81d70e5cd..20e14e71be9 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/_view_instrument_match.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/_view_instrument_match.py @@ -16,7 +16,7 @@ from logging import getLogger from threading import Lock from time import time_ns -from typing import Dict, List, Optional, Sequence +from typing import Any, Dict, List, Mapping, Optional, Sequence from opentelemetry.metrics import Instrument from opentelemetry.sdk.metrics._internal.aggregation import ( @@ -29,10 +29,22 @@ from opentelemetry.sdk.metrics._internal.measurement import Measurement from opentelemetry.sdk.metrics._internal.point import DataPointT from opentelemetry.sdk.metrics._internal.view import View +from opentelemetry.util.types import AnyValue _logger = getLogger(__name__) +def _to_hashable(value: AnyValue) -> Any: + if isinstance(value, (str, int, float, bool)): + return value + elif isinstance(value, Sequence): + # can optimize further + return tuple(_to_hashable(v) for v in value) + elif isinstance(value, Mapping): + return tuple([(k, _to_hashable(v)) for k, v in sorted(value.items())]) + return None + + class _ViewInstrumentMatch: def __init__( self, @@ -42,7 +54,7 @@ def __init__( ): self._view = view self._instrument = instrument - self._attributes_aggregation: Dict[frozenset, _Aggregation] = {} + self._attributes_aggregation: Dict[Any, _Aggregation] = {} self._lock = Lock() self._instrument_class_aggregation = instrument_class_aggregation self._name = self._view._name or self._instrument.name @@ -102,7 +114,7 @@ def consume_measurement( else: attributes = {} - aggr_key = frozenset(attributes.items()) + aggr_key = _to_hashable(attributes) if aggr_key not in self._attributes_aggregation: with self._lock: diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py index 752b9067b77..2de046b330b 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py @@ -67,7 +67,7 @@ from json import dumps from os import environ from types import ModuleType -from typing import List, MutableMapping, Optional, cast +from typing import List, MutableMapping, Optional, Sequence, Union, cast from urllib import parse from opentelemetry.attributes import BoundedAttributes @@ -78,6 +78,7 @@ ) from opentelemetry.semconv.resource import ResourceAttributes from opentelemetry.util._importlib_metadata import entry_points, version +from opentelemetry.util.types import Attributes as AllAttributes from opentelemetry.util.types import AttributeValue psutil: Optional[ModuleType] = None @@ -90,7 +91,7 @@ pass LabelValue = AttributeValue -Attributes = typing.Mapping[str, LabelValue] +Attributes = AllAttributes logger = logging.getLogger(__name__) CLOUD_PROVIDER = ResourceAttributes.CLOUD_PROVIDER @@ -162,7 +163,9 @@ class Resource: _schema_url: str def __init__( - self, attributes: Attributes, schema_url: typing.Optional[str] = None + self, + attributes: Attributes, + schema_url: typing.Optional[str] = None, ): self._attributes = BoundedAttributes(attributes=attributes) if schema_url is None: diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 3ac45806358..9df1506db5b 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -842,9 +842,7 @@ def _new_links(self, links: Sequence[trace_api.Link]): def get_span_context(self): return self._context - def set_attributes( - self, attributes: Mapping[str, types.AttributeValue] - ) -> None: + def set_attributes(self, attributes: Mapping[str, types.AnyValue]) -> None: with self._lock: if self._end_time is not None: logger.warning("Setting attribute on ended span.") @@ -853,7 +851,7 @@ def set_attributes( for key, value in attributes.items(): self._attributes[key] = value - def set_attribute(self, key: str, value: types.AttributeValue) -> None: + def set_attribute(self, key: str, value: types.AnyValue) -> None: return self.set_attributes({key: value}) @_check_span_ended diff --git a/opentelemetry-sdk/tests/logs/test_log_record.py b/opentelemetry-sdk/tests/logs/test_log_record.py index 4a0d58dc9b1..8374094ee04 100644 --- a/opentelemetry-sdk/tests/logs/test_log_record.py +++ b/opentelemetry-sdk/tests/logs/test_log_record.py @@ -50,25 +50,26 @@ def test_log_record_to_json(self): "schema_url": "", }, }, - indent=4, + indent=None, ) + attr = { + "mapping": {"key": "value"}, + "none": None, + "sequence": [1, 2], + "str": "string", + } actual = LogRecord( timestamp=0, observed_timestamp=0, body="a log line", resource=Resource({"service.name": "foo"}), - attributes={ - "mapping": {"key": "value"}, - "none": None, - "sequence": [1, 2], - "str": "string", - }, + attributes=attr, ) - self.assertEqual(expected, actual.to_json(indent=4)) + self.maxDiff = None self.assertEqual( actual.to_json(indent=None), - '{"body": "a log line", "severity_number": null, "severity_text": null, "attributes": {"mapping": {"key": "value"}, "none": null, "sequence": [1, 2], "str": "string"}, "dropped_attributes": 0, "timestamp": "1970-01-01T00:00:00.000000Z", "observed_timestamp": "1970-01-01T00:00:00.000000Z", "trace_id": "", "span_id": "", "trace_flags": null, "resource": {"attributes": {"service.name": "foo"}, "schema_url": ""}}', + expected, ) def test_log_record_to_json_serializes_severity_number_as_int(self): diff --git a/opentelemetry-sdk/tests/metrics/test_view_instrument_match.py b/opentelemetry-sdk/tests/metrics/test_view_instrument_match.py index 38d36758f39..776dccc0431 100644 --- a/opentelemetry-sdk/tests/metrics/test_view_instrument_match.py +++ b/opentelemetry-sdk/tests/metrics/test_view_instrument_match.py @@ -22,6 +22,7 @@ from opentelemetry.context import Context from opentelemetry.sdk.metrics._internal._view_instrument_match import ( + _to_hashable, _ViewInstrumentMatch, ) from opentelemetry.sdk.metrics._internal.aggregation import ( @@ -113,7 +114,7 @@ def test_consume_measurement(self): ) self.assertEqual( view_instrument_match._attributes_aggregation, - {frozenset([("c", "d")]): self.mock_created_aggregation}, + {_to_hashable({"c": "d"}): self.mock_created_aggregation}, ) view_instrument_match.consume_measurement( @@ -129,8 +130,8 @@ def test_consume_measurement(self): self.assertEqual( view_instrument_match._attributes_aggregation, { - frozenset(): self.mock_created_aggregation, - frozenset([("c", "d")]): self.mock_created_aggregation, + _to_hashable({}): self.mock_created_aggregation, + _to_hashable({"c": "d"}): self.mock_created_aggregation, }, ) @@ -159,8 +160,8 @@ def test_consume_measurement(self): self.assertEqual( view_instrument_match._attributes_aggregation, { - frozenset( - [("c", "d"), ("f", "g")] + _to_hashable( + {"c": "d", "f": "g"} ): self.mock_created_aggregation }, ) @@ -190,7 +191,7 @@ def test_consume_measurement(self): ) self.assertEqual( view_instrument_match._attributes_aggregation, - {frozenset({}): self.mock_created_aggregation}, + {_to_hashable({}): self.mock_created_aggregation}, ) # Test that a drop aggregation is handled in the same way as any @@ -219,7 +220,7 @@ def test_consume_measurement(self): ) ) self.assertIsInstance( - view_instrument_match._attributes_aggregation[frozenset({})], + view_instrument_match._attributes_aggregation[_to_hashable({})], _DropAggregation, ) @@ -484,7 +485,7 @@ def test_setting_aggregation(self): self.assertIsInstance( view_instrument_match._attributes_aggregation[ - frozenset({("c", "d")}) + _to_hashable({"c": "d"}) ], _LastValueAggregation, ) diff --git a/opentelemetry-sdk/tests/resources/test_resources.py b/opentelemetry-sdk/tests/resources/test_resources.py index b080519a867..ac98c74492c 100644 --- a/opentelemetry-sdk/tests/resources/test_resources.py +++ b/opentelemetry-sdk/tests/resources/test_resources.py @@ -240,25 +240,30 @@ def test_service_name_using_process_name(self): def test_invalid_resource_attribute_values(self): with self.assertLogs(level=WARNING): + guid = uuid.uuid4() resource = Resource( { SERVICE_NAME: "test", "non-primitive-data-type": {}, - "invalid-byte-type-attribute": ( + "bytes-type-attribute": ( b"\xd8\xe1\xb7\xeb\xa8\xe5 \xd2\xb7\xe1" ), "": "empty-key-value", None: "null-key-value", - "another-non-primitive": uuid.uuid4(), + "another-non-primitive": guid, } ) self.assertEqual( resource.attributes, { SERVICE_NAME: "test", + "bytes-type-attribute": ( + b"\xd8\xe1\xb7\xeb\xa8\xe5 \xd2\xb7\xe1" + ), + "non-primitive-data-type": {}, }, ) - self.assertEqual(len(resource.attributes), 1) + self.assertEqual(len(resource.attributes), 3) def test_aggregated_resources_no_detectors(self): aggregated_resources = get_aggregated_resources([]) diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index 7b23c11fa1f..8e8f903e6c3 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -25,6 +25,7 @@ from typing import Optional from unittest import mock from unittest.mock import Mock, patch +from uuid import uuid4 from opentelemetry import trace as trace_api from opentelemetry.attributes import BoundedAttributes @@ -143,7 +144,7 @@ def run_general_code(shutdown_on_exit, explicit_shutdown): # test default shutdown_on_exit (True) out = run_general_code(True, False) - self.assertTrue(out.startswith(b"1")) + self.assertTrue(out.startswith(b"1"), out) # test that shutdown is called only once even if Tracer.shutdown is # called explicitly @@ -709,6 +710,12 @@ def test_attributes(self): root.set_attribute("http.response.status_code", 200) root.set_attribute("http.status_text", "OK") root.set_attribute("misc.pi", 3.14) + root.set_attribute("mapping-key", {"key": "value"}) + root.set_attribute("array-key", [1, 2, 3]) + root.set_attribute( + "complex-key", + [{"key1": "value1"}, {"key2": 42}, "key3", [1, 2, 3]], + ) # Setting an attribute with the same key as an existing attribute # SHOULD overwrite the existing attribute's value. @@ -721,7 +728,7 @@ def test_attributes(self): list_of_numerics = [123, 314, 0] root.set_attribute("list-of-numerics", list_of_numerics) - self.assertEqual(len(root.attributes), 9) + self.assertEqual(len(root.attributes), 12) self.assertEqual(root.attributes["http.request.method"], "GET") self.assertEqual( root.attributes["url.full"], @@ -746,64 +753,73 @@ def test_attributes(self): self.assertEqual( root.attributes["list-of-numerics"], (123, 314, 0) ) + self.assertEqual(root.attributes["mapping-key"], {"key": "value"}) + self.assertEqual(root.attributes["array-key"], (1, 2, 3)) + self.assertEqual( + root.attributes["complex-key"], + ( + {"key1": "value1"}, + {"key2": 42}, + "key3", + (1, 2, 3), + ), + ) attributes = { "attr-key": "val", "attr-key2": "val2", "attr-in-both": "span-attr", + "mapping-key": {"key": "value"}, + "array-key": [1, 2, 3], + "complex-key": [ + {"key1": "value1"}, + {"key2": 42}, + "key3", + [1, 2, 3], + ], } with self.tracer.start_as_current_span( "root2", attributes=attributes ) as root: - self.assertEqual(len(root.attributes), 3) + self.assertEqual(len(root.attributes), 6) self.assertEqual(root.attributes["attr-key"], "val") self.assertEqual(root.attributes["attr-key2"], "val2") self.assertEqual(root.attributes["attr-in-both"], "span-attr") + self.assertEqual(root.attributes["mapping-key"], {"key": "value"}) + self.assertEqual(root.attributes["array-key"], (1, 2, 3)) + self.assertEqual( + root.attributes["complex-key"], + ( + {"key1": "value1"}, + {"key2": 42}, + "key3", + (1, 2, 3), + ), + ) def test_invalid_attribute_values(self): with self.tracer.start_as_current_span("root") as root: with self.assertLogs(level=WARNING): - root.set_attributes( - {"correct-value": "foo", "non-primitive-data-type": {}} - ) - - with self.assertLogs(level=WARNING): - root.set_attribute("non-primitive-data-type", {}) - with self.assertLogs(level=WARNING): - root.set_attribute( - "list-of-mixed-data-types-numeric-first", - [123, False, "string"], - ) - with self.assertLogs(level=WARNING): - root.set_attribute( - "list-of-mixed-data-types-non-numeric-first", - [False, 123, "string"], - ) - with self.assertLogs(level=WARNING): - root.set_attribute( - "list-with-non-primitive-data-type", [{}, 123] - ) - with self.assertLogs(level=WARNING): - root.set_attribute("list-with-numeric-and-bool", [1, True]) - + root.set_attributes({"correct-value": "foo", "": 42}) with self.assertLogs(level=WARNING): root.set_attribute("", 123) with self.assertLogs(level=WARNING): root.set_attribute(None, 123) + with self.assertLogs(level=WARNING): + root.set_attribute(42, "foo") + with self.assertLogs(level=WARNING): + root.set_attribute("foo", uuid4()) self.assertEqual(len(root.attributes), 1) self.assertEqual(root.attributes["correct-value"], "foo") 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( + "bytes-type-attribute", + b"\xd8\xe1\xb7\xeb\xa8\xe5 \xd2\xb7\xe1", + ) + self.assertTrue("bytes-type-attribute" in root.attributes) root.set_attribute("valid-byte-type-attribute", b"valid byte") self.assertTrue( @@ -907,20 +923,27 @@ def test_invalid_event_attributes(self): with self.tracer.start_as_current_span("root") as root: with self.assertLogs(level=WARNING): root.add_event( - "event0", {"attr1": True, "attr2": ["hi", False]} + "event0", {"attr1": True, "attr2": ["hi", False], "": 42} ) with self.assertLogs(level=WARNING): - root.add_event("event0", {"attr1": {}}) + root.add_event("event0", {"attr1": {}, "attr3": uuid4()}) with self.assertLogs(level=WARNING): - root.add_event("event0", {"attr1": [[True]]}) + root.add_event("event0", {"attr1": [[True]], "": 42}) with self.assertLogs(level=WARNING): - root.add_event("event0", {"attr1": [{}], "attr2": [1, 2]}) + root.add_event( + "event0", {"attr1": [{}], "attr2": [1, 2], "": 42} + ) self.assertEqual(len(root.events), 4) - self.assertEqual(root.events[0].attributes, {"attr1": True}) - self.assertEqual(root.events[1].attributes, {}) - self.assertEqual(root.events[2].attributes, {}) - self.assertEqual(root.events[3].attributes, {"attr2": (1, 2)}) + self.assertEqual( + root.events[0].attributes, + {"attr1": True, "attr2": ("hi", False)}, + ) + self.assertEqual(root.events[1].attributes, {"attr1": {}}) + self.assertEqual(root.events[2].attributes, {"attr1": ([True],)}) + self.assertEqual( + root.events[3].attributes, {"attr1": ({},), "attr2": (1, 2)} + ) def test_links(self): id_generator = RandomIdGenerator()