Update json and proto encoder to always accept None type, cleanup code / tests a little bit

.changelog/5305.added

@@ -0,0 +1 @@
+ `opentelemetry-exporter-otlp-proto-common`, `opentelemetry-exporter-otlp-json-common`: encoders now always accept null, and encode it as an empty `AnyValue` in accordance with the spec.

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

@@ -6,12 +6,9 @@
 from __future__ import annotations
 
 import logging
-from collections.abc import Collection, Mapping, Sequence
+from collections.abc import Mapping, Sequence
 from os import environ
-from typing import (
-    Any,
-    cast,
-)
+from typing import Any
 
 from opentelemetry.proto_json.common.v1.common import AnyValue as JSONAnyValue
 from opentelemetry.proto_json.common.v1.common import (
@@ -47,7 +44,7 @@
 )
 from opentelemetry.sdk.resources import Resource
 from opentelemetry.sdk.util.instrumentation import InstrumentationScope
-from opentelemetry.util.types import Attributes
+from opentelemetry.util.types import _ExtendedAttributes
 
 _logger = logging.getLogger(__name__)
 
@@ -71,9 +68,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):
@@ -87,45 +84,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:
@@ -137,18 +109,15 @@ def _encode_trace_id(trace_id: int) -> bytes:
 
 
 def _encode_attributes(
-    attributes: Attributes | None,
-    allow_null: bool = False,
+    attributes: _ExtendedAttributes | None,
 ) -> 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,
         ),
         dropped_attributes_count=readable_log_record.dropped_attributes,
         severity_number=getattr(

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

@@ -9,7 +9,6 @@
 from unittest.mock import patch
 
 from opentelemetry.exporter.otlp.json.common._internal import (
-    _encode_array,
     _encode_attributes,
     _encode_instrumentation_scope,
     _encode_key_value,
@@ -57,6 +56,11 @@
 _COMMON_LOGGER_NAME = "opentelemetry.exporter.otlp.json.common._internal"
 
 
+class CallingStrRaisesException:
+    def __str__(self):
+        raise ValueError("Cannot encode")
+
+
 class TestCommonEncoder(unittest.TestCase):
     def test_encode_value(self):
         cases = [
@@ -151,29 +155,23 @@ 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)
-        self.assertEqual(
-            result,
-            [
-                JSONAnyValue(int_value=1),
-                JSONAnyValue(),
-                JSONAnyValue(int_value=2),
-            ],
+        result = _encode_value([1, None, 2])
+        expected = JSONAnyValue(
+            array_value=JSONArrayValue(
+                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)
+        self.assertEqual(result, expected)
+        self.assertEqual(result.array_value.values[1].to_dict(), {})
 
     def test_encode_key_value(self):
         result = _encode_key_value("mykey", "myval")
@@ -254,7 +252,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": CallingStrRaisesException(), "b": 2}
+            )
 
         self.assertEqual(len(error.records), 1)
         self.assertEqual(error.records[0].msg, "Failed to encode key %s: %s")
@@ -268,10 +268,14 @@ def test_encode_attributes_error_skips_bad_key(self):
             ],
         )
 
-    def test_encode_attributes_error_list_none(self):
+    def test_encode_attributes_error_list_unencodable_item(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", CallingStrRaisesException(), "test"],
+                    "b": 2,
+                }
             )
 
         self.assertEqual(len(error.records), 1)