You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "kamalcph (via GitHub)" <gi...@apache.org> on 2024/07/22 15:36:18 UTC
[PR] KAFKA-17168: Remove the logPrefix to print the thread name. [kafka]
kamalcph opened a new pull request, #16657:
URL: https://github.com/apache/kafka/pull/16657
User can configure the log4j conversion-pattern to print the thread name if 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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-17168: Remove the logPrefix to print the thread name [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #16657:
URL: https://github.com/apache/kafka/pull/16657#issuecomment-2244878486
@kamalcph could you please fix the build error? it seems we have to rename "logger" to "LOGGER"
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-17168: Remove the logPrefix to print the thread name [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 merged PR #16657:
URL: https://github.com/apache/kafka/pull/16657
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-17168: Remove the logPrefix to print the thread name [kafka]
Posted by "satishd (via GitHub)" <gi...@apache.org>.
satishd commented on code in PR #16657:
URL: https://github.com/apache/kafka/pull/16657#discussion_r1686938593
##########
storage/src/main/java/org/apache/kafka/storage/internals/log/RemoteStorageThreadPool.java:
##########
@@ -34,7 +34,7 @@
import static org.apache.kafka.server.log.remote.storage.RemoteStorageMetrics.REMOTE_STORAGE_THREAD_POOL_METRICS;
public class RemoteStorageThreadPool extends ThreadPoolExecutor {
- private final Logger logger;
+ private final Logger logger = LoggerFactory.getLogger(RemoteStorageThreadPool.class);
Review Comment:
Can you make this as static to avoid creating multiple logger instances as we do not need any context for each instance? I understand that we create only a single instance for now but it is good to make this logger static even if we create multiple instances in future.
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-17168: Remove the logPrefix to print the thread name [kafka]
Posted by "satishd (via GitHub)" <gi...@apache.org>.
satishd commented on code in PR #16657:
URL: https://github.com/apache/kafka/pull/16657#discussion_r1686938593
##########
storage/src/main/java/org/apache/kafka/storage/internals/log/RemoteStorageThreadPool.java:
##########
@@ -34,7 +34,7 @@
import static org.apache.kafka.server.log.remote.storage.RemoteStorageMetrics.REMOTE_STORAGE_THREAD_POOL_METRICS;
public class RemoteStorageThreadPool extends ThreadPoolExecutor {
- private final Logger logger;
+ private final Logger logger = LoggerFactory.getLogger(RemoteStorageThreadPool.class);
Review Comment:
Can you make this as static to avoid creating multiple logger instances as we do not need any context for each instance?
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-17168: Remove the logPrefix to print the thread name [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #16657:
URL: https://github.com/apache/kafka/pull/16657#discussion_r1687339581
##########
storage/src/main/java/org/apache/kafka/storage/internals/log/RemoteStorageThreadPool.java:
##########
@@ -34,7 +34,7 @@
import static org.apache.kafka.server.log.remote.storage.RemoteStorageMetrics.REMOTE_STORAGE_THREAD_POOL_METRICS;
public class RemoteStorageThreadPool extends ThreadPoolExecutor {
- private final Logger logger;
+ private final Logger logger = LoggerFactory.getLogger(RemoteStorageThreadPool.class);
Review Comment:
> I understand that we create only a single instance for now but it is good to make this logger static even if we create multiple instances in future.
that is a good point. The default provider `reload4j` maintain the logger instance for each class name, so the instance should be same even though we don't use `static`. However, it seems slf4j docs does not describe such behavior so we can't assume other providers are same
for another, we should apply the `static` to code base. 1) unify the code style and 2) avoid multiple logger instances in the future.
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-17168: Remove the logPrefix to print the thread name [kafka]
Posted by "kamalcph (via GitHub)" <gi...@apache.org>.
kamalcph commented on PR #16657:
URL: https://github.com/apache/kafka/pull/16657#issuecomment-2247268147
Test failures are unrelated.
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org