fix(otel): share one AnyValue decoder across all OTLP paths - #187
fix(otel): share one AnyValue decoder across all OTLP paths#187Leroyyyyyyyyy 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
e007c14 to
c65f82d
Compare
|
Rebased onto latest main. CI hasn't run on this PR — looks like it's waiting on the first-time-contributor workflow approval. Could a maintainer kick it off? Happy to address anything it turns up. |
krisztianfekete
left a comment
krisztianfekete
left a comment
There was a problem hiding this comment.
Thanks, added two comments, can you please take a look at them?
| if key in self._GENAI_EVENT_KEYS and key not in attributes: | ||
| value_obj = attr.get("value", {}) | ||
| if "stringValue" in value_obj: | ||
| attributes[key] = value_obj["stringValue"] |
There was a problem hiding this comment.
Here we still hand roll stringValue only handling. Strands puts gen_ai.input.messages in span events, and newer GenAI semconv makes messages a complex array, so that promotion still silently drops anything that isn't a plain string. Can you please fix this as well?
|
|
||
| def _extract_agentevals_metadata(resource_attrs: list[dict]) -> dict: | ||
| """Extract agentevals-specific metadata from OTLP resource attributes.""" | ||
| flat = flatten_otlp_attributes(resource_attrs) |
There was a problem hiding this comment.
flatten_otlp_attributes used to guarantee scalar or absent. After this change can return a list or dict, and _extract_agentevals_metadatafeeds agentevals.session_name straight into self._active_session_for_name.get(session_name) in ws_server.py and otlp_processing.py for logs.
Can we read session_name and eval_set_id with a string only accessor, the way _extract_conversation_id already does in otlp_processing.py?
2 participants
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