You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2020/09/01 07:42:06 UTC

[GitHub] [kafka] cnZach opened a new pull request #9236: MINOR: Log warn message with details when there's kerberos login issue

cnZach opened a new pull request #9236:
URL: https://github.com/apache/kafka/pull/9236


   …though we will still retry
   
   Currently, we just capture the exception and retry later, we can't figure out what happened at runtime, it would be helpful to log a warn message with details.
   
   *Summary of testing strategy (including rationale)
   for the feature or bug fix. Unit and/or integration
   tests are expected for any behaviour change and
   system tests should be considered for larger changes.*
   Minor logging change, so no extra test is required. 
   
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation 
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] rajinisivaram commented on a change in pull request #9236: MINOR: Log warn message with details when there's kerberos login issue

Posted by GitBox <gi...@apache.org>.
rajinisivaram commented on a change in pull request #9236:
URL: https://github.com/apache/kafka/pull/9236#discussion_r488519370



##########
File path: clients/src/main/java/org/apache/kafka/common/security/kerberos/KerberosLogin.java
##########
@@ -213,6 +213,7 @@ public LoginContext login() throws LoginException {
                             break;
                         } catch (Exception e) {
                             if (retry > 0) {
+                                log.warn("[Principal={}]: Error when trying to renew with TicketCache, but will retry ", principal, e);

Review comment:
       @cnZach Looks like the intention was to print out the exception with stack trace? You would want to use:
   ```
   public void warn(String msg, Throwable t);
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] omkreddy commented on pull request #9236: MINOR: Log warn message with details when there's kerberos login issue

Posted by GitBox <gi...@apache.org>.
omkreddy commented on pull request #9236:
URL: https://github.com/apache/kafka/pull/9236#issuecomment-685444628


   ok to test


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] cnZach commented on a change in pull request #9236: MINOR: Log warn message with details when there's kerberos login issue

Posted by GitBox <gi...@apache.org>.
cnZach commented on a change in pull request #9236:
URL: https://github.com/apache/kafka/pull/9236#discussion_r481880310



##########
File path: clients/src/main/java/org/apache/kafka/common/security/kerberos/KerberosLogin.java
##########
@@ -237,6 +238,7 @@ public LoginContext login() throws LoginException {
                             break;
                         } catch (LoginException le) {
                             if (retry > 0) {
+                                log.warn("[Principal={}]: Error when trying to re-Login, but will retry ", principal, e);

Review comment:
       oops, yes, corrected.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] omkreddy commented on a change in pull request #9236: MINOR: Log warn message with details when there's kerberos login issue

Posted by GitBox <gi...@apache.org>.
omkreddy commented on a change in pull request #9236:
URL: https://github.com/apache/kafka/pull/9236#discussion_r481854947



##########
File path: clients/src/main/java/org/apache/kafka/common/security/kerberos/KerberosLogin.java
##########
@@ -237,6 +238,7 @@ public LoginContext login() throws LoginException {
                             break;
                         } catch (LoginException le) {
                             if (retry > 0) {
+                                log.warn("[Principal={}]: Error when trying to re-Login, but will retry ", principal, e);

Review comment:
       e => le ?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] cnZach commented on pull request #9236: MINOR: Log warn message with details when there's kerberos login issue

Posted by GitBox <gi...@apache.org>.
cnZach commented on pull request #9236:
URL: https://github.com/apache/kafka/pull/9236#issuecomment-692409942


   > @cnZach can you rebase this PR against trunk? That will trigger the build.
   
   Hi @omkreddy , do I need to change anything for this PR?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] omkreddy commented on pull request #9236: MINOR: Log warn message with details when there's kerberos login issue

Posted by GitBox <gi...@apache.org>.
omkreddy commented on pull request #9236:
URL: https://github.com/apache/kafka/pull/9236#issuecomment-687065096


   @cnZach  can you rebase this PR against trunk? That will trigger the build.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] omkreddy commented on a change in pull request #9236: MINOR: Log warn message with details when there's kerberos login issue

Posted by GitBox <gi...@apache.org>.
omkreddy commented on a change in pull request #9236:
URL: https://github.com/apache/kafka/pull/9236#discussion_r491437035



##########
File path: clients/src/main/java/org/apache/kafka/common/security/kerberos/KerberosLogin.java
##########
@@ -213,6 +213,7 @@ public LoginContext login() throws LoginException {
                             break;
                         } catch (Exception e) {
                             if (retry > 0) {
+                                log.warn("[Principal={}]: Error when trying to renew with TicketCache, but will retry ", principal, e);

Review comment:
       slf4j logger methods logs exception if the last argument is throwable. so this is fine.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] cnZach commented on pull request #9236: MINOR: Log warn message with details when there's kerberos login issue

Posted by GitBox <gi...@apache.org>.
cnZach commented on pull request #9236:
URL: https://github.com/apache/kafka/pull/9236#issuecomment-686447109


   Not sure what happened... Can you trigger the test again?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] cnZach commented on a change in pull request #9236: MINOR: Log warn message with details when there's kerberos login issue

Posted by GitBox <gi...@apache.org>.
cnZach commented on a change in pull request #9236:
URL: https://github.com/apache/kafka/pull/9236#discussion_r489215673



##########
File path: clients/src/main/java/org/apache/kafka/common/security/kerberos/KerberosLogin.java
##########
@@ -213,6 +213,7 @@ public LoginContext login() throws LoginException {
                             break;
                         } catch (Exception e) {
                             if (retry > 0) {
+                                log.warn("[Principal={}]: Error when trying to renew with TicketCache, but will retry ", principal, e);

Review comment:
       Hi @rajinisivaram , do you mean we should use  `log.warn(s"[Principal=$principal]: Error when trying to renew with TicketCache, but will retry ", e); ` ?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] omkreddy merged pull request #9236: MINOR: Log warn message with details when there's kerberos login issue

Posted by GitBox <gi...@apache.org>.
omkreddy merged pull request #9236:
URL: https://github.com/apache/kafka/pull/9236


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] cnZach commented on pull request #9236: MINOR: Log warn message with details when there's kerberos login issue

Posted by GitBox <gi...@apache.org>.
cnZach commented on pull request #9236:
URL: https://github.com/apache/kafka/pull/9236#issuecomment-685306709


   Hi @omkreddy or @rajinisivaram , what do you think about this? I just want to print out the error. It will be helpful for troubleshooting kerberos login issue.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org