Skip to content

[java] GuardLogStatement can have more detailed example #3144

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

Closed
KroArtem opened this issue Feb 28, 2021 · 4 comments · Fixed by #3155
Closed

[java] GuardLogStatement can have more detailed example #3144

KroArtem opened this issue Feb 28, 2021 · 4 comments · Fixed by #3155
Assignees
Labels
an:enhancement An improvement on existing features / rules in:documentation Affects the documentation
Milestone

Comments

@KroArtem
Copy link
Contributor

KroArtem commented Feb 28, 2021

Affects PMD Version:
6.31

Rule: GuardLogStatement

Description:

GuardLogStatement says that you should check for logLevel or do not check for logLevel but do not create String via concatenation. It would be useful to have one more example on how to avoid if statement and to use lazy logging
for log4j2 or slf4j working with parameters

Whenever using a log level, one should check if the loglevel is actually enabled, or
otherwise skip the associate String creation and manipulation.
EXAMPLE:
    // Add this for performance
    if (log.isDebugEnabled() { ...
        log.debug("log something" + " and " + "concat strings");

Exception Stacktrace:

# Copy-paste the stack trace here

Code Sample demonstrating the issue:


Steps to reproduce:

Please provide detailed steps for how we can reproduce the bug.

  1. ... (e.g. if you're using maven: mvn clean verify)
  2. ...

Running PMD through: [CLI | Ant | Maven | Gradle | Designer | Other]

@KroArtem KroArtem added the a:bug PMD crashes or fails to analyse a file. label Feb 28, 2021
@oowekyala oowekyala added an:enhancement An improvement on existing features / rules in:documentation Affects the documentation and removed a:bug PMD crashes or fails to analyse a file. labels Mar 1, 2021
@adangel adangel self-assigned this Mar 4, 2021
@adangel adangel changed the title GuardLogStatement can have more detailed example [java] GuardLogStatement can have more detailed example Mar 4, 2021
@adangel
Copy link
Member

adangel commented Mar 4, 2021

@adangel
Copy link
Member

adangel commented Mar 4, 2021

Proposed documentation:

Whenever using a log level, one should check if the loglevel is actually enabled, or
otherwise skip the associate String creation and manipulation.

An alternative to checking the log level are substituting parameters, formatters or lazy logging
with lambdas. The available alternatives depend on the actual logging framework.

The examples are now:

    // Add this for performance
    if (log.isDebugEnabled() {
        log.debug("log something" + " and " + "concat strings");
    }
    // Avoid the guarding if statement with substituting parameters
    log.debug("log something {} and {}", param1, param2);
    // Avoid the guarding if statement with formatters
    log.debug("log something %s and %s", param1, param2);
    // Avoid the guarding if statement with lazy logging and lambdas
    log.debug("log something expensive: {}", () -> calculateExpensiveLoggingText());

@KroArtem What do you think?

@KroArtem
Copy link
Contributor Author

KroArtem commented Mar 7, 2021

@adangel , yep, I think this is the most appropriate clarification. Since different logging frameworks have different options, enumerating all possible ways to avoid guarding statement is a good thing.

@durimkryeziu
Copy link

I have a log statement as in the example docs like: log.debug("log something {} and {}", param1, param2); and PMD still complains? Why so?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
an:enhancement An improvement on existing features / rules in:documentation Affects the documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants