I was trying to wrap a Logback logger in order to provide some handy methods and already defined default keys of the logged json output and I came up with something like this.

Do you spot any problem with it, or do you have any suggestion?

public class Log {



    private Logger logger = LoggerFactory.getLogger("jsonLogger");



    private String msg;

    private long start;

    private Map<String, Object> kvs;



    private Log(String msg) {

        this.msg = msg;

        this.start = System.currentTimeMillis();

        this.kvs = new HashMap<>();

    }



    public static Log msg(String msg) {

        return new Log(msg);

    }



    public Log kv(String key, Object value) {

        kvs.put(key, value);

        return this;

    }



    public void log() {

        kvs.put("elapsed", System.currentTimeMillis() - start);

        List<StructuredArgument> args = new ArrayList<>();

        for (String k : kvs.keySet()) {

            args.add(StructuredArguments.kv(k, kvs.get(k)));

        }

        logger.info(msg, args.toArray());

    }



}

In this way you can log just a message, custom key-values and have a way to keep track of the execution between a function:

// just a message

Log.msg("my message").log();



// message with custom kv or complex objects

Log.msg("my message").kv("foo", "bar").kv("arr", new String{"a", "b", "c"}).log();



// elapsed time

Log logger = Log.msg("my message").kv("foo", "bar");

Thread.sleep(1240);

logger.log();

i do not think this is really a good idea (sorry to say that)...

why is is not a good idea? it's breaking the concept of segregaton of concerns:

1. a Logger is responsible for logging


2. a Profiler is responsible for measuring execution time


3. a Logger is NOT responsible for keeping track of items (who is responsible for cleaning these entries?)

another issue i see is that you create a new logger whenever you want to write something into you log, i don't see any reason for that...

what would i suggest?

create a timesheet

public class Timesheet{



   private final Map<TimeStamp, KeyValue kv> entries;

   private final Logger logger;



    public Timesheet(Logger logger){

        this.logger = logger

        entries= ... 

    }



    public Timesheet kv(Object key, Object value) {

        TimeStamp timestamp = new TimeStamp();

        entries.put(timestamp , new KeyValue(key, value));

        log.debug("adding key/value {}/{} at {}", key, value, timestamp );

        return this;

    }



    public Timesheet msg(Object msg) {

        logger.debug(msg);

        return this;

    }



    public Timesheet elapsed() {

        logger.debug("time elapsed {}", calculateTimeElapsed());

        return this;

    }



}

not all is implemented here, it should just girve you an idea of how you could do it...

Timesheet timesheet = new Timesheet(LoggerFactory.getLogger("jsonLogger")); //now availible for all classes

timesheet.msg("hello");

timesheet.kv("foo", "bar");

Thread.sleep(1234);

timesheet.elapsed();

some final words...

i think it is not a good idea of using KeyValue pairs - if you know what kind of entries these are, give them a proper name!

i think if you would have choosen a more compliant name (Log is really confusing) your idea would not have been so bad...