C0urante commented on code in PR #12023:
URL: https://github.com/apache/kafka/pull/12023#discussion_r846829654
##########
clients/src/main/java/org/apache/kafka/common/utils/LogContext.java:
##########
@@ -16,778 +16,28 @@
*/
package org.apache.kafka.common.utils;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-import org.slf4j.Marker;
-import org.slf4j.helpers.FormattingTuple;
-import org.slf4j.helpers.MessageFormatter;
-import org.slf4j.spi.LocationAwareLogger;
-
/**
* This class provides a way to instrument loggers with a common context which
can be used to
* automatically enrich log messages. For example, in the KafkaConsumer, it is
often useful to know
* the groupId of the consumer, so this can be added to a context object which
can then be passed to
* all of the dependent components in order to build new loggers. This removes
the need to manually
* add the groupId to each message.
*/
-public class LogContext {
+public class LogContext extends AbstractLogContext {
private final String logPrefix;
public LogContext(String logPrefix) {
+ super(logPrefix == null ? null : message -> logPrefix + message);
Review Comment:
We still do pay the penalty of an extra method invocation with this style,
but if optimization over cleanliness were that much of a priority in the first
place, the existing
[LogContext.AbstractKafkaLogger::addPrefix](https://github.com/apache/kafka/blob/6e9cd0c7f58a24b5ecef9746881ebcccd7633a7e/clients/src/main/java/org/apache/kafka/common/utils/LogContext.java#L65-L67)
method would be dropped in favor of inlining the prefixing logic directly
wherever it's needed.
Considering we're only performing this extra method invocation on a code
path where string concatenation is guaranteed to take place shortly afterward,
it seems like a small cost to pay.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
