Skip to content
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

DRILL-7417: Add user logged in/out event in info level logs #1880

Closed
wants to merge 0 commits into from

Conversation

sohami
Copy link
Contributor

@sohami sohami commented Oct 23, 2019

No description provided.

@sohami
Copy link
Contributor Author

sohami commented Oct 23, 2019

Sample output logs:
WebUser:
2019-10-22 13:47:24,888 [qtp480678786-70] INFO o.a.d.e.s.r.a.DrillRestLoginService - WebUser alice logged in from 172.30.8.49:60558
2019-10-22 13:47:30,508 [qtp480678786-64] INFO o.a.d.e.s.rest.LogInLogOutResources - WebUser alice logged out from 172.30.8.49:60567

JDBC/ODBC:
2019-10-22 13:48:16,977 [UserServer-1] INFO o.a.drill.exec.rpc.user.UserServer - User alice logged in from /10.10.100.163:59846
2019-10-22 13:48:19,858 [UserServer-1] INFO o.a.drill.exec.rpc.user.UserServer - User alice logged out from /10.10.100.163:59846

Copy link
Member

@arina-ielchiieva arina-ielchiieva left a comment

Choose a reason for hiding this comment

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

@sohami thanks for the PR, overall looks good, please address some minor code review comments.

@@ -78,7 +78,7 @@ public UserIdentity login(String username, Object credentials, ServletRequest re
// Authenticate the user with configured Authenticator
userAuthenticator.authenticate(username, credentials.toString());

logger.debug("WebUser {} is successfully authenticated", username);
logger.info("WebUser {} logged in from {}:{}", username, request.getRemoteHost(), request.getRemotePort());
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
logger.info("WebUser {} logged in from {}:{}", username, request.getRemoteHost(), request.getRemotePort());
logger.info("WebUser {} logged in from {}: {}", username, request.getRemoteHost(), request.getRemotePort());

@@ -122,7 +122,8 @@ private UserIdentity spnegoLogin(Object credentials) {

// Get the client user short name
final String userShortName = new HadoopKerberosName(clientName).getShortName();

logger.info("WebUser {} logged in from {}:{}", userShortName, request.getRemoteHost(),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
logger.info("WebUser {} logged in from {}:{}", userShortName, request.getRemoteHost(),
logger.info("WebUser {} logged in from {}: {}", userShortName, request.getRemoteHost(),

@sohami
Copy link
Contributor Author

sohami commented Oct 24, 2019

@arina-ielchiieva - Made the changes to add import for logger.

@arina-ielchiieva
Copy link
Member

+1, please squash the commits.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants