-
Notifications
You must be signed in to change notification settings - Fork 821
Add safety check for bedrock ConverseStream responses #3990
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
Add safety check for bedrock ConverseStream responses #3990
Conversation
65e10cb to
fde933a
Compare
05bcdf9 to
0861f9f
Compare
...trumentation-botocore/src/opentelemetry/instrumentation/botocore/extensions/bedrock_utils.py
Show resolved
Hide resolved
…ntelemetry/instrumentation/botocore/extensions/bedrock_utils.py
|
@liustve Please fix the CI failures and next time please run the tests locally :) |
|
@liustve I don't think you pushed a fix for the test: |
Apologies, had to fix a few things in my local dev environment, for some reason the tests were using another version of OTel botocore. Also revised the test to better capture the intended behavior. |
instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_bedrock.py
Show resolved
Hide resolved
…est_botocore_bedrock.py
|
@xrmx added another defensive check for |
*Description of changes:* Ports the changes from open-telemetry/opentelemetry-python-contrib#3990 Add defensive check for Bedrock responses missing expected structure. When no messageStart event with assistant role is received, the response may lack the standard output.message format, causing the occasional KeyError exceptions. For testing, I was unable to reproduce the exact scenario that would trigger this bug, however, this safety check would fix this issue without affecting normal instrumentation behavior. By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Description
Add defensive check for Bedrock responses missing expected structure. When no messageStart event with assistant role is received, the response may lack the standard output.message format, causing the occasional KeyError exceptions.
Fixes #3958
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
I was unable to reproduce the exact scenario that would trigger this bug , this safety check would fix this without affecting normal instrumentation behavior.
Does This PR Require a Core Repo Change?
Checklist:
See contributing.md for styleguide, changelog guidelines, and more.