Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(instrumentation): add span attr for Bedrock prompt caching #2789

Merged
merged 4 commits into from
Mar 24, 2025

Conversation

thisthat
Copy link
Contributor

@thisthat thisthat commented Mar 18, 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.

Follow up for the suggestion made by @nirga: #2788 (review)


Important

Adds span attribute for prompt caching in Bedrock instrumentation and tests for cache read/write scenarios.

  • Behavior:
    • Adds gen_ai.prompt_caching span attribute in prompt_caching_handling() in prompt_caching.py.
    • Sets attribute to "read" or "write" based on cache token counts.
  • Tests:
    • Adds test_prompt_cache in test_anthropic.py to verify span attributes for cache reads and writes.
    • Includes test_prompt_cache.yaml for VCR testing with specific request/response scenarios.

This description was created by Ellipsis for ee0a081. It will automatically update as commits are pushed.

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.

❌ Changes requested. Reviewed everything up to ee0a081 in 2 minutes and 13 seconds

More details
  • Looked at 226 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. packages/opentelemetry-instrumentation-bedrock/opentelemetry/instrumentation/bedrock/prompt_caching.py:29
  • Draft comment:
    If both READ and WRITE headers are present, the second set_attribute call will override the first. If this behavior is intended, please add a comment clarifying it.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50%
    None
2. packages/opentelemetry-instrumentation-bedrock/tests/traces/test_anthropic.py:325
  • Draft comment:
    Unreachable code: The call to prompt_caching_call() after the return in the inner function is never executed. Please remove or reposition it.
  • Reason this comment was not posted:
    Marked as duplicate.

Workflow ID: wflow_zIMS26lgZOTeLwWl


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Member

@nirga nirga left a comment

Choose a reason for hiding this comment

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

Thanks @thisthat!

@nirga nirga merged commit 8501aa9 into traceloop:main Mar 24, 2025
9 checks passed
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.

2 participants