Repository navigation
[opentelemetry-instrumentation-genai-anthropic] Record gen_ai.tool.definitions - #652
Conversation
def0cfd to
5408a52
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The current implementation can exhaust a one-shot tools iterator before the underlying create() call reads it, potentially changing request behavior.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates the opentelemetry-instrumentation-genai-anthropic package to record gen_ai.tool.definitions on Anthropic spans by extracting the tools request parameter and mapping it into util-genai ToolDefinition models, with coverage across sync/async and streaming/non-streaming code paths.
Changes:
- Extend request parameter extraction to retain
toolsand add aget_tool_definitions()mapper for Anthropic tool shapes. - Populate
InferenceInvocation.tool_definitions(gated by content-capture mode) so spans can includegen_ai.tool.definitions. - Add unit/integration tests (sync + async) asserting presence/absence of the attribute based on content capture, plus mapping tests for tool variants.
File summaries
| File | Description |
|---|---|
| instrumentation/opentelemetry-instrumentation-genai-anthropic/src/opentelemetry/instrumentation/genai/anthropic/messages_extractors.py | Adds tools to extracted params and introduces get_tool_definitions() mapping logic. |
| instrumentation/opentelemetry-instrumentation-genai-anthropic/src/opentelemetry/instrumentation/genai/anthropic/patch.py | Threads mapped tool definitions into the invocation when content capture is enabled. |
| instrumentation/opentelemetry-instrumentation-genai-anthropic/tests/test_messages_extractors.py | Adds focused unit tests for extract_params(...tools=...) and get_tool_definitions() mappings. |
| instrumentation/opentelemetry-instrumentation-genai-anthropic/tests/test_sync_messages.py | Adds sync streaming + create assertions for gen_ai.tool.definitions and its omission when capture is off. |
| instrumentation/opentelemetry-instrumentation-genai-anthropic/tests/test_async_messages.py | Adds async streaming + create assertions for gen_ai.tool.definitions and its omission when capture is off. |
| instrumentation/opentelemetry-instrumentation-genai-anthropic/.changelog/652.fixed | Adds a changelog fragment for recording gen_ai.tool.definitions. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Pull request dashboard statusMerged · refreshed 2026-09-14 01:46 UTC Status above doesn't look right?
|
| invocation.tool_definitions = ( | ||
| get_tool_definitions(params.tools) if capture_content else None | ||
| ) | ||
| invocation.attributes = attributes |
There was a problem hiding this comment.
The gen_ai.tool.definitions attribute is recommended (not opt-in). Per the semconv schema, only the large optional properties (description and parameters) should be excluded by default, while name and type are required.
Please populate invocation.tool_definitions unconditionally with the required properties (name, type), while keeping the content-capture guard around description and parameters in the instrumentation for performance reasons.
There was a problem hiding this comment.
It seems like gen_ai.tool.definitions is still opt-in at this moment (semconv), I'm wondering if we're moving this to recommended in the near future.
I think one inconsistency here is that gen_ai.tool.definitions is opt-in but description and parameters have a nested level of opt-in based on that. But ContentCapturingMode isn't granular enough to distinguish this opt-in and opt-in's opt-in.
Maybe in this PR, I can proceed with only emitting name and type when content capture is off, then open an issue to discuss more granularity for ContentCapturingMode? Or we can put this PR on hold until we have a solution
There was a problem hiding this comment.
Sorry for the confusion, you're absolutely right. I created an issue to follow up in the conventions - open-telemetry/semantic-conventions-genai#512
I think your PR is correct as is.
There was a problem hiding this comment.
🟢 Approval recommended
The change is narrowly scoped, preserves SDK behavior (including one-shot iterables), and is backed by targeted sync/async tests covering the new telemetry attribute.
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 0 new
- Review effort level: Lite
lmolkova
left a comment
lmolkova
left a comment
There was a problem hiding this comment.
LGTM, please resolve conflicts and we should be good to go.
Thanks!
…definitions A tool-calling request produced a span with no trace of the tools it offered the model. extract_params reads the request arguments one by one and had no slot for tools, so the list never reached the code that records telemetry. Add the slot and map Anthropic's tool shapes onto the semconv models. A custom tool carries input_schema and becomes a FunctionToolDefinition. Server tools and toolsets become a GenericToolDefinition keyed by their versioned type; a toolset carries no name, so its type stands in for the name the schema requires. Recording follows OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT, matching genai-openai. The attribute is opt_in in spans.yaml and sits in the attributes.gen_ai.content group. Assisted-by: Claude Opus 5
…ools iterator tools is typed as an iterable, so a caller may pass a generator. Reading tool definitions drained it before the SDK serialized the request, and the request went out with an empty tool list. Materialize it in the wrappers, ahead of both readers. The SDK materializes the value itself when it builds the request body, so the request is unchanged. Placement matters and differs between the two entry points: create reads the request after the wrapper runs, but stream serializes it while building the manager, before the invocation is created. Doing this while building the invocation would leave stream recording no tools. Assisted-by: Claude Opus 5
c04e6b4 to
dfd6b8e
Compare
Thanks! I've resolved the conflict |
github-project-automation
Bot
moved this to Done
in GenAI Semantic Conventions and Instrumentation libraries
Description
The Anthropic instrumentation never recorded
gen_ai.tool.definitions, so atool-calling request produced a span with no trace of the tools it offered the
model.
extract_paramsreads the request arguments one by one and had no slotfor
tools, so the list never reached the code that records telemetry.A custom tool carries
input_schemaand maps to aFunctionToolDefinition.Server tools (
web_search_20250305,bash_20250124) and toolsets(
computer_toolset_20260801) map to aGenericToolDefinitionkeyed by theirversioned
type; a toolset carries noname, so itstypestands in for thenamethe schema requires.The instrumentation records the attribute only when
OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENTis set, matchinggenai-openai.spans.yamlmarks the attributeopt_inand places it in theattributes.gen_ai.contentgroup, so following the content setting is thenarrower reading.
Known gaps:
genai-langchain,genai-agno,genai-portkeyandgenai-smolagentssettool_definitionsregardless of that setting, so onecall yields a different attribute depending on the provider. Separately,
get_content_attributesin util-genai serializesdescriptionandparameterson its capture-disabled path, against the registry note thatnon-required properties stay out by default. Both belong in util-genai and are
tracked in #655; once that lands, this gate can move or drop. Tools reached
through
mcp_serversstay uncovered, since they never appear in thetoolsarray.
Type of change
How has this been tested?
Unit tests cover
get_tool_definitionsacross the custom, server-tool andunnamed-toolset shapes, tools that expose attributes instead of dict keys, and
the empty and
Nonecases. Sync and async tests assert the serializedattribute on
createandstreamspans with content capture on, and itsabsence with capture off alongside the input and output messages. Both pairs
replay existing cassettes, since the attribute comes from the request and the
recorded response does not affect it.
anthropicoldest leg (anthropic==0.51.0): 184 passed, 7 skippedanthropiclatest leg (anthropic==1.4.0): 191 passedanthropicconformance: 5 scenarios pass weaver live-checktox -e precommit(ruff, ruff-format, rstcheck) andpyrightcleanChecklist