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/10/09 15:33:48 UTC

[GitHub] [kafka] bmaggi opened a new pull request #9407: MINOR: Merge log error to avoid double error

bmaggi opened a new pull request #9407:
URL: https://github.com/apache/kafka/pull/9407


   When using an error tracking system, 2 errors means 2 different alerts.
   It's best to group the logs and have one error with all information.
   
   For example when using with [Sentry](https://sentry.io/welcome/), this double line of log.error will create 2 different Issues.
   One can merge the issues but it will be simpler to have a single error log line
   
   I feel this is a trivial change, so following [Contribution rules](https://cwiki.apache.org/confluence/display/KAFKA/Contributing+Code+Changes#ContributingCodeChanges-PullRequest)
   I didn't create a Jira ticket but simply open a pull request
   
   If I'm wrong, please feel free to indicate me the correct process.


----------------------------------------------------------------
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] bmaggi commented on pull request #9407: MINOR: Merge log error to avoid double error

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


   🤔 not sure how failing tests are related to the patch 🤔 


----------------------------------------------------------------
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] kkonstantine merged pull request #9407: KAFKA-10611: Merge log error to avoid double error

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


   


----------------------------------------------------------------
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] kkonstantine commented on a change in pull request #9407: MINOR: Merge log error to avoid double error

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



##########
File path: connect/runtime/src/main/java/org/apache/kafka/connect/runtime/WorkerTask.java
##########
@@ -184,8 +184,7 @@ private void doRun() throws InterruptedException {
 
             execute();
         } catch (Throwable t) {
-            log.error("{} Task threw an uncaught and unrecoverable exception", this, t);
-            log.error("{} Task is being killed and will not recover until manually restarted", this);
+            log.error("{} Task threw an uncaught and unrecoverable exception, task is being killed and will not recover until manually restarted", this, t);

Review comment:
       I'd suggest keeping the format slightly closer to what we had before. Specifically:
   
   `Task threw an uncaught and unrecoverable exception. Task is being killed and will not recover until manually restarted`




----------------------------------------------------------------
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] bmaggi commented on pull request #9407: MINOR: Merge log error to avoid double error

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


   Thanks for the comments.
   
   Following suggestions: 
   * I created a JIRA ticket [KAFKA-10611](https://issues.apache.org/jira/browse/KAFKA-10611) on the subject 
   * I also added a new commit using the suggested text (closer to the old version) 


----------------------------------------------------------------
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] rhauch commented on a change in pull request #9407: MINOR: Merge log error to avoid double error

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



##########
File path: connect/runtime/src/main/java/org/apache/kafka/connect/runtime/WorkerTask.java
##########
@@ -184,8 +184,7 @@ private void doRun() throws InterruptedException {
 
             execute();
         } catch (Throwable t) {
-            log.error("{} Task threw an uncaught and unrecoverable exception", this, t);
-            log.error("{} Task is being killed and will not recover until manually restarted", this);
+            log.error("{} Task threw an uncaught and unrecoverable exception, task is being killed and will not recover until manually restarted", this, t);

Review comment:
       I also like @kkonstantine's suggested format.




----------------------------------------------------------------
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