go-logging icon indicating copy to clipboard operation
go-logging copied to clipboard

global SetLevel

Open jbenet opened this issue 9 years ago • 6 comments

We've multiple loggers, but would be nice to set the level of all of them in one place (e.g. normally all are at Error, but can bump one down specifically to debug that portion.

logging.SetLevel(logging.ERROR, "") // or "*"

jbenet avatar Oct 01 '14 08:10 jbenet

That sounds like a great feature.

I like the idea of thinking of this as the argument beeing a prefix, eg. "".

The level logic would not set all loggers beneath it to a level but the opposite; when fetching the level, stop when a defined level is found, splitting the module name by '/'.

op avatar Oct 01 '14 15:10 op

@op sgtm. In that case, I suggest prefixing everything with /. Many systems make the mistake of removing the leading slash (favoring things like "foo/bar", "foo/bar/baz"), but pathing systems have the leading / for good reasons, like namespacing/mounting.

Extending this analogy, i could totally see this making sense:

log1 := logging.MustGetLogger("/")
log2 := logging.MustGetLogger("/foo")
log3 := logging.MustGetLogger("/foo/bar")

logging.SetLevel(logging.ERROR, "/foo") // log2 and log2 are now at logging.ERROR

Or:

log1 := logging.RootLogger()  // has prefix of "/"
log2 := logging.NewLogger(log1, "foo")  // has prefix /foo
log3 := logging.NewLogger(log2, "bar") // has prefix /foo/bar

logging.SetLevel(logging.ERROR, "/foo") // log2 and log2 are now at logging.ERROR

Logger Tree

Note that up there I'm constructing loggers explicitly, whereas you prefer static loggers (GetLogger). It's your module and probably should think thrice before breaking the interface, but I think constructing Loggers explicitly makes sense. This is particularly apparent when instantiating objects in a module twice, and wanting to redirect their logging separately.

I think go-logging could work phenomenally well with a logger-tree model similar to how google's context module works. If you haven't seen it, check it out. it's really a great piece of interface design. http://godoc.org/code.google.com/p/go.net/context

jbenet avatar Oct 02 '14 06:10 jbenet

@jbenet That's worth a thought. Why do you want to have relative paths for loggers though? I believe that might lead to code that is harder to reason about. Very flexible, I agree.

I've been thinking of going the opposite route. Somehow, make it possible to just say logging.New(), and have that automatically figure out the package path and re-use that.

Keep the ideas coming. :)

op avatar Oct 02 '14 17:10 op

Why do you want to have relative paths for loggers though? I believe that might lead to code that is harder to reason about. Very flexible, I agree.

On the contrary. Static initialziation is very hard to reason about / deal with when changing the package structure or instantiation of things. I'd follow the context example.

logging.New(), and have that automatically figure out the package path and re-use that.

seems too magic to me. sometimes i don't care about logging per package, but --say-- per incoming request. (useful to change logging level on specific requests across packages)

jbenet avatar Oct 07 '14 22:10 jbenet

One more reason. being able to do this (on a per-request, or whatever, basis):

log := logger.WithPrefix("[Peer: %s] ", local.peer.ID())

log.Debug("trying to do something with %s and %s, with %s", thisThing, thatThing, anotherPeer.ID())
// [Peer: abcbacbacbbacbabc] trying to do something with aaaaa and bbbbb, with dfdffdfdfdffdfdfdfd

log.Error("something bad happened: %s", err)  
// [Peer: abcbacbacbbacbabc] something bad happened: all the things broke.

Instead of, everywhere:

log.Debug("[Peer: %s] trying to do something with %s and %s, with %s", local.peer.ID(), thisThing, thatThing, anotherPeer.ID())
// [Peer: abcbacbacbbacbabc] trying to do something with aaaaa and bbbbb, with dfdffdfdfdffdfdfdfd

log.Error("[Peer: %s] something bad happened: %s", local.peer.ID(), err)  
// [Peer: abcbacbacbbacbabc] something bad happened: all the things broke.

jbenet avatar Oct 08 '14 03:10 jbenet

Instead of "WithPrefix", I would check the design of structlog and go that way:

log := log.Bind("Peer", local.peer.ID())
log = log.Bind("Other", foobar)
log.Debug("doing something")

so basically store a map[string]interface{} and dump that as a prefix. You need a value semantic for return values, but it's more powerful because you can even overwrite an object by simply rebinding it. Moreover, it opens the door to having a json.formatter to dump logs in a machine-parsable format.

rasky avatar Dec 09 '14 02:12 rasky