Don't force using a log.Logger about analytics-go

[achille-roussel]

Having the method called Logf does make it incompatible with the standard logger, we could call the method Printf (which makes it then incompatible with the testing package... sounds like a design flaw), but we can work around with a trick like:

• converting a standard logger to an analytics.Logger

type Logger interface {
    // Compatible with the testing logger
    Logf(string, ...interface{})
}

func StdLogger(logger *log.Logger) Logger {
    return LoggerFunc(logger.Printf)
}

• converting a testing logger to an analytics.Logger

type Logger interface {
    // Compatible with the standard logger
    Printf(string, ...interface{})
}

func TestingLogger(t *testing.T) Logger {
    return LoggerFunc(t.Logf)
}

I'd say I like the first approach better so it doesn't create a dependency on the testing package outside of test code.

[f2prateek]

What if we just accepted an io.Writer?

[achille-roussel]

I think having the concept of a logger is better, that way the program can set a specific log format (with timestamps in a special format or prefixed with "analytics" to make it easy to filter for example).

[f2prateek]

Cool makes sense - how about something that distinguishes b/w the log type.

type Logger interface {
    Errorf(string, ...interface{})
    Printf(string, ...interface{})
}

Or a bit more complex as we do in the java library (https://github.com/segmentio/analytics-java/blob/master/analytics/src/main/java/com/segment/analytics/Log.java#L5-L11).

This gives more power, and lets users format the message depending on the error level (e.g. red for errors if a tty is attached).

[achille-roussel]

I'm down for a change of this kind but this interface is compatible with neither testing.T nor log.Logger.
If we change it to:

type Logger interface {
    Logf(string, ...interface{})
    Errorf(string, ...interface{})
}

It is then compatible with testing.T and has the side effect of making the test fail if Errorf is called (which may be cool, or may be weird if Errorf isn't called if verbose is set to false for example).

I what we need the most is a compatibility shim to the standard logger but it only has Printf, Panicf or Fatalf: https://golang.org/pkg/log/
We need to define what the mapping would be then, do we want something like this:

analytics	log
Logger.Logf	Logger.Printf
Logger.Errorf	Logger.Fatalf

or something like this (I like it better as it doesn't crash the program):

analytics	log
Logger.Logf	Logger.Printf("INFO: " + format, args...)
Logger.Errorf	Logger.Printf("ERROR: " + format, args...)

[f2prateek]

Yeah agreed, definitely prefer the latter in that case.

I think I'd prefer one of these 2 interfaces for the logger:

type Logger interface {
    // Compatible with the standard logger
    Printf(string, ...interface{})
}

// Provide adapter for test logger.

type Logger interface {
    // No way to make this compatible with the standard logger  without an adapter so lets just make it compatible with the test logger.
    Logf(string, ...interface{})
    Errorf(string, ...interface{})
}

// Provide adapter for standard logger.