BoundedAttributes: _clean_extended_attribute returns None for both invalid input and valid AnyValue(None), causing invalid attributes to be written instead of dropped

[tsushanth]

Bug Description

_clean_extended_attribute() in opentelemetry-api/src/opentelemetry/attributes/__init__.py uses None as a dual-purpose return value:

1. Sentinel for invalid input — returned when the key is empty/non-string, or the value cannot be serialized as an AnyValue.
2. Valid attribute value — AnyValue(None) is a legitimate attribute value per the OpenTelemetry spec, and _clean_extended_attribute_value can legitimately return None when the input is None.

The non-extended path (_clean_attribute) correctly guards against this with if value is None: return immediately after the call. The extended path (extended_attributes=True) has no such guard in either __setitem__ or _set_items. As a result:

• An invalid attribute (e.g. empty-string key "") is written into the backing dict with value None under the invalid key, instead of being silently dropped.
• self.dropped is never incremented for the rejected attribute.

Affected Code

opentelemetry-api/src/opentelemetry/attributes/__init__.py

_clean_extended_attribute (lines ~212–229) returns None for invalid input:

if not (key and isinstance(key, str)):
    _logger.warning("invalid key `%s`. must be non-empty string.", key)
    return None

__setitem__ (lines ~284–291) — missing guard for the extended path:

if self._extended_attributes:
    value = _clean_extended_attribute(key, value, self.max_value_len)
    # ← no `if value is None: return` here
else:
    value = _clean_attribute(key, value, self.max_value_len)
    if value is None:      # ← guard exists only for non-extended path
        return

Same omission in _set_items (lines ~302–307).

Reproduction

from opentelemetry.attributes import BoundedAttributes

ba = BoundedAttributes(maxlen=10, extended_attributes=True, immutable=False)
ba[""] = "hello"

# Expect: empty-string key rejected, dropped counter incremented
assert "" not in ba, f"Invalid key '' was written into dict: {dict(ba._dict)}"
assert ba.dropped == 1, f"dropped counter not incremented: {ba.dropped}"

Actual: "" is present in ba._dict with value None, and ba.dropped == 0.

Root Cause

None is an ambiguous return value: it cannot distinguish "this input was invalid, drop it" from "this is a valid AnyValue whose value happens to be None". The non-extended path sidesteps this accidentally because _clean_attribute never accepts None as a valid attribute value, so the if value is None guard works there. The extended path does not have the same assumption.

Suggested Fix

Introduce a private sentinel:

_INVALID_ATTRIBUTE = object()

Return _INVALID_ATTRIBUTE (instead of None) from _clean_extended_attribute on invalid key or unserializable value. Then guard both __setitem__ and _set_items:

if self._extended_attributes:
    value = _clean_extended_attribute(key, value, self.max_value_len)
    if value is _INVALID_ATTRIBUTE:
        with self._lock:
            self.dropped += 1
        return

And in _set_items:

cv = _clean_extended_attribute(key, value, self.max_value_len)
if cv is _INVALID_ATTRIBUTE:
    continue

This keeps None available as a valid AnyValue while giving the caller an unambiguous signal for rejection.

Component

opentelemetry-api — opentelemetry/attributes/__init__.py

[bhumikadangayach]

I'd like to take this on. The repro and suggested fix are clear — I'll introduce the _INVALID_ATTRIBUTE sentinel as proposed, guard both setitem and _set_items in the extended-attributes path, and add a regression test covering the empty-key case (plus the AnyValue(None) case to confirm it's still correctly preserved as a valid value). Will open a PR once it's tested and green.

[herin049]

This should be addressed once #5266 is merged.

[bhumikadangayach]

Thanks for the pointer, @herin049 — looked at #5266 and agree, this looks like it gets resolved as a natural consequence of the BoundedAttributes refactor there rather than needing a standalone fix. I'll hold off on implementing the sentinel approach to avoid duplicating/conflicting with that work. Happy to help verify once #5266 lands that this specific case (empty-key rejection + dropped counter) is covered, if that'd be useful.