Add caller (file:line number) to non-tracing entries?
Any opinions on this? It's available from runtime.Caller. Following the current design, it could be enabled by having a function like log.IncludeCaller(bool) and then every entry would include a field caller with value file:line number). Could also implement this as the default log library handles flags, e.g. log.SetFlags(log.LstdFlags | log.Lshortfile).
I'm happy to create a PR for this if you'd like.
It's currently enabled for Error() calls. Personally I don't rely on file or line numbers at all, with pkg/errors wrapping them I find it easy to pinpoint, but an option to enable it for non-error calls couldn't hurt!
For reference it's https://github.com/apex/log/commit/89ce6f11b2dc2a7603f3429d7b3076c526980841. The hacky splitting stuff is because pkg/errors only exposes the obscure Format string :(
Sounds good. :) I'll throw something together.
I decided to not add the runtime.* stuff because I almost always return errors, only my top-most app level has logging, so at that point the file/line is pretty irrelevant, happy to hear what other people think there though. I'm not sure it's super useful.
At least for me (and where I work), one of use cases is that it helps to quickly track exactly where that log message occurred. If a log message is unique, then you can grep for that—but even so, instead of having to grep, you have a reference to where it's from.
I probably should have named the other fields something else, think this should take precedence if enabled? the pkg/error one would be a bit more informative, or a full stack I suppose would be ideal there but that's a lot of noise
Implementation-wise, it might be easier to let the WithError() source take precedence. From my cursory look, I was thinking of a change like this:
func NewEntry(log *Logger) *Entry {
e := &Entry{
Logger: log,
}
if logger, ok := Log.(*Logger); ok {
if logger.IncludeSource {
// Set source field
}
}
return e
}
Then when WithError() creates its own Fields, then the above source will effectively be ignored.
that never happened did it?
Nope, and my needs changed at the time, IIRC.