Expand attribute value type to support complex values everywhere and cleanup surrounding code

.changelog/5266.added

@@ -0,0 +1 @@
+`opentelemetry-api`, `opentelemetry-sdk`, `opentelemetry-exporter-otlp-proto-common`, `opentelemetry-exporter-otlp-json-common`: add support for extended attribute values everywhere.

docs/conf.py

@@ -26,14 +26,21 @@
 
 settings.configure()
 
-# Provide AnyValue in opentelemetry.attributes module's namespace so the
-# "AnyValue" forward reference in opentelemetry.util.types._ExtendedAttributes
+# TODO: Is there a better way to do this ? I have to  do this re-import thing
+# in like a dozen
+# Provide AnyValue to a bunch of module's namespaces.
+# "AnyValue" forward reference in opentelemetry.util.types.AnyValue
 # resolves when sphinx_autodoc_typehints calls typing.get_type_hints() on
 # BoundedAttributes (whose __globals__ is the attributes module). Docs-only.
 import opentelemetry.attributes  # noqa: E402
+import opentelemetry.metrics  # noqa: E402
 from opentelemetry.util.types import AnyValue as _AnyValue  # noqa: E402
 
-opentelemetry.attributes.AnyValue = _AnyValue
+opentelemetry.metrics._internal.AnyValue = (
+    opentelemetry.metrics._internal.instrument.AnyValue
+) = opentelemetry.metrics._internal.observation.AnyValue = (
+    opentelemetry.attributes.AnyValue
+) = _AnyValue
 
 
 source_dirs = [
@@ -186,10 +193,6 @@
         "py:class",
         "AnyValue",
     ),
-    (
-        "py:class",
-        "_ExtendedAttributes",
-    ),
     ("py:class", "Token"),
 ]
 

exporter/opentelemetry-exporter-otlp-json-common/src/opentelemetry/exporter/otlp/json/common/_internal/__init__.py

@@ -6,11 +6,8 @@
 from __future__ import annotations
 
 import logging
-from collections.abc import Collection, Mapping, Sequence
-from typing import (
-    Any,
-    cast,
-)
+from collections.abc import Mapping, Sequence
+from typing import Any
 
 from opentelemetry.proto_json.common.v1.common import AnyValue as JSONAnyValue
 from opentelemetry.proto_json.common.v1.common import (
@@ -52,9 +49,9 @@ def _encode_resource(resource: Resource) -> JSONResource:
 
 
 # pylint: disable-next=too-many-return-statements
-def _encode_value(value: Any, allow_null: bool = False) -> JSONAnyValue | None:
-    if allow_null and value is None:
-        return None
+def _encode_value(value: Any) -> JSONAnyValue:
+    if value is None:
+        return JSONAnyValue()
     if isinstance(value, bool):
         return JSONAnyValue(bool_value=value)
     if isinstance(value, str):
@@ -68,45 +65,20 @@ def _encode_value(value: Any, allow_null: bool = False) -> JSONAnyValue | None:
     if isinstance(value, Sequence):
         return JSONAnyValue(
             array_value=JSONArrayValue(
-                values=cast(
-                    list[JSONAnyValue],
-                    _encode_array(value, allow_null=allow_null),
-                )
+                values=[_encode_value(v) for v in value]
             )
         )
     if isinstance(value, Mapping):
         return JSONAnyValue(
             kvlist_value=JSONKeyValueList(
-                values=[
-                    _encode_key_value(str(k), v, allow_null=allow_null)
-                    for k, v in value.items()
-                ]
+                values=[_encode_key_value(str(k), v) for k, v in value.items()]
             )
         )
     raise TypeError(f"Invalid type {type(value)} of value {value}")
 
 
-def _encode_key_value(
-    key: str, value: Any, allow_null: bool = False
-) -> JSONKeyValue:
-    return JSONKeyValue(
-        key=key, value=_encode_value(value, allow_null=allow_null)
-    )
-
-
-def _encode_array(
-    array: Collection[Any], allow_null: bool = False
-) -> list[JSONAnyValue | None]:
-    if not allow_null:
-        # Let the exception get raised by _encode_value()
-        return [_encode_value(v, allow_null=allow_null) for v in array]
-
-    return [
-        _encode_value(v, allow_null=allow_null)
-        if v is not None
-        else JSONAnyValue()
-        for v in array
-    ]
+def _encode_key_value(key: str, value: Any) -> JSONKeyValue:
+    return JSONKeyValue(key=key, value=_encode_value(value))
 
 
 def _encode_span_id(span_id: int) -> bytes:
@@ -118,18 +90,15 @@ def _encode_trace_id(trace_id: int) -> bytes:
 
 
 def _encode_attributes(
-    attributes: Attributes | None,
-    allow_null: bool = False,
+    attributes: Attributes,
 ) -> list[JSONKeyValue]:
     if not attributes:
         return []
     json_attributes = []
     for key, value in attributes.items():
         # pylint: disable=broad-exception-caught
         try:
-            json_attributes.append(
-                _encode_key_value(key, value, allow_null=allow_null)
-            )
+            json_attributes.append(_encode_key_value(key, value))
         except Exception as error:
             _logger.exception("Failed to encode key %s: %s", key, error)
     return json_attributes

exporter/opentelemetry-exporter-otlp-json-common/src/opentelemetry/exporter/otlp/json/common/_internal/_log_encoder/__init__.py

@@ -50,13 +50,10 @@ def _encode_log(readable_log_record: ReadableLogRecord) -> JSONLogRecord:
         if readable_log_record.log_record.trace_id
         else None,
         flags=int(readable_log_record.log_record.trace_flags),
-        body=_encode_value(
-            readable_log_record.log_record.body, allow_null=True
-        ),
+        body=_encode_value(readable_log_record.log_record.body),
         severity_text=readable_log_record.log_record.severity_text,
         attributes=_encode_attributes(
-            cast(Attributes, readable_log_record.log_record.attributes),
-            allow_null=True,
+            cast(Attributes, readable_log_record.log_record.attributes)
         ),
         dropped_attributes_count=readable_log_record.dropped_attributes,
         severity_number=getattr(

exporter/opentelemetry-exporter-otlp-json-common/tests/test_common_encoder.py

@@ -8,7 +8,6 @@
 from logging import ERROR
 
 from opentelemetry.exporter.otlp.json.common._internal import (
-    _encode_array,
     _encode_attributes,
     _encode_instrumentation_scope,
     _encode_key_value,
@@ -35,6 +34,11 @@
 from opentelemetry.sdk.util.instrumentation import InstrumentationScope
 
 
+class CannotEncode:
+    def __str__(self):
+        raise ValueError("Cannot encode")
+
+
 class TestCommonEncoder(unittest.TestCase):
     def test_encode_value(self):
         cases = [
@@ -129,29 +133,20 @@ def test_encode_value_mapping(self):
         result_dict = result.to_dict()
         self.assertIn("kvlistValue", result_dict)
 
-    def test_encode_value_none_not_allowed(self):
-        with self.assertRaises(TypeError):
-            _encode_value(None)
-
-    def test_encode_value_none_allowed(self):
-        result = _encode_value(None, allow_null=True)
-        self.assertIsNone(result)
+    def test_encode_null(self):
+        result = _encode_value(None)
+        self.assertEqual(result, JSONAnyValue())
 
     def test_encode_array_with_nulls(self):
-        result = _encode_array([1, None, 2], allow_null=True)
+        result = _encode_value([1, None, 2])
         self.assertEqual(
-            result,
+            result.array_value.values,
             [
                 JSONAnyValue(int_value=1),
                 JSONAnyValue(),
                 JSONAnyValue(int_value=2),
             ],
         )
-        self.assertEqual(result[1].to_dict(), {})
-
-    def test_encode_array_none_raises(self):
-        with self.assertRaises(TypeError):
-            _encode_array([1, None, 2], allow_null=False)
 
     def test_encode_key_value(self):
         result = _encode_key_value("mykey", "myval")
@@ -232,7 +227,9 @@ def test_encode_attributes_empty(self):
 
     def test_encode_attributes_error_skips_bad_key(self):
         with self.assertLogs(level=ERROR) as error:
-            result = _encode_attributes({"a": 1, "bad_key": None, "b": 2})
+            result = _encode_attributes(
+                {"a": 1, "bad_key": CannotEncode(), "b": 2}
+            )
 
         self.assertEqual(len(error.records), 1)
         self.assertEqual(error.records[0].msg, "Failed to encode key %s: %s")
@@ -246,10 +243,10 @@ def test_encode_attributes_error_skips_bad_key(self):
             ],
         )
 
-    def test_encode_attributes_error_list_none(self):
+    def test_encode_attributes_error_list_unencodable(self):
         with self.assertLogs(level=ERROR) as error:
             result = _encode_attributes(
-                {"a": 1, "bad_key": ["test", None, "test"], "b": 2}
+                {"a": 1, "bad_key": ["test", CannotEncode(), "test"], "b": 2}
             )
 
         self.assertEqual(len(error.records), 1)

exporter/opentelemetry-exporter-otlp-json-common/tests/test_log_encoder.py

@@ -127,7 +127,7 @@ def test_encode_log_dict_body_with_nulls(self):
             JSONAnyValue(
                 kvlist_value=JSONKeyValueList(
                     values=[
-                        JSONKeyValue(key="error"),
+                        JSONKeyValue(key="error", value=JSONAnyValue()),
                         JSONKeyValue(
                             key="array_with_nones",
                             value=JSONAnyValue(
@@ -149,12 +149,7 @@ def test_encode_log_no_body(self):
         log = make_log(body=None)
         result = encode_logs([log])
         lr = _get_first_log_record(result)
-        self.assertIsNone(lr.body)
-
-        lr_dict = result.to_dict()["resourceLogs"][0]["scopeLogs"][0][
-            "logRecords"
-        ][0]
-        self.assertNotIn("body", lr_dict)
+        self.assertEqual(lr.body, JSONAnyValue())
 
     def test_encode_log_extended_attributes(self):
         log = make_log(
@@ -184,7 +179,7 @@ def test_encode_log_empty_record(self):
         self.assertEqual(lr.observed_time_unix_nano, TIME + 1000)
         self.assertIsNone(lr.severity_text)
         self.assertIsNone(lr.severity_number)
-        self.assertIsNone(lr.body)
+        self.assertEqual(lr.body, JSONAnyValue())
         self.assertEqual(lr.attributes, [])
 
     def test_encode_log_event_name(self):

exporter/opentelemetry-exporter-otlp-proto-common/src/opentelemetry/exporter/otlp/proto/common/_internal/__init__.py

@@ -27,7 +27,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__)
 
@@ -51,9 +51,9 @@ def _encode_resource(resource: Resource) -> PB2Resource:
     return PB2Resource(attributes=_encode_attributes(resource.attributes))
 
 
-def _encode_value(value: Any, allow_null: bool = False) -> PB2AnyValue | None:
-    if allow_null is True and value is None:
-        return None
+def _encode_value(value: Any) -> PB2AnyValue:
+    if value is None:
+        return PB2AnyValue()
     if isinstance(value, bool):
         return PB2AnyValue(bool_value=value)
     if isinstance(value, str):
@@ -66,45 +66,19 @@ def _encode_value(value: Any, allow_null: bool = False) -> PB2AnyValue | None:
         return PB2AnyValue(bytes_value=value)
     if isinstance(value, Sequence):
         return PB2AnyValue(
-            array_value=PB2ArrayValue(
-                values=_encode_array(value, allow_null=allow_null)
-            )
+            array_value=PB2ArrayValue(values=[_encode_value(v) for v in value])
         )
-    elif isinstance(value, Mapping):
+    if isinstance(value, Mapping):
         return PB2AnyValue(
             kvlist_value=PB2KeyValueList(
-                values=[
-                    _encode_key_value(str(k), v, allow_null=allow_null)
-                    for k, v in value.items()
-                ]
+                values=[_encode_key_value(str(k), v) for k, v in value.items()]
             )
         )
     raise Exception(f"Invalid type {type(value)} of value {value}")
 
 
-def _encode_key_value(
-    key: str, value: Any, allow_null: bool = False
-) -> PB2KeyValue:
-    return PB2KeyValue(
-        key=key, value=_encode_value(value, allow_null=allow_null)
-    )
-
-
-def _encode_array(
-    array: Sequence[Any], allow_null: bool = False
-) -> Sequence[PB2AnyValue]:
-    if not allow_null:
-        # Let the exception get raised by _encode_value()
-        return [_encode_value(v, allow_null=allow_null) for v in array]
-
-    return [
-        _encode_value(v, allow_null=allow_null)
-        if v is not None
-        # Use an empty AnyValue to represent None in an array. Behavior may change pending
-        # https://github.com/open-telemetry/opentelemetry-specification/issues/4392
-        else PB2AnyValue()
-        for v in array
-    ]
+def _encode_key_value(key: str, value: Any) -> PB2KeyValue:
+    return PB2KeyValue(key=key, value=_encode_value(value))
 
 
 def _encode_span_id(span_id: int) -> bytes:
@@ -115,23 +89,17 @@ def _encode_trace_id(trace_id: int) -> bytes:
     return trace_id.to_bytes(length=16, byteorder="big", signed=False)
 
 
-def _encode_attributes(
-    attributes: _ExtendedAttributes,
-    allow_null: bool = False,
-) -> list[PB2KeyValue] | None:
+def _encode_attributes(attributes: Attributes) -> list[PB2KeyValue] | None:
     if attributes:
         pb2_attributes = []
         for key, value in attributes.items():
             # pylint: disable=broad-exception-caught
             try:
-                pb2_attributes.append(
-                    _encode_key_value(key, value, allow_null=allow_null)
-                )
+                pb2_attributes.append(_encode_key_value(key, value))
             except Exception as error:
                 _logger.exception("Failed to encode key %s: %s", key, error)
-    else:
-        pb2_attributes = None
-    return pb2_attributes
+        return pb2_attributes
+    return None
 
 
 def _get_resource_data(

exporter/opentelemetry-exporter-otlp-proto-common/src/opentelemetry/exporter/otlp/proto/common/_internal/_log_encoder/__init__.py

@@ -39,17 +39,16 @@ def _encode_log(readable_log_record: ReadableLogRecord) -> PB2LogRecord:
         if readable_log_record.log_record.trace_id == 0
         else _encode_trace_id(readable_log_record.log_record.trace_id)
     )
-    body = readable_log_record.log_record.body
     return PB2LogRecord(
         time_unix_nano=readable_log_record.log_record.timestamp,
         observed_time_unix_nano=readable_log_record.log_record.observed_timestamp,
         span_id=span_id,
         trace_id=trace_id,
         flags=int(readable_log_record.log_record.trace_flags),
-        body=_encode_value(body, allow_null=True),
+        body=_encode_value(readable_log_record.log_record.body),
         severity_text=readable_log_record.log_record.severity_text,
         attributes=_encode_attributes(
-            readable_log_record.log_record.attributes, allow_null=True
+            readable_log_record.log_record.attributes
         ),
         dropped_attributes_count=readable_log_record.dropped_attributes,
         severity_number=getattr(