rewrite-logging-frameworks icon indicating copy to clipboard operation
rewrite-logging-frameworks copied to clipboard

Remove loglevel guards when using parameterized logging

Open timtebeek opened this issue 2 years ago • 1 comments

What problem are you trying to solve?

Loglevel guards ( such as if (LOG.isDebugEnabled())) are a performance improvement to prevent building up logging statement arguments when the log line would be discarded. Parameterized logging is a similar approach at limiting the impact of logging on performance, by only formatting the logging statement when the loglevel is enabled. Code should likely only have to use one of the two approaches: either loglevel guards, or parameterized logging. So when paramterized logging statements are used, the loglevel guards can be removed.

What precondition(s) should be checked before applying this recipe?

The loglevel guard should only be removed if

  • all statements in the body are logging statements
  • all logging statements have arguments that are not method invocations (maybe allow Exception.getMessage()), to prevent side effects

Describe the situation before applying the recipe

if (LOG.isDebugEnabled()) {
    LOG.debug("Swallowed an IOException caused by client connectivity: {}", cause.getMessage(), cause);
}

Describe the situation after applying the recipe

-if (LOG.isDebugEnabled()) {
-    LOG.debug("Swallowed an IOException caused by client connectivity: {}", cause.getMessage(), cause);
+LOG.debug("Swallowed an IOException caused by client connectivity: {}", cause.getMessage(), cause);
-}

Any additional context

As discovered working through

  • https://github.com/micronaut-projects/micronaut-core/pull/10161#issuecomment-1835649110

timtebeek avatar Dec 01 '23 09:12 timtebeek

Some getters might be expensive to compute, so the basic recipe should probably only rewrite:

if (LOG.isDebugEnabled()) {
    LOG.debug("Hello {}!", name);
}

while something like:

if (LOG.isDebugEnabled()) {
    LOG.debug("Hello {}!", person.getName());
}

should only be rewritten based on some recipe parameter (e.g. rewriteGetters: true).

Remark that in the Log4j API (but not in SLF4J), the second example can always be rewritten using lambdas:

if (LOG.isDebugEnabled()) {
    LOG.debug("Hello {}!", () -> person.getName());
}

ppkarwasz avatar Dec 29 '23 18:12 ppkarwasz