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.

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

Remove loglevel guards when using parameterized logging #127

Open
timtebeek opened this issue Dec 1, 2023 · 1 comment
Open

Remove loglevel guards when using parameterized logging #127

timtebeek opened this issue Dec 1, 2023 · 1 comment
Labels
recipe Recipe Requested

Comments

@timtebeek
Copy link
Contributor

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

@timtebeek timtebeek added the recipe Recipe Requested label Dec 1, 2023
@timtebeek timtebeek moved this to Recipes Wanted in OpenRewrite Dec 1, 2023
@ppkarwasz
Copy link
Contributor

ppkarwasz commented Dec 29, 2023

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());
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
recipe Recipe Requested
Projects
Status: Recipes Wanted
Development

No branches or pull requests

2 participants