1

This is the thing: I want to improve the Logging system in Our Apps.

We have multiple services Running on Cloud (irrelevant).

Those services each one Log everything in their Logs, for example:

public class class1 {

protected Logger logger = 
Logger.getLogger(Service1 .class.getName());


  public method1 (){

   ...
   logger.info("....."); //It creates a new String
   ...
   logger.debug("..."); //It creates a new String
   .
   .
   .
  }

  public method2 (){

   ...
   logger.info("....."); //It creates a new String
   ...
   logger.debug("..."); //It creates a new String
   .
   .
   .
  }
}

I want to improove this code and standardize the Logging System, but I'm not a Java Expert and I need help to know what's the best approach for this?

I got an Idea, here I go:

I'm thinking to create a Utility Class that manage all Logging Messaging (I think this approach would be with StringBuffer for synchronize) or create an StringBuilder on each method and reuse it, something like this:

public class class1 {

  protected Logger logger = 
  Logger.getLogger(Service1 .class.getName());


  public method1 (){
    StringBuilder msg = new StringBuilder();

    msg.append(" ...").append(" xx ").append(" yy "); //new message
    logger.info(msg); 
    ...
    msg.setLength(0); //Reset
    msg.append(" ...").append(" xx ").append(" yy "); //new message
    logger.debug("..."); 
    .
    .
    .
  }

  public method2 (){

    StringBuilder msg = new StringBuilder();

    msg.append(" ...").append(" xx ").append(" yy "); //new message 
    reusing builder
    logger.info(msg); 
    ...
    msg.setLength(0); //Reset
    msg.append(" ...").append(" xx ").append(" yy "); //new message 
    reusing builder
    logger.debug(msg);
    .
    .
    .
  }
}

What do you think?

PD: We are using Log4j and my english is not perfect. This approach is to avoid Sonar fails for doing this:

logger.debug("..." + " " + someVariable + " " + otherVariable);

I hope you understand to me =)

EJoshuaS - Stand with Ukraine
  • 11,977
  • 56
  • 49
  • 78
fernando Pineda
  • 21
  • 4
  • All of your proposed alternatives still create Strings. For example, `" xx "` and `" yy "`. – John Bollinger Feb 02 '18 at 16:29
  • 1
    My Favorite is to use `String.format("Error from %s class: %s", "MyClass", ex.getMessage())` since it allows to see the log message all at once and is NPE free. – LMC Feb 02 '18 at 16:29
  • 1
    You'd be reinventing the wheel, and using an anti-pattern to fix an anti-pattern. I recommend you use some kind of aspect-oriented mechanism for logging, alongside leveraging your logging framework's format-driven functionalities to standardize a format. – Mena Feb 02 '18 at 16:30
  • Don't try to improve things when you admit yourself that you're "not a Java Expert". You will only make things worse. – Kayaman Feb 02 '18 at 16:39
  • @Kayaman this is the reason because I'm asking, **right**? – fernando Pineda Feb 02 '18 at 16:42
  • Once common technique is to check if debug level is enabled before actually creating message string and logging it: `if ( logger.isDebugEnabled() ) logger.debug(//create your expensive string);` – tsolakp Feb 02 '18 at 17:06
  • @tsolakp that's very rarely needed, thanks to placeholders. – Kayaman Feb 02 '18 at 17:20
  • @Kayaman. Do you mean placeholders like in String.format? – tsolakp Feb 02 '18 at 17:22
  • @tsolakp yeah. It was once a common anti-pattern to pepper `if(logger.isDebugEnable())` everywhere for perceived performance improvement. The code is a lot more readable with placeholders too. – Kayaman Feb 02 '18 at 17:24
  • @Kayaman. I don't argue that `String.format` being more readable but how does it relate to performance of creating String that is passed to `logger.debug` versus just using concatenation or `StringBuilder`? – tsolakp Feb 02 '18 at 17:30
  • @tsolakp I meant placeholders (take a look at the accepted answer), I wasn't talking about `String.format()`. You seem to be confused. Of course if you're using some ancient logging library, it might not have placeholders, but if you choose to use ancient libraries it's your own fault. – Kayaman Feb 02 '18 at 17:31
  • @Kayaman. That's why my original question wanted to clarify. Looks like Logger placeholders are same as String.format when non MessageSupplier method is called. With `MessageSupplier` I am wondering if we will be creating too many objects with those lambda expressions. – tsolakp Feb 02 '18 at 17:45
  • @tsolakp well they're sort of the same. They just avoid the unnecessary String building if the logging level isn't enabled. Additional things like checking the logging level or providing suppliers are very rarely needed. There's no need to overcomplicate things, it's just logging. There are more important performance hotspots to worry about. – Kayaman Feb 02 '18 at 17:53

1 Answers1

3

If you are able to use log4j 2.x, you can let the library take care of it by using parameters:

logger.debug("... {} {}", someVariable, otherVariable);

See Logger javadocs for more info.

Note that even if you have a dependency bound to 1.x, the log4j team explicitly changed the package structure for 2.x so as to avoid collisions. You can use both together freely without fear of compatibility issues (just make sure that you are importing the correct Logger class).

rmlan
  • 4,387
  • 2
  • 21
  • 28