[KAFKA-6702]Wrong className in LoggerFactory.getLogger method#4772
Conversation
|
Sorry, there is something wrong in my local git, so i push a new commit and delete the old one. |
|
LGTM |
hachikuji
left a comment
hachikuji
left a comment
There was a problem hiding this comment.
LGTM. Thanks for the patch!
| */ | ||
| public class LogAndContinueExceptionHandler implements DeserializationExceptionHandler { | ||
| private static final Logger log = LoggerFactory.getLogger(StreamThread.class); | ||
| private static final Logger log = LoggerFactory.getLogger(LogAndContinueExceptionHandler.class); |
There was a problem hiding this comment.
IIRC, using StreamThread is on purpose here (same below). @guozhangwang can you chime in?
If StreamThread is on purpose, we might want to add a comment.
There was a problem hiding this comment.
Would be helpful to understand the reason as well. The problem with a mismatch is that it's harder to track down the source of a log message and it messes up line numbers when printed.
There was a problem hiding this comment.
The original motivation is to maintain the information of which thread was hitting this error, but the current implementation does not maintain that any more.. I'm not sure if it was lost somewhere in previous commit or it is never the case. But this is what I've in mind originally:
final String logPrefix = String.format("stream-thread [%s] ", threadClientId);
final LogContext logContext = new LogContext(logPrefix);
final Logger log = logContext.logger(LogAndContinueExceptionHandler.class);
There was a problem hiding this comment.
Are these classes public? Could we move them into StreamThread?
There was a problem hiding this comment.
Yes they are designed to be public.
There was a problem hiding this comment.
I see. There may not be a nice way to wire in the thread id from a static context. If we can't get the thread id, I'd suggest accepting the change here so that at least the message points to the right file.
5 participants
https://issues.apache.org/jira/browse/KAFKA-6702
[KAFKA-6702]Wrong className in LoggerFactory.getLogger method