On 03/15/2010 01:45 PM, Vladimir Prus wrote:
Looking at the proposed libraries, I see two major issues:
* There's no default setup that is comparable with the two mechanisms above. BOOST_LOG_TRIVIAL has only severity and it appears adding component information is complicated. In a sense, it looks like the library provides building blocks, but not a solution I can immediately use.
Right. There is no silver bullet for everyone. I learned that watching the first review of the library by John Torjo, participating his second review and collecting the requirements and feedback on my library. This review confirms it even further. Therefore, it's useful to provide a flexible library with a few most obvious and frequent cases available at hand and a great potential of extension and configurability. Regarding the trivial logging. As the naming implies, it is trivial, and is intended to be a drop-in replacement for std::cerr, but with boosters. (pardon my pun). :) But I understand that one or two options could be of use, such as the file name.
* It appears that out of the box, the proposed library does not address this component/severity style of logging that I've explained above. The proposed solutions at:
http://permalink.gmane.org/gmane.comp.lib.boost.devel/200793
sound relatively complex.
I can't provide logger features for every case. Sometimes users will have to extend the library. Writing a class template with a couple constructors and a trivial method doesn't look very complex to me.
* The example of specifying filter without lambda functions given in http://permalink.gmane.org/gmane.comp.lib.boost.devel/200793 is frankly scary. I believe that in most languages and environments I worked with, lambda functions are just convenient alternative to defining the same function explicitly. Why is in Boost.Log, lambda expressions are blessed with extra convenience?
It's not scary, it's verbose. If you want to get down to the guts of the library, you're welcome. You get the full control at the cost of verbosity. But most people won't need it and will just use the shorthands, such as "extract" and "attr". I see no problem with it.
* Why can't I use the library without named parameters? Surely, there are low-tech ways to specify things.
What are these? Boost.Parameter is used in various places of the library, but its primary goal is to provide abstraction between components that would otherwise be tightly coupled. For instance, loggers consist of number of features, each of which is independent from others. Named parameters elegantly allow to interact with a particular feature without bothering other features.
* Why does channel_logger has a *single* parameter, and still requires that you pass it via named parameter. That is:
src::channel_logger<> cl("foobar");
produces incomprehensible error message.
Yes, that should be fixed.
* The library went too far with namespaces. Why are *six* nested namespaces necessary? This appears to be just asking for extra typing for no good reason.
Use namespace aliases. Most of the time you will only need the sources namespace. Other namespaces were introduced to keep code clarity and avoid name clashes.
* Why is trivial logging printing the sequental number of each log message? I don't think this helps anything.
Record identifiers proved to be very useful in communication, surrounding the piece of log. It's much easier to write "see, there's that error in line 2764" than to go for lengthy explanations of how to find it in the file.
Also, it prints the thread "number", on my system a hex string. I am not sure thread id should be part of trivial logging.
Why not? Why can't a multithreaded application use trivial logging?
Finally, I think by default, everybody things of logging as glorified std::cout, so trivial logging better print messages to console, not to a file.
I disagree. First, you don't need the library to spam the console. Second, I consider spamming the console to be a bad idea in the first place. In my practice there are cases when writing _anything_ on the console is forbidden.
* Why does the library have light_rw_mutex? I would really prefer if everything thread-related be located inside Boost.Thread, so that at least the code can be readily audited.
light_rw_mutex can be more efficient than shared_mutex. If it gets to Boost.Thread, I'll be first to use it.
* It appears that the default behaviour of fmt::attr is to throw is the attribute with such name is not found. This seems like a bad choice to me. A logging library should never fail -- it is the last resort at diagnosing a problem in the field, and if it throws by default, it's useless.
That was my thought initially. Then this thread had happened: http://article.gmane.org/gmane.comp.lib.boost.devel/185956 You can suppress exceptions with exception handlers on different levels of the library.
I'd recommend working with maintainer of boost/throw_exception.hpp to add noreturn attributes on gcc, and silence the warning in other ways on other compilers.
AFAIK, it already has that attribute. Not sure why it doesn't work for you. What compiler do you use?
* Building the library with threading=single results in unresolved references to pthread functions. I belive that library should not use any sync mechanisms in this case.
Yes, I've been told that. Presumably, the references are from Boost.ASIO, which is used by the syslog backend. Not sure what would be the best solution - to drop single-threaded builds or to exclude syslog in ST builds.