[boost] [log] Boost.Log Formal Review

Subject: [boost] [log] Boost.Log Formal Review
From: sam.gundry_at_[hidden]
Date: 2010-03-16 20:52:26

(This is my first Boost library review so take it easy...)

> Please explicitly state in your review whether the library should be accepted.

Yes. There are some concerns but I think it is very nearly complete, as far as my usage is concerned.

These concerns are elicited in detail below but for now my three major ones are:
- __LINE__, __FILE__ as attributes
- Boost.DateTime formatting is sloooowwww
- Ability to strip/compile out log messages in their entirety

> What is your evaluation of the design?

I haven't applied it heavily yet but so far the design seems logical. Separating the sinks and sources provides good flexibility. I'm yet to delve into usage of the filters though.

> What is your evaluation of the implementation?

I can't speak for the correctness of the implementation, but from a performance perspective, I've compared it to log4cxx (see the sourceforge thread Andrey has referred to) and it is comparable and/or better in most of my usages. Given the flexibility, I am prepared to pay for a little performance hits.

A concern, which again Andrey has mentioned during this review, is the speed of Boost.DateTime formatting. From our comparisons, it is about 3-5x slower than log4cxx.  Is it possible to have a bare-minimum date time formatter similar to log4cxx's implemented? I wrote a quick little wrapper using a customised strftime (for microseconds) with the most basic caching (which log4cxx's formatter uses) and performance was comparable.

> What is your evaluation of the documentation?

For most the part, very good. Definitely sufficient/adequate for acceptance. The design and installation pages, etc, are extremely clear and easy to read.  Personally, I would prefer a few more concrete examples. For example, (a minor one, I know) but how do you specify keywords via a settings file.

> What is your evaluation of the potential usefulness of the library?

High. An efficient, highly functional and configurable logging library has much potential.

I've found __LINE__ and __FILE__ missing as attributes. Andrey advised these are part of named_scope but the formatting is currently missing. Ideally - and I don't know if this is technically possible but - I would prefer these as just another attribute. Instead of requiring a BOOST_LOG_FUNCTION() call when you want to use them, you should be able to specify it as per usual, something a long the lines of:

backend->set_formatter( fmt::format(%1% [%2%:%3%] %4%)
               % fmt::attr< SeverityLevel >("Severity")
               % fmt::attr< fmt::scope::file >("File")
               % fmt::attr< fmt::scope::line >("SrcLineNumber")
               % fmt::message()
       );

And then BOOST_LOG_SEV(...) << " a message" has the file and line attribute values already. Note, no need for BOOST_LOG_FUNCTION.

With respect to compiling out the log messages: it would be desirable for security, code protection and/or performance if the ability to specify certain log records to be stripped out of the source code.

A hack such as follows - which is obviously extremely inflexible - allows the users to define LOG_NTRACE to compile out all trace messages:
#       if defined (LOG_NTRACE)
#                       define LOG_TRACE(logger, stream) (void(0)) ;
#       else
#                       define LOG_TRACE(logger, stream) BOOST_LOG_SEV(logger, trace) << stream;
#      endif

Something a lot smarter and flexible built into the library is desirable. Say, templated on your custom severity levels (or, indeed, any arbitrary attribute) and generating disable/enable macros.

> Did you try to use the library?  With what compiler?  Did you have any problems?

Yes. gcc 4.3.3, msvc 9, and qcc/gcc 4.x.x (can't remember) on QNX.  No problems.

> How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?

Initially, at the end of last year, I compared performance and functionality to log4cxx. However, for this review, I have had a quick read of the docs (not the code) and attempted integrating it for our purposes, about 4 hours worth.

> Are you knowledgeable about the problem domain?

Not hugely. Middle-of-the-road. Neither here nor there. 5/10. Relatively average.

A big thanks to Andrey for all his effort. Good luck with the acceptance!

Cheers,
Sam