Avoid potentially "expensive" rendering of message Object to String w…

…hen log level is disabled SLF4J-497
qos-ch · Jun 15, 2021 · 940c462 · 940c462
1 parent 6230948
commit 940c462
Show file tree

Hide file tree

Showing 2 changed files with 89 additions and 12 deletions.
diff --git a/jcl-over-slf4j/src/main/java/org/apache/commons/logging/impl/SLF4JLocationAwareLog.java b/jcl-over-slf4j/src/main/java/org/apache/commons/logging/impl/SLF4JLocationAwareLog.java
@@ -104,7 +104,9 @@ public boolean isFatalEnabled() {
      *          the message to log. Converted to {@link String}
      */
     public void trace(Object message) {
-        logger.log(null, FQCN, LocationAwareLogger.TRACE_INT, String.valueOf(message), null, null);
+        if (isTraceEnabled()) {
+            logger.log(null, FQCN, LocationAwareLogger.TRACE_INT, String.valueOf(message), null, null);
+        }
     }
 
     /**
@@ -117,7 +119,9 @@ public void trace(Object message) {
      *          the exception to log
      */
     public void trace(Object message, Throwable t) {
-        logger.log(null, FQCN, LocationAwareLogger.TRACE_INT, String.valueOf(message), null, t);
+        if (isTraceEnabled()) {
+            logger.log(null, FQCN, LocationAwareLogger.TRACE_INT, String.valueOf(message), null, t);
+        }
     }
 
     /**
@@ -128,7 +132,9 @@ public void trace(Object message, Throwable t) {
      *          the message to log. Converted to {@link String}
      */
     public void debug(Object message) {
-        logger.log(null, FQCN, LocationAwareLogger.DEBUG_INT, String.valueOf(message), null, null);
+        if (isDebugEnabled()) {
+            logger.log(null, FQCN, LocationAwareLogger.DEBUG_INT, String.valueOf(message), null, null);
+        }
     }
 
     /**
@@ -141,7 +147,9 @@ public void debug(Object message) {
      *          the exception to log
      */
     public void debug(Object message, Throwable t) {
-        logger.log(null, FQCN, LocationAwareLogger.DEBUG_INT, String.valueOf(message), null, t);
+        if (isDebugEnabled()) {
+            logger.log(null, FQCN, LocationAwareLogger.DEBUG_INT, String.valueOf(message), null, t);
+        }
     }
 
     /**
@@ -152,7 +160,9 @@ public void debug(Object message, Throwable t) {
      *          the message to log. Converted to {@link String}
      */
     public void info(Object message) {
-        logger.log(null, FQCN, LocationAwareLogger.INFO_INT, String.valueOf(message), null, null);
+        if (isInfoEnabled()) {
+            logger.log(null, FQCN, LocationAwareLogger.INFO_INT, String.valueOf(message), null, null);
+        }
     }
 
     /**
@@ -165,7 +175,9 @@ public void info(Object message) {
      *          the exception to log
      */
     public void info(Object message, Throwable t) {
-        logger.log(null, FQCN, LocationAwareLogger.INFO_INT, String.valueOf(message), null, t);
+        if (isInfoEnabled()) {
+            logger.log(null, FQCN, LocationAwareLogger.INFO_INT, String.valueOf(message), null, t);
+        }
     }
 
     /**
@@ -176,7 +188,9 @@ public void info(Object message, Throwable t) {
      *          the message to log. Converted to {@link String}
      */
     public void warn(Object message) {
-        logger.log(null, FQCN, LocationAwareLogger.WARN_INT, String.valueOf(message), null, null);
+        if (isWarnEnabled()) {
+            logger.log(null, FQCN, LocationAwareLogger.WARN_INT, String.valueOf(message), null, null);
+        }
     }
 
     /**
@@ -189,7 +203,9 @@ public void warn(Object message) {
      *          the exception to log
      */
     public void warn(Object message, Throwable t) {
-        logger.log(null, FQCN, LocationAwareLogger.WARN_INT, String.valueOf(message), null, t);
+        if (isWarnEnabled()) {
+            logger.log(null, FQCN, LocationAwareLogger.WARN_INT, String.valueOf(message), null, t);
+        }
     }
 
     /**
@@ -200,7 +216,9 @@ public void warn(Object message, Throwable t) {
      *          the message to log. Converted to {@link String}
      */
     public void error(Object message) {
-        logger.log(null, FQCN, LocationAwareLogger.ERROR_INT, String.valueOf(message), null, null);
+        if (isErrorEnabled()) {
+            logger.log(null, FQCN, LocationAwareLogger.ERROR_INT, String.valueOf(message), null, null);
+        }
     }
 
     /**
@@ -213,7 +231,9 @@ public void error(Object message) {
      *          the exception to log
      */
     public void error(Object message, Throwable t) {
-        logger.log(null, FQCN, LocationAwareLogger.ERROR_INT, String.valueOf(message), null, t);
+        if (isErrorEnabled()) {
+            logger.log(null, FQCN, LocationAwareLogger.ERROR_INT, String.valueOf(message), null, t);
+        }
     }
 
     /**
@@ -224,7 +244,9 @@ public void error(Object message, Throwable t) {
      *          the message to log. Converted to {@link String}
      */
     public void fatal(Object message) {
-        logger.log(null, FQCN, LocationAwareLogger.ERROR_INT, String.valueOf(message), null, null);
+        if (isErrorEnabled()) {
+            logger.log(null, FQCN, LocationAwareLogger.ERROR_INT, String.valueOf(message), null, null);
+        }
     }
 
     /**
@@ -237,7 +259,9 @@ public void fatal(Object message) {
      *          the exception to log
      */
     public void fatal(Object message, Throwable t) {
-        logger.log(null, FQCN, LocationAwareLogger.ERROR_INT, String.valueOf(message), null, t);
+        if (isErrorEnabled()) {
+            logger.log(null, FQCN, LocationAwareLogger.ERROR_INT, String.valueOf(message), null, t);
+        }
     }
 
     /**

diff --git a/jcl-over-slf4j/src/test/java/org/apache/commons/logging/InvokeJCLTest.java b/jcl-over-slf4j/src/test/java/org/apache/commons/logging/InvokeJCLTest.java
@@ -27,6 +27,7 @@
 
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertEquals;
 
 import org.junit.Test;
 
@@ -85,4 +86,56 @@ public void testPrintAPI() {
         log.fatal(null, e);
         log.fatal("fatal message", e);
     }
+
+    @Test
+    public void testAvoidConvertingObjectToString() {
+        Log log = LogFactory.getLog(InvokeJCLTest.class);
+        Exception e = new Exception("just testing");
+
+        TestMessage fatalMsg = new TestMessage("fatal msg");
+        TestMessage errorMsg = new TestMessage("error msg");
+        TestMessage warnMsg = new TestMessage("warn msg");
+        TestMessage infoMsg = new TestMessage("info msg");
+        TestMessage debugMsg = new TestMessage("debug msg");
+        TestMessage traceMsg = new TestMessage("trace msg");
+
+        log.fatal(fatalMsg);
+        log.fatal(fatalMsg, e);
+        assertEquals(2, fatalMsg.invokedCount);
+
+        log.error(errorMsg);
+        log.error(errorMsg, e);
+        assertEquals(2, errorMsg.invokedCount);
+
+        log.warn(warnMsg);
+        log.warn(warnMsg, e);
+        assertEquals(2, warnMsg.invokedCount);
+
+        log.info(infoMsg);
+        log.info(infoMsg, e);
+        assertEquals(2, infoMsg.invokedCount);
+
+        log.debug(debugMsg);
+        log.debug(debugMsg, e);
+        assertEquals(0, debugMsg.invokedCount);
+
+        log.trace(traceMsg);
+        log.trace(traceMsg, e);
+        assertEquals(0, traceMsg.invokedCount);
+    }
+
+    static class TestMessage {
+
+        private final String msg;
+        int invokedCount = 0;
+
+        TestMessage(String msg) {this.msg = msg;}
+
+        @Override
+        public String toString() {
+            invokedCount++;
+            return msg;
+        }
+    }
+
 }