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

Changing the defaults #18

Open
ansel1 opened this issue Dec 2, 2015 · 5 comments
Open

Changing the defaults #18

ansel1 opened this issue Dec 2, 2015 · 5 comments

Comments

@ansel1
Copy link

ansel1 commented Dec 2, 2015

I've been using logxi for a while. One of the things that continues to itch is that the defaults are very hard to control.

logxi establishes it's settings by reading environment variables in a package init() function. All loggers created after that pick up those settings. It's not possible to reliable modify those environment variables or change logxi's built-in defaults before logxi's init() function is executed.

What we're doing is running logxi's ProcessLogix*Env() functions, then recreating all our loggers to pick up the new settings, but that has a big drawback. It means there needs to be a single block of code that knows about all the loggers that need to be recreated. That discourages the use of package-private loggers.

I'm not sure what or if there's a good solution. I see there's a logger map protected by a mutex with a comment about future support for runtime configuration changes...but since most code has direct references to logger instances...there would need to be an additional, concurrent-safe indirection between logger references and logger configurations that could change.

For now, I'm considering just forking logxi and added the bootstrap code to load environment defaults before logxi's init sequence.

@mgutz
Copy link
Owner

mgutz commented Dec 3, 2015

I'm not sure if this helps. The way I use logxi for package-private logging is to create a logger with a namespace.

//// models/init.go
var logger  = log.New("my-models")
//// controllers/init.go
var logger = log.New("my-controllers")

You can then control how each package logs

LOGXI=my-models=DBG,my-controllers=ERR

@ansel1
Copy link
Author

ansel1 commented Dec 3, 2015

That's what we do too, but we want different defaults for LOGXI and LOGXI_FORMAT when running tests. Our makefile can set these, but the makefile is only useful when you want to run all the project's tests. When we are focussing on individual packages or tests, we're using "go test ..." and its tougher to manage a different set of environment variables just for those test runs. For our other configuration options (also configured via environment variables), we use a .env loader at runtime, but that happens too late for logxi...by the time we've run that, logxi has already configured itself.

Maybe I can just use the Suppress() function to turn off logs completely during tests.

@mgutz
Copy link
Owner

mgutz commented Dec 6, 2015

I need to refactor how logxi reads environment vars. The unit tests are more complicated than they need to be because of it. Do you want to take a stab at refactoring it? I have limited time.

@ansel1
Copy link
Author

ansel1 commented Dec 7, 2015

Thoughts on how to refactor? I'm a little torn on the best approach. There are good reasons to init logging in init() functions (mainly so logging can happen as early as possible in boot up). If I add a more robust way to reconfigure existing loggers, it seems I'll have to add at least one extra mutex into the logger calls, which will impact performance. Or move loggers to channels to avoid the lock, but again, not sure what the performance impact will be...

There's currently a "concurrent writer" which has a mutex...maybe I could move that mutex up the stack, to a "ConcurrentLogger"? That would avoid adding another mutex.

@ansel1
Copy link
Author

ansel1 commented Dec 7, 2015

Another simpler approach would be passing defaults to log.New().

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