Fluent interface for Logger

Question

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();

Answer 1

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...