From: Darryl Green (darryl.green_at_[hidden])
Date: 2005-11-15 04:52:05
Hartmut Kaiser <hartmut.kaiser <at> gmail.com> writes:
> Please always state in your review, whether you think the library hould be
> accepted as a Boost library!
I don't think the library is ready for acceptance yet, but I do think that it
embodies many of the elements required to make a widely useful logging library,
including some apparently unique/novel ones, and that such a library would make
a great addition to Boost.
While I think the library could do with some further refinement, the scope of
the library needs to be limited sufficiently that it is possible to produce a
high-quality implementation. Once the library reaches that quality while
offering sufficient flexibility and extensibility it should be accepted. Later
submissions of add-ons/enhancements can be included or mini-reviewed as
appropriate (new appenders, enhanced/specialised filtering etc).
So - thats a "not yet" going on abstain (given that my name is on many of the
source files I'm not sure that the review manager should actually count my vote
> Additionally please consider giving feedback on the following general
> - What is your evaluation of the design?
Under the features there is a pretty good design waiting to get out, however it
needs to be revisited in a number of areas:
1) A given message is directed to a particular named log, from where it is sent
to 1 or more named appenders. It is easy to connect/disconnect appenders to
logs. It is also easy to set the log level. However, I find the log level
facility useless, because in general I want to filter the messages to a
particular appender, not globally - if one of my appenders is the system error
log, I always want errors, all errors, and nothing else, going to it. If I have
some sort of console debug output, I might want to turn on debug level output
to it alone (in general from only a few logs). I can do this reasonably well if
I connect the "system error log" to "*.error" and adjust what goes to
the "debug console" by specifying eg. "foo.bar.debug" output to go there, or
more likely "foo.bar.*" and turning off "foo.bar.debug" if it is too noisy. If
some library author decides that log levels are the way to go, I can't get my
app to direct that libraries errors to the error log. Alternatively, if I write
a library, and I want to ensure that it is useable in this scenario and in some
level-based logging scheme, I would presumably need to log an error with a
level of err to a log called "mylib.error" and require that the app using mylib
connect appenders to "mylib.*" and set levels on "mylib.*" if the app is
using "level based" filtering. While I find level-based filtering limited, I do
think the ability to apply some sort of filter other than just a bool
enable/disable is useful. The type of filter used should be customisable - a
very simple fast filter (bool or just a non-empty appender set) would suit many
uses, while some might require a very elaborate check against some sort of
filter settings data structure. Either this can be encapsulated as a policy, or
(and I think this would be less intrusive and as effective) the enabled_logger
etc interface could be tweaked and documented to support something like:
#define BOOST_LOGC(log_name, cond) if (logger_keeper keep = logger_and_bool
(log_name, (cond) )); else (*keep.stream())
// needs a mutex
BOOST_LOGC(some_log, thread_enabled()) << "Thread " << pthread_self() << " is
if thread-based filtering is desired by default just replace BOOST_LOG with:
#define BOOST_LOG(log_name) if (logger_keeper keep = logger_and_bool(log_name,
thread_enabled() )); else (*keep.stream())
BOOST_LOG(some_log) << "Thread " << pthread_self() << " is enabled.";
Extending logger_and_bool to logger_and_predicate and the appropriate c'tors
for the keeper/enabled_log would allow the predicate to perform checks that
require access to the logger itself as well. These tests are (potentially)
heavyweight but so long as a simple call to BOOST_LOG simply bypasses them
there is no cost to the user who doesn't need them.
For example, Gennady would presumably like some form of fully extensible set of
attributes to apply filtering rules to based on log name. He could simply
maintain whatever map<name_string, attr_type> he needs and write appropriate
predicates to filter on this. Of course that could be too slow, in which case a
more efficient mapping from log to attributes would be needed - by allowing
customisation/extension of the logger_impl - see below.
2) While the library aims to offer a customisation point through replacing the
manager type, the manager concept is un(der)documented, and the various
interfaces that depend on the manager type are too closely coupled to allow
much customisation anyway. Note that this may be ok, if the design of the
standard manager is flexible enough without extensive customisation, and any
remaining customisation points/concepts are clearly documented. In any case,
the use of the customisation needs to be illustrated by example (ie. an
alternate manager replacing all the default interfaces).
3) Closely related to the above, I would like to see cleaner separation between
the components that make up this library. This is something that is hard to do
when the implementation is a feature driven moving target (I've been trying to
do it on and off). This separation needs to maintain or increase the existing
loose coupling between compilation units that use the library to log, those
that set/modify filtering and those that do actual output of logged messages,
while reflecting that separation in the implementation. Currrently appenders do
this, but the logging and filtering is too internally intertwined. If the
library formalised that separation allowing each module to be customised,
library code could use logging independent of how the application (or other
libraries) used it/controlled it (and without forcing library recompilation).
4) Configuring modifiers using the index to order them is awkward and error-
prone. The concept of some sort of indexed insertion into a sequence of
modifiers doesn't even seem to be one that there is a real use for - an implied
by order of insertion approach would work at least as well.
5) While applying modifiers to logs rather than to appenders is (potentially)
more efficient as it avoids doing the modification N times for N appenders, in
reality I have yet to encounter a case where the log format wasn't associated
with the destination, rather than the source (ie. a log file should have a
consistent format). An extreme example is when the log is a structured log such
as the windows event log. A simple example is the newline modifier. If an
appender is expecting to receive log messages it is up to the appender to
delimit the messages in the appropriate way - be that by packing it into a
syslog UDP packet or just sticking a newline on the end. I don't think the
newline modifier should exist at all, but in other cases it does make sense to
include log specific information. I'm not sure that for typical use where N is
small that the efficency gains are likely to be significant, but it would also
be possible to improve efficiency issue in cases where it is a real concern by
using a multiplexing appender (that forwarded formatted output multiple "real"
The modifier concept also blurs the line between insertion of information and
formatting - if a log should always contain some particular information at the
start of it what is wrong with just writing it to the enabled_logger's stream
when it is created. This also leverages all the built-in and extended output
and formatting facilities of std streams (including rather more
internationalisation support than found in strings). This fits in with the user
extensible logger attributes, predicates etc above. I would like to see 2
points at which the content can be modified - when the stream is created, so
that informartion that should be a prefix on all logged messages via a
particular log, regardless of destination can be inserted there, and another
associated with the appender, rather than any particular log, so that any
content that must always be inserted in messages to that log can be
> - What is your evaluation of the implementation?
There are some areas of the core library, especially robust thread safety, that
need attention. The most glaring of these is the
m_deleted hack to try to work around the use of a reference in place of a copy
of the logger (*not* the logger_impl - copying a logger is cheap - it is only a
shared_ptr). There is also an outstanding fixme re. the problem of logger_impls
being kept alive by their presence in the logger collection even after they are
The appenders supplied are, in general, not much more than examples. While this
is fine in that user-written appenders are a great customisation point, I think
it is really essential that at least a few robust, high performance appenders
be offered. I think it is obvious from the above design comments that I think
an overall cleanup/refactor reflecting the final design is in order.
> - What is your evaluation of the documentation?
The documentation is a nice friendly tutorial. However, there is a severe lack
of design documentation supporting rationale, and no real reference section.
> - What is your evaluation of the potential usefulness of the library?
A good logging facility is essential in most (I'm tempted to say all)
applications of any complexity, and current solutions tend to introduce
undesirable coupling or lack features/flexibility. The library is potentially
very useful not just as a developer's debugging tool but as a part of any
application that needs configurable logging of any sort.
> - Did you try to use the library? With what compiler? Did you have any
Yes (an earlier version) gcc 3.3 and 3.4. I didn't have any real problems with
it, but I haven't tried to install and use "out of the box" as I was modifying
it at the time.
> - How much effort did you put into your evaluation? A glance? A quick
> reading? In-depth study?
I've made a reasonably in-depth study of and changes to versions of this
library. I'd say I'm very familiar with it.
> - Are you knowledgeable about the problem domain?
Reasonably so - I've written logging components as part of various systems
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk