Skip to content

fix(llmobs): capture Anthropic tool definitions, calls, and results on Bedrock InvokeModel - #19944

Open
jessicagamio wants to merge 5 commits into
mainfrom
jessica.gamio/feat-FRMLO-226-bedrock-invokemodel-tool-calls
Open

fix(llmobs): capture Anthropic tool definitions, calls, and results on Bedrock InvokeModel#19944
jessicagamio wants to merge 5 commits into
mainfrom
jessica.gamio/feat-FRMLO-226-bedrock-invokemodel-tool-calls

Conversation

@jessicagamio

Copy link
Copy Markdown
Contributor

Description

The Bedrock InvokeModel integration loses tool information for Anthropic models in three places:

  • Output — only the first content block is read, and only its text field. A tool_use block produces an empty output, and blocks after the first are discarded (so text is lost when a response starts with tool_use or thinking).
  • Input — only text and image blocks are handled, with no fallback. tool_use, tool_result, and thinking blocks in conversation history are dropped, so agent-loop traces read as if the model answered without using a tool.
  • Tool definitions — read only from the Converse toolConfig field, so InvokeModel never reports them.

Converse/ConverseStream already handle all of this, so behavior differed by API for the same model.

The cause is duplication: the same Anthropic payload was parsed by three separate copies of the logic, and each new Anthropic block type (tool_use, thinking) only ever got added to the Anthropic SDK integration. This consolidates that parsing into _integrations/utils.py — one content-block extractor, one ToolCall/ToolResult builder pair, one tool-definition extractor — shared by the Anthropic and Bedrock integrations, then uses it on both the request and response paths of InvokeModel.

Commits are independently revertible in order, if reviewers want narrower scope:

  1. fix — output: tool calls and all content blocks
  2. refactor — share the output extractor with anthropic.py
  3. refactor — share tool block parsing between input and output
  4. fix — input: capture tool blocks
  5. fix — capture tool definitions

Testing

24 new unit tests in tests/llmobs/test_integrations_utils.py covering each block type, parallel tool calls, JSON-string tool arguments, replayed agent-loop history, and the non-Anthropic response shapes that share the same entry point.

Verified against live Bedrock with a two-turn agent loop on Claude Haiku 4.5 — tool definitions, tool call, and tool result all captured with matching tool_id across turns.

llmobs::anthropic 108 passed / 0 failed and contrib::botocore 185 passed / 0 failed.

Risks

The refactor moves logic out of a working integration (anthropic.py) to fix a different one. The existing Anthropic suite is unchanged and passing, which is the check on that.

Output messages for Anthropic InvokeModel now carry role: "assistant", matching the Anthropic SDK and Converse integrations. One assertion in test_bedrock_llmobs.py was updated for this.

Additional Notes

Commit 5 adds one line to contrib/internal/botocore/services/bedrock.py to pass tools through in the invoke request params — the LLMObs layer has no other access to the request body.

Not included: InvokeModelWithResponseStream drops tool arguments in a separate function, and max_tokens metadata still reads the legacy max_tokens_to_sample field.

…edrock InvokeModel

The Bedrock InvokeModel integration read only the first content block of an
Anthropic response, and only its `text` field. Claude's Messages API returns a
list of tagged blocks, so blocks after the first were discarded and a `tool_use`
block produced an empty string.

Adds `get_messages_from_anthropic_content()` to the shared integration utils and
uses it for the Anthropic branch of `_extract_output_message`. Tool calls are now
captured on InvokeModel spans, matching Converse, and responses that begin with a
`tool_use` or `thinking` block keep their assistant text.

Output messages for Anthropic InvokeModel now carry `role: "assistant"`, matching
the Anthropic and Converse integrations.
…e SDK integration

`anthropic.py` had its own copy of the content-block extraction logic, identical
in purpose to the one now in `_integrations/utils.py`. Two parsers for the same
payload is how the Bedrock InvokeModel path drifted: it was written for the
March 2024 single-text-block shape and never updated as Anthropic added
`tool_use` and `thinking` blocks, while `anthropic.py` was.

Delegates `AnthropicIntegration._extract_output_message` and
`_format_tool_result_content` to the shared helpers so there is one
implementation to maintain. No behavior change.

This commit is separable: dropping it leaves the Bedrock fix intact, at the cost
of keeping the duplicate parser in `anthropic.py`.
…d output paths

`ToolCall` and `ToolResult` construction from Anthropic content blocks was written
out separately in the input and output extraction paths, including the JSON-string
normalization of `tool_use.input`. Extracts `anthropic_tool_call_from_block()` and
`anthropic_tool_result_from_block()` so both paths build them the same way, and
removes the now-redundant `_format_tool_result_content` pass-through.

Also restores the stricter `list` guard on the shared content extractor. The
earlier `Iterable` check accepted dicts, which would have iterated keys instead of
returning no messages.

No behavior change.
…ut messages

`_extract_input_message` handled only `text` and `image` blocks, with no fallback,
so `tool_use`, `tool_result`, and `thinking` blocks in Anthropic input messages were
dropped entirely. Agent loops replay the assistant's tool call and the user's tool
result as conversation history, so the span input showed only the text turns and the
trace read as if the model had answered without using a tool.

Handles those block types using the shared Anthropic builders, matching the
Anthropic SDK integration.
…ropic models

Tool definitions were only read from the Converse `toolConfig` field, so spans for
`InvokeModel` calls never reported them. Anthropic models carry tool definitions in
the request body under `tools`, in Anthropic's own format rather than Converse's
`toolSpec` wrapper.

Passes `tools` through in the invoke request params and reuses the Anthropic
definition extractor, which moves from the Anthropic integration into the shared
integration utils so both callers build definitions the same way.
@jessicagamio
jessicagamio requested review from a team as code owners August 28, 2026 22:02
@jessicagamio
jessicagamio requested a review from Yun-Kim August 28, 2026 22:02
@datadog-datadog-prod-us1-2

datadog-datadog-prod-us1-2 Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Tests

🎉 All green!

🧪 All tests passed
❄️ No new flaky tests detected

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 0261b2f | Docs | View more details | Give us feedback!

@pr-commenter

pr-commenter Bot commented Aug 28, 2026

Copy link
Copy Markdown

Benchmarks

Benchmark execution time: 2026-08-28 22:29:58

Comparing candidate commit 0261b2f in PR branch jessica.gamio/feat-FRMLO-226-bedrock-invokemodel-tool-calls with baseline commit f4fe3db in branch main.

📊 Benchmarking dashboard

Found 0 performance improvements and 8 performance regressions! Performance is the same for 578 metrics, 10 unstable metrics, 1 known flaky benchmarks, 17 flaky benchmarks without significant changes.

Explanation

This is an A/B test comparing a candidate commit's performance against that of a baseline commit. Performance changes are noted in the tables below as:

  • 🟩 = significantly better candidate vs. baseline
  • 🟥 = significantly worse candidate vs. baseline

We compute a confidence interval (CI) over the relative difference of means between metrics from the candidate and baseline commits, considering the baseline as the reference.

If the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD), the change is considered significant.

Feel free to reach out to #apm-benchmarking-platform on Slack if you have any questions.

More details about the CI and significant changes

You can imagine this CI as a range of values that is likely to contain the true difference of means between the candidate and baseline commits.

CIs of the difference of means are often centered around 0%, because often changes are not that big:

---------------------------------(------|---^--------)-------------------------------->
                              -0.6%    0%  0.3%     +1.2%
                                 |          |        |
         lower bound of the CI --'          |        |
sample mean (center of the CI) -------------'        |
         upper bound of the CI ----------------------'

As described above, a change is considered significant if the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD).

For instance, for an execution time metric, this confidence interval indicates a significantly worse performance:

----------------------------------------|---------|---(---------^---------)---------->
                                       0%        1%  1.3%      2.2%      3.1%
                                                  |   |         |         |
       significant impact threshold --------------'   |         |         |
                      lower bound of CI --------------'         |         |
       sample mean (center of the CI) --------------------------'         |
                      upper bound of CI ----------------------------------'

scenario:httppropagationinject-ids_only

  • 🟥 execution_time [+2.504µs; +2.711µs] or [+15.170%; +16.425%]

scenario:iastaspects-add_aspect

  • 🟥 execution_time [+12.347µs; +14.865µs] or [+12.438%; +14.974%]

scenario:iastaspects-join_aspect

  • 🟥 execution_time [+52.101µs; +59.893µs] or [+24.421%; +28.073%]

scenario:iastaspects-title_aspect

  • 🟥 execution_time [+70.486µs; +74.479µs] or [+26.146%; +27.628%]

scenario:iastaspectsospath-ospathbasename_aspect

  • 🟥 execution_time [+142.647µs; +150.784µs] or [+36.769%; +38.866%]

scenario:iastaspectssplit-rsplit_aspect

  • 🟥 execution_time [+38.055µs; +43.429µs] or [+27.018%; +30.833%]

scenario:telemetryaddmetric-1-count-metric-1-times

  • 🟥 execution_time [+624.130ns; +669.907ns] or [+23.204%; +24.906%]

scenario:tracer-small

  • 🟥 execution_time [+29.320µs; +32.135µs] or [+9.015%; +9.880%]

Unstable benchmarks

These benchmarks have a confidence interval too wide to call a change; treat them as noise rather than signal.

scenario:coreapiscenario-context_with_data_listeners

  • unstable execution_time [-699.532ns; +764.518ns] or [-6.411%; +7.007%]

scenario:coreapiscenario-core_dispatch_1_listener

  • unstable execution_time [-33.707ns; +32.500ns] or [-5.491%; +5.294%]

scenario:coreapiscenario-core_dispatch_50_listeners

  • unstable execution_time [-1458.030ns; +1817.172ns] or [-8.659%; +10.792%]

scenario:coreapiscenario-core_dispatch_exception_listeners

  • unstable execution_time [-869.153ns; +1617.537ns] or [-6.787%; +12.631%]

scenario:coreapiscenario-core_dispatch_listeners

  • unstable execution_time [-267.036ns; +378.517ns] or [-7.376%; +10.456%]

scenario:coreapiscenario-core_dispatch_no_args_listeners

  • unstable execution_time [-190.887ns; +315.286ns] or [-6.639%; +10.966%]

scenario:coreapiscenario-core_dispatch_with_results_1_listener

  • unstable execution_time [-78.854ns; +65.982ns] or [-6.696%; +5.603%]

scenario:coreapiscenario-core_dispatch_with_results_50_listeners

  • unstable execution_time [-4200.905ns; +3806.384ns] or [-10.225%; +9.265%]

scenario:coreapiscenario-core_dispatch_with_results_listeners

  • unstable execution_time [-830.429ns; +750.980ns] or [-10.117%; +9.149%]

scenario:packagesupdateimporteddependencies-import_many_stdlib_cached

  • unstable execution_time [-57595.016ns; +59494.200ns] or [-9.284%; +9.590%]

Known flaky benchmarks

These benchmarks are marked as flaky and will not trigger a failure. Modify FLAKY_BENCHMARKS_REGEX to control which benchmarks are marked as flaky.

scenario:iastaspects-ljust_noaspect

  • 🟥 execution_time [+56.674µs; +60.383µs] or [+20.110%; +21.426%]

Known flaky benchmarks without significant changes:

  • scenario:errortrackingflasksqli-baseline
  • scenario:flasksimple-iast-get
  • scenario:iastaspects-casefold_aspect
  • scenario:iastaspects-casefold_noaspect
  • scenario:iastaspects-index_aspect
  • scenario:iastaspects-lower_aspect
  • scenario:iastaspects-replace_aspect
  • scenario:iastaspects-swapcase_aspect
  • scenario:iastaspects-title_noaspect
  • scenario:iastaspects-translate_aspect
  • scenario:iastaspects-translate_noaspect
  • scenario:iastaspects-upper_noaspect
  • scenario:packagespackageforrootmodulemapping-cache_off
  • scenario:packagespackageforrootmodulemapping-cache_on
  • scenario:sethttpmeta-all-enabled
  • scenario:span-start
  • scenario:telemetryaddmetric-record-100-metrics

@jessicagamio
jessicagamio requested a review from ncybul August 28, 2026 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant