Attach user identity to router request logs#15126
Conversation
| } | ||
| emitQueryTime(requestTimeNs, success, sqlQueryId, queryId); | ||
|
|
||
| AuthenticationResult authenticationResult = AuthorizationUtils.authenticationResultFromRequest(req); |
There was a problem hiding this comment.
this would throw an exception if DRUID_AUTHENTICATION_RESULT attribute is not set. can we not get the request directly and avoid possibility of that exception being thrown?
There was a problem hiding this comment.
Since we are attaching an authenticator even if the authenticators are null, I'd expect the request would have an auth result every time. The QueryLifecycle also uses the same method and so if there were a failure, it would fail before the request comes back to the router. What do you think?
There was a problem hiding this comment.
Yeah. I did check that, too. Just being more risk averse, I guess. I am going to approve. Feel free to merge once CI is green. Those failures are likely code coverage issues.
|
Thanks for the review @abhishekagarwal87 |
* Attach user identity to router request logs * Add test * More tests
* Attach user identity to router request logs * Add test * More tests (cherry picked from commit 7802078)
* Attach user identity to router request logs * Add test * More tests (cherry picked from commit 7802078) Co-authored-by: Atul Mohan <[email protected]>
* Attach user identity to router request logs * Add test * More tests
3 participants
Description
This PR attaches user identity to the query request logs on the router. Having the user identity present in the router request logs is useful when druid security is enabled only on the routers.
Sample query request logs on the router would now look like this:
{"query/time":94,"success":false,"identity":"a2l007"} {"query":"select * from \"testdata\"","context":{"sqlQueryId":"sqlQuerytest1","sqlOuterLimit":1001,"queryId":"sqlQuerytest1"}}Key changed/added classes in this PR
AsyncQueryForwardingServletThis PR has: