Add logger interface and stop relying on Logrus directly

[sagikazarmark]

Fixes #868

Before merging: I placed the interface in pkg/log package because that's the standard place for reusable stuff. If you want it somewhere else, let me know.

Further (possible) improvements:

• implement the whole set of log functions (currently only the used ones are implemented, adding new ones later would mean breaking the interface)
• remove WithField and replace it with a constructor (used in one place only). It would result in a cleaner interface which does not reference itself, thus external implementations would not have to import dex itself to provide integration

[ericchiang]

thanks for doing this!

[sagikazarmark]

@ericchiang let me answer here, as most of your comments are related to a single problem.

The interface I added includes the following function:

WithField(key string, value interface{}) Logger

The same function in Logrus looks like this:

WithField(key string, value interface{}) *Entry

The problem is the interface has a reference to itself in order to remove dependency on Logrus. Because of this:

• the interface must be importable by external packages in order to implement it, thus it cannot be an internal package
• Logrus itself cannot satisfy the interface, so we need a wrapper

That WithField function is used in a single place:

dex/server/server.go

Line 489 in 8935a14

c, err = openConnector(s.logger.WithField("connector", conn.Name), conn)

It's inside the library part, that's why I added the function to the interface. But I didn't really have any heuristics about the implementation, I just created and implemented the interface.

So if dex code is not supposed to be used from external packages, the logger can be moved to an internal package (although then dex code should probably be moved to internal as well).

In case WithField method can somehow be replaced, then the interface wouldn't have to reference itself, it could be moved to an internal package and logrus would satisfy the interface without a wrapper.

[ericchiang]

I vote we drop our uses of WithField to simplify the Logger interface.

[sagikazarmark]

Well, I'm generally in favor of structured logging, but given there is a single place where it is used right now, I'm inclined to remove it as well, at least for now.

[sagikazarmark]

@ericchiang I removed the wrapper type and the structural logging.

I still think if any dex code is supposed to be called externally, then the logger interface should be exported too, but if you want, I can move it under internal/.

[ericchiang]

one nit.

dex code shouldn't be imported by other projects. We change the API and refactor all the time (like what this PR is doing). I won't block this PR on pkg/log instead of internal/log though.

lgtm, thanks for helping out here! 😃