Re: [boost] [Review] Boost.Logging: formal review
-----Original Message----- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Gennadiy Rozental Sent: den 15 december 2005 22:39 To: boost@lists.boost.org Subject: Re: [boost] [Review] Boost.Logging: formal review
1. Enabled/disabled logger, logging macros and efficiency
The logging macro BOOST_LOG should accept two arguments instead of one. The logger and the log message. The syntax
(1) BOOST_LOG(some_log) << "my message " << foo() << bar();
should be replaced with
(2) BOOST_LOG(some_log, "my message " << foo() << bar());
and the macro being defined something like this
(3) #define BOOST_LOG(logger, msg) \ if(logger.isEnabled()) { /* code that logs msg */ ... }
Thus, disabled loggers will never take longer than simple if-statement, as long as the isEnabled() method is fast, which IMO should take no longer than comparing booleans.
1. I think this is also the case in original interface.
In the original solution (1) the method foo() will always be called, which is inefficient when the logger is disabled, in solution (2), foo() will never be called when the logger is disabled.
I think you are mistaken
Yes, my mistake.
Further more, the macro could insert extra information such as __LINE__, __FILE__ etc.
This is also true for original interface.
Yes true -- my mistake again (I didn't look enough at the implementation of the macro).
All in all I prefer message not be inside brackets. It looks much more natural and I don't see any theoretical problens to get the same performance. But I also believe that macro shouldn't be a primary interface. So if you prefer message inside the breakets - it's your choice.
However, there is one additional benefit of placing the message inside brackets. One can remove the log statement in a build. Other than that, I guess its style preferences.
2. Filtering
There has been some discussion about the ability of a library to be able to do more filtering than simple log levels, notably by Gennadiy Rozental.
Gennadiy writes:
At the bare minimum it should support:
entry level - levels set is ordered set of values indication importance of information provided. Filtering is based on threshold. Examples: DEBUG, INFO, MAJOR
entry category - categories set is a set of unique values indicating kind of information provided Filtering is based on masking. Examples: ARGS, RETURN_VALUE, ERROR_LOG, DATA_FLOW, PROG_FLOW
entry keyword - keyword set is set of user defined keywords (most frequently strings) identifying area of the program. Filtering is based on match of keywords. Keywords usually are used to mark specific part of application.
I agree that it is very important for a logging library to support this. However, I do not think that the solution is for the logging library to be aware of such special values. The library should be as simplistic as
My position that library should be configurable by any class satisfying Filter concept. This particular filters could probably be supplied by the library as an example and as most widely useful.
My proposed solution is to basically let loggers be somewhat entry (Gennadiy's definition above) unaware. Instead, let the developer be
I do not see how it's possible. IMO framework should employ some MPL magic and construct proper entry structure based on set of Filters passed as template policy parameters.
entry aware. By that I mean that the developer should log messages that belong to a specific entry category/level to a specific logger. For
This is never be acceptable IMO. Even with 3 filters, each ahving 5 possible values you looking into 125 different loggers.
Support for log functions on top of a basic framework solution is needed to alleviate this problem. I wouldn't mind that the library provides log funtions that take various filter functions or predicates to determine wether to send a log statement to a logger. In this case, the logger would be enabled and the log filter function would send log statements to the logger if the log statement meets some criteria. Still, the underlying logger framework would be the same; logger object should still be enabled or disabled/enabled, there would still be a few different logger objects for different levels and possible other user defined criteria. IMO it is very important for an administrator/programmer of a system to be able to enable/disable logs, know what loggers and appenders exists and be able to configure said entities. If filter is only done on a per log statement, how do you even know as a administrator of a system what log filters exists? Which filter criteria should be enabled and which filter criteria should be disabled? If all log statements are sent to one logger object, the logger object becomes a monolith of functionality and complexity. Runtime performance might degrade considerable if various filters is to be checked, not only if the log statements should be logged, but also if one wants to send log statements to different appenders/sinks/destinations. Cheers, Richard The content of this e-mail is intended only for the confidential use of the person(s) to whom it is addressed. If the reader of this message is not such a person, you are hereby notified that you have received this communication in error and that reading it, copying it, or in any way disseminating its content to any other person, is strictly prohibited. If you have received this message in error, please notify the author by replying to the e-mail immediately. NeoNet AB and its subsidiaries (NeoNet Securities AB, NeoNet Securities Inc. and NeoNet Technology AB) are unable to exercise control over the content of information contained in transmissions made via the Internet and hereby excludes any warranty as to the quality or accuracy of any information contained in this message and any liability of any kind for the information contained in it, or for its transmission, reception, storage or use in any way whatsoever.
On 12/19/05, Richard Glanmark <Richard.Glanmark@neonet.biz> wrote:
All in all I prefer message not be inside brackets. It looks much more natural and I don't see any theoretical problens to get the same performance. But I also believe that macro shouldn't be a primary interface. So if you prefer message inside the breakets - it's your choice.
However, there is one additional benefit of placing the message inside brackets. One can remove the log statement in a build. Other than that, I guess its style preferences.
I personally prefer the all-enclosed macro style for this reason as well as others. But I think one of Gennadiy's points is that the primary interface to the Log should not be defined in terms of macros. A standard set of macros can be provided by the Boost.Log implementation for the sake of convenience and casual use. But there should be a well documented public interface that users with more complex requirements can use to write their own logging macros, in whichever style they prefer.
My position that library should be configurable by any class
satisfying Filter concept. This particular filters could probably be supplied by the library as an example and as most widely useful.
My proposed solution is to basically let loggers be somewhat entry (Gennadiy's definition above) unaware. Instead, let the developer be
I do not see how it's possible. IMO framework should employ some MPL magic and construct proper entry structure based on set of Filters passed as template policy parameters.
entry aware. By that I mean that the developer should log messages that belong to a specific entry category/level to a specific logger. For
This is never be acceptable IMO. Even with 3 filters, each ahving 5 possible values you looking into 125 different loggers.
Support for log functions on top of a basic framework solution is needed to alleviate this problem. I wouldn't mind that the library provides log funtions that take various filter functions or predicates to determine wether to send a log statement to a logger. In this case, the logger would be enabled and the log filter function would send log statements to the logger if the log statement meets some criteria.
Still, the underlying logger framework would be the same; logger object should still be enabled or disabled/enabled, there would still be a few different logger objects for different levels and possible other user defined criteria.
I don't think it makes sense to require a user to have separate loggers to handle e.g. DEBUG, WARNING, and ERROR levels. I also don't think it makes sense to require separate logs for different logging keywords (I think thats the term Gennadiy used). The user may use separate loggers for each of these separate logical channels, but it should not be a requirement. IMO it is very important for an administrator/programmer of a system to
be able to enable/disable logs, know what loggers and appenders exists and be able to configure said entities.
But if the programmer has to provide a separate logger for each logging level and conceptual channel of log information, each of which is configured independently, isn't this management problem far worse? If filter is only done on a per log statement, how do you even know as a
administrator of a system what log filters exists? Which filter criteria should be enabled and which filter criteria should be disabled?
Can you expound a bit on what you mean by this? If all log statements are sent to one logger object, the logger object
becomes a monolith of functionality and complexity. Runtime performance might degrade considerable if various filters is to be checked, not only if the log statements should be logged, but also if one wants to send log statements to different appenders/sinks/destinations.
I don't think the functional area is all that large. I see the central concepts as: Entry (or Message): the set of information that makes up a single message to log. Might include a "level" (e.g. debug, info, warning, error), a "category" (e.g. arguments, program_flow, etc), a "keyword" (e.g. "Network", "UI", "Disk Cache", etc) and of course a text string. Filter: determines whether or not an Entry should be logged. Probably implemented as a relatively simple functor. Sink: ultimate destination for Entries. Implementations for wirting to files, system log facilities, etc can be provided. Log: provides logging methods. Implements the Filter concept and contains zero or more Sinks which are the targets of Entries for which the Filter returns true. The basic implementation of all of these should be relatively simple. I think the bulk of the complexity will come in writing specific Sinks that interface with various system-specific APIs, and perhaps in a Configuration class which can be used to initialize a Log and load it with a set of Sinks at runtime. And perhaps UNICODE/I18N issues. Also quoting from Gennadiy's review of John's submission (sorry to keep pointing to your review, Gennadiy, but it is as always a very astute one): Log may have multiple appenders attached (BTW - I very much prefer term used
by iostreams library - sink). what if I want different entry formatting/prefixing/filtering in different outputs. I may have errors_output which contains only errors. I may have end_user_log that contains only major information for end user with detailed time and date of the entry.
I think the idea of the Log having multiple Sinks, each of which may perform custom formatting on the output and perhaps even ignore some Entries is important here. This says to me that the Log concept is a very simple one: simply check the Filter and (if true) forward on to the Sinks. No modifiers happen here. The Sinks may themselves do further Filtering (e.g. in the case of the error_log above) but they receive the Entries exactly as the user passed them to the Logger. I've been sketching up a very simple implementation of some of these ideas in my spare time. Is there interest in fleshing these ideas out more fully on or off-list? -- Caleb Epstein caleb dot epstein at gmail dot com
[...] I agree with most of the comments you made.
I've been sketching up a very simple implementation of some of these ideas in my spare time. Is there interest in fleshing these ideas out more fully on or off-list?
I am all for it ;) Gennadiy
Hi Caleb, I was also looking for the implementation of a such a logger. When you specify a sink does it include a log message formatter that user can specify or sink itself is a message formatter. I think users should have capability to format the log message as they want for a sink. -- Nitin Motgi nitin dot motgi at gmail dot com. On 12/19/05, Gennadiy Rozental <gennadiy.rozental@thomson.com> wrote:
[...]
I agree with most of the comments you made.
I've been sketching up a very simple implementation of some of these ideas in my spare time. Is there interest in fleshing these ideas out more fully on or off-list?
I am all for it ;)
Gennadiy
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
-- Nitin Motgi NITIN . DOT. MOTGI .AT. GMAIL .DOT. COM
On 12/19/05, Gennadiy Rozental <gennadiy.rozental@thomson.com> wrote:
I am all for it ;)
OK, so to that end, please find attached a sketchy implementation of some of these concepts. The concepts are: - Entry - represents a message to be logged. This can be a single std::string or a more complex structure, whatever the user likes. Should model OutputStreamable, but this requirement is up to the Sink. Filters may place additional requirements on the Entry. - Filter - a functor that is used to determine if an Entry should be sent to a Logger's Sink list. A model of std::unary_function<Entry, bool> - Sink - a destination for Entries. Concrete implementations might write to a file, debug window, etc. A model of std::unary_function<Entry, void> - Logger - the object to which Entries are sent by the user. Contains a Filter and a list of Sinks. When the user sends an Entry to the Logger, it will call the Filter functor and, if it returns true, pass it to each of the Sinks it holds. I've attached a sample implementation (log.h) which contains the following classes in "namespace logging" (I ran into collisions with math.h when I tried "namespace log"): - template<typename Entry> null_filter: a Filter that always returns true. - template<typename Entry> basic_file_sink: a Sink implementation that opens a std::ofstream and writes Entries to it - template<typename Entry> basic_ostream_sink: a Sink that holds an std::ostream reference and writes Entries to it. - template<typename Entry> basic_logger: the implementation of the Logger concept. Holds a Filter and a std::list of Sinks, where Filter is implemented as boost::function<bool(Entry)> and Sink is implemented as boost::function<void(Entry)>. Provides non-const reference getters for the filter and sinks members, and of course implements the all important "write" method that does the real work of checking the Filter and passing the Entry to the Sinks if the Filter returned true. I've also provided (in tuple_log.h) an implementation of the Entry concept which attempts to implement Gennadiy's ideas of level, category, and keyword. It is in "namespace tuple_log" and is simply a boost::tuple<level, category, std::string, std::string> where level and category are just wrapped enums with streaming operations added (the proposed BOOST_ENUM would be handy here). Along with the tuple-style Entry class there is an associated filter functor. It can be used to filter messages based on level (>= some threshold), category (matching a bitmask), and keyword (keyword appears in an "include" list and does not appear in an "exclude" list). There is a test program (log.cpp) that exercises these classes and attempts to do some benchmarking. Tested on g++ 3.3.4 on Linux and MSVC8 on Win2000. Some open issues I'm looking for help with: - I'm more or less ignorant when it comes to wide-strings, wide-streams and the like. Would these interfaces need to be paramterized on CharT, or would/should it be possible to support both narrow and wide input with a single interface? - I decided that the Entry concept would represent the entire message-to-be-logged, including any metadata that goes along with it ( e.g. level, category, keyword, thread ID, timestamp, __FILE__, __LINE__, etc). The Filter implementation is consequently parameterized on Entry, which means you need a fully-formed Entry (including the message portion) to call "basic_logger::is_enabled" to check to see if an Entry should even be formatted for writing. Thoughts and feedback are most welcome, -- Caleb Epstein caleb dot epstein at gmail dot com
Caleb Epstein wrote:
OK, so to that end, please find attached a sketchy implementation of some of these concepts.
[snip] This code is very easy to read... maybe because it's just a skeleton, but it looks very nice. Haven't thought about everything carefully yet. I really like the < and OR filters (level, category). I think these are really useful. We might want to be able to chain filters on a log.
I think the idea of the Log having multiple Sinks, each of which may perform custom formatting on the output and perhaps even ignore some Entries is important here. This says to me that the Log concept is a very simple one: simply check the Filter and (if true) forward on to the Sinks. No modifiers happen here. The Sinks may themselves do further Filtering (e.g. in the case of the error_log above) but they receive the Entries exactly as the user passed them to the Logger.
Yes, I've come to the same idea myself. That's how it'll be. Best, John -- John Torjo, Contributing editor, C/C++ Users Journal -- "Win32 GUI Generics" -- generics & GUI do mix, after all -- http://www.torjo.com/win32gui/surfaces.html - Sky's the limit! -- http://www.torjo.com/cb/ - Click, Build, Run!
I've been sketching up a very simple implementation of some of these ideas in my spare time. Is there interest in fleshing these ideas out more fully on or off-list?
I'm all for it as well! But how 'bout after New Year's Eve? I'm quite beat, and I want a vacation... That said, Merry Christmas to you all, and A Happy New Year! Best, John -- John Torjo, Contributing editor, C/C++ Users Journal -- "Win32 GUI Generics" -- generics & GUI do mix, after all -- http://www.torjo.com/win32gui/surfaces.html - Sky's the limit! -- http://www.torjo.com/cb/ - Click, Build, Run!
John Torjo wrote:
I've been sketching up a very simple implementation of some of these ideas in my spare time. Is there interest in fleshing these ideas out more fully on or off-list?
I'm all for it as well! But how 'bout after New Year's Eve? I'm quite beat, and I want a vacation...
That said, Merry Christmas to you all, and A Happy New Year!
And the same to you. If you have a use for it, attached is a new version of the wildcarded name matching logic + tests intended to: 1) Support detaching objects with rules (ie name matching) from logs 2) Avoid the rules table always getting bigger Presumably predicates could be associated with logs using the same mechanism as used for appenders and enabledness (really a degenerate predicate anyway). Regards Darryl Green. // test_rules.cpp // Boost Logging Template library // // Author: Darryl Green // // Copyright (C) 2005 Darryl Green (darryl.green@aanet.com.au) // // Permission to copy, use, sell and distribute this software is granted // provided this copyright notice appears in all copies. // Permission to modify the code and to distribute modified code is granted // provided this copyright notice appears in all copies, and a notice // that the code was modified is included with the copyright notice. // // This software is provided "as is" without express or implied warranty, // and with no claim as to its suitability for any purpose. // See http://www.boost.org for updates, documentation, and revision history. // The rules are used to match appenders to logs // rules are applied in teh order they were specified #include <boost/log/log_impl.hpp> #include <map> #include <vector> #include <algorithm> #include <iostream> #include <iterator> #include "rules.hpp" using namespace boost::logging::detail; struct fails { fails(std::string msg) : m_msg(msg) {} const char *what() { return m_msg.c_str(); } std::string m_msg; }; void test1() { rule_set A; rule_set B; A.modify("a*+"); B.modify("a*+"); if (A.modify("a+")) throw fails("A already accepts a"); // should do nothing if (A.modify("a.b+")) throw fails("A already accepts a.b"); // should do nothing if (!(A.match("a") && A.match("a.b") && A.match("a.b.c"))) throw fails("A should match a & a.b & a.b.c"); if (A.match("b") || A.match("b.a") || A.match("ba")) throw fails("A should NOT match b or b.a or ba"); if (!(B.match("a") && B.match("a.b") && B.match("a.b.c"))) throw fails("B should match"); if (B.match("b") || B.match("b.a") || B.match("ba")) throw fails("B should NOT match"); A.modify("a.b-"); if (A.match("a.b")) throw fails("A should NOT match a.b"); if (!A.match("a.b.c")) throw fails("A should match a.b.c"); A.modify("a.c*-"); if (A.match("a.c")) throw fails("A should NOT match a.c"); if (A.match("a.c.b")) throw fails("A should NOT match a.c.b"); if (A.modify("a.c.x-")) throw fails("A already rejects a.c.x"); // should do nothing if (A.modify("a.c.*-")) throw fails("A already rejects a.c.*"); // should do nothing // make a "hole" in the rejections if (!A.modify("a.c.x*+")) throw fails("A not accepting a.c.x"); // and check it works if (!A.match("a.c.xanadu")) throw fails("A should match a.c.xanadu"); std::copy(A.rules().begin(), A.rules().end(), std::ostream_iterator<std::string>(std::cout, "\n") ); // if we turn off "*" there should be no rules left A.modify("*-"); if (!A.rules().empty()) throw fails("A not empty"); } int main (int argc, char *argv[]) { try { test1(); } catch (fails& err) { std::cout << " FAILED : " << err.what() << std::endl; return 1; } std::cout << "PASSED" << std::endl; return 0; } // rules.cpp // Boost Logging Template library // // Author: Darryl Green // // Copyright (C) 2005 Darryl Green (darryl.green@aanet.com.au) // Copyright (C) 2004-2005 John Torjo (john@torjo.com) // // Permission to copy, use, sell and distribute this software is granted // provided this copyright notice appears in all copies. // Permission to modify the code and to distribute modified code is granted // provided this copyright notice appears in all copies, and a notice // that the code was modified is included with the copyright notice. // // This software is provided "as is" without express or implied warranty, // and with no claim as to its suitability for any purpose. // See http://www.boost.org for updates, documentation, and revision history. // The rules are used to match appenders to logs // rules are applied in teh order they were specified #include "rules.hpp" #include <boost/log/log_impl.hpp> #include <map> #include <vector> #include <algorithm> #include <iostream> namespace boost { namespace logging { namespace { typedef logging_types::log_name_string_type rule_type; typedef std::vector<rule_type> rule_table; struct compiled_rule { compiled_rule(const rule_type& r) { rule_type pattern = r; pattern.resize(pattern.size()-1); match_all = (pattern == "*") || pattern.empty(); if (!match_all) { fixed_start = *pattern.begin() != '*'; fixed_end = *pattern.rbegin() != '*'; rule_type::size_type pos = 0, next; // split spec on wildcard char (*) while ( pos < pattern.size() ) { next = pattern.find('*', pos); if (next == rule_type::npos) next = pattern.size(); if (next - pos > 0) searches.push_back( pattern.substr(pos, next-pos)); pos = next+1; } match_all = searches.empty(); } } bool matches(const rule_type& r) const { if (match_all) return true; if (fixed_start && r.find(*searches.begin()) != 0) return false; // each segment in searches must match, // but there can be anything in between rule_type::size_type at = 0; for (rule_table::const_iterator begin = searches.begin(), end = searches.end(); begin != end; ++begin) { rule_type::size_type next = r.find(*begin, at); if (next == rule_type::npos) return false; at = next + begin->length(); } if (fixed_end) { char ch = *r.rbegin(); if (ch == '-' || ch == '+') return at == r.size()-1; return at == r.size(); } return true; } private: rule_table searches; // substrings to match bool match_all; // short cct common case bool fixed_start; // first substring must match at start bool fixed_end; // last substring must match at end }; struct matches { matches(const compiled_rule& s) : spec(s) { } bool operator () (const rule_type& r) const { return spec.matches(r); } private: const compiled_rule& spec; }; } // anon namespace namespace detail { bool rule_set::modify(const rule_type& r) { rule_type tmp = r; if (*tmp.rbegin() != '-' && *tmp.rbegin() != '+') tmp.push_back('+'); compiled_rule new_rule(tmp); // remove rules that are subranges of the new one m_rules.erase ( std::remove_if(m_rules.begin(), m_rules.end(), matches(new_rule)), m_rules.end()); // add if this rule is a subrange of an existing rule of oposite sign // add if it is a +ve rule that isn't a subrange of an existing rule // (ie it is a subrange of the implied "everything off" rule) char sign = *tmp.rbegin(); bool add_rule = sign != '-'; // see if rule is a subrange for (rule_table::iterator rit = m_rules.begin(), end = m_rules.end(); rit != end; ++rit) { if (compiled_rule(*rit).matches(tmp)) add_rule = sign != *rit->rbegin(); } if (add_rule) m_rules.push_back(tmp); return add_rule; } bool rule_set::clear() { bool not_empty = !m_rules.empty(); m_rules.clear(); return not_empty; } bool rule_set::match(const logging_types::log_name_string_type& target) const { bool result = false; for (rule_table::const_iterator rit = m_rules.begin(), end = m_rules.end(); rit != end; ++rit) { if (compiled_rule(*rit).matches(target)) { result = *rit->rbegin() != '-'; } } return result; } const rule_table& rule_set::rules() const { return m_rules; } } // detail // public interface } // logging } // boost // rules.hpp // Boost Logging library // rules match logs by name to modify them (attach appenders, enable/disable etc) // // Author: Darryl Green // // Copyright (C) 2005 Darryl Green (darryl.green@aanet.com.au) // Copyright (C) 2004-2005 John Torjo (john@torjo.com) // // Permission to copy, use, sell and distribute this software is granted // provided this copyright notice appears in all copies. // Permission to modify the code and to distribute modified code is granted // provided this copyright notice appears in all copies, and a notice // that the code was modified is included with the copyright notice. // // This software is provided "as is" without express or implied warranty, // and with no claim as to its suitability for any purpose. // See http://www.boost.org for updates, documentation, and revision history. // The rules are used to match appenders to logs // rules are applied in the order they were specified #ifndef BOOST_LOGGING_RULES_HPP #define BOOST_LOGGING_RULES_HPP #include <boost/log/log_impl.hpp> namespace boost { namespace logging { namespace detail { struct rule_set { typedef logging_types::log_name_string_type rule_type; typedef std::vector<rule_type> rule_table; bool modify( const logging_types::log_name_string_type& r); bool clear(); bool match(const logging_types::log_name_string_type& target) const; const rule_table& rules() const; rule_table m_rules; }; } } } #endif No virus found in this outgoing message. Checked by AVG Free Edition. Version: 7.1.371 / Virus Database: 267.14.7/214 - Release Date: 23/12/2005
participants (7)
-
Andrew Schweitzer -
Caleb Epstein -
Darryl Green -
Gennadiy Rozental -
John Torjo -
nitin motgi -
Richard Glanmark