John Torjo wrote:

Sebastian Redl wrote:

...

Quick question before I dig deeper into the docs.

Say I have a library that does some logging. Naturally, the logger of this library should be configured by the user of the library, not the library itself. How would I best do that in this library?

Please see attached. OK, so basically the library asks the user to initialize the log. Makes sense. Is there a way to quickly duplicate the configuration of one logger into another?

Sebastian Redl

*About test_rolling_file: *This is my bad - I needed some input, so I decided to read the file itself ("test.cpp"), and then log some of its lines. The problem is that when ran from bjam, it runs from the project's parent directory - in other words, if it were to read from "test_rolling_file/test.cpp", there wouldn't be any errors. I will fix this after the review. *About test_simple_tss: *- your process can't access the Output Debug Window, and it's taking too long to do logging. Sleeping a bit more and not writing to the Output Debug Window will do the trick (see attached file). Note: on my machine it runs fine as it was in the original version Note2: To see that the above tests work, you can always open all_tests.sln, and compile and run them standalone. Best, John -- http://John.Torjo.com -- C++ expert http://blog.torjo.com ... call me only if you want things done right /** Boost Logging library Author: John Torjo, www.torjo.com Copyright (C) 2007 John Torjo (see www.torjo.com for email) Distributed under the Boost Software License, Version 1.0. (See accompanying file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) See http://www.boost.org for updates, documentation, and revision history. See http://www.torjo.com/log2/ for more details */ /* Tests on_dedicated_thread */ #include <boost/test/minimal.hpp> #include <boost/logging/format_fwd.hpp> BOOST_LOG_FORMAT_MSG( optimize::cache_string_one_str<> ) #include <boost/logging/format_ts.hpp> #include <boost/logging/format/formatter/thread_id.hpp> #include <boost/thread/thread.hpp> #include <boost/thread/xtime.hpp> using namespace boost::logging; typedef logger_format_write< default_, default_, writer::threading::on_dedicated_thread > logger_type; BOOST_DECLARE_LOG_FILTER(g_log_filter, filter::no_ts ) BOOST_DECLARE_LOG(g_l, logger_type) #define L_ BOOST_LOG_USE_LOG_IF_FILTER(g_l(), g_log_filter()->is_enabled() ) BOOST_DEFINE_LOG_FILTER(g_log_filter, filter::no_ts ) BOOST_DEFINE_LOG(g_l, logger_type) void do_sleep(int ms) { using namespace boost; xtime next; xtime_get( &next, TIME_UTC); next.nsec += (ms % 1000) * 1000000; int nano_per_sec = 1000000000; next.sec += next.nsec / nano_per_sec; next.sec += ms / 1000; next.nsec %= nano_per_sec; thread::sleep( next); } int MESSAGES_PER_THREAD = 200; int THREAD_COUNT = 5; void use_log_thread() { for ( int i = 0; i < MESSAGES_PER_THREAD; ++i) { L_ << detail::get_thread_id() << " message " << (i+1) ; do_sleep(1); } } std::ostringstream g_out; void test_logged_messages() { typedef std::map<detail::thread_id_type, int> coll; coll messages; std::istringstream in( g_out.str()); std::string line; while ( std::getline( in, line ) ) { // each logged message has this syntax: // thread_id message idx detail::thread_id_type thread_id; std::string word; int idx = 0; std::istringstream line_in(line); line_in >> thread_id >> word >> idx; BOOST_CHECK(word == "message"); // for each thread - we should be reading an increasing index 1, 2, 3, ... BOOST_CHECK( messages[thread_id] + 1 == idx); messages[thread_id] = idx; // we only have THREAD_COUNT threads logging! BOOST_CHECK( (int)messages.size() <= THREAD_COUNT); } // at this point, each thread should have written all MESSAGES_PER_THREAD messages BOOST_CHECK( (int)messages.size() == THREAD_COUNT); for ( coll::const_iterator b = messages.begin(), e = messages.end() ; b != e; ++b) BOOST_CHECK( b->second == MESSAGES_PER_THREAD); } void test_on_dedicated_thread() { g_l()->writer().add_formatter( formatter::append_newline() ); g_l()->writer().add_destination( destination::stream(g_out) ); // g_l()->writer().add_destination( destination::dbg_window() ); g_l()->mark_as_initialized(); for ( int i = 0 ; i < THREAD_COUNT; ++i) boost::thread t( &use_log_thread); // allow for all threads to finish int sleep_ms = MESSAGES_PER_THREAD * THREAD_COUNT * 5 /* just in case*/; std::cout << "sleeping for " << sleep_ms << " milliseconds" << std::endl; do_sleep( sleep_ms ); test_logged_messages(); } int test_main(int, char *[]) { test_on_dedicated_thread(); return 0; } // End of file

Steven Watanabe wrote:

AMDG

Gennadiy Rozental wrote:

...

The formal review of Logging library, proposed by John Torjo, begins today and will run till Feb 13:

Here's part 1 of my review from reading the docs only. More to follow after I look through the implementation.

in main_intro.html there is a line that says Click to see the code which sounds like it ought to be a link but isn't.

Yup, sorry - fixed. Will be online shortly.

scenarios_code.html#scenarios_code_mom: I don't think that write is the correct name: g_l()->writer().write("%time%($hh:$mm.$ss.$mili) [%idx%] |\n", "cout file(out.txt) debug"); This doesn't write anything. It seems like this should be called something more like set_format_and_destination()

Yup you're right - will fix it after review.

I don't like the idea of two stage initialzation implied by g_l()->mark_as_initialized(); Is there any way that the initialization can be done where the log is defined?

See http://torjo.com/log2/doc/html/caching.html for the rationale. However, for a named_logger, we might be able to avoid caching - that is, initialize it in one step. Note that in the future I want to make caching a policy - I just didn't have time to do it before the review.

Concepts as Namespaces: I strongly dislike this way of expressing it. What you mean is that you put implementations of that concept in a specific namespace. The concept, per se, has no relation to the namespace. This description is very misleading, IMO.

Actually I think this is way too cool. First of all, it's very easy to find everything that is related to a concept: - in the documentation - just click on that namespace - when using code completion In fact once you get used to the library, you'll see that gets easier and easier to read the code/docs. I don't know if any other lib has done this before - but please give it a shot first ;)

What does the suffix ts stand for? Oh. I see. Thread Safety. Could this be spelled out completely? It's not going to appear all over the code so there is no reason to make the name as short as possible.

Ok, I can update this - after the review.

boost::logging::formatter::idx: Please! don't abbreviate this. "index" is only *two* characters longer.

Will do - after the review

Workflow.html: You'll use this when you want a optimize string class. Or, when using tags s/optimzed/optimized/ s/a/an/

Is there a way to have destination specific formatters?

Yes. When using a custom route (msg_route::with_route). If you make optimize::cache_string_several_str<> your msg formatter. See http://torjo.com/log2/doc/html/no__levels__with__route_8cpp-example.html

Throughout the docs some #include's directives are links to the corresponding headers and other are not.

My mistake - please see http://torjo.com/log2/doc/html/headers_to_include.html That's all you need to know.

I find the use of preprocessor directives vs. templates for customization not very intuitive. For example, the options controlling caching are macros. I'm not convinced that this should be a global setting. The fast and

About cache - see above.

slow compile time options make sense I think because they are transparent to the user. (BTW, would I need to #ifdef out the #include <boost/logging/format.hpp> to have the compile times improve?)

What do you mean?

AT the bottom of macros.html: #define BOOST_LOG_TSS_USE_CUSTOM = my_thread_specific_ptr The "=" is wrong I think.

At this time it's ok. However I guess you're right - after the review I can update the code not to require that '='.

If BOOST_LOG_NO_TSS is defined is it an error to use the thread-specific-storage filters?

Yes.

filter::debug_enabled/release_enabled: How is debug/release determined. NDEBUG?

Yes

in namespaceboost_1_1logging_1_1manipulator.html there is a reference to destination::shared_memory - writes into shared memory (using boost::shmem::named_shared_object) is this obsolete?

I need to rewrite it to use interprocess.

namespaceboost_1_1logging_1_1manipulator.html#manipulator_share_data: It doesn't look like the example here will compile.

You're right - instead of "write_to_file" I should have written "my_file"

Second, is there any particular reason not to use shared_ptr directly?

Yes, see this : http://torjo.com/log2/doc/html/namespaceboost_1_1logging_1_1manipulator.html...

structboost_1_1logging_1_1manipulator_1_1is__generic.html: I'm don't understand the reference to the template operator=.

Not sure I understand what you mean.

namespaceboost_1_1logging_1_1formatter_1_1convert.html "explain that you can extend the following - since they're namespaces!!! so that you can "inject" your own write function in the convert_format::prepend/or whatever namespace, and then it'll be automatically used!" I'm almost certain that this is wrong. In order to be found by a template

It's not - see http://torjo.blogspot.com/2007/10/template-construct.html

Concepts: For the following concepts I either couldn't figure them out or had to hunt all over the documentation. This information should be specified in the Concepts section

I ssume you've read the Workflow, right?

Filter: There is some way to get a boolean out of a filter. All the details are specified by the logging macro.

The macro uses the filter. The filter can have a function that returns a boolean. It can have no parameters (for instance, if it's a simple filter), or for instance one parameter if it's a filter based on levels.

Level: I can't figure out from the documentation what I would need to do to model this concept.

http://torjo.com/log2/doc/html/namespaceboost_1_1logging_1_1level.html (also, see scenario below)

Writer: Seems to be a specialization of a Unary Function Object?

Yes

Scenario: This doesn't seem like a concept. I'm a little confused now. In addition I don't understand why ts and usage are disjoint. There seems to be significant overlap between the two. Is there any way that they could be merged?

"scenario" is a way for you to decide the right logger, based on your needs: - "usage" - you can find the right logger, based on usage, like: using namespace boost::logging::scenario::usage; typedef use< // the filter is always accurate (but slow) filter_::change::always_accurate, // filter does not use levels filter_::level::no_levels, // the logger is initialized once, when only one thread is running logger_::change::set_once_when_one_thread, // the logger favors speed (on a dedicated thread) logger_::favor::speed> finder; - "ts" - you can find your logger, based on your thread-safety needs, like: using namespace boost::logging::scenario::ts; typedef use< filter_::use_tss, level_::no_levels, logger_::use_tss> finder;

Gather: What exactly are the requirements? The docs say "a function that will gather the data - called .out()" What are the requirements on the return type? Further in scenarios_code.html#scenarios_code_mon there is the definition void *out(const char* msg) { m_msg = msg; return this; } Is out() called implicitly or not? Why is "this" returned as a void*? Confused...

The requirements are to return a void* pointer. This is needed when http://torjo.com/log2/doc/html/caching.html and you want to cache the filter as well. If I remove the usage of http://torjo.com/log2/doc/html/caching.html#caching_BOOST_LOG_BEFORE_INIT_CA... then you can return anything you like. For more details, please take a look at cache_before_init_macros.hpp, line 50

Tag: The only requirement on a tag is that it must be CopyConstructable (Is it Assignable, too?). tag::holder is a effectively a fusion::set of tags.

Not really - theres'a bit of logic in that class.

It's not clear from either the tag or holder documentation how to extract tags in a formatter.

Not sure what you mean.

One final comment about the Concepts:

Manipulators: I think this framework is overly complex. It would be

Why?

better to make manipulators Equality Comparable Unary Function Objects. In the case of destinations the return type is ignored, for for formatters, the result type should be the same as the argument type. For both the argument type is determined from the

"for formatters the result should be the same as the argument type" - why?

logger. You can use type erasure to allow them to be treated polymorphically at runtime.

Isn't this what I'm doing? Best, John -- http://John.Torjo.com -- C++ expert http://blog.torjo.com ... call me only if you want things done right

Como says:

...

como -a test.cpp Comeau C/C++ 4.3.9 (Apr 4 2007 16:51:45) for _MS_WINDOWS_x86_Beta Copyright 1988-2007 Comeau Computing. All rights reserved. MODE:strict warnings C++ noC++0x_extensions

"test.cpp", line 9: error: no instance of overloaded function "X::foo" matches

To me this looks like a bug in como. Side-note: both VS2005 and gcc 4.1 compile this code. And yes, this is the type of overloading I was counting on. Best, John -- http://John.Torjo.com -- C++ expert http://blog.torjo.com ... call me only if you want things done right

...

...

To me this looks like a bug in como.

Side-note: both VS2005 and gcc 4.1 compile this code. And yes, this is the type of overloading I was counting on.

It is not a bug in como. Section 14.6.4 of the standard specifies this behavior.

14.6.4.2 "For the part of the lookup using associated namespaces (3.4.2), only function declarations with external linkage found in either the template definition context or *the template instantiation context *are found." That to me sounds like a bug in como. Best, John -- http://John.Torjo.com -- C++ expert http://blog.torjo.com ... call me only if you want things done right

Steven Watanabe wrote:

AMDG

John Torjo wrote:

...

14.6.4.2 "For the part of the lookup using associated namespaces (3.4.2), only function declarations with external linkage found in either the template definition context or *the template instantiation context *are found."

That to me sounds like a bug in como.

Note that this section is talking about associated namespaces, aka Argument Dependent Lookup. What the reference to the template instantiation context means, is that the compiler will find the declarations of functions *From namespaces associated with the argument types* as long as those declarations exist either

a) At the point when the template is defined. b) At the point the template is instantiated.

Although in practice (b) is a superset of (a), in theory these two sets of declarations are distinct because of export.

Yup, you're right - there is a solution, the 5th comment on : https://www.blogger.com/comment.g?blogID=699828515576400912&postID=4064173770571925791 I will implement it after the review. Best, John -- http://John.Torjo.com -- C++ expert http://blog.torjo.com ... call me only if you want things done right

I have seen already some quotes to the C++ standard such as this one and wonder whether it's legal. Isn't the standard a non-free document, it's as far as I know not even accessible without paying for it (in contrast to C#, which resulted in online docs for this language and the library even in Open Source IDEs which isn't still the case for C++).

Seems like a copyright violation ...

As far as I know the copyright law, if you're quoting a small text, it's ok. However, you gurus in copyright law please correct me if I'm wrong. Best, John -- http://John.Torjo.com -- C++ expert http://blog.torjo.com ... call me only if you want things done right

On Feb 6, 2008 3:38 AM, Jens Seidel <jensseidel@users.sf.net> wrote:

...

I have seen already some quotes to the C++ standard such as this one and wonder whether it's legal. [snip]

Seems like a copyright violation ...

I think that it's ok, mainly because it's a small thing. Agreed. I would have expected it to come under "fair use".

If not, there's always the public comment draft standards which, afaik, are on unrestricted distribution. I believe that strictly speaking, that is not so. My understanding is

From: Scott McMurray that draft standards are copyright works too, but are issued with a license which allows unrestricted copying /for the purpose of commenting on the proposed new standard/. Copying for other purposes is not allowed. (If this is not right, I'm sure that a member of the standards committee will be along shortly to correct me.) -- Martin Bonner Senior Software Engineer/Team Leader PI SHURLOK LTD Telephone: +44 1223 441434 / 203894 (direct) Fax: +44 1223 203999 Email: martin.bonner@pi-shurlok.com www.pi-shurlok.com disclaimer

That's fine, but a concept is not a namespace. A concept is a set of requirements. I have no problem with putting everything associated with a particular concept in a single namespace. What I object to is your terminology.

Ok, my bad - will fix after the review. You're right - I didn't express my intention clear enough.

...

...

Second, is there any particular reason not to use shared_ptr directly?

Yes, see this : http://torjo.com/log2/doc/html/namespaceboost_1_1logging_1_1manipulator.html...

That doesn't answer my question.

I wanted a standard way of implementing a manipulator that holds non constant data. Users will usually look at existing code and then develop their-own. Using shared_ptr<> could easily be overlooked, and people could would end up with a badly implemented manipulator. Also, non_const_context makes sure the shared_ptr doesn't point to null - syntactic sugar. To answer your question : you can use shared_ptr, but the recommended way is to use non_const_context.

...

...

Level: I can't figure out from the documentation what I would need to do to model this concept.

http://torjo.com/log2/doc/html/namespaceboost_1_1logging_1_1level.html

(also, see scenario below)

What if I want to write my own level holder? All that gives is a list of level holders and levels that you provide.

Depends on what you want - the library is very flexible when it comes to filters. The simplest level holders looks like this: // the higher the level, the more critical the message struct level_filter { level_filter(int default_level = 0) : m_level(default_level) {} bool is_enabled(int level) const { return level >= m_level; } void set_enabled(int level) { m_level = level; } private: int m_level; }; Then, you can use it in code ... typedef logger_format_write< > logger_type; BOOST_DECLARE_LOG(g_l, logger_type) BOOST_DECLARE_LOG_FILTER(g_log_filter, level_filter ) #define L_(level) BOOST_LOG_USE_LOG_IF_FILTER(g_l(), g_log_filter()->is_enabled(level) ) // logging messages const int dbg_ = 500; const int err_ = 1000; L_(dbg_) << "debug message"; L_(err_) << "error msg";

...

The requirements are to return a void* pointer. This is needed when http://torjo.com/log2/doc/html/caching.html and you want to cache the filter as well.

Then, are there two cases? The version of out returning an ostream and the version that processes the results directly and returns void*?

I will address this in another email.

...

If I remove the usage of http://torjo.com/log2/doc/html/caching.html#caching_BOOST_LOG_BEFORE_INIT_CA... then you can return anything you like. For more details, please take a look at cache_before_init_macros.hpp, line 50

...

Tag: The only requirement on a tag is that it must be CopyConstructable (Is it Assignable, too?). tag::holder is a effectively a fusion::set of tags.

Not really - theres'a bit of logic in that class.

Ok. That was the best guess I could come up with from namespaceboost_1_1logging_1_1tag.html. (structboost_1_1logging_1_1tag_1_1holder.html is completely useless)

...

...

It's not clear from either the tag or holder documentation how to extract tags in a formatter.

Not sure what you mean.

Suppose I write my own tag.

struct my_tag { const char* data; };

Now, I also need a formatter capable of processing it, right?

Yes.

a) what should the argument type to the formatter be? b) How do I get the value of my_tag?

I've attached an example of creating your custom formatter. I will put it on svn after the review.

If manipulators are effectively function objects, I would like to be able to just plug in existing function objects like those created by bind. If there isn't anything more to specialize then you should re-use existing concepts.

There's more to manipulators than just being a functor: - they need to have some inner typedefs, and they provide the possibility to be configured.

...

...

better to make manipulators Equality Comparable Unary Function Objects. In the case of destinations the return type is ignored, for for formatters, the result type should be the same as the argument type. For both the argument type is determined from the

"for formatters the result should be the same as the argument type" - why?

Because the result may have to be passed on to other formatters, of the same static type right? Or am I completely misunderstanding how formatters work?

They don't work like that. A formatter gets a string as argument to its operator(), and it manipulates that string. So each manipulator works isolated from the other formatters.

...

...

logger. You can use type erasure to allow them to be treated polymorphically at runtime.

Isn't this what I'm doing?

Maybe so, but I don't understand the need to use inheritance at all.

When talking about inheritance - can you tell me which part you're referring to, so I can explain? Best, John

In Christ, Steven Watanabe

_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

-- http://John.Torjo.com -- C++ expert http://blog.torjo.com ... call me only if you want things done right /** Boost Logging library Author: John Torjo, www.torjo.com Copyright (C) 2007 John Torjo (see www.torjo.com for email) Distributed under the Boost Software License, Version 1.0. (See accompanying file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) See http://www.boost.org for updates, documentation, and revision history. See http://www.torjo.com/log2/ for more details */ #include <boost/logging/format_fwd.hpp> struct my_tag { my_tag(const char * data = "" /* default value */) : data(data) {} const char* data; }; namespace bl = boost::logging; typedef bl::tag::holder< bl::default_, my_tag> tag_holder; BOOST_LOG_FORMAT_MSG( tag_holder ) #include <boost/logging/format.hpp> #include <boost/logging/format/formatter/tags.hpp> typedef bl::logger_format_write< > logger_type; typedef bl::filter::no_ts filter_type; struct my_tag_formatter : bl::formatter::class_<my_tag_formatter, bl::formatter::implement_op_equal::no_context > { void operator()(tag_holder & val) const { typedef tag_holder::string_type string_type; // automatic conversion - tag holder provides this const my_tag & tag = val; string_type & str = val; // print the tag str = tag.data + str; } }; #define L_(data) BOOST_LOG_USE_LOG_IF_FILTER(g_l(), g_log_filter()->is_enabled() ) .set_tag( my_tag(data) ) BOOST_DEFINE_LOG_FILTER(g_log_filter, filter_type ) BOOST_DEFINE_LOG(g_l, logger_type) void custom_tag_class_example() { g_l()->writer().add_formatter( bl::formatter::time("$hh:$mm ") ); g_l()->writer().add_formatter( my_tag_formatter() ); g_l()->writer().add_formatter( bl::formatter::append_newline() ); g_l()->writer().add_destination( bl::destination::cout() ); g_l()->mark_as_initialized(); int i = 1; L_("tag_a ") << "this is so cool " << i++; L_("tag_b ") << "this is so cool again " << i++; } int main() { custom_tag_class_example(); }

AMDG

What does that mean?

...

To answer your question : you can use shared_ptr, but the recommended way is to use non_const_context.

And in addition, non_const_context is a slightly higher level of abstraction.

Yes :)

The disadvantage I see is that copying such a manipulator may cause surprises and using shared_ptr directly makes it more obvious that copying doesn't quite have normal semantics.

http://torjo.com/log2/doc/html/namespaceboost_1_1logging_1_1manipulator.html...

...

Depends on what you want - the library is very flexible when it comes to filters. The simplest level holders looks like this:

// the higher the level, the more critical the message struct level_filter { level_filter(int default_level = 0) : m_level(default_level) {} bool is_enabled(int level) const { return level >= m_level; } void set_enabled(int level) { m_level = level; } private: int m_level; };

So the minimum needed to interoperate with LOG_IF_LEVEL is bool is_enabled(int level)?

Yes.

...

They don't work like that. A formatter gets a string as argument to its operator(), and it manipulates that string. So each manipulator works isolated from the other formatters.

In place manipulation. Doh. (Sounds like I've been doing too much functional programming recently)

:) Best, John -- http://John.Torjo.com -- C++ expert http://blog.torjo.com ... call me only if you want things done right

AMDG John Torjo wrote:

Steven Watanabe wrote:

...

Gather: What exactly are the requirements? The docs say "a function that will gather the data - called .out()" What are the requirements on the return type? Further in scenarios_code.html#scenarios_code_mon there is the definition void *out(const char* msg) { m_msg = msg; return this; } Is out() called implicitly or not? Why is "this" returned as a void*? Confused...

The requirements are to return a void* pointer. This is needed when http://torjo.com/log2/doc/html/caching.html and you want to cache the filter as well.

If I remove the usage of http://torjo.com/log2/doc/html/caching.html#caching_BOOST_LOG_BEFORE_INIT_CA... then you can return anything you like. For more details, please take a look at cache_before_init_macros.hpp, line 50

I just thought of a way to avoid this requirement. if(!is_enabled); else for(int i = 0; i == 0;) for(bool (*f)(); i < 3; ++i) if(i == 0) { struct local { bool do_filter() { return(/is_enabled/); } }; f = &local::is_enabled; } else if(i == 2) { // cache filter. } else //do logging Wow, that's brilliant! Thanks! Best, John -- http://John.Torjo.com -- C++ expert http://blog.torjo.com ... call me only if you want things done right

Hi Loïc ,

Hello,

I've just had a fast look at the doc. I'm using for my work the previous version of this library (which was not accepted in boost, but nevertheless fits my needs quite well), and my two minutes reading of the doc of this new version seems to imply it is more powerfull, more configurable, but also more complex to understand.

Indeed so :)

I'd like to know if there is a document specifying what are the functionnal differences between v1 & v2, as well as a transition path for existing code using v1.

Basically it's in my TODO list. The v2 of the library is *very* extensible/flexible, thus docs can go on an on. So, to answer your question: - there's no doc yet - I will provide one after the review Quick summary: - On the usage side: BOOST_LOG macro from v1. You'll need to define your BOOST_LOG similar to this: #define BOOST_LOGL(lvl) BOOST_LOG_USE_LOG_IF_LEVEL(g_l(), g_log_level(), lvl ) BOOST_DEFINE_LOG_FILTER(g_log_level, boost::logging::level::holder ) // holds the application log level BOOST_DEFINE_LOG(g_l, logger_type) // logger_type - depends on what logger you want - On the initialization : As you see, this is quite different from v1. Just in case you have problems, showing me the initialization code could help. Best, John

Regards,

-- http://John.Torjo.com -- C++ expert http://blog.torjo.com ... call me only if you want things done right

The new version seems to improve things somewhat in that it all seems to be in the headers, however I still strongly dislike the use of a preprocessor macro to tell the headers whether or not to use Unicode. There are two problems with this:

I've re-looked at the implementation. I would say a few parts can be made Unicode-independent rather easy. However to make the char_type part of the logger will require a bit of work. To summarize: - I don't remember anyone asking for this before - It's certainly doable. I can implement this after the review. Best, John -- http://John.Torjo.com -- C++ expert http://blog.torjo.com ... call me only if you want things done right

Hi Scott, Thanks for the review.

To raise the bar you might include requirements such as gleaning maximum information from the point of logging, with minimum developer fuss and delivering that information to a storage+viewing mechanism for convenient post-processing and analysis, e.g.;

I would say I provided that.

- the library doesnt appear to use the __LINE__ facillity

Actually it doesn't use it, unless you want to - if you take a look at the use_tags.cpp example, you'll see that the lib can actually log __FILE__/__LINE__, __FUNCTION__, and more.

- how do I log the value of a variable and then search for that variable by name?

Not sure what you mean.

- how do I search for the next reference with any assurance that the same name is used?

Not sure what you mwan.

- where is the default mode that just works "out of the box"

How about this: http://torjo.com/log2/doc/html/scenarios_code.html#scenarios_code_mom

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

Useful. This looks after most of the needs relating to developers debug output and also more permanent support output. It does not appear to consider what often occurs with such output - it may be the best or only record of system activity and will become the focus of more complex post-processing and analysis. The library does not specifically cater to such activity.

Not sure what you mean. Why do you think I neglected that?

* Do you think the library should be accepted as a Boost library? Be sure to say this explicitly so that your other comments don't obscure your overall opinion.

No. As an example of a developers debug logging facillity this is one of the better ones. A Boost logging facillity should be targeted slightly differently. Solving the requirements of developers debugging should be a side benefit of solving the wider issue of recording system activity for any of the system stakeholders.

Again, not sure what you mean - and please explain why my lib doesn't solve the above issue. Thanks. Best, John -- http://John.Torjo.com -- C++ expert http://blog.torjo.com ... call me only if you want things done right

Hi Scott.

Some examples of representation;

After completion there were 3 leftovers and ... After completion there were <3> leftovers and ... After completion there were <3!unsigned int> leftovers and ... After completion there were <3!unsigned int/matching_profile> leftovers and ...

What you suggest is a whole new layer built on top of what the log library provides - as proposed & currently implemented. YMMV but what it does provide is something I find 'seriously lacking' in the currently available set of C++ libraries. Stating that it should not be accepted only due to it not containing that additional layer of support seems wrong, provided of course that what it does provide passes well through this public review. :-) The layer you request can, and would most likely be useful to, be build on top of what this library provides, either as a part of this or a separate library, but requiring that it 'solve the world' before accepting it seems too much. I can only imagine the next guy saying that it should not be accepted because it does not provide an out-of-the-box industry-grade network logging server that can be easily integrated with customer's standard Kerberos based authentication/authorization server to provide security over who can access those logs. :-) That said, I really hope to find the time to review the library. :-) Having such a library in boost seems like it would be a great plus. It would provide an arguably high-quality logging solution somewhere where lots of developers look first for high-quality solutions and would automatically gain it a large number of potential high-quality users/supporters/developers/reviewers as boost libraries have always had great support & maintenance and I do not recall many that got unmaintained and none that got absolutely lost & untouchable... Most, if not all, logging libraries I tried suffer greatly from not having that. Either their design grew stale and stuck in different backward compatibility issues or they stopped being developed all together. Using them always seemed like a not such a big step away from 'rolling your own', which then you most often actually do... Best regards, Jurko Gospodnetić

Hi Scott.

You make a series of valid points. My enduring difficulty is that the phrase "whole new layer built on top" is technically correct, results in the whole new layer never being created and terabytes of free-format log files "out there".

I could not agree more. :-)

I also suspect that if the "new layer" issue was addressed up front it would push the design around a bit. There could be a cleaner separation between a layer that transports bytes from the point of logging to the place of storage and the layer that codes and decodes application data to and from the byte stream.

I would suggest a compromise here though as what this library is supposed to provide as currently implemented is greatly needed as it stands - even without this additional formatting layer. In most cases when logging comes up as a requirement for me those logs need to be conditionally and partially enabled at run-time, are used only for manual inspection without any automated tools or where quick shell scripts using grep/sed/awk suffice and there is not great big-brother enterprise infrastructure that these logs need to integrate into. So in those cases such standardized tools, even though potentially nice, are not strictly required. Perhaps as I am not really knowledgeable in that 'enterprise domain' I do not really see how such an additional layer (a new one) could ever integrate into existing enterprise environments. Such places are most often too rigid to change their existing ways/formats/tools... Thus any such future layer seems like it would potentially help only new projects. That said, I'd really like if Boost Logging or an additional extension library would provide such a layer, however I would not impose that on John as a must. On the other hand... I understand the need and would give a *nudge* in persuading him to take it on. :-))) It seems to me that there is more chance of that layer getting implemented if the library is made public as a part of boost which would gain it more users than forcing John to implement the additional layer. He could very well get tangled up in real-life work in the mean time, and not be able to finish the library (at least not on time), and the already done parts (quite useful by themselves) would not get enough users of the right caliber for a new maintainer to be found. Best regards, Jurko Gospodnetić

...

I also suspect that if the "new layer" issue was addressed up front it would push the design around a bit. There could be a cleaner separation between a layer that transports bytes from the point of logging to the place of storage and the layer that codes and decodes application data to and from the byte stream.

I would suggest a compromise here though as what this library is

I could not agree more :)

supposed to provide as currently implemented is greatly needed as it stands - even without this additional formatting layer. In most cases when logging comes up as a requirement for me those logs need to be conditionally and partially enabled at run-time, are used only for manual inspection without any automated tools or where quick shell scripts using grep/sed/awk suffice and there is not great big-brother enterprise infrastructure that these logs need to integrate into. So in those cases such standardized tools, even though potentially nice, are not strictly required.

Very true!

That said, I'd really like if Boost Logging or an additional extension library would provide such a layer, however I would not impose that on John as a must. On the other hand... I understand the need and would give a *nudge* in persuading him to take it on. :-)))

Oh well - I can't promise anything. I might actually take on this challenge - 'cause I really love logging - so as soon as I have some free time.

He could very well get tangled up in real-life work in the mean time,

Oh yes - busyness is a second nature to me ;) Best, John -- http://John.Torjo.com -- C++ expert http://blog.torjo.com ... call me only if you want things done right

the log file. Either useful data is buried (cannot be extracted) or inconsistently represented.

Some examples of representation;

After completion there were 3 leftovers and ... After completion there were <3> leftovers and ... After completion there were <3!unsigned int> leftovers and ... After completion there were <3!unsigned int/matching_profile> leftovers and

The last representation : what does "/matching_profile" mean? Based on what you've just printed above, how would you see the 3 value above? Like this: unsigned int leftovers_count; If so, the first 3 representations would be rather easy to implement.

...

You might argue that your library does not prevent such output. Which would

Indeed so ;)

be true. What I am suggesting is that a Boost logging library should provide explicit solutions to such issues. With such formalizations in place a much

Actually, I would expect a Boost Format library to handle those issues.

more ambitious viewer can be developed, giving a richer user experience than notepad or nano. Analysis can be more accurate, reliable and ambitious.

I'm all for that. However, I'd rather have a layer on top of Boost Logging, or completely separated, to handle your specific case, than to force the users onto the above scenario.

b) A customer requires a low-cost, low-tech, highly robust audit trail of all activity on their system. It is decided to log all events into text files. Each event involves a time, a unique identifier and a list of zero or more runtime values (i.e. parameters).

About "event involving a time, unique id, list of ..." - could we be talking about tags? I believe those should actually be what you want: http://torjo.com/log2/doc/html/namespaceboost_1_1logging_1_1tag.html

If I could write something like; #include <boost/logging.hpp>

void test_mul_levels_one_logger() { L_ << "hello world";}

I see you've posted a different email regarding to this - I'll answer that in a different email. Best, John -- http://John.Torjo.com -- C++ expert http://blog.torjo.com ... call me only if you want things done right

Hi John,

Paused for food. A picnic with friends over the local surf break, on a balmy night in the South Pacific. Ah but back to more important things ;-)

That sounds pretty cool ;)

...

The last representation : what does "/matching_profile" mean?

It was intended to be an example of an encoded variable reference. The last component being the supplied variable name.

No, like this (choice of identifier not helpful);

unsigned int matching_profile;

As a side-note - you do know that if you truly want this: "After completion there were <3!unsigned int/matching_profile> leftovers", you'll need a macro somewhere, like: L_ << "After completion there were " << V_(macthing_profile) << " leftovers"; However, there should be a quite different solution to your problem: - when logging, automatically log the filename & line (or at least some unique ID from which you can get them) . This is possible with the current version of the lib. - then, have a viewer that parses both a log, and the source code for each logged line. Then, it can match a certain logged message with the actual line of C++ code. It can then interpret it. Thus, even if you have: After completion there were 3 leftovers and ... It will know 3 is of type "unsigned int", and is called matching_profile.

...

About "event involving a time, unique id, list of ..." - could we be talking about tags? I believe those should actually be what you want: http://torjo.com/log2/doc/html/namespaceboost_1_1logging_1_1tag.html

Think it's fair to say no. While technically I suspect tags could be applied to the problem at best it would be unwieldy.

The crucial piece seemed to be the declaration of the tags macro where the log message is decorated with information from the runtime context. This might be on a per-module or global basis.

I'm guessing that in my circumstance there would be a set of declared tags for each event type.

Could you give me a specific example? Because we're talking in too generic terms ;) Best, John -- http://John.Torjo.com -- C++ expert http://blog.torjo.com ... call me only if you want things done right

...

- then, have a viewer that parses both a log, and the source code for each logged line. Then, it can match a certain logged message with the actual line of C++ code. It can then interpret it.

Or it can integrate with the IDE. There is a lot more potential hiding under the encoding issue. Seamless navigation to source code line

As a side-note, right now you can log into the IDE, in case that IDE is for Windows - see destination::dbg_window()

is a simple example. A form of this is already achievable with your current library (I believe), just route the debug messages to the MS IDE and format the file+line information as that environment expects.

Yes ;)

Highlight the debug message and a function key will then put you on the spot. I'd assume that others have already wired that together.

Actually just pressing Enter on a line is enough :)

I have recently been through the third iteration of a significant logging development. One of the biggest things to emerge was the value of a standard encoding. Another was the difficulty of selling the idea ;-)

By standard encoding - do you mean how information is formatted, before being outputted, or this : http://en.wikipedia.org/wiki/Character_encoding ? Best, John -- http://John.Torjo.com -- C++ expert http://blog.torjo.com ... call me only if you want things done right

Scott Woods <scott.suzuki <at> gmail.com> writes:

You might argue that your library does not prevent such output. Which would be true. What I am suggesting is that a Boost logging library should provide explicit solutions to such issues. With such formalizations in place a much more ambitious viewer can be developed, giving a richer user experience than notepad or nano.

Again I have the perception that your library does not preclude such output but also includes no specific solution. Which does not rule out your library in such circumstances but it does mean that supporting work would be required.

In my humble opinion, while youe point might be important in itself, it is kinda irrelevant to the submission under review. Absence of log format protocals, graphical viewer for logs or log query language shouldn't affect our decision IMO, unless library is not fexible enough and can't support what you have in mind. If you are interrested in bringing something like this to review I am sure it will have some interress. IMO the relationship here is similar to XML and all garden veriety of technologies build on top of it. Gennadiy

[This is a continuation of my previous premature post - is there a pill for that?]

...

- where is the default mode that just works "out of the box"

How about this: http://torjo.com/log2/doc/html/scenarios_code.html#scenarios_code_mom

If I could write something like; #include <boost/logging.hpp> using ... void test_mul_levels_one_logger() { log() << "hello world"; } and this resulted in a file with the content; 2007-2-7@13:33:22 "hello world" The file would have the same name as the application and would appear in either a configured folder, a default folder or the working folder. That would be closer to "out of the box" for me. Again there wasnt anything in your library precluding this. I would just have to create the supporting code.

...

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

Useful. This looks after most of the needs relating to developers debug output and also more permanent support output. It does not appear to consider what often occurs with such output - it may be the best or only record of system activity and will become the focus of more complex post-processing and analysis. The library does not specifically cater to such activity.

Not sure what you mean. Why do you think I neglected that?

See below.

...

* Do you think the library should be accepted as a Boost library? Be sure to say this explicitly so that your other comments don't obscure your overall opinion.

No. As an example of a developers debug logging facillity this is one of the better ones. A Boost logging facillity should be targeted slightly differently. Solving the requirements of developers debugging should be a side benefit of solving the wider issue of recording system activity for any of the system stakeholders.

Again, not sure what you mean - and please explain why my lib doesn't solve the above issue. Thanks.

The short answer; there is no encoding specification. There is a de facto standard; a stream of readable bytes separated into distinct records by newline characters. A standard encoding would allow downstream tools (analysis and viewing) to be much more robust and also application independent. Cheers, Scott

----- Original Message ----- From: "John Torjo" <john.groups@torjo.com> To: <boost@lists.boost.org> Sent: Thursday, February 07, 2008 4:06 PM Subject: Re: [boost] Review of Logging library begins today February 4th

...

How 'bout this:

// no levels #include <boost/logging/out_of_the_box/no_levels.hpp>

[snip] You pretty much nailed it. Are you saying that these headers or something like them will be part of Boost.Logging?

On Wed, Feb 06, 2008 at 01:57:38PM +1300, Scott Woods wrote:

If I could write something like;

#include <boost/logging.hpp>

using ...

void test_mul_levels_one_logger() { log() << "hello world"; }

and this resulted in a file with the content;

2007-2-7@13:33:22 "hello world"

Aigh! Please, no! If anything, please default to ISO date and time: "2007-02-07 13:33:22". -Steve

Jurko Gospodnetić wrote:

Hi.

...

Aigh! Please, no! If anything, please default to ISO date and time: "2007-02-07 13:33:22".

I second that...

Updated, will commit after the review Best, John -- http://John.Torjo.com -- C++ expert http://blog.torjo.com ... call me only if you want things done right

Hi John. Just some more notes I took while reading the documentation. Some are corrections and some are questions. * Change-log has some old revision notes (0.4 - 0.7) ordered incorrectly. * Term 'namespace' as you use it when not referring to C++ namespaces should most likely be changed to 'concept', but I believe someone mentioned this. * Some nitpicking. * Many lists inconsistently have items with and some without a trailing dot. * Many lists inconsistently have lowercase/uppercase leading letters. * Feature list at http://torjo.com/log2/doc/html/main_intro.html. * Lose the 'as you'll see' at the end of the thread-safety feature. Such extra commenting is OK for some 'monologue' part of the documentation like some rationale explanation but does not seem to fit here. * Generally do not write the docs using first-person style. That way the docs become too personal and may irritate the reader because he takes the information in it too personally. Again... may be OK for some parts but for most of it phrases like 'I do not know...' should be replaced with 'There is no way to know...' or something similar. * Example: 'Note that I encountered a very big problem, when implementing scoped logs: I don't know how you gather your message, when using the logs. In other words, I don't know your Usage Syntax. So I had to make a few assumptions, as you'll see.'. * http://torjo.com/log2/doc/html/scoped_logs.html * 'as first' ==> 'as the first'. * Phrase 'You cannot use any variable from your scope, nor any other variables' when describing BOOST_SCOPED_LOG seems unclear. Most likely remove it as its point is actually implied by the first item on that same list. * 'contactenates' ==> 'concatenates'. * '...are allowed' ==> 'Multiple scoped logs are allowed'. * http://torjo.com/log2/doc/html/caching.html * 'which, until you initialize your logs, do some logging' ==> 'which do some logging before you initialize your logs'. * 'and it its body' ==> 'and in its body'. * 'context do rely on' ==> 'context do rely on'. * mark_as_initialized() function. * Could use a link from here to the appropriate reference manual part of the documentation. * If the user does not call this function explicitly, when will it get called? And why do you keep calling it explicitly in all the example code? * Seems like a bad name for what it is intended to do. If it actually does change the logger's state to 'initialized' then it should be an internal implementation detail function and not made public. If it only raises a flag meaning 'do not cache log messages for this logger' then it should be renamed to something like disable_caching() or enable_caching( false ). * Documentation here gets a little mixed up with what you are supposed to call this function on: the log or the cache. Also it is not clear what is supposed to be considered initialized. * E.g. 'when cache is marked as initialized' is not something I can understand intuitively... how can a 'cache' be initialized... what could this 'cache initialization' be since log messages have already been cached and later on there is no need for this 'cache'. * In another place you state 'when the log is marked as initialized'. * Inconsistent final paragraph and example order in BOOST_LOG_BEFORE_INIT_LOG_ALL and BOOST_LOG_BEFORE_INIT_CACHE_FILTER documentation. * How do BOOST_LOG_BEFORE_INIT_LOG_ALL, BOOST_LOG_BEFORE_INIT_CACHE_FILTER and BOOST_LOG_BEFORE_INIT_IGNORE_BEFORE_INIT macros work together? Since BOOST_LOG_BEFORE_INIT_LOG_ALL is on by default do the other two simply override this one? Or is the user expected to disable this one somehow? * Update copyright notices to 2008. * Common Usage paragraph has a line saying 'Click to see the code' which is not a link to anything. :-) The same example is duplicated as Scenario 1 and there this line is linked correctly. Is this your doc tool bug or is there actual content duplication here? * http://torjo.com/log2/doc/html/boost_logging_requirements.html * States scoped logging will be implemented soon, while it already has been. * 'statemets' ==> 'statements'. * 'Thoghths' ==> 'Thoughts'. * How would one achieve the following: you have an application logging messages organized into different groups (categories/whatever...) and you want to say at run-time: log this group to this file and to debug output, do not log that group at all and log that third group only to this file. Would this require separate logger objects? Or can filters somehow decide which destinations they disable/enable? * What are the exception safety guarantees? What can throw std::bad_alloc for example? What other exceptions can be thrown by the Boost Log library? If ordinary logging can throw, is it safe to use in destructors and how can scoped logging be safe then at all? * http://torjo.com/log2/doc/html/namespaceboost_1_1logging_1_1tag.html * What is that '#include <boost/logging/format_fwd.hpp>' line in the 'Tags - explained' part. * '(from optimize namespace)' should be linked to the corresponding namespace/concept reference documentation. And now it's 6:19 here... and I really need to get some sleep too. :-) Will try to finish reading the docs tomorrow but will not be able to try it out and prepare a review until later next week. Hope this helps. Best regards, Jurko Gospodnetić

Hi John.

...

* Some nitpicking. * Many lists inconsistently have items with and some without a trailing dot. * Many lists inconsistently have lowercase/uppercase leading letters.

this took a while to fix...

Hehe... this was the one item I actually expected to be brushed off with a 'what the f***'-style comment... Hats off for the effort... :-)))

...

* If the user does not call this function explicitly, when will it get called? And why do you keep calling it explicitly in all the example code?

it needs to always be called explicitly

...

* Seems like a bad name for what it is intended to do. If it actually does change the logger's state to 'initialized' then it should be an internal implementation detail function and not made public. If it only raises a flag meaning 'do not cache log messages for this logger' then it should be renamed to something like disable_caching() or enable_caching( false ).

It actually gets all messages that were cached, and writes them to their logger destinations.

Perhaps there is a different way to implement this that would prevent errors such as forgetting to 'mark a logger as initialized' and also not cause the developer to ask himself 'why is this library so complicated, why do I need to call something called 'mark_as_initialized'... Some ideas (note that these are just things 'off the top of my head' so might not be represent a good design as they stand :-)): - better naming... documenting that a logger may be in two states - enabled and disabled, that caching is used while the logger is in the disabled state and renaming mark_as_initialized() to enable()/set_enabled()/set_state()/something to that effect... - effectively two states but implemented by allowing the user to prepare some sort of a LoggerParameters object that would contain all the Logger settings he needs and then passing that LoggerParameters object to a Logger constructor. Then constructing a Logger in such a way would automatically be in the enabled state and you can still have a separate constructor that enables caching if needed. That way someone who does not care about multiple-threads would not have to even consider things like 'my logger has not been initialized yet' (and this does seem as the most common use case) and those that do would have an understandable interface for making sure their separate threads get synchronized correctly. - another variation (and the one that seems like the best choice) would be to make the logger active by default and only disable it (i.e. enable caching) explicitly. That way the most common use case would be simple and intuitive... and when someone needs caching - they will have no objection to having to enable it - and will even be glad that they have that functionality available. :-)

...

* How do BOOST_LOG_BEFORE_INIT_LOG_ALL, BOOST_LOG_BEFORE_INIT_CACHE_FILTER and BOOST_LOG_BEFORE_INIT_IGNORE_BEFORE_INIT macros work together? Since BOOST_LOG_BEFORE_INIT_LOG_ALL is on by default do the other two simply override this one? Or is the user expected to disable this one somehow?

The other two can override it.

If you implement 'optional' caching as suggested above then would not all these macros be made redundant? This would also be a step towards pleasing all the 'macro-are-evil' people... :-) (Note, I am one of them... just have not still taken the time to fully understand why each of them is used in Boost Logging. :-) )

fixed fixed fixed fixed fixed

Thanks... :-)

...

* How would one achieve the following: you have an application logging messages organized into different groups (categories/whatever...) and you want to say at run-time: log this group to this file and to debug output, do not log that group at all and log that third group only to this file. Would this require separate logger objects? Or can filters somehow decide which destinations they disable/enable?

You'd need a different logger per category.

Hmmm... ok... I have not taked the time to research this... but my fear with this is that this might cause logging support to have a large footprint. This also seems to be related to one of the objections made by Andrey Semashev - with supporting more detailed filtering criteria. His context entities would seem to correspond to what I named 'categories/whatever...' above.

...

* What are the exception safety guarantees? What can throw std::bad_alloc for example? What other exceptions can be thrown by the

There are a few objects that can throw std::bad_alloc - the classes that use TSS and array::shared_ptr_holder. I can pre-allocate some memory for the shared_ptr_holder and it will be a bit harder for the TSS classes.

Making it not throw is one thing - useful but perhaps not strictly necessary. In any case could you just document which functions may throw what (possibly stating where needed that they throw whatever gets thrown by some user callback - if such things are used).

...

* http://torjo.com/log2/doc/html/namespaceboost_1_1logging_1_1tag.html * What is that '#include <boost/logging/format_fwd.hpp>' line in the 'Tags - explained' part.

what you need to include in order to use tags

Perhaps add that note to the docs. :-).

Note: I'll upload the changes to http://torjo.com/log2/doc/v3/html/

The link does not work for me. Perhaps you just did find the time to upload it yet. Best regards, Jurko Gospodnetić

Hi John, Yet another suggestion. Documentation that was structured something like; * Overview Of A Configurable Logging Facillity * Canned Configurations * Just Using A File * In The Presence Of An IDE * Sending To Syslog * ... * The Default Configuration * Advanced Configuration While the configurability of your library is perhaps its greatest selling point, without the bundling of "kickstart" code and a presentation such as that shown above it forces first timers to learn how to configure. I remember clicking your "Fast And Furious" with a mixture of guilt and relief only to be faced with material that still assumed a deeper understanding. There is an analogy to be drawn with the STL. I was a wary adopter of strings and maps, only using them in the simplest ways. Documentation for such usage was plentiful. But it's the deeper potential of these STL templates that has made me a convert. Cheers.

John Torjo wrote:

...

...

* What is your evaluation of the design?

In short, questionable. More elaborate notes follow:

* To my mind, the named destination design approach is not intuitive.

Right, because that's how you implemented you lib. That is so biased.

Well, believe me, I tried to be as objective as I could. Look at it from another side, I wouldn't have started my lib if I was happy with another (yours?) solution. I'm missing some features and that's what I expressed in my review. And since it is your library review I propose to concentrate on your solution, not mine. Note to the review manager: You are free to decide whether to take my vote into consideration or not. Any decision would be fine with me.

First, if that were true, we should ban scripts, 'cause you might do typos.

Every source of errors is evil. Especially the ones that show in run time. If we can get rid of them, why not?

Second, how many times do you initialize a logger? Oh yes, just once...

That's a tricky question. I'd like to have one such place. But first, as I've noted below I seem to have such code in each module. Second, even this single piece of code will be modified some day, and the one who modifies it may introduce an error.

Third, this allows you to initialize the logging from a configuration - which your lib can't quite do.

I was talking about this kind of code: g_l()->writer().add_destination( destination::named("cout out debug") .add( "cout", destination::cout()) .add( "debug", destination::dbg_window() ) .add( "out", destination::file("out.txt")) ); Ok, I agree that it is _possible_ to initialize it after reading the file, but not convenient (at least for me). I don't see why would I need to duplicate these "cout out debug". Actually, I don't see why would I need these names in the first place.

...

time. It also complicates making the list of destinations configurable (e.g. loading logging settings of the user's app from a file).

* The approach to adding formatters is not very intuitive.

That's why I have the named syntax - which of course, you dislike.

Named formatter syntax, I missed that... You have a point here, although I would really like a lambda syntax support along with it.

...

* Lifetime of the objects provided by users, such as streams, should be controlled by the library.

That's easy - I'll have a destination::stream_ptr - it's on my TODO list.

Why would you need yet another smart pointer?

However, what if you just have a raw pointer to an existing stream, and you just can't get a shared_ptr?

How would you get such raw pointer? Anyway, in case if someone else is controlling the stream life time you can always provide an empty deleter to shared_ptr and keep your headache.

...

* Filtering looks way too minimalistic to me.

What? I really don't get this. A filter is not always static - it's an object, and you can manipulate it any way you'd like.

I saw it coming... I didn't mean "static" in C++ terms. What I meant is that filters have no clue on what they are filtering. The is_enabled function cannot make its decision on the context of execution. For example, you cannot make a filter that will discard all log messages from objects of class A and pass through messages from objects of class B. Unless they use different loggers, that is.

I can't believe you take this flexibility as a shortcoming. But please, explain how you see the concept of filtering.

Well, the example above illustrates it. Filtering is intended to selectively discard the unneeded information from logs. The criteria of such selection can be very complex and change during run time. For example, I run an ftp server and I want to see how much connections come from France. I'd like to be able to configure my server log filtering (or statistics) to log all connections from french IPs to a separate file. I don't see how I would implement it with your concept of filtering.

...

* I don't see the reason of all these macros for declaring and defining logs, tags, etc. I would rather prefer to see functions or objects forward declarations instead.

http://torjo.com/log2/doc/html/defining_your_logger_filter.html#declare_defi... http://torjo.com/log2/doc/html/defining_your_logger_filter.html

I've read it and I'm not convinced.

...

Anyway, I don't like functions like this, they are always a source of mistakes.

And what would the alternative be?

The logger gets initialized after at least one destination is added. But you didn't answer, will I be able to modify the set of destinations in run time?

...

* I see that there is no easy way to use once-initialized logs in module A (exe or dll) in another module B.

I really don't get what you're trying to say here. The dll_and_exe just shows that from EXE, you can use logs you define your own (from the EXE), and logs from another module - the DLL.

What code duplication are you talking about?

The log.cpp file in both exe and dll. What I want is: 1. Initialize logging somewhere in the beginning of the execution. For example, in the main. 2. Use the initialized loggers everywhere in the app - in exe itself and other dlls that may have not a slightest idea of where and how these logs are actually stored.

...

* The most critical note, from my point of view. I didn't find any trace of attributes or something like that. The library focuses on processing

Because you didn't look - it's tags, and it's way more powerful than what you have. http://torjo.com/log2/doc/html/namespaceboost_1_1logging_1_1tag.html

Been there. As far as I understood, it's yet an other way make formatting more flexible, and I didn't figure out when do I actually need it.

Why? Because your "attributes" are always there, and are fixed. Not to say that turning them off can be a pain.

Well, I want them to be there. Moreover, sometimes I want them to be everywhere the execution comes, whether it be another function, class or module. This is a powerful way of execution trace markup.

Tags, on the other hand, are flexible - you can add your own tags (tag classes), and adding tags or turning them off is just one or two lines of code.

That's fine, but what can I do with tags except to format them into the log message? Can I use them in filters? Can I use them to automatically generate values (for example, automatically log an actual count of objects of class A)?

...

* The design is not OOP-friendly. I can't create a logger object that is specific for a request that my application is currently processing (or

.... initialize it

That's the point. Since loggers also contain formatters and writers I would have to duplicate their initialization in each class if I want to have an object-specific logger. That's a burden, not to mention the performance loss.

...

can I?). This might be very useful if I wanted to log the context of execution (the request parameters, again, as attributes or a similar feature) in each log record. One can try to emulate such feature with a

Please elaborate on what you're trying to do here. It's way too vague...

Remember the ftp server example above? If I had a class that would represent an ftp connection (say, CFTPConnection), I would prefer to have a separate logger in each instance of this class. Why? Because then I could add connection-specific attributes to it, such as client IP, current state (idle, downloading, uploading, etc.)., connection duration, whatever. All this information would be implicitly available while I merely log at some checkpoints ("Requested file X", "Transmission complete"...). I can't do that with your solution.

...

* The "optimize" namespace and "favor" stuff is a questionable. IMO, the code should not contain things like this. Why would I chose to use non-optimized features? Because optimized ones lack some features or

Oh boy. The "favor"

I'm not against optimizations. If you feel these tools may provide benefits - fine. If there is a string type that performs better than STL but has restriction (honestly, who would need to optimize this, especially if you employ in-place formatting?), ok, place it into tools subdirectory. But please, don't call it optimized_string or optimized::string. Call it fixed_string, limited_string or preallocated_string if it limits the maximum buffer size. If you provide favor::speed, at least tell me what does it mean. Or better - make the name mean something. E.g. use_record_caching or whatever does it actually mean.

...

* Maybe a compilable configuration should be provided to reduce user's code build times. This would be a better solution than to depend on a macro that has different default values in release and debug.

Like BOOST_LOG_COMPILE_FAST_ON, BOOST_LOG_COMPILE_FAST_OFF? Which are already there

Exactly. I was saying that a precompiled library would be a better way than these macros.

...

* A better header segregation is needed. And it would be good to see in docs what I have to include to use each component of the library.

You mean: http://torjo.com/log2/doc/html/headers_to_include.html

Not exactly. What I meant is that on each component's description page there should be an appropriate #include. But I guess this info is available on the reference page. About a better header segregation. What I meant is that if I need logger, would have to include logger.hpp, if I need tag::file_line, I would include tag/file_line.hpp, etc.

...

* I can see that filtering may unnecessarilly block threads. That could be fixed with read/write mutexes.

What? Please explain.

detail/filter.hpp:146. ts::is_enabled acquires a scoped lock of a mutex. The function only reads the value, but I assume it may read it in multiple threads, doesn't it? If so, you could have used r/w mutex and acquire a read lock which doesn't block if there is a write lock.

...

* Use __LINE__ with care when compiling on MSVC 7.0+ with /ZI. You may run into problems when forming unique variable names. Use __COUNTER__ instead in such cases.

I think I have. Please give me an example of where I used this badly.

detail/scoped_log.hpp. BTW, it is used with BOOST_LOG_CONCATENATE, which I couldn't find anywhere but this file.

...

* Don't count on that pthread_t is ostreamable. Some platforms define it as a fundamental type, some as a structure. It may well be a char* which will most likely cause crashes in your code.

Thanks, didn't know that. What do you suggest? Dumping it as a (void*) pointer?

You might try to detect in compile time whether it is integral, pointer or structure. The first case is straightforward, in second you could cast it to uintptr_t, the last one is trickier. I guess the most portable way would be to dump it as sequence of sizeof(pthread_t) bytes (in hex, that is).

...

...

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

I have a dual feeling about this. On one hand it provides a good set of features to implement logging in a relatively small and simple application. On the other hand, would I bother using it instead std::cout? Maybe.

I would say it would work on any size application. Why just small and simple application?

The reasons I have noted above. The large application usually consists of many modules and generates more logs. I have identified inconveniences in both support of modules and filtering logs.

...

...

* Do you think the library should be accepted as a Boost library?

No, at least at its current shape. I expect something different and more elaborate from the logging library. I believe that logging is not only writing strings into a file but it is an event reporting system. The library should be flexible enough to be able to support statistics gathering and alarming. The library should provide a flexible filtering

Please explain why my lib doesn't allow for statistics gathering and alarming.

Because your solution is geared to processing of strings. And statistics and alarming involve pieces of the application data, such as number of requests processed, amount of data transmitted, number of active connections, failure code and parameters, etc.

...

mechanism based on attributes of the logging records. I cannot see this

flexible filtering - why isn't this flexible enough? What do you want that is not in there?

See above.