[WEB][42108] Best practice use logger.error

[JulienBertrand]

\newpage

Best practice use logger.error

Platform

OS	OS version	Langage
{Windows}	{}	{JAVA}

Main caracteristics

ID	Title	Category	Sub-category
CRJVM005-42108	{Wrong use of logger.error for logging }	{Logging}	{Aggregator Logger}

Severity / Remediation Cost

• Case 1:

Severity	Remediation Cost
Minor	Minor

Rule short description

• Case 1: Wrong use of logger.error for aggregator logger

Rule complete description

Text

{When running an application, loggers in error severity shouldn't be run in the main body of a method. In production, an application with many users can result in a very large amount of logs and costly in performance. Incorrect use of loggers and their severity can result in too many network calls and memory overflow of splunk servers. So, It's recommented to use the error severity of your logger in a catch or failure zone of the program.}

JAVA Example

// Uncompliant code:
public void save(){
    double currentValue = generateValue();
    if (currentValue != 0) {
        logger.error("Before Save");
        saveCurrentValue(currentValue);
    }
}

// Compliant code:
public void save(){
    double currentValue = generateValue();
    if (currentValue != 0) {
        try{
            logger.debug("Before Save");
            saveCurrentValue(currentValue);
        } catch(Exception e){
            logger.error(e.getStackTrace());
        }
    }
}

Implementation principle

• 1- detect the call of a logger
• 2- detect the type of severity used on this logger
• 3- detect that the logger is not called before an exception is triggered (for example: not called in the catch of a try/catch)
• 4- If logger error called outside a catch, report the line

Source for understanding the difference in the use of logger severity: https://logging.apache.org/log4j/1.2/apidocs/org/apache/log4j/Level.html

[jhertout]

Even if it seems to be something for ecoCode in this rule, I can see several problems:

• We do not have anything (document, measure or other...) that show us the impact in term of energy for the rule
• The rule is oriented to talk about "performance". Even if it can be related to energy, it is not the same thing. At least the rule should be reworded.
• The rule is to generic for me "loggers in error severity shouldn't be run in the main body of a method" and in the exemple we are in a "if", seems contradictory.
• The link in the description: https://logging.apache.org/log4j/1.x/apidocs/org/apache/log4j/Level.html is only the Javadoc of the Level class, I do not understand why it is relevant with the issue.

For me, this rule needs more work before being implemented in ecoCode.