C0urante commented on a change in pull request #11903:
URL: https://github.com/apache/kafka/pull/11903#discussion_r829392255
##########
File path:
connect/transforms/src/main/java/org/apache/kafka/connect/transforms/RegexRouter.java
##########
@@ -57,7 +61,10 @@ public R apply(R record) {
final Matcher matcher = regex.matcher(record.topic());
if (matcher.matches()) {
final String topic = matcher.replaceFirst(replacement);
+ log.trace("Rerouting from topic '{}' to new topic '{}'",
record.topic(), topic);
Review comment:
I don't see a reason to.
If trace-level logging is disabled, the cost of this line will be two method
invocations (one for `Record::topic`, which has a [trivial
implementation](https://github.com/apache/kafka/blob/7afdb069bf5539ec404d9305239849ac35ad2d82/connect/api/src/main/java/org/apache/kafka/connect/connector/ConnectRecord.java#L69-L71),
and one for `Logger::trace`), plus a check to see if trace-level logging is
enabled. No message formatting will take place.
If we wrap this with `Logger::isTraceEnabled`, we'll reduce that to a single
method invocation and check to see if trace-level logging is enabled. But if
trace-level logging is enabled, we'd end up doubling the checks to see if the
logger is enabled, and add an extra method invocation. There's a good writeup
on the performance implications of this style of checking
[here](https://logging.apache.org/log4j/1.2/manual.html#performance).
It seems unlikely that any of this will have a significant impact on
performance, especially considering that this same code path already contains a
regex check a few lines above. There's also the existing logging in `Cast`
that's mentioned in the description that takes place at trace level and doesn't
have a guard for `Logger::isTraceEnabled`.
Ultimately, if we're concerned about performance here, it'd probably be much
more effective to cache the mappings of input topic -> output topic than to add
guards around trace-level log statements that don't do involve any string
concatenation or expensive `toString` calls.
--
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]
