8000 fix(bedrock): handle non-text contentBlockDelta events in converse_stream by octavifs · Pull Request #3404 · traceloop/openllmetry · GitHub
[go: up one dir, main page]

Skip to content

Conversation

octavifs
Copy link
@octavifs octavifs commented Oct 9, 2025
  • I have added tests that cover my changes.
  • If adding a new instrumentation or changing an existing one, I've added screenshots from some observability platform showing the change.
  • PR name follows conventional commits format: feat(instrumentation): ... or fix(instrumentation): ....
  • (If applicable) I have updated the documentation accordingly.

Fixes #2961

This PR fixes the KeyError when using the bedrock converse api with streaming, on events other than text, and adds a test to validate the fix for handling non-text contentBlockDelta events in Bedrock's converse_stream API.

Background

According to AWS Bedrock API documentation, the contentBlockDelta field can contain EITHER of the following:

  • citation
  • reasoningContent
  • toolUse
  • text

The previous code assumed text would always be present, causing KeyError exceptions when the model uses tools or returns other content types.

Changes

  • Fixes KeyError on contentBlockDelta containing attributes other than "text"
  • Add test_anthropic_converse_stream_with_tool_use in test_anthropic.py
  • Record VCR cassette with tool use interaction (cleaned of credentials)

Test Details

The new test:

  1. Sends a weather query with a get_weather tool definition
  2. Streams the response which triggers the model to use the tool
  3. Validates that contentBlockDelta events with toolUse (instead of text) don't crash the instrumentation
  4. Ensures proper span recording throughout

Related


Important

Fixes KeyError in converse_stream by handling non-text contentBlockDelta events and adds a test to validate the fix.

  • Behavior:
    • Fixes KeyError in converse_stream in __init__.py by checking for 'text' in contentBlockDelta events.
    • Handles non-text events like toolUse without crashing.
  • Tests:
    • Adds test_anthropic_converse_stream_with_tool_use in test_anthropic.py to validate handling of non-text contentBlockDelta events.
    • Uses a VCR cassette test_anthropic_converse_stream_with_tool_use.yaml to simulate tool use interaction.
  • Misc:
    • Ensures proper span recording and no logs emitted when use_legacy_attributes is True.

This description was created by Ellipsis for d511cd6. You can customize this summary. It will automatically update as commits are pushed.

Summary by CodeRabbit

  • Bug Fixes

    • Improved stability of Bedrock Anthropic streaming by only accumulating text when present in content deltas, preventing errors with missing or non-text events.
    • Enhanced robustness when tool use occurs during streaming, avoiding crashes and ensuring smooth response handling.
  • Tests

    • Added comprehensive coverage for Anthropic converse streaming with tool use, including a new cassette.
    • Validates correct handling of non-text events and confirms expected tracing attributes and spans without emitting legacy logs.

- citation
- reasoningContent
- toolUse
- text

https://docs.aws.amazon.com/bedrock/latest/APIReference/API_runtime_ContentBlockDeltaEvent.html

So we can't assume that text will always exist within a event["contentBlockDelta"]["delta"] object, as this is not the case.
Add test to validate handling of non-text contentBlockDelta events in
Bedrock's converse_stream API. According to AWS documentation,
contentBlockDelta can contain one of: citation, reasoningContent,
toolUse, or text.

The new test `test_anthropic_converse_stream_with_tool_use` ensures
that when the model uses tools, the instrumentation correctly handles
contentBlockDelta events that contain toolUse instead of text,
preventing KeyError exceptions.

Changes:
- Add test_anthropic_converse_stream_with_tool_use in test_anthropic.py
- Record VCR cassette with tool use interaction
- Clean cassette to remove AWS credentials and security tokens

This test validates the fix in commit b84cd7c where the code was
updated to check for text existence before accessing it:
if "contentBlockDelta" in event and "text" in event["contentBlockDelta"].get("delta", {})
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link
coderabbitai bot commented Oct 9, 2025

Walkthrough

Adds a guard in Bedrock instrumentation to append streaming text only when present in contentBlockDelta.delta.text. Introduces a new test for Anthropic converse_stream with tool use and a corresponding cassette, validating handling of non-text deltas and ensuring spans and attributes are recorded.

Changes

Cohort / File(s) Summary
Bedrock streaming guard
packages/opentelemetry-instrumentation-bedrock/opentelemetry/instrumentation/bedrock/__init__.py
Tightens condition in _handle_converse_stream to append to response only when event.contentBlockDelta.delta.text exists.
Tests: Anthropic converse_stream with tool use
packages/opentelemetry-instrumentation-bedrock/tests/traces/test_anthropic.py
Adds test to iterate stream, collect text deltas when present, detect toolUse deltas, and assert span attributes and absence of legacy logs.
Cassettes: Anthropic tool-use stream
packages/opentelemetry-instrumentation-bedrock/tests/traces/cassettes/test_anthropic/test_anthropic_converse_stream_with_tool_use.yaml
Adds VCR cassette capturing Bedrock runtime POST with toolConfig and event-stream response for weather tool invocation.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Test as Test
  participant Client as Bedrock Runtime Client
  participant Bedrock as Bedrock Service
  participant Instr as Instrumentation

  rect rgb(240,245,255)
    Test->>Client: converse_stream(request with toolConfig)
    Client->>Bedrock: HTTP POST /model/... (stream)
    Bedrock-->>Client: EventStream (contentBlockDelta / toolUse / text / ...)

    loop For each event
      Client-->>Instr: on_event(event)
      alt Has contentBlockDelta.delta.text
        Instr->>Instr: append text to response_msg
      else Non-text delta (e.g., toolUse)
        Instr->>Instr: skip text accumulation
      end
    end
  end

  Client-->>Test: Stream iteration completes
  Instr-->>Instr: record span with model/vendor/request type
  note over Instr: No legacy attributes logs emitted
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • nirga
  • doronkopit5
  • dinmukhamedm

Poem

I twitch my ears at streaming breeze,
Guarding text from missing keys.
Tools may chatter, clouds may brew—
I nibble deltas, only true.
Traces hop, spans neatly align,
Bugs burrow out, the carrots shine. 🥕🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly describes the primary change by indicating a bug fix in the Bedrock instrumentation’s converse_stream to handle non-text contentBlockDelta events, accurately reflecting the code update without extraneous detail.
Linked Issues Check ✅ Passed The changes fully implement the fix described in issue #2961 by adding the necessary checks for “delta” and “text” within contentBlockDelta to prevent KeyErrors, and the added test reproduces and validates handling of toolUse events as specified.
Out of Scope Changes Check ✅ Passed All modified files and tests directly relate to fixing the KeyError in converse_stream and validating non-text contentBlockDelta events; there are no unrelated or extraneous code changes in this PR.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between e66894f and d511cd6.

📒 Files selected for processing (3)
  • packages/opentelemetry-instrumentation-bedrock/opentelemetry/instrumentation/bedrock/__init__.py (1 hunks)
  • packages/opentelemetry-instrumentation-bedrock/tests/traces/cassettes/test_anthropic/test_anthropic_converse_stream_with_tool_use.yaml (1 hunks)
  • packages/opentelemetry-instrumentation-bedrock/tests/traces/test_anthropic.py (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/cassettes/**/*.{yaml,yml,json}

📄 CodeRabbit inference engine (CLAUDE.md)

Never commit secrets or PII in VCR cassettes; scrub sensitive data

Files:

  • packages/opentelemetry-instrumentation-bedrock/tests/traces/cassettes/test_anthropic/test_anthropic_converse_stream_with_tool_use.yaml
**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.py: Store API keys only in environment variables/secure vaults; never hardcode secrets in code
Use Flake8 for code linting and adhere to its rules

Files:

  • packages/opentelemetry-instrumentation-bedrock/tests/traces/test_anthropic.py
  • packages/opentelemetry-instrumentation-bedrock/opentelemetry/instrumentation/bedrock/__init__.py
🧬 Code graph analysis (1)
packages/opentelemetry-instrumentation-bedrock/tests/traces/test_anthropic.py (2)
packages/opentelemetry-instrumentation-bedrock/tests/conftest.py (2)
  • instrument_legacy (90-102)
  • brt (36-42)
packages/traceloop-sdk/traceloop/sdk/utils/in_memory_span_exporter.py (1)
  • get_finished_spans (40-43)
🪛 Ruff (0.13.3)
packages/opentelemetry-instrumentation-bedrock/tests/traces/test_anthropic.py

982-982: Unused function argument: instrument_legacy

(ARG001)

🔇 Additional comments (4)
packages/opentelemetry-instrumentation-bedrock/opentelemetry/instrumentation/bedrock/__init__.py (1)

391-392: LGTM! Fi 8000 x correctly handles non-text contentBlockDelta events.

The guard now checks for the presence of both contentBlockDelta and text in delta before appending, preventing KeyError when streaming events contain toolUse, citation, or reasoningContent instead of text.

packages/opentelemetry-instrumentation-bedrock/tests/traces/test_anthropic.py (2)

980-1064: LGTM! Comprehensive test for non-text contentBlockDelta handling.

The test properly validates:

  • Safe handling of contentBlockDelta events with toolUse instead of text
  • Text accumulation only when text field is present
  • Detection of tool usage
  • Correct span attributes and legacy attribute behavior

982-982: Static analysis false positive.

The instrument_legacy parameter is a pytest fixture used implicitly during test setup and teardown. The Ruff warning about it being unused can be safely ignored.

packages/opentelemetry-instrumentation-bedrock/tests/traces/cassettes/test_anthropic/test_anthropic_converse_stream_with_tool_use.yaml (1)

1-132: LGTM! Cassette appears properly scrubbed.

The VCR cassette correctly captures the Bedrock converse-stream interaction with tool use. Request/response payloads and headers are base64 encoded, and no plaintext secrets or PII are visible.

As per coding guidelines: Verify that AWS credentials are sourced from environment variables in the test setup and not hardcoded in the cassette.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor
@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed everything up to d511cd6 in 47 seconds. Click for details.
  • Reviewed 248 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/opentelemetry-instrumentation-bedrock/opentelemetry/instrumentation/bedrock/__init__.py:391
  • Draft comment:
    Nice fix: the updated conditional checks if the 'delta' field contains a 'text' key before appending, which prevents KeyError on non-text contentBlockDelta events. This aligns with the AWS Bedrock API documentation.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is purely informative, as it praises the fix and explains what it does without suggesting any changes or improvements. It doesn't ask for confirmation or suggest any specific action.
2. packages/opentelemetry-instrumentation-bedrock/tests/traces/test_anthropic.py:980
  • Draft comment:
    The new test 'test_anthropic_converse_stream_with_tool_use' effectively validates that non-text (e.g. toolUse) events do not crash the instrumentation and correctly set span attributes. This improves coverage of edge cases.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is purely informative, describing what the new test does and its benefits. It doesn't provide any actionable feedback or suggestions for improvement.

Workflow ID: wflow_qQ6LlCPvhh9sBvsQ

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

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.

🐛 Bug Report:

2 participants

0