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

Logger Composability Guidelines #267

Open
mitsuhiko opened this issue Apr 7, 2018 · 6 comments
Open

Logger Composability Guidelines #267

mitsuhiko opened this issue Apr 7, 2018 · 6 comments

Comments

@mitsuhiko
Copy link

This is not necessarily a feature request but maybe a request for guidelines. Currently a lot of crates provide something like try_init or init where a logger is set as the global handler. As there can only be one it means that once that happens nobody can change it (unlike for instance the panic hook which can be overridden).

As a result crates like sentry that want to get logger data in addition to dispatching to another logger run into the issue where the user needs to completely change their code to get it hooked. For instance the pretty_env_logger and many others do not even provide a convenient way to say "get me a logger with default config" but don't register it.

Would it make sense to encourage loggers to provide a standardized API that returns a logger but does not register it? A similar issue also happens with the global level where users themselves need to figure out what the highest level is that multiple loggers might need.

@sfackler
Copy link
Member

sfackler commented Apr 7, 2018

I think it would make sense to update the "implementing a logger" section of the docs to say that you should provide a constructor for your Log impl in addition to the direct init convenience methods, yeah.

@sfackler
Copy link
Member

sfackler commented Apr 8, 2018

One interesting question is who should control the max log level. Should the Log implementation's constructor set it up, or is that the job of whoever's using it?

@mitsuhiko
Copy link
Author

@sfackler i do wonder if it wouldn't make sense to have the Log have a new trait method that returns the global log level as option and if one is returned then it's being set when the logger is registered.

@sfackler
Copy link
Member

sfackler commented Apr 9, 2018

That would handle part of it, but there's still the case where the logger is reconfiguring itself at runtime.

@dpc
Copy link

dpc commented May 3, 2018

In slog loggers (which are called Drain) impl a trait. That seems like an idiomatic way to do it. This way they can be generalized over, composed, etc.

@KodrAus
Copy link
Contributor

KodrAus commented Nov 15, 2021

It's been a long time here. slog and log4rs have always offered a complete API for building logging pipelines, and libraries like tracing have also since come along and scaffolded out a lot of API work in this area. I don't imagine we'd want to replicate much of that here in log, but I think the universal advice is for libraries implementing Log to expose concrete types that callers can construct and work with directly.

EFanZh pushed a commit to EFanZh/log that referenced this issue Jul 23, 2023
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

4 participants