Make Logger a concrete type instead of an interface

[akshayjshah]

zapcore.Facility is the extension point we're offering to third-party
developers, so there's no need to make the logger an interface. In fact, an
interface will be more constraining, since we won't be able to add any methods
after version 1.0.

Along the way, this fixes our woes with AddCaller not working properly (#40).

[mention-bot]

@akshayjshah, thanks for your PR! By analyzing the history of the files in this pull request, we identified @jcorbin, @akabos and @prashantv to be potential reviewers.

[bufdev]

I REALLY don't like this, if I could I would HIGHLY fight against this change. I think that how golang made their Logger a struct is highly limiting - there's a lot of instances where writers of libraries want to abstract away what type of logging package is used, and end up wanting an interface anyways, and the overhead vtable lookup cost is very small.

https://godoc.org/github.com/sirupsen/logrus#StdLogger
https://github.com/grpc/grpc-go/blob/master/grpclog/logger.go
https://github.com/peter-edge/dlog-go

[ansel1]

@peter-edge if library authors want to abstract the logger they use, isn't it their business to define the Logger interface that library can consume? I assume the use case you are describing is: I'm a library, I want to log stuff, I want to use zap as my default logging library, but I want to allow others who use me to set a different logger. In that use case, can't I ( the library's author ) define an interface which zap.Logger happens to conforms to? That's a common pattern. You see that a lot in packages which use the stdlogger by default.

[bufdev]

It's a common pattern that most libraries (especially written by new gophers) ignore. I've seen way too many libraries locked into https://github.com/golang/glog because "oh google is using it, it must be the way to go" (and not realizing how much glog does) to not be wary of this pattern. I think one of the biggest mistakes golang made in the stdlib was to make Logger a struct, not an interface - it encourages this.

Zap could become the new glog in that sense, and I'm really nervous of that. If anything, make Logger an interface (so people gravitate towards that), and then have an UnsugaredLogger or something.

[bufdev]

Also this is why I added grpclog back in the way - a lot of people, even experienced gophers, do the lock-in, and then it messes with everyone.

[akshayjshah]

Peter and I are both internal to Uber, so we get to cheat and video chat :) For posterity, conclusions from our discussion:

• Since we already have zapcore.Facility and any zap.Logger interface would still reference zapcore.Field and zapcore.CheckedEntry, there's not much value in making it an interface.
• However, we can ship two interfaces for the sugared logger, one that includes the print/printf methods and one for the structured methods. Users of the sugared logger who don't need Desugar can depend on the interfaces instead of the concrete type.
• Docs should feature the sugared logger.

[jcorbin]

This may be the biggest pain point for pre 1.0 -> 1.0 conversions: a global s/zap.Logger/*zap.Logger/ (I mean sure, the yet to settle construction shape changes too, but that's at least a contained change)

[akshayjshah]

Luckily, that change is go fix-able.