8000 Add account ID and region to AWS trace log output by whummer · Pull Request #9084 · localstack/localstack · GitHub
[go: up one dir, main page]

Skip to content

Add account ID and region to AWS trace log output #9084

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

Merged
merged 2 commits into from
Sep 7, 2023
Merged

Conversation

whummer
Copy link
Member
@whummer whummer commented Sep 7, 2023

Motivation

There've been some support cases and issue reports where the root cause was a mismatch in account ID or region specified in the client requests. With multi-account being more widely adopted by our users, the account ID a request is made against is becoming an important part of the trace debug information.

Changes

This PR adds the AWS account ID / region name to the trace log outputs.

Format before:

2023-09-07T10:20:45.044  INFO --- [   asgi_gw_0] localstack.request.aws     : AWS s3.CreateBucket => 200; CreateBucketRequest({'ACL': None, ...

Format after:

2023-09-07T10:21:39.171  INFO --- [   asgi_gw_0] localstack.request.aws     : AWS s3.CreateBucket => 200; 000000000000/us-east-1 - CreateBucketRequest({'ACL': None, ...

Happy for any suggestions regarding the message format. Also, we could consider hiding this behind a feature flag, or dumping the account ID only if it is different from the default account ID (000000000000), if we think that the output is becoming too spammy otherwise. No strong opinion from my side, we just need a mechanism to enable this output when debugging a user issue.

@whummer whummer requested a review from thrau as a code owner September 7, 2023 08:29
@whummer whummer requested review from alexrashed and dfangl September 7, 2023 08:29
@whummer whummer added the semver: patch Non-breaking changes which can be included in patch releases label Sep 7, 2023
@github-actions
Copy link
github-actions bot commented Sep 7, 2023

LocalStack Community integration with Pro

       2 files         2 suites   1h 26m 42s ⏱️
2 169 tests 1 671 ✔️ 498 💤 0
2 170 runs  1 671 ✔️ 499 💤 0

Results for commit c3a6ed2.

♻️ This comment has been updated with latest results.

Copy link
Member
@alexrashed alexrashed left a comment

Choose a reason for hiding this comment

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

Nice! Pretty neat how easy it is to enhance this. The changes are looking good, they are only added for the trace log level, but they will be quite useful! 🚀

Copy link
Member
@dfangl dfangl left a comment

Choose a reason for hiding this comment

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

I think this is fine! It is already in the current logs, but has to be read from the headers, which can be quite tedious. This will make these mismatches noticeable on the first sight, which I really like!

Copy link
Member
@viren-nadkarni viren-nadkarni left a comment

Choose a reason for hiding this comment

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

We could consider using ; as a separator rather than - to keep consistent, but otherwise this is a very useful change!

Co-authored-by: Viren Nadkarni <viren.nadkarni@localstack.cloud>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0