Re: [boost] [log] Comments

Subject: Re: [boost] [log] Comments
From: Andrey Semashev (andrey.semashev_at_[hidden])
Date: 2010-03-15 13:24:49

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.