-
Notifications
You must be signed in to change notification settings - Fork 259
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
Transform liblog into a logging facade #3
Comments
This seems like a pretty great idea! I don't personally have many strong opinions about logging or what direction this library should go in, and I also don't mind deprecating this library as-is (just going into maintenance mode) to make room for another. Some thoughts:
This seems like a good idea.
Some use cases of logging may hurt because of this, but I'd be willing to believe that they're fairly few and somewhat niche.
This also seems like a good idea.
The one use case I know of for this is a unit testing framework where you want to capture each test's output individually. As you mentioned though you can build this on top of a global framework so it's probably not that bad to lose.
The lack of teardown seems somewhat worrying to me, but the one-time initialization is probably fine. The strategy I'm going to try to take elsewhere is that once
This seems somewhat reasonable. If I've invested time and effort into using a logging infrastructure I'm probably willing to set it up to get it running. |
Yeah, thinking about it a bit more, you'd definitely want teardown to flush buffers at the very least. I'm not a huge fan of having post-atexit calls panic. Seems like they should just be noops to avoid weird transient failures that are hard to reproduce. |
Adding at_exit cleanup wasn't that bad, though the implementation involves some "interesting" uses of atomics: https://github.com/sfackler/log-ng/commit/0bc99ed5b0e2b0f417257921cda78c5ef3612f7b.
I'd prefer to keep the implementation in the |
That's a good point, I wouldn't mind moving this crate more towards a different design, may want to discuss with a few others though :) |
I would like it if in addition to the dynamic logging level, there was a static logging level -- a compile time choice of what types of logging are even available. This should recover all the niche uses of the old system and impose no burdens the common case. If |
A static logging level seems like a great idea. I've been worrying a bit about
|
@Ericson2314 is this something like what you had in mind? https://github.com/sfackler/log-ng/commit/f40621c311ab82af8ded28b79566ccdcec427d99 |
Glad you like it! That's most of it. My thing about What do we use |
I believe We could potentially do something like this, though it's pretty bizzaro looking: macro_rules! debug {
($($arg:tt)*) => (
match () {
#[cfg(not(any(log_level = "off",
log_level = "error",
log_level = "warn",
log_level = "info")))]
() => log!(::log_ng::LogLevel::Debug, $($arg)*),
#[cfg(any(log_level = "off",
log_level = "error",
log_level = "warn",
log_level = "info")))]
() => (),
}
)
} |
That's a great trick! I'm now firmly in scrapping debug_assert. If debug_assert is /used/ by the standard library, not merely defined in it. liblog can be used, and just compiled out for release. |
Keep in mind that |
Well debugging and logging are related, broadly. And having one library to manage all optional sanity checks seems convenient. |
Hmm, I'm trying #[macro_use] #[no_link]
extern crate log_ng;
#[cfg(any(log_level = "off",
log_level = "error",
log_level = "warn",
log_level = "info",
log_level = "debug"))]
extern crate log_ng; But it doesn't seem to be working. Perhaps those |
Are you passing one of those to rustc? |
no, but the macros are asking for things in |
The macros expand inline into crates that use them, and that's where the cfg takes effect. |
Oh no. Is there anyway to delay that? |
See issue rust-lang#3 for background. Closes rust-lang#3 Closes rust-lang#7 Closes rust-lang#11
See issue rust-lang#3 for background. Closes rust-lang#3 Closes rust-lang#7 Closes rust-lang#11
See issue rust-lang#3 for background. Closes rust-lang#3 Closes rust-lang#7 Closes rust-lang#11
See issue rust-lang#3 for background. Closes rust-lang#3 Closes rust-lang#7 Closes rust-lang#11
See issue rust-lang#3 for background. Closes rust-lang#3 Closes rust-lang#7 Closes rust-lang#11
Fixed rust-lang#3 Signed-off-by: Jiahao XU <[email protected]>
There are a couple of distinct kinds of programs, each of which has different needs from a logger:
rustc
need a simple logging framework that provides some reasonable level of configurable filtering when trying to debug a problem.stderr
at all. They may, however, have the ability to log information through a serial port or network interface.The current liblog implementation covers the first and last use cases, but misses the middle two.
There are two competing concerns here: we want a single logging interface that libraries can use - it would be crazy to expect someone to provide different versions of a single crate that only differ by their choice of logging framework! But at the same time, we want the libraries to be usable in all of the contexts mentioned above, with their logging intact.
One solution is to turn
liblog
into a logging "facade". It provides the infrastructure to allow libraries to send log messages, while allowing the downstream application to chose the actual logging framework that will process the log messages. One example is the slf4j library for Java, which allows libraries to log in a way that will work with any of the many logging frameworks available in Java: java.util.logging, log4j, Logback, etc.I have a prototype conversion of liblog to a facade in the log-ng project. It consists of three crates:
log_ng
- the core facadebasic
- a port of liblog's currentRUST_LOG
based log implementationlog4r
- a prototype of a more complex logger in the vein of log4j and Logback.Documentation is available here: http://sfackler.github.io/log-ng/doc/log_ng/
Changes from
liblog
While converting
liblog
, I did make some changes to the current functionality:u32
, consisting ofOff
,Error
,Warn
,Info
,Debug
, andTrace
. Note thatTrace
is new. I don't really see the use in having 255 distinct levels, and it simplifies parsing and pretty-printing of the level if we can restrict the level to an enum.enabled
before callinglog
. It turned out when I was experimenting with logger implementations thatlog
can pretty trivially perform the filtering itself. If argument computation is expensive, thelog_enabled!
macro is still available, which will call through toenabled
.liblog
implementation does provide the ability to set the logger, but since it's task local, it's impossible to make sure the right logger is installed in each thread - think of threads started byTaskPool
for example. Having thread-local loggers also results in a log of duplicated data and computation. TheRUST_LOG
environment variable has to be reparsed every time a program logs on a new thread, for example. In addition, it's not totally clear to me that anyone actually wants to have different loggers on different threads. Even if one does, it's pretty easy to make a thread local logger implementation for the logging facade.ndebug
cfg is checked only indebug!
andtrace!
, not inlog!
as well. I'm not sure what the right thing to do is here, but it seems like completely forbidding the use of the debug and trace levels with optimizations enabled is over-aggressive, especially since the global maximum log level check makes them pretty cheap when those levels are disabled. We may want to havedebug!
andtrace!
always enabled, and have separate debug build only versions.Interesting notes/design decisions
atexit
but it would require storing the logger in anArc
and adding a couple extra atomic reads and writes every time a message is logged. I'm not sure what the right answer is here. I don't really think people will want to swap their loggers out dynamically, but cleanup at exit seems like a nice thing to do to avoid spamming Valgrind.Remaining problems/blockers
TestMain
function or junit's@BeforeClass
annotation to allow for global setup before any tests run. This is useful in other contexts as well for other global resource setup.Thoughts?
The text was updated successfully, but these errors were encountered: