[boost] Re: Logging Library -- Formal Review Request

From: Darryl Green (darryl.green_at_[hidden])
Date: 2005-05-04 02:30:08

Hi John,

John Torjo <john.lists <at> torjo.com> writes:

> 
> I've seen that some of you have requested "levels", and yes, I should 
> add them. I hope I'll have time to implement them soon (less than a month).
> 

I think you (or anyone) should be able to implement levels by changing the
manager only, although I guess that depends on what "implement levels" really
means.

I also think appenders/modifers should be able to be removed as well as added.
It seems to me that the manager should deal with appender add/remove (or
connect/disconnect if you prefer) in a way similar to enable/disable (or levels).

However, to be able to do that appenders (or connections to them) need to be
comparable. It should be possible to write a manager to do this (eg one that
indexes appenders by name), but the logger interface won't let me.

Given the various requests/discussion surrounding exactly how some parts of
logging should best be done, I still think the simplified manager-log_impl
interface I proposed previously, where the manager provides an
appender/modifier vector to the log_impl, has both performance advantages
and allows more flexibility in manager implementation/features. However,
it probably isn't general or clean enough an interface, and this could
be addressed by providing a concept something like the following, to
replace the (not customisable at present) logger_impl.

struct sink
{
  void write(const log_types::stream &s);
  bool enabled(log_types::level l);
};

The rest of the interface (actual type) is log_manager defined. 
A sink with the current semantics + level support implemented by extending
the enabled flag to be a level (something you previously talked me out of, but
I'm definitely happy to see it return) would look something like:

struct sink
{
  template <class IT>
  sink(log_types::level l, IT first, IT last) : m_level(l)
  {
     set_sinks(first, last);
  }

  // used by manager to update level similar to enabled updates now
  void set_level(log_types::level l)
  {
    m_level = l;
  }

  // used by manager to add or remove appenders and/or modifers
  // ordering of these is manager (IT type actually) defined - 
  // the sink/logger doesn't care.
  template <class IT>
  void set_sinks(IT first, IT last)
  {
    sinks_ptr p = new sink_vec(first, last);
    // needs a lock here, or magic shared_ptr::replace magic :-)
    m_sinks = p;
  }

  void write(const log_manager::stream &s)
  {
     {
       // needs lock here or some other magic 
       sinks_ptr p = m_sinks;
     }
     // no need to lock here - allow whatever level
     // of concurrency appenders support
     if (!p) return;
     log_manager::string msg = s.str();
     for (sink_func *fp = p->first(); fp != p->last(), fp++)
     {
        (*fp)(msg);
     }
  }
  bool enabled(log_types::level l)
  {
     return l >= m_level;
  }

  typedef boost::function< void(log_manager::string&) > sink_func;
  typedef std::vector< sink_func > sink_vec;
  typedef boost::shared_ptr<sink_vec> sinks_ptr;

  sinks_ptr m_sinks;
  log_types::level m_level;
  // mutex needed if no replace/atomic_ptr;
};

I'm assuming above that you intend to introduce a feature similar to the
one your original postings re. your logging library convinced me to drop - one
where the enabledness is not a bool but a scalar used as a threshold against
some level specified in the log command? Something like:

BOOST_ENABLE_LOG(my_log, WARN);
BOOST_LOG(my_log, ERROR) << "This is an error";
BOOST_LOG(my_log, DEBUG) << "This is debug";

which would output:

"This is an error" (above specified level) but not "This is debug" (below level)?

I hope the code above is legible - its just typed into gmane from memory +
reference to your latest  code, so don't expect it to be correct!

Regards
Darryl.