fix(otel): share one AnyValue decoder across all OTLP paths - #187
Open
Leroyyyyyyyyy wants to merge 1 commit into
Open
fix(otel): share one AnyValue decoder across all OTLP paths#187Leroyyyyyyyyy wants to merge 1 commit into
Leroyyyyyyyyy wants to merge 1 commit into
Conversation
Three AnyValue decoders had drifted apart. extraction's
flatten_otlp_attributes silently dropped array/kvlist/bytes attributes,
and the OTLP JSON loader json.dumps()'d the raw proto wrapper instead of
decoding it. gen_ai.response.finish_reasons therefore surfaced as
'{"values": [{"stringValue": "stop"}]}' on one path and vanished on
another, where both should yield ["stop"].
Move the already-correct recursive decoder out of api/otlp_processing.py
into a dependency-free module and route all three call sites through it.
The new module imports only the standard library, so extraction,
loader.otlp and api.otlp_processing can all share it without creating an
import cycle.
bytesValue is returned unchanged: MessageToDict base64-encodes protobuf
bytes fields, so callers already receive a str today. Decoding it here
would change existing behaviour, which is out of scope for this fix.
Adds coverage for array/kvlist/bytes attributes, which previously had
none on either path.
Fixes agentevals-dev#173
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #173
Problem
Three places decoded the OTLP
AnyValueunion, and only one did it fully:arrayValue/kvlistValuebytesValueextraction.flatten_otlp_attributesloader.otlp.OtlpJsonLoader._extract_attributesjson.dumpsof the raw proto wrapperapi.otlp_processing._parse_otlp_any_valueFeeding one span attribute —
gen_ai.response.finish_reasonscarryingarrayValue: ["stop"]— through the receiver paths produced three different answers before this change:OtlpJsonLoader'{"values": [{"stringValue": "stop"}]}'["stop"]OtlpJsonLoader'{"values": [{"stringValue": "stop"}]}'["stop"]["stop"]["stop"]["stop"]Note that even after a
json.loads, the loader's value is still the proto wrapper — not the decoded list.Approach
Moved the already-correct recursive decoder into a new
agentevals/otlp_anyvalue.pyand pointed all three call sites at it. The gRPC receiver needs no change: it handsMessageToDictoutput to the sameprocess_traces.The new module imports only the standard library. That is deliberate:
extractionimportsloader.base, which eagerly initialises theloaderpackage (and thereforeloader.otlp), so havingloader.otlpimport fromextractionwould create a real import cycle. A leaf module has no edge back into the package and cannot participate in one.Two behaviours are intentionally preserved:
bytesValueis returned unchanged.MessageToDictbase64-encodes protobuf bytes fields and OTLP/JSON does the same, so call sites already receive astr. Decoding to real bytes would be a behaviour change beyond this fix.{}—is_any_value()keeps the prior semantics of both flatteners.The loader's dict-shaped attribute branch (
_flatten_nested_dict, for ClickHouse-style nested JSON) is untouched; only the OTLP array branch now shares the decoder.Testing
array/kvlist/bytesattributes had no test coverage on either path — which is how the mismatch survived. Added 7 tests acrosstests/test_extraction.pyandtests/test_otlp_loader.py, including the nestedarrayValue-of-kvlistValueshape used for tool calls.758 passed, 6 skipped (unit suite)
ruff check . / ruff format --check . clean