Minor cleanup around tool invocation tool call args / results - #292
Minor cleanup around tool invocation tool call args / results#292DylanRussell wants to merge 5 commits into
Conversation
| else: | ||
| arguments = input_str | ||
|
|
||
| tool_invocation.arguments = ( |
There was a problem hiding this comment.
they are json dumped only for the time being until we can populate complex attributes on spans. I'd prefer to either keep deserialization in instrumentations or make utils deserialize them when they come as string (since it's common part for all instrumentations). If we do the latter, we should also do this in other instrumentation libs and eventually util-genai will switch to complex attrs AND will start deserailizing these at the same time.
There was a problem hiding this comment.
How does complex attributes help here ? The hard problem here is converting arguments (which can be essentially anything) into the narrow otel attributes type, complex attributes just expands the type a tiny bit
At the moment I see only 3 instrumentations with tool invocations:
^^ I don't really see what common code to pull out of this into the utils library.. I also still prefer pulling that serialization out of the utils library where it's kind of hidden.
So in general you prefer the tool arguments / results are not serialized and somehow better represented ?
There was a problem hiding this comment.
semconv tell to have them not serialized
It's expected to be an object - in case a serialized string is available to the instrumentation, the instrumentation SHOULD do the best effort to deserialize it to an object.
how I see it can work:
- instrumentations either
- deserialize them into
ToolCallArguments- we'll add a type even though it's just an alias to AnyValue - keep them as string since they've got string from model
- deserialize them into
- utils in the future once complex attributes on spans are supported:
- if it's string, try deserializing it into json object (AnyValue)
- if it's a
ToolCallArgumentsalready, just set it - ignore otherwise
- utils now:
- if it's a string, stamp as is
- serialize into string otherwise
There was a problem hiding this comment.
Okay i think we are mostly aligned then, just leave it up to instrumentations to translate arguments into AnyValue and accept that as the param ?
Maybe lets just revisit once complex attributes is available as that should be pretty soon..
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
This PR updates tool invocation telemetry to stop automatically JSON-serializing tool call arguments/results in util-genai, and shifts responsibility for serialization + content-capture gating into the instrumentations.
Changes:
- Change
ToolInvocationto store arguments/results asAnyValueand to omit them when content capture is disabled. - Update LangChain / Google GenAI / OpenAI Agents instrumentations to conditionally serialize tool args/results using
gen_ai_json_dumps. - Update changelogs and adjust related unit tests.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| util/opentelemetry-util-genai/tests/test_toolcall.py | Removes JSON-serialization assertion and tweaks the content-capture-disabled test inputs |
| util/opentelemetry-util-genai/src/opentelemetry/util/genai/_tool_invocation.py | Switches tool arg/result handling and attribute typing toward AnyValue without util-side serialization |
| util/opentelemetry-util-genai/.changelog/292.changed | Notes ToolInvocation now accepts AnyValue for args/results |
| instrumentation/opentelemetry-instrumentation-google-genai/.../tool_call_wrapper.py | Uses gen_ai_json_dumps for tool args/results when capturing content |
| instrumentation/opentelemetry-instrumentation-genai-openai-agents/.../processor.py | Gates tool result capture and serializes non-strings with gen_ai_json_dumps |
| instrumentation/opentelemetry-instrumentation-genai-langchain/.../callback_handler.py | Avoids parsing args when not capturing; serializes non-strings with gen_ai_json_dumps |
| instrumentation/opentelemetry-instrumentation-genai-langchain/.changelog/292.changed | Notes removal of redundant JSON encoding of tool arguments |
| self._start(self._get_base_attributes()) | ||
|
|
||
| def _get_base_attributes(self) -> dict[str, AttributeValue]: | ||
| def _get_base_attributes(self) -> dict[str, AnyValue]: |
| def _get_metric_attributes(self) -> dict[str, AnyValue]: | ||
| attrs: dict[str, AnyValue] = { |
| self.arguments | ||
| if self.should_capture_content_on_span | ||
| and self.arguments is not None | ||
| else None, |
| self.tool_result | ||
| if self.should_capture_content_on_span | ||
| and self.tool_result is not None | ||
| else None, | ||
| ), | ||
| ) | ||
| attributes: dict[str, AttributeValue] = { | ||
| attributes: dict[str, AnyValue] = { |
| self.should_capture_content_on_span = should_capture_content_on_spans() | ||
| self.name = name | ||
| self.tool_result: AnyValue | None = None | ||
| self.tool_result: AnyValue = None |
| # instrumentation library before assigning these attributes | ||
| # to the invocation. | ||
| self.arguments: AnyValue | None = None | ||
| self.arguments: AnyValue = None |
|
|
||
| tool_invocation.arguments = ( | ||
| arguments | ||
| if isinstance(arguments, str) | ||
| else gen_ai_json_dumps(arguments) |
| tool_invocation.tool_call_id = getattr(output, "tool_call_id", None) | ||
| tool_invocation.tool_result = getattr(output, "content", None) | ||
| raw_result = getattr(output, "content", output) | ||
| if self._telemetry_handler.should_capture_content(): | ||
| tool_invocation.tool_result = ( | ||
| raw_result | ||
| if isinstance(raw_result, str) | ||
| else gen_ai_json_dumps(raw_result) |
| if self._handler.should_capture_content() and output is not None: | ||
| invocation.tool_result = ( | ||
| output if isinstance(output, str) else str(output) | ||
| output | ||
| if isinstance(output, str) | ||
| else gen_ai_json_dumps(output) | ||
| ) |
| span_exporter, handler = _make_span_exporter_and_handler() | ||
| invocation = handler.tool("get_weather") | ||
| invocation.arguments = {"location": "Paris"} | ||
| invocation.arguments = "abasfasf" |
Pull request dashboard statusWaiting on the author · refreshed 2026-07-28 02:18 UTC Move out of draft to request review. Doesn't look right?
|
Description
Updates langchain instrumentation so it doesn't bother encoding arguments because ultimately they are json dumped into an attribute, also updated the code to ignore arguments when the content capture flag is not set..
I removed the gen ai json dumps of tool call args / results from the util library and instead updated it to take an AnyValue type, I think it's better for instrumentations to decide how they want results / arguments serialized and to do so themselves.
Most likely in the next otel python release AnyValue will soon be used everywhere and will include extended attributes so I used that for tool call args / results and attribute values, this PR will fail CI until that release happens..
Type of change
How has this been tested?
Unit tests
Checklist
See CONTRIBUTING.md
for the style guide, changelog guidance, and more.