fix(exporter/otlp): support non-standard types as attribute values and log body#5239
fix(exporter/otlp): support non-standard types as attribute values and log body#5239grvmishra788 wants to merge 8 commits into
Conversation
…c-methods - Use str(Path(...)) as expected value so tests pass on Windows (backslash separator) - Add pylint disable for too-many-public-methods on TestCommonEncoder (21 methods)
| ] | ||
| ) | ||
| ) | ||
| # Third-party instrumentation can inject arbitrary types that cannot be exhaustively |
There was a problem hiding this comment.
instrumentation should not do this, and ideally has type checking or something to ensure that it isn't doing this.. I would log an error here too
There was a problem hiding this comment.
Added _logger.error() before the str() fallback in both json-common and proto-common
There was a problem hiding this comment.
Personally, I don't think this added behavior should be something that lives in the exporter at all. The OTel Python internals should always be operating with only the types outlined in the spec. If we do decide we want to support automatic coercion of attribute types, it should be performed at a much higher level (e.g. at .set_attribute(...)). There is another discussion entirely about whether we do want to support this behavior.
3 participants
Description
Passing a
pathlib.Path(or any other type outside the OTLPAnyValuespec) as a span/log attribute value or log body caused the OTLP exporter to crash:Both the proto-common and json-common OTLP encoders (
_encode_value()) had no fallback for unrecognised types. This PR adds astr()best-effort fallback before raising, so non-standard types are encoded as their string representation instead of crashing the exporter. Ifstr()itself raises, the original exception propagates unchanged.The fix mirrors the existing
_clean_extended_attribute_value()behaviour in the SDK and is applied consistently across both encoder implementations.Fixes #5210.
Type of change
How Has This Been Tested?
None-handling tests (allow_null=True/False) and list-with-Noneerror paths all pass unchanged.Does This PR Require a Contrib Repo Change?
Checklist: