opentelemetry-configuration: treat present-null config value as empty mapping

.changelog/5454.fixed

@@ -0,0 +1 @@
+`opentelemetry-configuration`: a declarative config key present with an empty (null) value on an object-typed node (e.g. `always_on:`, a `- service:` detector, or a metric `console:` exporter) is now treated the same as an explicit empty config (`always_on: {}`) instead of failing type dispatch or silently skipping the node. Both `dict`-typed nodes and dataclasses constructible with no arguments are covered.

opentelemetry-configuration/src/opentelemetry/configuration/_conversion.py

@@ -12,7 +12,7 @@
 from __future__ import annotations
 
 from collections.abc import Mapping
-from dataclasses import fields, is_dataclass
+from dataclasses import MISSING, fields, is_dataclass
 from enum import Enum
 from types import UnionType
 from typing import Any, TypeVar, Union, get_args, get_origin, get_type_hints
@@ -33,19 +33,50 @@ def _unwrap_optional(type_hint: Any) -> Any:
     return type_hint
 
 
+def _is_empty_constructible_dataclass(unwrapped: Any) -> bool:
+    """True if ``unwrapped`` is a dataclass type instantiable with no args.
+
+    A dataclass with only optional fields (all have a default or
+    default_factory) can be built as ``cls()``; one with a required field
+    cannot, and coercing a present null into it would raise ``TypeError``.
+    """
+    return (
+        isinstance(unwrapped, type)
+        and is_dataclass(unwrapped)
+        and all(
+            field.default is not MISSING
+            or field.default_factory is not MISSING
+            for field in fields(unwrapped)
+        )
+    )
+
+
 def _convert_value(value: Any, type_hint: Any) -> Any:
     """Convert a value according to its type hint.
 
     Recursively converts dicts to dataclasses and lists of dicts to lists of
     dataclasses. Other values (primitives, enums, ``dict[str, Any]`` aliases)
     pass through unchanged.
     """
-    if value is None:
-        return None
-
     unwrapped = _unwrap_optional(type_hint)
     origin = get_origin(unwrapped)
 
+    if value is None:
+        # A mapping key present with an empty (null) YAML value parses to
+        # ``None``, which is otherwise indistinguishable from an absent key.
+        # For object-typed nodes the declarative-config spec treats a present
+        # null as "select this with an empty config" — e.g. ``always_on:`` is
+        # equivalent to ``always_on: {}`` and a metric ``console:`` exporter to
+        # ``console: {}``. Substitute an empty mapping and let the dict/
+        # dataclass handling below build it, so downstream ``is not None`` type
+        # dispatch selects it. Scalar fields — and dataclasses with required
+        # fields, which cannot be defaulted — keep ``None``, so an absent
+        # optional section stays unset.
+        if origin is dict or _is_empty_constructible_dataclass(unwrapped):
+            value = {}
+        else:
+            return None
+
     # list[X] — recurse on each element
     if origin is list and isinstance(value, list):
         args = get_args(unwrapped)

opentelemetry-configuration/tests/file/test_loader.py

@@ -282,11 +282,11 @@ class TestConfigLoaderEndToEnd(unittest.TestCase):
         trace_id_ratio_based: {ratio: 0.5}
 """
 
-    def _load(self) -> OpenTelemetryConfiguration:
+    def _load(self, yaml: str | None = None) -> OpenTelemetryConfiguration:
         with tempfile.NamedTemporaryFile(
             mode="w", suffix=".yaml", delete=False
         ) as fh:
-            fh.write(self._YAML)
+            fh.write(self._YAML if yaml is None else yaml)
             path = fh.name
         try:
             return load_config_file(path)
@@ -327,6 +327,35 @@ def test_typed_config_feeds_factory_function(self):
         self.assertIsInstance(processors[0], BatchSpanProcessor)
         self.assertIsInstance(processors[0].span_exporter, ConsoleSpanExporter)
 
+    def test_null_valued_required_field_node_survives_conversion(self):
+        # Regression test for #5451: ``jaeger_remote_development`` is nullable
+        # in the schema (so ``jaeger_remote_development:`` passes validation and
+        # is NOT rejected before conversion), yet its model has required
+        # fields. Coercing the present null into it would raise TypeError, so
+        # it must be left as None instead. The rest of the config still loads.
+        config = self._load(
+            """
+file_format: '1.0'
+tracer_provider:
+  processors:
+    - batch:
+        exporter:
+          console:
+  sampler:
+    jaeger_remote_development:
+"""
+        )
+
+        # Schema accepted the null, and conversion left the required-field
+        # node unset rather than crashing.
+        self.assertIsNone(
+            config.tracer_provider.sampler.jaeger_remote_development
+        )
+        # A sibling nullable dict-typed node (console:) was still coerced.
+        self.assertEqual(
+            config.tracer_provider.processors[0].batch.exporter.console, {}
+        )
+
 
 class TestFileFormatValidation(unittest.TestCase):
     """Validate the file_format version per the configuration spec."""

opentelemetry-configuration/tests/test_conversion.py

@@ -11,6 +11,7 @@
 from opentelemetry.configuration._common import _additional_properties
 from opentelemetry.configuration._conversion import _dict_to_dataclass
 from opentelemetry.configuration.models import ExemplarFilter
+from opentelemetry.configuration.models import Sampler as SamplerConfig
 
 
 @dataclass
@@ -42,6 +43,13 @@ class _WithEnum:
     filter: ExemplarFilter | None = None
 
 
+@dataclass
+class _WithMapping:
+    # ``dict[str, Any]``-typed node, like the sampler/detector leaf configs.
+    option: dict[str, Any] | None = None
+    name: str | None = None
+
+
 class TestDictToDataclass(unittest.TestCase):
     def test_raises_on_non_dataclass(self):
         # _dict_to_dataclass is internal and assumes cls is a dataclass.
@@ -72,11 +80,25 @@ def test_converts_list_of_dataclasses(self):
         self.assertEqual(result.middle.items[0].value, 1)
         self.assertEqual(result.middle.items[1].value, 2)
 
-    def test_none_value_preserved(self):
+    def test_present_null_dataclass_coerced_to_empty_instance(self):
+        # A present-null value on a dataclass-typed node that can be built with
+        # no arguments (e.g. a metric ``console:`` exporter) becomes a
+        # defaulted instance, not None. Regression test for #5451.
         result = _dict_to_dataclass({"middle": None, "name": "test"}, _Outer)
-        self.assertIsNone(result.middle)
+        self.assertIsInstance(result.middle, _Middle)
+        self.assertIsNone(result.middle.inner)
         self.assertEqual(result.name, "test")
 
+    def test_present_null_dataclass_with_required_field_stays_none(self):
+        # ``jaeger_remote_development`` is nullable in the schema but its model
+        # (ExperimentalJaegerRemoteSampler) has required fields, so it cannot
+        # be defaulted. A present null must stay None rather than raising a
+        # TypeError trying to instantiate it. Regression test for #5451.
+        result = _dict_to_dataclass(
+            {"jaeger_remote_development": None}, SamplerConfig
+        )
+        self.assertIsNone(result.jaeger_remote_development)
+
     def test_missing_optional_fields_default_to_none(self):
         result = _dict_to_dataclass({}, _Outer)
         self.assertIsNone(result.middle)
@@ -108,3 +130,23 @@ def test_enum_value_already_enum_passes_through(self):
             {"filter": ExemplarFilter.trace_based}, _WithEnum
         )
         self.assertIs(result.filter, ExemplarFilter.trace_based)
+
+    def test_present_null_mapping_coerced_to_empty_dict(self):
+        # A key present with a null YAML value (e.g. ``option:``) on a
+        # dict-typed node means "select with empty config", so it must become
+        # an empty mapping rather than None. Regression test for #5451.
+        result = _dict_to_dataclass({"option": None}, _WithMapping)
+        self.assertEqual(result.option, {})
+
+    def test_explicit_empty_mapping_preserved(self):
+        result = _dict_to_dataclass({"option": {}}, _WithMapping)
+        self.assertEqual(result.option, {})
+
+    def test_absent_mapping_stays_none(self):
+        # An omitted optional section must remain unset, unlike a present null.
+        result = _dict_to_dataclass({"name": "x"}, _WithMapping)
+        self.assertIsNone(result.option)
+
+    def test_present_null_scalar_stays_none(self):
+        result = _dict_to_dataclass({"name": None}, _WithMapping)
+        self.assertIsNone(result.name)

opentelemetry-configuration/tests/test_meter_provider.py

@@ -9,6 +9,7 @@
 import unittest
 from unittest.mock import MagicMock, patch
 
+from opentelemetry.configuration._conversion import _dict_to_dataclass
 from opentelemetry.configuration._meter_provider import (
     configure_meter_provider,
     create_meter_provider,
@@ -186,6 +187,23 @@ def test_console_exporter(self):
         self.assertIsInstance(reader, PeriodicExportingMetricReader)
         self.assertIsInstance(reader._exporter, ConsoleMetricExporter)
 
+    def test_null_valued_console_exporter_from_parsed_config(self):
+        # Regression test for #5451: a dataclass-typed exporter written as
+        # ``console:`` (present, null) must behave like ``console: {}`` rather
+        # than requiring an explicit empty mapping.
+        config = _dict_to_dataclass(
+            {
+                "readers": [
+                    {"periodic": {"exporter": {"console": None}}},
+                ]
+            },
+            MeterProviderConfig,
+        )
+        provider = create_meter_provider(config)
+        reader = provider._metric_readers[0]
+        self.assertIsInstance(reader, PeriodicExportingMetricReader)
+        self.assertIsInstance(reader._exporter, ConsoleMetricExporter)
+
     def test_periodic_reader_default_interval(self):
         config = self._make_periodic_config(
             PushMetricExporterConfig(console=ConsoleMetricExporterConfig())

opentelemetry-configuration/tests/test_resource.py

@@ -7,6 +7,7 @@
 import unittest
 from unittest.mock import MagicMock, patch
 
+from opentelemetry.configuration._conversion import _dict_to_dataclass
 from opentelemetry.configuration._exceptions import ConfigurationError
 from opentelemetry.configuration._resource import create_resource
 from opentelemetry.configuration.models import (
@@ -376,6 +377,17 @@ def test_explicit_service_name_overrides_env_var(self):
             resource = create_resource(config)
         self.assertEqual(resource.attributes[SERVICE_NAME], "explicit-svc")
 
+    def test_null_valued_detector_from_parsed_config(self):
+        # Regression test for #5451: a detector written as ``- service:``
+        # (present, null) is parsed to ``{"service": None}``. It must run the
+        # detector just like ``- service: {}`` rather than being skipped.
+        config = _dict_to_dataclass(
+            {"detection_development": {"detectors": [{"service": None}]}},
+            ResourceConfig,
+        )
+        resource = create_resource(config)
+        self.assertIn(SERVICE_INSTANCE_ID, resource.attributes)
+
     def test_service_detector_not_run_when_absent(self):
         resource = create_resource(ResourceConfig())
         self.assertNotIn(SERVICE_INSTANCE_ID, resource.attributes)

opentelemetry-configuration/tests/test_tracer_provider.py

@@ -10,6 +10,7 @@
 from unittest.mock import MagicMock, patch
 
 from opentelemetry import trace as trace_api
+from opentelemetry.configuration._conversion import _dict_to_dataclass
 from opentelemetry.configuration._tracer_provider import (
     configure_tracer_provider,
     create_tracer_provider,
@@ -249,6 +250,39 @@ def test_no_sampler_raises_configuration_error(self):
         with self.assertRaises(ConfigurationError):
             self._make_provider(SamplerConfig())
 
+    def test_null_valued_always_on_from_parsed_config(self):
+        # Regression test for #5451: a leaf sampler written as ``always_on:``
+        # (present, null) is parsed to ``None`` by the YAML loader. It must be
+        # treated the same as ``always_on: {}`` rather than failing type
+        # dispatch.
+        sampler_config = _dict_to_dataclass({"always_on": None}, SamplerConfig)
+        provider = self._make_provider(sampler_config)
+        self.assertIs(provider.sampler, ALWAYS_ON)
+
+    def test_null_valued_parent_based_delegates_from_parsed_config(self):
+        # Regression test for #5451: the exact parent_based config from the
+        # issue, where every leaf delegate is written with a null value.
+        sampler_config = _dict_to_dataclass(
+            {
+                "parent_based": {
+                    "root": {"always_on": None},
+                    "remote_parent_sampled": {"always_on": None},
+                    "remote_parent_not_sampled": {"always_off": None},
+                    "local_parent_sampled": {"always_on": None},
+                    "local_parent_not_sampled": {"always_off": None},
+                }
+            },
+            SamplerConfig,
+        )
+        provider = self._make_provider(sampler_config)
+        sampler = provider.sampler
+        self.assertIsInstance(sampler, ParentBased)
+        self.assertIs(sampler._root, ALWAYS_ON)
+        self.assertIs(sampler._remote_parent_sampled, ALWAYS_ON)
+        self.assertIs(sampler._remote_parent_not_sampled, ALWAYS_OFF)
+        self.assertIs(sampler._local_parent_sampled, ALWAYS_ON)
+        self.assertIs(sampler._local_parent_not_sampled, ALWAYS_OFF)
+
     def test_user_defined_sampler_loaded_via_entry_point(self):
         mock_sampler = MagicMock(spec=Sampler)
         mock_class = MagicMock(return_value=mock_sampler)