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 2021/03/02 13:10:47 UTC
[GitHub] [kafka] dengziming opened a new pull request #10247: MINOR: Fix log format
dengziming opened a new pull request #10247:
URL: https://github.com/apache/kafka/pull/10247
*More detailed description of your change*
Accidentally saw a log:
```
[2021-03-02 20:39:56,418] DEBUG FindCoordinator request failed due to {} (org.apache.kafka.clients.consumer.internals.AbstractCoordinatorTest$DummyCoordinator:863)
org.apache.kafka.common.errors.AuthenticationException: Authentication failed
```
I fixed it to:
```
[2021-03-02 20:51:39,050] ERROR FindCoordinator request failed due to Authentication failed (org.apache.kafka.clients.consumer.internals.AbstractCoordinatorTest$DummyCoordinator:863)
```
I think the author wants to call `Logger.debug(String format, Object arg)`, but called `Logger.debug(String msg, Throwable t)`. In fact, there are 3 overloadings `Logger.debug()` so we need to be careful.
*Summary of testing strategy (including rationale)*
test locally
### 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] dengziming closed pull request #10247: MINOR: Fix log format in AbstractCoordinator
Posted by GitBox <gi...@apache.org>.
dengziming closed pull request #10247:
URL: https://github.com/apache/kafka/pull/10247
----------------------------------------------------------------
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] dengziming commented on pull request #10247: MINOR: Fix log format in AbstractCoordinator
Posted by GitBox <gi...@apache.org>.
dengziming commented on pull request #10247:
URL: https://github.com/apache/kafka/pull/10247#issuecomment-788975239
@chia7712 Thank you,There are 5 overloadings of log.debug,
----------------------------------------------------------------
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] ijuma commented on a change in pull request #10247: MINOR: Fix log format in AbstractCoordinator
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10247:
URL: https://github.com/apache/kafka/pull/10247#discussion_r585668358
##########
File path: clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractCoordinator.java
##########
@@ -860,7 +860,7 @@ public void onSuccess(ClientResponse resp, RequestFuture<Void> future) {
@Override
public void onFailure(RuntimeException e, RequestFuture<Void> future) {
- log.debug("FindCoordinator request failed due to {}", e);
+ log.debug("FindCoordinator request failed due to {}", e.getMessage());
Review comment:
It's not clear what was intended here. Is the stacktrace useful or not? If the `{}` is removed, then the stacktrace would be printed. If we make the change in this PR, we don't include the stacktrace. @hachikuji Thoughts since you have done a lot of coordinator debugging?
----------------------------------------------------------------
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] chia7712 commented on a change in pull request #10247: MINOR: Fix log format in AbstractCoordinator
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #10247:
URL: https://github.com/apache/kafka/pull/10247#discussion_r585634195
##########
File path: clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractCoordinator.java
##########
@@ -860,7 +860,7 @@ public void onSuccess(ClientResponse resp, RequestFuture<Void> future) {
@Override
public void onFailure(RuntimeException e, RequestFuture<Void> future) {
- log.debug("FindCoordinator request failed due to {}", e);
+ log.debug("FindCoordinator request failed due to {}", e.getMessage());
Review comment:
How about using `e.toString()`? It shows the class name of exception. I feel it is useful also.
----------------------------------------------------------------
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] dengziming commented on a change in pull request #10247: MINOR: Fix log format in AbstractCoordinator
Posted by GitBox <gi...@apache.org>.
dengziming commented on a change in pull request #10247:
URL: https://github.com/apache/kafka/pull/10247#discussion_r586046025
##########
File path: clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractCoordinator.java
##########
@@ -860,7 +860,7 @@ public void onSuccess(ClientResponse resp, RequestFuture<Void> future) {
@Override
public void onFailure(RuntimeException e, RequestFuture<Void> future) {
- log.debug("FindCoordinator request failed due to {}", e);
+ log.debug("FindCoordinator request failed due to {}", e.getMessage());
Review comment:
Thank you, the #10232 LGTM, will close 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] ableegoldman commented on a change in pull request #10247: MINOR: Fix log format in AbstractCoordinator
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on a change in pull request #10247:
URL: https://github.com/apache/kafka/pull/10247#discussion_r585856635
##########
File path: clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractCoordinator.java
##########
@@ -860,7 +860,7 @@ public void onSuccess(ClientResponse resp, RequestFuture<Void> future) {
@Override
public void onFailure(RuntimeException e, RequestFuture<Void> future) {
- log.debug("FindCoordinator request failed due to {}", e);
+ log.debug("FindCoordinator request failed due to {}", e.getMessage());
Review comment:
@guozhangwang has a PR open in which he fixes this on the side. See https://github.com/apache/kafka/pull/10232/files#r584478802
----------------------------------------------------------------
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] dengziming commented on pull request #10247: MINOR: Fix log format in AbstractCoordinator
Posted by GitBox <gi...@apache.org>.
dengziming commented on pull request #10247:
URL: https://github.com/apache/kafka/pull/10247#issuecomment-788899739
@ableegoldman , Hi, PTAL.
----------------------------------------------------------------
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] dengziming edited a comment on pull request #10247: MINOR: Fix log format in AbstractCoordinator
Posted by GitBox <gi...@apache.org>.
dengziming edited a comment on pull request #10247:
URL: https://github.com/apache/kafka/pull/10247#issuecomment-788975239
@chia7712 Thank you,There are 5 overloadings of log.debug, the code in your comment isn't the same with mine, WDYT?
----------------------------------------------------------------
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