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

If you're using SLF4J API, which is what it looks like you're using, I would recommend a slightly different approach.

To start, I believe you would be better off extending the Logger class. This would be beneficial because you would inherit the classes provided and can override the necessary classes.

Usually in Java when we are creating helper classes, we override the base class and then write whatever code we would like to supplement the already existing functionality.

Also, in the future please note that you should explain what you've written a bit more in the question, so that reviewers have an easier time understanding your code and thus can better help suggest improvements.

I hope this feedback helps you improve your code. If you have any questions, feel free to leave a comment.

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