Remove logxi dependency

[richp10]

Personal preference is logrus but maybe best to keep generic "log" - up to you, but I strongly think logxi has to go as it created all sorts of problems.

There, I have that off my chest!

[bkono]

Ah, this is the first big one to go tackle. I'm in 100% agreement that the hard dependency on logxi should be removed. I'm also very much in the camp that:

a reusable package should avoid as much as possible to log anything, and return errors instead where it makes sense and let the caller worry about logging.
ref: https://www.0value.com/about-go-logging

Given the nature of this library, there are both interesting data points and things that are genuinely useful to log... just not in the way the package currently does it. Following that train of thought, I think it makes sense to categorize what is currently logged, what is worth logging, and what a consumer actually wants to have access to. In my cursory review there are a number of points for debug logging that may or may not be of interest, a few fatals to force a fail fast scenario (though it may not be fast enough, and can lead to unpleasant runtime behavior), the query logging features (both debug style and slow indicators), and a final grouping of Warn and Error that in reality appear to make up for the fact that the api itself isn't returning error information to act on.

If we don't want to make any breaking changes to the api, my initial vote is to provide a simple func or Logger interface, and a few SetLogger* options (possibly on runner.DB?) for debug/trace, warn/error, and sql statement logging, using the pretty well accepted func (string, interface{}...) style that can easily be met with both Printf and structured logging libraries either directly or with a really minimal adapter function. In addition, we can simply default them to noop or nil (depending on impl) and avoid the potentially expensive writes completely without needing the additional logxi isLevel* set of logger methods.

Some additional items of interest if/when we want to make bigger changes:

• Fix the APIs so that errors are returned instead of logging an error and providing back nil. Get a little more idiomatic Go in terms of treating errors as data to operate on.
• Provide metrics as a type of event source. The slow query logging is interesting, but actually getting that data out in a form other than a log would be far more useful, especially if looking to tie it into an application's other metric monitoring tools (datadog, promethus, influx, etc).
• Review how and when sql queries are logged. This was one of my few painful-in-production moments, when I realized the slow query logging was coupled to actually spitting out the sql string leading to a data leak in the logs.

The above is of course purely IMHO, and I'm open to alternative views or approaches. If we do decide to go down this path, I'm more than happy to get an experimental PR going to discuss what this looks like in practice.

Thoughts? cc @pascallouisperez @cbelsole @richp10

[richp10]

Sounds good. Although I did not use it much before giving up and replacing - I found the logging to be insufficiently granular. Lots of slow query log messages cluttering up without an obvious way to tweak the threshold of what is meant by slow. Your proposed approach above sounds good to me, but in terms of decision I defer to experience.

[bkono]

@richp10 @pascallouisperez @cbelsole Did a fairly simplistic first pass of this to get the discussion rolling. Still needs some more testing, examples of using other log packages, etc... but its a starting point. In this PR all specs are passing, and logxi is removed as a dependency completely (confirmed via vgo vendor, building, etc). #10.

Apologies in advance, I did it off my vgo branch, so its probably best to look at just the last commit in isolation to avoid the influx of /vendor/.