Expand attribute value type to support complex values everywhere and cleanup surrounding code - #5266
Expand attribute value type to support complex values everywhere and cleanup surrounding code#5266DylanRussell wants to merge 78 commits into
attribute value type to support complex values everywhere and cleanup surrounding code#5266Conversation
There was a problem hiding this comment.
Pull request overview
This PR expands OpenTelemetry Python’s attribute value model across the API, SDK, and OTLP exporters to support “complex” values everywhere (e.g., None, heterogeneous arrays, and maps), aligning behavior with the referenced OTEP and updating surrounding utilities/tests accordingly.
Changes:
- Broadens
AnyValue/AttributeValue/Attributestyping and updates public API surfaces (tracing/logs/events/metrics) to accept complex attribute values. - Refactors attribute cleaning/storage (
BoundedAttributes) and metric attribute-keying (_hash_attributes) to handle complex values consistently. - Updates OTLP proto/json encoding to represent
Noneas an emptyAnyValueand adjusts unit tests/benchmarks for the new semantics.
Reviewed changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| opentelemetry-sdk/tests/trace/test_trace.py | Updates span attribute/event tests for complex values and bytes behavior. |
| opentelemetry-sdk/tests/resources/test_resources.py | Updates resource attribute validation tests for new cleaning/stringify behavior. |
| opentelemetry-sdk/tests/metrics/test_view_instrument_match.py | Updates metrics tests to use _hash_attributes aggregation keys. |
| opentelemetry-sdk/tests/metrics/test_measurement_consumer.py | Adjusts a concurrency test to pass attributes explicitly. |
| opentelemetry-sdk/src/opentelemetry/sdk/util/instrumentation.py | Switches instrumentation scope attributes typing to Attributes. |
| opentelemetry-sdk/src/opentelemetry/sdk/resources/init.py | Aligns resource attribute typing/imports with new Attributes definition. |
| opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/_view_instrument_match.py | Introduces _hash_attributes and deep-copies measurement attributes prior to aggregation. |
| opentelemetry-sdk/src/opentelemetry/sdk/environment_variables/init.py | Clarifies docs for attribute value length limits with string/bytes focus. |
| opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/_exceptions.py | Updates exception-attribute merging to the unified Attributes type. |
| opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/init.py | Updates logging translation checks/warnings for AnyValue casting. |
| opentelemetry-sdk/src/opentelemetry/sdk/_events/init.py | Updates event logger provider attribute typing to Attributes. |
| opentelemetry-sdk/benchmarks/metrics/test_benchmark_metrics_histogram.py | Expands benchmarks to cover complex/mapping/array attribute shapes. |
| opentelemetry-api/tests/logs/test_proxy.py | Updates logs proxy tests to Attributes. |
| opentelemetry-api/tests/events/test_proxy_event.py | Updates events proxy tests to Attributes. |
| opentelemetry-api/tests/attributes/test_attributes.py | Reworks attribute tests around new _clean_attribute_value/BoundedAttributes. |
| opentelemetry-api/src/opentelemetry/util/types.py | Redefines AnyValue/AttributeValue and widens Attributes accordingly; removes _ExtendedAttributes. |
| opentelemetry-api/src/opentelemetry/trace/span.py | Updates span API type hints to use collections.abc.Mapping and types.Attributes in NoOp implementation. |
| opentelemetry-api/src/opentelemetry/attributes/init.py | Major refactor: new recursive cleaner and BoundedAttributes as a dict subclass. |
| opentelemetry-api/src/opentelemetry/_logs/_internal/init.py | Updates log API typing from _ExtendedAttributes to Attributes. |
| opentelemetry-api/src/opentelemetry/_events/init.py | Updates event API typing from _ExtendedAttributes to Attributes. |
| exporter/opentelemetry-exporter-otlp-proto-grpc/benchmarks/test_benchmark_trace_exporter.py | Updates patch target to TraceServiceStub. |
| exporter/opentelemetry-exporter-otlp-proto-common/tests/test_log_encoder.py | Refactors OTLP proto log encoder tests around new null/AnyValue behavior. |
| exporter/opentelemetry-exporter-otlp-proto-common/tests/test_attribute_encoder.py | Adjusts encoder failure tests to use an unencodable object. |
| exporter/opentelemetry-exporter-otlp-proto-common/src/opentelemetry/exporter/otlp/proto/common/_internal/_log_encoder/init.py | Stops using allow_null; encodes null as empty AnyValue. |
| exporter/opentelemetry-exporter-otlp-proto-common/src/opentelemetry/exporter/otlp/proto/common/_internal/init.py | Simplifies _encode_value and _encode_attributes around null-as-empty-AnyValue. |
| exporter/opentelemetry-exporter-otlp-json-common/tests/test_log_encoder.py | Updates JSON log encoder tests for null-as-empty-AnyValue body. |
| exporter/opentelemetry-exporter-otlp-json-common/tests/test_common_encoder.py | Updates JSON common encoder tests for null and unencodable values. |
| exporter/opentelemetry-exporter-otlp-json-common/src/opentelemetry/exporter/otlp/json/common/_internal/_log_encoder/init.py | Removes allow_null usage in JSON log encoding. |
| exporter/opentelemetry-exporter-otlp-json-common/src/opentelemetry/exporter/otlp/json/common/_internal/init.py | Simplifies _encode_value and _encode_attributes around null-as-empty-AnyValue. |
| docs/conf.py | Updates Sphinx type-hint resolution aliasing; now also touches metrics internals. |
| .changelog/5266.added | Adds changelog entry for extended/complex attribute support across packages. |
Comments suppressed due to low confidence (1)
opentelemetry-api/src/opentelemetry/trace/span.py:96
- The note in
Span.set_attributessays the behavior ofNonevalue attributes is undefined, but this PR explicitly adds support forNone/null attribute values across the API/SDK. Please update the docstring to reflect the new supported semantics (even ifNoneremains discouraged).
def set_attributes(
self, attributes: Mapping[str, types.AttributeValue]
) -> None:
"""Sets Attributes.
Sets Attributes with the key and value passed as arguments dict.
Note: The behavior of `None` value attributes is undefined, and hence
strongly discouraged. It is also preferred to set attributes at span
creation, instead of calling this method later since samplers can only
consider information already present during span creation.
"""
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| val, max_string_value_length | ||
| ) | ||
| return MappingProxyType(cleaned_mapping) | ||
| _logger.warning( |
There was a problem hiding this comment.
Can you add an exhaustiveness check?
We used this pattern to avoid raising an AssertionError at runtime
There was a problem hiding this comment.
It is valid for the code to reach the return None at the bottom ( if the following statement returns false: if ( type(value).__str__ is not object.__str__ or type(value).__repr__ is not object.__repr__ )
If we just revert that back to str we also wouldn't need the exhaustivity check
There was a problem hiding this comment.
But that is something that can happen right ? And in that case do we want to raise an error ?
The spec states that if we can't convert to string, we should fallback to an empty any value
If the source data cannot be serialized neither to a string nor to a byte sequence then it SHOULD by converted to an empty AnyValue.
None is returned because that ultimately is converted to an empty AnyValue..
I can add a comment explaining this.. but WDYT of that reasoning ?
|
Approved public API, looks as expected opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py:89: Attributes: Attribute value was changed: Mapping[str, LabelValue] -> Mapping[str, LabelValue] | None
opentelemetry-api/src/opentelemetry/util/types.py:9: AttributeValue: Attribute value was changed: str | bool | int | float | Sequence[str] | Sequence[bool] | Sequence[int] | Sequence[float] -> str | bool | int | float | bytes | Sequence['AnyValue'] | Mapping[str, 'AnyValue'] | None
opentelemetry-api/src/opentelemetry/util/types.py:19: Attributes: Attribute value was changed: Mapping[str, AttributeValue] | None -> Mapping[str, AnyValue] | None
public-symbols-check: exit 1 (1.87 seconds) /home/runner/work/opentelemetry-python/opentelemetry-python> python /home/runner/work/opentelemetry-python/opentelemetry-python/scripts/griffe_check.py pid=2356 |

Description
This PR updates the code to support complex values everywhere as described in this OTEP -- basically it adds support for
Nonetype,Bytestype, heterogeneous arrays, and maps as attribute values.I cleaned up the surrounding code, including refactoring
BoundedAttributes.. this was in part possible because we no longer have to support 2 sets of attribute values.BoundedAttributesis a dictionary sublcass used in the SDK for setting/storing/getting attributes. It's used for span attributes (and span event attributes and span link attributes), log attributes, instrumentation scope attributes, and resource attributes, but i think it's completely optional formetricpoints -- all the various metricpointclasses just accept a genericattributesvalue, and I think it's up to instrumentations to decide on whether to use it or not.. I don't see any guidance requiring people do it, and I don't see any usages ofBoundedAttributesin contrib.For the
bytestype I just pass that through as is.. Currently we do that for Extended attributes but not regular attributes (which I'm removing), for regular attributes it was decided to a utf-8 string.. Butbytesis a validAnyValuethat can be encoded asbytesin the OTLP proto, and we don't know how it was encoded, so I think passing it through makes the most sense..Some minor follow up work is tracked in #5304
I believe the failing Public Symbols check is acceptable, it is due to:
Attributestype (additive change, should be acceptable.)AttributesAsKeytype (unused in this repo and contrib, doesn't make sense as a type..).pyifile in the SDK (opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.pyi).Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Unit tests
Does This PR Require a Contrib Repo Change?
Yes I had to submit open-telemetry/opentelemetry-python-contrib#4646 -- if someone attempts to use an old (anything less than the next release of)
opentelemetry-instrumentation-loggingwith the next release ofopentelemetry-apiit will not work.. I'm confused about what to do to resolve that issue though..Checklist: