Skip to content

Conversation

@sanjanasekar
Copy link

Implements bedrock-request-id-logging spec requirements

  • Add INFO level logging for Bedrock converse and converse_stream requestIds
  • Use safe dictionary access for ResponseMetadata and RequestId
  • Log format: 'Bedrock converse[_stream] requestId: {request_id}'
  • All tests pass (92 bedrock tests verified)

Changes Made

  • Modified src/strands/models/bedrock.py to add requestId logging in both streaming and non-streaming paths
  • Added logging after self.client.converse_stream(**request) call (line ~1015)
  • Added logging after self.client.converse(**request) call (line ~1050)
  • Uses safe dictionary access: response.get('ResponseMetadata', {}).get('RequestId')

Testing

  • All 92 bedrock model tests pass
  • Feature implementation verified and ready for production use

Closes: bedrock-request-id-logging spec

- Add INFO level logging for Bedrock converse and converse_stream requestIds
- Use safe dictionary access for ResponseMetadata and RequestId
- Log format: 'Bedrock converse[_stream] requestId: {request_id}'
- Implements bedrock-request-id-logging spec requirements
if streaming:
response = self.client.converse_stream(**request)
request_id = response.get('ResponseMetadata', {}).get('RequestId')
logger.info(f"Bedrock converse_stream requestId: {request_id}")
Copy link
Member

Choose a reason for hiding this comment

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

Hi, thanks for this.

If we are going to do this we should also make sure we are logging on exceptions, as that is generally when people want to inspect the request ids anyway.

I am also a bit curious about the need for info here, for exceptional cases I would understand info, but for happy path curious if debug is acceptable for your needs? More so probing at this point than recommending a change.

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants