Use off-the-shelf logger logrus

[bboreham]

As discussed in #664

We have our own formatting routine, because the standard logrus one has surprising behaviour like adding extra spaces on the end of lines and inserting colour-changing escape sequences.

One side-effect of this change is that all logging now comes on stderr, rather than some on stdout and some on stderr as before. The weave script needs two changes to accommodate this. Closes #1039.

The previous style of having Debug.Println() or Error.Println() isn't supported by logrus, so such code changes to Log.Debugln() and Log.Errorln() respectively. We provide an Info reference so Info.Println() still works.

[rade]

I am curious what the resolution to #329 would look like with this change. To date that's the only justification provided for dragging in an external logging library.

[bboreham]

#329 is implemented at 11ef0fe

(actually more than was asked for, since #329 is only about DNS)

[inercia]

LGTM.

I was wondering if logrus could make things easier for identifying the module/package where the log line was generated, so we could get rid of things like [mdns] in "[mdns] ..." lines...

[bboreham]

@inercia not sure whether you meant easier in the code that generates the log line or easier for the person reading the log file. I'll answer the former.

The typical pattern I've seen is using runtime.Caller, but it requires that you know how many stack frames to go up before you hit the "real" call. Which is worse than where we are now, IMO.

It should be possible to use logrus' "fields" feature to set the module and bring it out again in the formatter.

[inercia]

@bboreham Yes, the runtime.Caller would be too hackish for me, so I guess we could use log.WithFields(log.Fields{"module": "mdns",})... or maybe we should create a common fun LogM(m string) { return Log.WithFields(log.Fields{"module": m}) }

[bboreham]

I suggest (a) that should be filed as a separate issue and (b) we should carefully consider what the log-file format of such messages should be. It is very useful to be able to reliably search them, so a format like module=mdns; is more search-friendly and less human-friendly, and [mdns] more the other way round.

[inercia]

I agree with both points. Parseability is probably not really necessary. I was more concerned with a) a common calling convention and b) a common log line formatting...

[bboreham]

Quick note: I should fix the way millisecond numbers are currently stripped of trailing zeros.

[bboreham]

I fixed the milliseconds and rebased off master.

[paulbellamy]

Why not get rid of common.Info?

[bboreham]

It was an extra ~50 edits, and Info.Printf seemed a reasonable way to do logging, so I aliased Log to make it work.

I see the argument that it's more consistent to lose it. I could be swayed.

[paulbellamy]

sway sway sway

[inercia]

I would remove it... 👍

[rade]

remove it.

[bboreham]

Removed.