Skip to content

Conversation

@thpierce
Copy link
Contributor

@thpierce thpierce commented Oct 16, 2025

Description

_decode_tool_use was only used when _tool_json_input_buf was found, but we were decoding the entire _content_block after adding _tool_json_input_buf to it. The _content_block overall which could contain non-JSON elements (e.g. {}), causing failures. To fix this, we have removed _decode_tool_use helper function and inlined JSON decoding logic directly into content_block_stop handler in _process_anthropic_claude_chunk, where we only use it to decode _tool_json_input_buf before appending to _content_block.

Fixes #3874

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Added unit tests. Before my change, inputs 1 & 2 failed with TypeError: the JSON object must be str, bytes or bytearray, not dict
  • Tested locally, see test details here: opentelemetry-instrumentation-botocore fails to parse tool use input #3874 - modified script to do pip install -e opentelemetry-python-contrib/instrumentation/opentelemetry-instrumentation-botocore and the issue disappeared/expected behaviour was achieved.

Does This PR Require a Core Repo Change?

  • Yes. - Link to PR:
  • No.

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated

Copy link
Contributor

@xrmx xrmx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the proper fix should look like something like #3874 (comment)

_decode_tool_use was only used when _tool_json_input_buf was found, but we were decoding the entire _content_block after adding _tool_json_input_buf to it. The _content_block overall which could contain non-JSON elements (e.g. {}), causing failures. To fix this, we have removed _decode_tool_use helper function and inlined JSON decoding logic directly into content_block_stop handler in _process_anthropic_claude_chunk, where we only use it to decode _tool_json_input_buf before appending to _content_block.
@thpierce
Copy link
Contributor Author

I think the proper fix should look like something like #3874 (comment)

Applied proposed fix and tested - seems to work!

@thpierce thpierce changed the title Handle dict input in _decode_tool_use for Bedrock streaming Only decode JSON input buffer in Anthropic Claude streaming Oct 17, 2025
thpierce added a commit to aws-observability/aws-otel-python-instrumentation that referenced this pull request Oct 17, 2025
_decode_tool_use was only used when _tool_json_input_buf was found, but we were decoding the entire _content_block after adding _tool_json_input_buf to it. The _content_block overall which could contain non-JSON elements (e.g. {}), causing failures. To fix this, we have removed _decode_tool_use helper function and inlined JSON decoding logic directly into content_block_stop handler in _process_anthropic_claude_chunk, where we only use it to decode _tool_json_input_buf before appending to _content_block.

Patch based on open-telemetry/opentelemetry-python-contrib#3875 with code copied directly from https://github.com/open-telemetry/opentelemetry-python-contrib/blob/v0.54b1/instrumentation/opentelemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/extensions/bedrock_utils.py#L289
thpierce added a commit to aws-observability/aws-otel-python-instrumentation that referenced this pull request Oct 17, 2025
_decode_tool_use was only used when _tool_json_input_buf was found, but we were decoding the entire _content_block after adding _tool_json_input_buf to it. The _content_block overall which could contain non-JSON elements (e.g. {}), causing failures. To fix this, we have removed _decode_tool_use helper function and inlined JSON decoding logic directly into content_block_stop handler in _process_anthropic_claude_chunk, where we only use it to decode _tool_json_input_buf before appending to _content_block.

Patch based on open-telemetry/opentelemetry-python-contrib#3875 with code copied directly from https://github.com/open-telemetry/opentelemetry-python-contrib/blob/v0.54b1/instrumentation/opentelemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/extensions/bedrock_utils.py#L289
Copy link
Contributor

@xrmx xrmx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@xrmx xrmx moved this to Ready for review in @xrmx's Python PR digest Oct 20, 2025
@xrmx
Copy link
Contributor

xrmx commented Oct 20, 2025

@thpierce please fix tox -e lint

Fix lint: `opentelemetry-instrumentation-botocore/tests/test_botocore_bedrock.py:2990:4: C0415: Import outside toplevel (opentelemetry.instrumentation.botocore.extensions.bedrock_utils.InvokeModelWithResponseStreamWrapper) (import-outside-toplevel)`
@thpierce
Copy link
Contributor Author

@xrmx - fixed. Ran tox -e lint:

...
  lint: OK (9.05 seconds)
  congratulations :) (9.25 seconds)

thpierce added a commit to aws-observability/aws-otel-python-instrumentation that referenced this pull request Oct 21, 2025
_decode_tool_use was only used when _tool_json_input_buf was found, but
we were decoding the entire _content_block after adding
_tool_json_input_buf to it. The _content_block overall which could
contain non-JSON elements (e.g. {}), causing failures. To fix this, we
have removed _decode_tool_use helper function and inlined JSON decoding
logic directly into content_block_stop handler in
_process_anthropic_claude_chunk, where we only use it to decode
_tool_json_input_buf before appending to _content_block.

Patch based on
open-telemetry/opentelemetry-python-contrib#3875
with code copied directly from
https://github.com/open-telemetry/opentelemetry-python-contrib/blob/v0.54b1/instrumentation/opentelemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/extensions/bedrock_utils.py#L289

Repeated testing in
open-telemetry/opentelemetry-python-contrib#3875
to confirm this works

By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice.
jj22ee pushed a commit to jj22ee/aws-otel-python-instrumentation that referenced this pull request Oct 22, 2025
…rvability#497)

_decode_tool_use was only used when _tool_json_input_buf was found, but
we were decoding the entire _content_block after adding
_tool_json_input_buf to it. The _content_block overall which could
contain non-JSON elements (e.g. {}), causing failures. To fix this, we
have removed _decode_tool_use helper function and inlined JSON decoding
logic directly into content_block_stop handler in
_process_anthropic_claude_chunk, where we only use it to decode
_tool_json_input_buf before appending to _content_block.

Patch based on
open-telemetry/opentelemetry-python-contrib#3875
with code copied directly from
https://github.com/open-telemetry/opentelemetry-python-contrib/blob/v0.54b1/instrumentation/opentelemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/extensions/bedrock_utils.py#L289

Repeated testing in
open-telemetry/opentelemetry-python-contrib#3875
to confirm this works

By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice.
jj22ee pushed a commit to jj22ee/aws-otel-python-instrumentation that referenced this pull request Oct 22, 2025
…rvability#497)

_decode_tool_use was only used when _tool_json_input_buf was found, but
we were decoding the entire _content_block after adding
_tool_json_input_buf to it. The _content_block overall which could
contain non-JSON elements (e.g. {}), causing failures. To fix this, we
have removed _decode_tool_use helper function and inlined JSON decoding
logic directly into content_block_stop handler in
_process_anthropic_claude_chunk, where we only use it to decode
_tool_json_input_buf before appending to _content_block.

Patch based on
open-telemetry/opentelemetry-python-contrib#3875
with code copied directly from
https://github.com/open-telemetry/opentelemetry-python-contrib/blob/v0.54b1/instrumentation/opentelemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/extensions/bedrock_utils.py#L289

Repeated testing in
open-telemetry/opentelemetry-python-contrib#3875
to confirm this works

By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice.
@thpierce
Copy link
Contributor Author

thpierce commented Oct 23, 2025

@xrmx - can you re-run workflows?

@thpierce
Copy link
Contributor Author

@xrmx - ping on this again. Possible to run workflows again?

@thpierce
Copy link
Contributor Author

Ah! Another lint issue - fixed:

> opentelemetry-python-contrib % tox -e precommit                                                                             
ROOT: will run in automatically provisioned tox, host .../python3.12 is missing [requires (has)]: tox-uv>=1
ROOT: provision> .tox/.tox/bin/python -m tox -e precommit
precommit: commands[0]> pre-commit run --color=always --all-files
ruff.....................................................................Passed
ruff-format..............................................................Passed
uv-lock..................................................................Passed
rstcheck.................................................................Passed
  precommit: OK (4.80=setup[0.01]+cmd[4.79] seconds)
  congratulations :) (5.00 seconds)
> tox -e lint
ROOT: will run in automatically provisioned tox, host .../python3.12 is missing [requires (has)]: tox-uv>=1
ROOT: provision> .tox/.tox/bin/python -m tox -e lint
  lint: OK (0.01 seconds)
  congratulations :) (0.19 seconds)

I don't know why pre-commit things were not being run before, I must have had something misconfigured, but seems to run now:

> opentelemetry-python-contrib % git commit -m "fix lint issue"
ruff.....................................................................Passed
ruff-format..............................................................Passed
uv-lock..............................................(no files to check)Skipped
rstcheck.............................................(no files to check)Skipped
[main 3422729f] fix lint issue

Copy link
Contributor

@tammy-baylis-swi tammy-baylis-swi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Run the jobs and lgtm too. Will need a Maintainer to proceed.

@xrmx xrmx merged commit d31e20e into open-telemetry:main Oct 29, 2025
647 checks passed
@github-project-automation github-project-automation bot moved this from Ready for review to Done in @xrmx's Python PR digest Oct 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

opentelemetry-instrumentation-botocore fails to parse tool use input

3 participants