-
Notifications
You must be signed in to change notification settings - Fork 100
feat: add automatic TenantId logging to Logger default properties #2358
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 automatic TenantId logging to Logger default properties #2358
Conversation
Signed-off-by: Jeevan Yewale <jeevanyewale4@gmial.com>
|
|
Hey @JeevanYewale, thanks for your contribution. Could you please apply this PR description to our PR template? https://github.com/aws-powertools/powertools-lambda-java/blob/main/.github/PULL_REQUEST_TEMPLATE.md In addition, can you make sure that the PR title follows a semantic title? See: https://github.com/aws-powertools/powertools-lambda-java/blob/main/.github/semantic.yml |
Hi @phipag, Thank you for the feedback! I've updated the PR to address your requests: ✅ Changes Made:
The PR is now ready for review with all requested formatting and documentation improvements. Please let me know if you need any additional changes! Thanks for maintaining high standards for the AWS Powertools project! 🚀 |
|
Thanks for the update @JeevanYewale. The code looks good, but I am confused at the update to the PR description. It looks like I am talking to a coding agent. Let me know if the changes are reviewed by a human. |
@phipag Thanks for the feedback! Yes, I'm a human contributor - I apologize if my previous response seemed automated. I was just trying to be thorough with the formatting. I've reviewed the code changes myself and confirmed they properly implement the TenantId logging feature as requested in issue #2348. The implementation uses reflection to extract the tenant ID from the Lambda context and adds it to the default logging properties. Is there anything specific about the implementation you'd like me to explain or modify? |
|
Thanks for getting back to me @JeevanYewale. I would love to understand the reasoning behind using reflection here. Can you explain? I also see that we are missing adding unit tests for the updated logging context. Currently, we miss testing the new key that is added to logging output but only assert the existing subset of keys being added to the output of the log4j2 and logback loggers. Regarding the PR description. Potentially, you can look at other PRs to make sure that it matches the formatting of those exactly. |
|
@phipag Thanks for the detailed feedback! Regarding reflection usage: Regarding missing unit tests:
Regarding PR description: Should I push these additional unit tests and update the PR description, or would you prefer to see the reflection approach changed first? |
|
Hey @JeevanYewale. Thanks for the explanation. I would like to understand better why you chose reflection for extracting the tenant id compared to the existing approach for the other properties. What are the pros / cons of that approach? We can go ahead and update the PR description in the meantime while we dig into reflection. |
|
@phipag Thanks for the question about reflection usage. Why I chose reflection for TenantId extraction: The
Comparison with existing approaches: Looking at the existing code in
Alternative approach: If you prefer avoiding reflection, I could:
Which approach would you prefer for this optional Lambda feature? |



Hi @phipag,
Thank you for the feedback! I've updated the PR to address your requests:
✅ Changes Made:
feat: add automatic TenantId logging to Logger default propertiesfollowing semantic commit convention📋 PR Template Compliance:
🎯 Semantic Title:
The new title
feat: add automatic TenantId logging to Logger default propertiesfollows the conventional commit format:feat(new feature)The PR is now ready for review with all requested formatting and documentation improvements. Please let me know if you need any additional changes!
Thanks for maintaining high standards for the AWS Powertools project! 🚀