-
Notifications
You must be signed in to change notification settings - Fork 282
feat: add cached token metrics support for Amazon Bedrock #531
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
base: main
Are you sure you want to change the base?
feat: add cached token metrics support for Amazon Bedrock #531
Conversation
- Add optional cacheReadInputTokens and cacheWriteInputTokens fields to Usage TypedDict - Update EventLoopMetrics to accumulate cached token metrics - Add OpenTelemetry instrumentation for cached token telemetry - Enhance metrics summary display to show cached token information - Maintain 100% backward compatibility with existing Usage objects - Add comprehensive test coverage for cached token functionality Resolves strands-agents#529
Hi maintainers, I noticed that PR #392 addresses similar cached token metrics functionality. After reviewing both implementations, I wanted to provide transparency about the overlap and seek guidance on the best path forward. Status Check:
Implementation Comparison:Similarities (95% overlap):
Key Differences:
Next Steps:I'm happy to:
The core functionality is essentially the same, so I'm flexible on whichever approach best serves the project. My primary goal is getting cached token metrics available for users. Please advise on the preferred path forward. Thanks for your time and consideration! Note: I apologize for not catching the existing PR #392 during my initial research. In the future, I'll do more thorough PR searches before starting implementation work. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contributions, would you mind adding the unit tests?
inputTokens: Required[int] | ||
outputTokens: Required[int] | ||
totalTokens: Required[int] | ||
cacheReadInputTokens: Optional[int] | ||
cacheWriteInputTokens: Optional[int] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Required is not necessary in my opinion.
Description
Add cached token metrics support for Amazon Bedrock to track cache read/write input tokens.
Changes Made
cacheReadInputTokens
andcacheWriteInputTokens
fields to Usage TypedDictTesting
Implementation Details
src/strands/types/event_loop.py
to add optional cached token fields using Required/Optional annotationssrc/strands/telemetry/metrics.py
to handle cached token accumulation and telemetrysrc/strands/telemetry/metrics_constants.py
result.metrics.accumulated_usage
works seamlessly with cached tokensResolves
Closes #529
Checklist