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 authenticated user to mdc by default #5499
Conversation
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! 👍 👍 👍
return log.authenticatedUser(); | ||
} | ||
return null; | ||
}), |
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.
Nit: we can use a ternary operator for consistent with others, something like this:
AUTHENTICATED_USER("authenticated.user", log ->
log.isAvailable(RequestLogProperty.AUTHENTICATED_USER) ? log.authenticatedUser() : null
),
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.
It looks useful.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5499 +/- ##
===========================================
+ Coverage 0 74.02% +74.02%
- Complexity 0 20845 +20845
===========================================
Files 0 1807 +1807
Lines 0 76745 +76745
Branches 0 9789 +9789
===========================================
+ Hits 0 56811 +56811
- Misses 0 15304 +15304
- Partials 0 4630 +4630 ☔ View full report in Codecov by Sentry. |
Motivation:
We use
ctx.log.authenticatedUser
to record the authenticated user in Central Dogma.https://github.com/line/centraldogma/blob/54205bfcee1167f84bc1ab1a69e08fa0c3e3f2f3/server/src/main/java/com/linecorp/centraldogma/server/internal/api/auth/ApplicationTokenAuthorizer.java#L74
It would be more convenient to provide
authenticatedUser
as a built in property since it is already a part of the armeria core APIs anyways.Modifications:
RequestLogProperty.AUTHENTICATED_USER
Result:
authenticatedUser
via mdc more conveniently