Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

CAMEL-12004 Add isDebugEnabled() guard for debug level logs #2093

Closed
wants to merge 1 commit into from
Closed

CAMEL-12004 Add isDebugEnabled() guard for debug level logs #2093

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 3 additions & 2 deletions camel-core/src/test/java/org/apache/camel/LanguageTestSupport.java
View file
Open in desktop
Expand Up @@ -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()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not needed when using {}

log.debug("Evaluated expression: {} on exchange: {} result: {}", expression, exchange, value);
}

return value;
}
Expand Down
4 changes: 3 additions & 1 deletion ...l-github/src/test/java/org/apache/camel/component/github/consumer/CommitConsumerTest.java
View file
Open in desktop
Expand Up @@ -60,7 +60,9 @@ public void process(Exchange exchange) throws Exception {
Message in = exchange.getIn();
RepositoryCommit commit = (RepositoryCommit) in.getBody();
User author = commit.getAuthor();
log.debug("Got commit with author: " + author.getLogin() + ": " + author.getHtmlUrl() + " SHA " + commit.getSha());
if (log.isDebugEnabled()) {
log.debug("Got commit with author: " + author.getLogin() + ": " + author.getHtmlUrl() + " SHA " + commit.getSha());
}
}
}
}
4 changes: 3 additions & 1 deletion ...nents/camel-jms/src/main/java/org/apache/camel/component/jms/reply/QueueReplyManager.java
View file
Open in desktop
Expand Up @@ -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()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could use {} style here

log.debug("Using shared queue: " + endpoint.getReplyTo() + " with fixed message selector [" + fixedMessageSelector + "] as reply listener: " + answer);
}
} else {
// use a dynamic message selector which will select the message we want to receive as reply
dynamicMessageSelector = new MessageSelectorCreator(correlation);
Expand Down
2 changes: 1 addition & 1 deletion ...nents/camel-spring/src/main/java/org/apache/camel/spring/spi/TransactionErrorHandler.java
View file
Open in desktop
Expand Up @@ -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()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

log debug enable is already checked above

log.debug("Transaction rollback (" + transactionKey + ") redelivered(" + redelivered + ") for "
+ ids + " due exchange was marked for rollbackOnlyLast and caught: ", cause);
} else {
Expand Down
5 changes: 3 additions & 2 deletions components/camel-test/src/main/java/org/apache/camel/test/junit4/LanguageTestSupport.java
View file
Open in desktop
Expand Up @@ -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()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, you do not recommend increasing complexity with the if statement. right?

log.debug("Evaluated expression: " + expression + " on exchange: " + exchange + " result: " + value);
}

assertTrue("Expression: " + expression + " on Exchange: " + exchange,
expectedValue.equals(value) || orThisExpectedValue.equals(value));
Expand Down
5 changes: 3 additions & 2 deletions components/camel-testng/src/main/java/org/apache/camel/testng/LanguageTestSupport.java
View file
Open in desktop
Expand Up @@ -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()) {
log.debug("Evaluated expression: " + expression + " on exchange: " + exchange + " result: " + value);
}

assertTrue(expectedValue.equals(value) || orThisExpectedValue.equals(value), "Expression: " + expression + " on Exchange: " + exchange);
}
Expand Down
8 changes: 6 additions & 2 deletions tooling/maven/camel-maven-plugin/src/main/java/org/apache/camel/maven/RunMojo.java
View file
Open in desktop
Expand Up @@ -697,8 +697,10 @@ private void terminateThreads(ThreadGroup threadGroup) {
// even log in future
Thread[] threadsArray = new Thread[1];
threadGroup.enumerate(threadsArray);
getLog().debug("strange; " + activeCount + " thread(s) still active in the group "
if (getLog().isDebugEnabled()) {
getLog().debug("strange; " + activeCount + " thread(s) still active in the group "
+ threadGroup + " such as " + threadsArray[0]);
}
}
}
}
Expand Down Expand Up @@ -806,7 +808,9 @@ private void addRelevantPluginDependenciesToClasspath(Set<URL> path) throws Mojo
// we must skip org.osgi.core, otherwise we get a
// java.lang.NoClassDefFoundError: org.osgi.vendor.framework property not set
if (classPathElement.getArtifactId().equals("org.osgi.core")) {
getLog().debug("Skipping org.osgi.core -> " + classPathElement.getGroupId() + "/" + classPathElement.getArtifactId() + "/" + classPathElement.getVersion());
if (getLog().isDebugEnabled()) {
getLog().debug("Skipping org.osgi.core -> " + classPathElement.getGroupId() + "/" + classPathElement.getArtifactId() + "/" + classPathElement.getVersion());
}
continue;
}

Expand Down
5 changes: 3 additions & 2 deletions ...ge-maven-plugin/src/main/java/org/apache/camel/maven/packaging/PackageDataFormatMojo.java
View file
Open in desktop
Expand Up @@ -190,8 +190,9 @@ public static void prepareDataFormat(Log log, MavenProject project, MavenProject
OutputStream fos = buildContext.newFileOutputStream(out);
fos.write(schema.getBytes());
fos.close();

log.debug("Generated " + out + " containing JSon schema for " + name + " data format");
if (log.isDebugEnabled()) {
log.debug("Generated " + out + " containing JSon schema for " + name + " data format");
}
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion ...el-package-maven-plugin/src/main/java/org/apache/camel/maven/packaging/PackageHelper.java
View file
Open in desktop
Expand Up @@ -47,7 +47,9 @@ public static boolean haveResourcesChanged(Log log, MavenProject project, BuildC
file = new File(r.getDirectory().substring(baseDir.length() + 1));
}
String path = file.getPath() + "/" + suffix;
log.debug("checking if " + path + " (" + r.getDirectory() + "/" + suffix + ") has changed.");
if (log.isDebugEnabled()) {
log.debug("checking if " + path + " (" + r.getDirectory() + "/" + suffix + ") has changed.");
}
if (buildContext.hasDelta(path)) {
log.debug("Indeed " + suffix + " has changed.");
return true;
Expand Down
5 changes: 3 additions & 2 deletions ...kage-maven-plugin/src/main/java/org/apache/camel/maven/packaging/PackageLanguageMojo.java
View file
Open in desktop
Expand Up @@ -224,8 +224,9 @@ public static void prepareLanguage(Log log, MavenProject project, MavenProjectHe
fos.close();

buildContext.refresh(out);

log.debug("Generated " + out + " containing JSon schema for " + name + " language");
if (log.isDebugEnabled()) {
log.debug("Generated " + out + " containing JSon schema for " + name + " language");
}
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions ...ge-maven-plugin/src/main/java/org/apache/camel/maven/packaging/SpringBootStarterMojo.java
View file
Open in desktop
Expand Up @@ -395,9 +395,9 @@ private Set<String> filterIncludedArtifacts(Set<String> artifacts) throws Depend
List<DependencyNode> nodes = visitor.getNodes();
for (DependencyNode dependencyNode : nodes) {
Artifact artifact = dependencyNode.getArtifact();

getLog().debug("Found dependency node: " + artifact.getGroupId() + ":" + artifact.getArtifactId() + ":" + artifact.getVersion() + " - scope=" + artifact.getScope());

if (getLog().isDebugEnabled()) {
getLog().debug("Found dependency node: " + artifact.getGroupId() + ":" + artifact.getArtifactId() + ":" + artifact.getVersion() + " - scope=" + artifact.getScope());
}
if (!Artifact.SCOPE_TEST.equals(artifact.getScope()) && !Artifact.SCOPE_PROVIDED.equals(artifact.getScope())) {
String canonicalName = artifact.getGroupId() + ":" + artifact.getArtifactId();
if (artifacts.contains(canonicalName)) {
Expand Down
4 changes: 3 additions & 1 deletion tooling/maven/guice-maven-plugin/src/main/java/org/apache/camel/guice/maven/RunMojo.java
View file
Open in desktop
Expand Up @@ -513,8 +513,10 @@ private void terminateThreads(ThreadGroup threadGroup) {
// even log in future
Thread[] threadsArray = new Thread[1];
threadGroup.enumerate(threadsArray);
getLog().debug("strange; " + activeCount + " thread(s) still active in the group "
if (getLog().isDebugEnabled()){
getLog().debug("strange; " + activeCount + " thread(s) still active in the group "
+ threadGroup + " such as " + threadsArray[0]);
}
}
}
}
Expand Down