Re: [boost] [log] Boost.Log formal review closing down

Subject: Re: [boost] [log] Boost.Log formal review closing down
From: Andrey Semashev (andrey.semashev_at_[hidden])
Date: 2010-03-17 16:26:59

On 03/17/2010 03:42 PM, vicente.botet wrote:
>
> ----- Original Message -----
> From: "vicente.botet"<vicente.botet_at_[hidden]>
> To:<boost_at_[hidden]>
> Sent: Wednesday, March 17, 2010 9:26 AM
> Subject: Re: [boost] [log] Boost.Log formal review closing down
>
>
>>
>> Hi,
>>
>> I though the review will continue until Sunday.
>>
>>
>> Here is a a part of my review. I will continue this evening.
>>
>>> - What is your evaluation of the design?
>>
>> First say that in general I like the library general architecture. Going into details:
>>
>> * Trivial log
>> Trivial log should be logged to std::cerr
>> Trivial log should contain in the record message only the application part and the severity. If the user want line number, a timestamp, thread identifier the library need to let the user configure this trivial logger.
>>
>> * Filters, Formatters and attributes:
>> An attribute is a different think than a filter. Filters contains attributes as part of its expressions.
>>
>> attr<  logging::severity_level>("Severity") should return the attribute value when called.
>>
>> The following expression
>>
>> attr<  logging::severity_level>("Severity")>= logging::info
>>
>> should return a filter. Can this make be possible and unanbigous?
>>
>> Note that I have removed the trivial namespace which didn't add any information in the preceding expressions.
>>
>> The same applies to formatters
>> fmt::stream
>> <<  fmt::attr<  unsigned int>("LineID")
>> <<  ":<"<<  fmt::attr<  logging::trivial::severity_level>("Severity")
>> <<  ">  "<<  fmt::message()
>>
>> should be just written
>>
>> fmt::stream
>> <<  attr<  unsigned int>("LineID")
>> <<  ":<"<<  attr<  logging::severity_level>("Severity")
>> <<  ">  "<<  message()
>>
>> or even the library could provide some functions returning the predefined attrributes:
>>
>> fmt::stream
>> <<  line_id()
>> <<  ":<"<<  severity_level()
>> <<  ">  "<<  message()
>>
>> Specific formats could be written as
>>
>> fmt::stream
>> <<  (format("%08x") % line_id())<<  ":<"<<  severity_level()<<  ">  "<<  message()
>>
>>
>> * Rotation on files.
>> Can rotation be based only on time? Can we rotate at 2:00AM every day?
>> How many rotation files are maintained? Can this number be configured?
>
> * Custom formatting functons
>   I find this code extremly complex
> void my_formatter(std::ostream&  strm, logging::record const&  rec)
> {
>      namespace lambda = boost::lambda;
>
>      unsigned int line_id;
>      if (logging::extract<  unsigned int>(
>          "LineID", rec.attribute_values(), lambda::var(line_id) = lambda::_1))
>      {
>          strm<<  line_id<<  ": ";
>      }
>
>      logging::trivial::severity_level severity;
>      if (logging::extract<  logging::trivial::severity_level>(
>          "Severity", rec.attribute_values(), lambda::var(severity) = lambda::_1))
>      {
>          strm<<  "<"<<  severity<<  ">  ";
>      }
>
>      strm<<  rec.message();
> }

   void my_formatter(std::ostream& strm, logging::record const& rec)
   {
     logging::extract< unsigned int >("LineID", rec.attribute_values(),
       lambda::var(strm) << lambda::_1 << ": ");

     logging::extract< severity_level >(
       "Severity",
       rec.attribute_values(),
       lambda::var(strm) << "<" << lambda::_1 << "> ");

     strm << rec.message();
   }

> *Attributes:
> I don't like that std::string is used as attribute key. I suspect that this is not the more efficient way to have dynamics attributes.
> I would prefer the library manage several kinds of attributes.
>
> Some attributes could be identified staticaly using a tag.
>
> tagged_attr<  unsigned int, tag_severity>
>
> Others using a std::string key
>
> named_attr<  unsigned int>("id")
>
> and others using an int index
>
> index_attr<  unsigned int>(3)
>
> The mapping could be done at two levels:
> The first level maps&type_info, to either the value_type extractor.
> - For tagged attributes the extractor is able to get the attribute value directly.
> - For named attributes and index attributes the extractor needs the additional name or index information to get attribute value.

That would complicate the library, both in terms of usage and 
implementation. I'd rather not introduce several different interfaces 
for the same thing.

> * Which is the interest of registering the attribute "ProcessID"

Some users expressed the need to start logging by appending to the log 
file leftover from the previous runs of the application. This attribute 
can help to detect the point of the application restart. The attribute 
may also be useful if log records are sent to a remote process, which 
may accumulate logs from a group of processes.