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

[bourbonkk]

Description

Fixes #5451

A YAML mapping key present with an empty (null) value (e.g. always_on:)
parses to None, which the config conversion layer could not distinguish
from an absent key. Since sampler and resource-detector type dispatch selects
a type via is not None, a config like:

sampler:
  always_on:

or the parent_based example from the issue failed with
ConfigurationError: Unsupported sampler type in config: Sampler(always_on=None, ...),
while always_on: {} worked. The same root cause silently skipped detectors
written as - service: (vs - service: {}), as noted by @emdneto.

Per the declarative-config spec, a key present with a null value on a
dict[str, Any]-typed node means "select this with an empty config", so
_convert_value now coerces such a None to an empty mapping. Scalar and
dataclass fields keep None, so an absent optional section stays unset
(existing test_none_value_preserved behavior is unchanged).

Type of change

• Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

uv run tox -e py312-test-opentelemetry-configuration (372 passed).

Added regression tests, verified they fail without the fix (with the exact
error from the issue) and pass with it:

• test_conversion.py: present-null mapping → {}, explicit {} preserved,
  absent key stays None, present-null scalar stays None.
• test_tracer_provider.py: {"always_on": None} and the full null-valued
  parent_based delegate config from the issue, driven through
  _dict_to_dataclass → create_tracer_provider.
• test_resource.py: null-valued detector entry ({"service": None}) runs
  the detector.

Does This PR Require a Contrib Repo Change?

• Yes. - Link to PR:
• No.

Checklist:

• Followed the style guidelines of this project
• Changelogs have been updated
• Unit tests have been added
• Documentation has been updated

🤖 Generated with Claude Code

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: bourbonkk / name: allen (bd1aa15)

[bourbonkk]

/easycla

[xrmx]

Works fine, thanks! I think we can improve the tests though

[emdneto]

The issue is not only for sampler, detectors, but for every dataclass configuration that can be instantiated with an empty configuration. Let's say: ConsoleMetricExporter. I think we should also cover the dataclasses besides dict. e.g.,

meter_provider:
  readers:
    - periodic:
        exporter:
          console:

I believe this should work without requiring us to pass console: {}.

[xrmx]

The issue is not only for sampler, detectors, but for every dataclass configuration that can be instantiated with an empty configuration. Let's say: ConsoleMetricExporter. I think we should also cover the dataclasses besides dict. e.g.,

meter_provider:
  readers:
    - periodic:
        exporter:
          console:

I believe this should work without requiring us to pass console: {}.

Great catch, I assumed that everything was becoming a dict. Something like the following may do but I really haven't grokked the code:

+    if value is None:
+        if origin is dict:
+            return {}
+        if origin is None and is_dataclass(unwrapped):
+            return unwrapped
+        return None

[bourbonkk]

Good point, thanks — you're right that it's broader than dict-aliased nodes. I've extended the coercion so a present null is also mapped to a defaulted instance when the annotated type is a dataclass that can be built with no arguments, which covers the console: exporter case:

def _coerce_present_null(unwrapped, origin):
    if origin is dict:
        return {}
    if (
        origin is None
        and 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)
        )
    ):
        return _dict_to_dataclass({}, unwrapped)
    return None

I kept the "all fields optional" guard so a dataclass with required fields (e.g. one that would need TypeError-raising positional args) is left as None instead of crashing — an absent optional section stays unset. Added regression tests for the null-valued metric console: exporter and for the required-field guard. PTAL.

[bourbonkk]

Adopted the structure — substitute {} and let the existing dict/dataclass conversion below build it, rather than a separate helper (c8da534).

I kept one guard on top of it: only coerce when the dataclass is constructible with no arguments. 21 config dataclasses have a required field — e.g. BatchSpanProcessor.exporter, PeriodicMetricReader.exporter, SimpleSpanProcessor.exporter, View.selector. For those, value = {} → _dict_to_dataclass({}, cls) → cls() raises TypeError on the missing positional arg. So e.g.:

processors:
  - batch:

would crash instead of producing the existing clean ConfigurationError: Unsupported ... type. Leaving required-field dataclasses as None keeps them on that error path, while console: and friends (all-optional) still get coerced. So it's:

if origin is dict or _is_empty_constructible_dataclass(unwrapped):
    value = {}
else:
    return None

Happy to drop the guard if you'd prefer required-field nodes to error too, but a raw TypeError seemed worse than the current ConfigurationError. PTAL.

[emdneto]

Adopted the structure — substitute {} and let the existing dict/dataclass conversion below build it, rather than a separate helper (c8da534).

I kept one guard on top of it: only coerce when the dataclass is constructible with no arguments. 21 config dataclasses have a required field — e.g. BatchSpanProcessor.exporter, PeriodicMetricReader.exporter, SimpleSpanProcessor.exporter, View.selector. For those, value = {} → _dict_to_dataclass({}, cls) → cls() raises TypeError on the missing positional arg. So e.g.:

processors:
  - batch:

would crash instead of producing the existing clean ConfigurationError: Unsupported ... type. Leaving required-field dataclasses as None keeps them on that error path, while console: and friends (all-optional) still get coerced. So it's:

if origin is dict or _is_empty_constructible_dataclass(unwrapped):
    value = {}
else:
    return None

Happy to drop the guard if you'd prefer required-field nodes to error too, but a raw TypeError seemed worse than the current ConfigurationError. PTAL.

I don't get your point here.

If I leave the batch configuration empty, it will trigger a ConfigurationError as expected.
e.g.,

opentelemetry.configuration._exceptions.ConfigurationError: Configuration does not match schema: None is not of type 'object' (at logger_provider -> processors -> 1 -> batch)

[bourbonkk]

@emdneto
You're right about batch/periodic — my examples were wrong. Those are type: object (non-nullable) in the schema, so - batch: is rejected at schema validation (None is not of type 'object') and never reaches conversion. So the guard is irrelevant for them.

I cross-checked all 21 required-field config dataclasses against the schema, and there's exactly one that is schema-nullable (type: [object, null]) and has required fields: ExperimentalJaegerRemoteSampler (endpoint, initial_sampler). So:

tracer_provider:
  sampler:
    jaeger_remote_development:

passes schema validation (it IS nullable), reaches conversion, and without the guard _dict_to_dataclass({}, ExperimentalJaegerRemoteSampler) → cls() raises:

TypeError: ExperimentalJaegerRemoteSampler.__init__() missing 2 required positional arguments: 'endpoint' and 'initial_sampler'

The guard leaves it None instead. I've added an end-to-end loader test (test_null_valued_required_field_node_survives_conversion) that loads exactly this config to prove the schema accepts the null and conversion survives, and switched the conversion unit test to use this real node instead of a synthetic one.

If you'd rather not carry the guard for a single experimental sampler, I'm happy to drop it and instead flag the schema inconsistency upstream (a nullable node whose object form has required properties). Let me know which you prefer.