[RFC] log.Logger interface

[mattfarina]

vsolver currently uses logrus. This can be really useful for testing. When vsolver is integrated into external applications the logger from that application should be used.

The proposal is switch vsolver to a standard log.Logger interface. In the testing use logrus to generate the output as needed. This can even be used in examples. logrus implements the log.Logger interface so it can be used.

This will help vsolver be a better library when imported into applications. It can use the native logging capabilities of the application.

Thoughts?

[sdboyer]

The solve tracer discussed in #5 will probably get its own logger, for which a log.Logger-style output would be sufficient. There's little question in my mind that that's the direction that should go. That tracer should be just as useful for a curious user as it would be for me/a developer.

However, there are other other classes of errors, and/or additional info about what the solver/SourceManager is doing, that may make more sense to express through a leveled logger. With all the disk state management we do, copious logging is a good idea - especially as things like #13 happen, and the source management problem space becomes temporally separate (its own goroutine(s)) from the solver itself.

So, yeah...no question that logrus is fugly where it is right now. But until the tracer is written and I see what information I want to leave in or out of that, I won't be quite clear on what to do with the rest of what logrus is currently covering, or what I want to do with the many other scattered logging-related TODOs.

[mattfarina]

@sdboyer Setting the logging mechanism is more of an application level piece than a library level piece. A library should not force the logger to use but instead work with a variety.

With that, let me propose this. Have a debug mode that is passed around. Then the implementing application can set the mode and pass in a logger. Then the logger writes debug output if the debug mode is set.

Then logrus only needs to be in the tests and maybe the example code.

[sdboyer]

Setting the logging mechanism is more of an application level piece than a library level piece. A library should not force the logger to use but instead work with a variety.

Deciding how logging data is ultimately directed is certainly an application level decision. Deciding how to encode events as log data, however, is not. There is a substantial conceptual difference between leveled and unleveled loggers, and the debate between their uses is long and storied.

With that, let me propose this. Have a debug mode that is passed around. Then the implementing application can set the mode and pass in a logger. Then the logger writes debug output if the debug mode is set.

You already have access to a debug mode: set the output level on the injected logger to logrus.DebugLevel.

I'm open to being convinced to drop the leveled logger (though your interest in doing so strikes me as odd, given that glide's msg package is a leveled logger). But all you've done so far is ask for capabilities that it already provides, and assert that it'd be a better arrangement of responsibilities to do it differently. I have a different viewpoint.

So, I need you to make a specific argument that addresses how a non-leveled, non-metadata-carrying logger is supposed to express...well, logging levels. Or, make a case for why logging levels aren't necessary - and ideally, tailor that argument to the particulars of this case. Additionally, please address the points that I've raised:

• what's being done with logrus now is going to be replaced with a trace logger which probably can be log.Logger-style...
• but there's still the matter of everything else that may or may not need logging
• that you do have the full capability of controlling the output, right now

[sdboyer]

I've thought about this some more. For what logrus is currently accomplishing, I think you're right - it'll be fine to just use a log.Logger, and have a flag for trace mode. I'll deal with the warnings, etc. later.

log.Logger is a struct, though, not an interface - that is what you want, right?

[sdboyer]

doing this in #21

[sdboyer]

A basically-OK version of this is now integrated into master. Here's the output now generated from go test -v: https://gist.github.com/sdboyer/eb578e95ad88db4821d19321bd345cc8