CAMEL-12004 Add isDebugEnabled() guard for debug level logs #2093
Conversation
|
Thanks for the pull request. Imho it would be better to convert the statements to parametrized log messages (where possible). That also saves the cost of the string concatenation (I do not see any potentially costly parameter creation) and does not add the complexity of an additional if statement to the code. |
|
I think the best way to deal with this problem is to use slf4j. But I don't know how costly the migration will be. I can look into the changes and try to parameterize them if its possible if this is the best solution in your opinion. |
| @@ -154,7 +154,7 @@ protected void processInTransaction(final Exchange exchange) throws Exception { | |||
| if (log.isDebugEnabled()) { | |||
| // log exception if there was a cause exception so we have the stack trace | |||
| Exception cause = exchange.getException(); | |||
| if (cause != null) { | |||
| if (cause != null && log.isDebugEnabled()) { | |||
There was a problem hiding this comment.
log debug enable is already checked above
| @@ -94,8 +94,9 @@ protected Object evaluateExpression(String expressionText, String expectedValue) | |||
| } else { | |||
| value = expression.evaluate(exchange, Object.class); | |||
| } | |||
|
|
|||
| log.debug("Evaluated expression: {} on exchange: {} result: {}", expression, exchange, value); | |||
| if (log.isDebugEnabled()) { | |||
There was a problem hiding this comment.
This is not needed when using {}
| @@ -131,7 +131,9 @@ protected AbstractMessageListenerContainer createListenerContainer() throws Exce | |||
| answer = new SharedQueueMessageListenerContainer(endpoint, fixedMessageSelector); | |||
| // must use cache level consumer for fixed message selector | |||
| answer.setCacheLevel(DefaultMessageListenerContainer.CACHE_CONSUMER); | |||
| log.debug("Using shared queue: " + endpoint.getReplyTo() + " with fixed message selector [" + fixedMessageSelector + "] as reply listener: " + answer); | |||
| if (log.isDebugEnabled()) { | |||
There was a problem hiding this comment.
We could use {} style here
| @@ -85,8 +85,9 @@ protected void assertExpression(String expressionText, String expectedValue, Str | |||
| } else { | |||
| value = expression.evaluate(exchange, Object.class); | |||
| } | |||
|
|
|||
| log.debug("Evaluated expression: " + expression + " on exchange: " + exchange + " result: " + value); | |||
| if (log.isDebugEnabled()) { | |||
There was a problem hiding this comment.
The rest is just unit tests and tooling so its not as important as runtime camel-core etc. However the changes are fine to do, but maybe favor using {} style
There was a problem hiding this comment.
In general, you do not recommend increasing complexity with the if statement. right?
I added the debug level guards to the one that I found with my script in the latest revision of source code.