Re: [boost] [log] Comments

Subject: Re: [boost] [log] Comments
From: Vladimir Prus (ghost_at_[hidden])
Date: 2010-03-15 14:10:50

On Monday 15 March 2010 20:24:49 Andrey Semashev wrote:

> 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.

First, it seems to me that if two major projects use component/severity scheme,
then presumably it's of general utility. Second, in the thread you have
mentioned, you yourself provide example solution that are far from trivial.
Maybe I'm missing something? In essence, I would like to have:

        magic_logger lg;
        DO_MAGIC(lg, "my_component", debug) << "hi there";

Could you show what will it take?

> > * 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.

As already expressed by Tom, logging library is fairly basic component
that should be useable by everybody. And lambda functions in current C++
in not something everybody know so forcing the user to pick between
using lambda, and much more verbose non-lambda style does not help.
Another aspect mentioned during review is that you roll your own lambda
implementation. Which means that folks that are familiar with boost::lambda
might run into things that don't work, or work differently. Further,
suppose one has access to a compiler that already implement native lambda.
He probably would prefer that to your lambda emulation, but you impose
a convenience tax on that.

Why cannot you provide and equally convenient ordinary functions?
 
> > * Why can't I use the library without named parameters? Surely,
> > there are low-tech ways to specify things.
> 
> What are these?

The simplest would be:

        file_log_parameters p;
        p.filter = ... ;
        p.formatter = ... ;
        init_log_to_file(p);

This is straightward for any user, produces clear error messages,
and is not much of a inconvenience.

> 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.

Could you give more examples?

> > * 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.

Of course I can use namespace aliases to alleviate the problem. However,
I don't see how namespaces are necessary. Are there actually things with
the same names in those different namespaces?

> > * 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.

What other libraries use this record id by default?

> > 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?

Because hex string, in a log, is completely useless. It is of any use if you
are attached to a still-running application, and can actually figure what
a thread id means in your program.

> > 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. 

Well, I do. Maybe because my console is usually not cmd.exe ;-)

> 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.

It seems like a fairly specialized requirement. What is trivial logging
supposed to be? If it is supposed to be printf logging on steriods, then it
should behave like printf as much as possible, including printing to console.

> > * 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. 

Can, or is?

> If it gets to  Boost.Thread, I'll be first to use it.

It's not like maintainer of Boost.Thread left on a space mission to outer
space and is not coming back ;-) Could you just propose this for inclusion?
We have too many bits hidden in detail namespace of various libraries.

> 
> > * 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 saw that -- my point is that *default* behaviour should be to go on no
matter what is wrong. 
 
> > 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?

gcc 4.4 with relatively recent svn trunk.

> 
> > * 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.

Well, if this the case, Boost.ASIO should be fixed not to contain its own
thread-related mechanisms -- which only reinforces my position about
rw_mutex.

- Volodya