Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SetHandler() sets handler for all logs in all packages importing same log15 path #16

Closed
sbward opened this issue Jul 24, 2014 · 3 comments

Comments

@sbward
Copy link

sbward commented Jul 24, 2014

If a package importing "gopkg.in/inconshreveable/log15.v2" imports another package which also uses "gopkg.in/inconshreveable/log15.v2", then any calls to l.SetHandler(...) for any logger will overwrite the handler in all loggers across both of those packages.

The reason is that there's a shared root in the package which always references the same underlying log.Handler even after calling l.New() on it.

Your documentation suggests initializing logging like this:

package yourlib

import "gopkg.in/inconshreveable/log15.v2"

var Log = log.New()

func init() {
    Log.SetHandler(log.DiscardHandler())
}

I tried this method and got burned because both my main package and my library imported "gopkg.in/inconshreveable/log15.v2". Suddenly my logs were gone because the library changed the singular root logger's handler to log.DiscardHandler.

I recommend changing the semantics of log15.New() to do a completely new object without a shared Root at the bottom, while allowing Logger.New() to perform shared state chaining.

@inconshreveable
Copy link
Owner

Sorry for the slow response.

You're right. This is very broken. The internal model I had for this in my head was:

A logger should inherit the handler of its parent until a call to SetHandler changes it.

Does that seem like a reasonable answer? It's a little bit different than what you propose in your PR

@sbward
Copy link
Author

sbward commented Jul 28, 2014

No worries, and yes, that does sound reasonable to me.

@inconshreveable
Copy link
Owner

Actual fix is here, the first one would have broken something else:

da1a931

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants