Review of Jose Armando Garcia Sancio's std.log

[Steven Schveighoffer]

On Tue, 06 Mar 2012 14:19:27 -0500, Jose Armando Garcia  
<jsancio at gmail.com> wrote:

> On Mon, Mar 5, 2012 at 1:55 PM, Steven Schveighoffer
> <schveiguy at yahoo.com> wrote:
>> On Mon, 13 Feb 2012 10:50:04 -0500, David Nadlinger <see at klickverbot.at>
>> wrote:
>>
>>> There are several modules in the review queue right now, and to get  
>>> things
>>> going, I have volunteered to manage the review of Jose's std.log  
>>> proposal.
>>> Barring any objections, the review period starts now and ends in three
>>> weeks, on March 6th, followed by a week of voting.
>>
>>
>>
>> Some notes:
>>
>> I dislike that logging affects function execution.  In particular, I  
>> don't
>> think the logging library should have any business throwing exceptions  
>> or
>> errors.  It should be invisible to the application.  The equivalent  
>> function
>> can be had by giving a wrapper function (i.e. log this message at the  
>> fatal
>> level, and then throw an error).  A use case I can see is printing  
>> several
>> fatal log messages before exiting.
>>
> Then don't use std.log.fatal. It is not like you are forced to use it.
> You can implement the above by using std.log.error

Then I can't access the fatal level.

When I used Log4Net to log application errors, I only logged fatal errors  
to the event log (using an event log backend).  In fact, in that case, I  
was *catching* uncaught exceptions.  There was no need to throw another  
exception at that point.  My point is, whether to throw an exception or  
not should be up to the application, and having a fatal level can be  
utilized in other ways than "this is just like error but throws an  
exception".

Again, the logging library should not be in the business of dictating  
application design.

If fatal and critical logged an error at the "Error" logging level, and  
threw an appropriate exception, that would be a different story, because  
then it's just a convenience function.  But you have made two levels of  
logging unavailable without throwing exceptions.

As I brought up in another part of this thread, I envision the following  
pattern emerging:

try
{
    fatal("connection aborted!");
}
catch(LoggingError)
{
}

... // graceful shutdown of rest of the application
throw new LoggingError("connection aborted!");

>> The log aliases use names that are too common.  I think log.info is a  
>> better
>> symbol for logging than just 'info', which could be a symbol in a  
>> myriad of
>> places.  Given that D's symbol lookup rules allow shadowing of global
>> symbols, this does not work out very well.
>>
> This is a tough one. Should we be relying on D's module abstraction.
> It is not scalable as a module designer and implementer to think about
> other modules. This is why a lot of programming languages implement
> the concept of namespaces.

The problem is that by default D pulls in a module's symbols into the  
current scope.  You have to go out of your way to *avoid* this.  By  
default you should be able to just import std.log and not have your local  
symbols shadow std.log's.

There are many solutions to this, as I have brought up elsewhere.

> import log = std.log;
> log.info("hello world");

I like this better:

import std.log;
// alias log.info info // if you desire
log.info("hello world");

>> Like others have stated, I think vlog is a) confusing, and b)  
>> unnecessary.
>>  Even reading the docs, I can't understand what it's used for, and why  
>> it
>> has such different syntax than the normal logging stuff.
>>
> I have tried to explain this before but it looks like I have failed. I
> find it useful. If you are interested on a different explaination:
> http://google-glog.googlecode.com/svn/trunk/doc/glog.html

Someone else pointed that out.  I think the documentation explanation  
there is much more complete than your version, and I think vlog is fine,  
it just needs a lot more documentation.

>> Do we have to make the logger a singleton?  I'd like to see cases where  
>> I
>> can have different log instances.  For example, an instance I can
>> enable/disable per class type, or an instance that logs to a diffferent
>> backend.  Or a non-shared instance which does not need to handle  
>> threading
>> issues (i.e. a per-thread file log). Does this help with the vlog issue?
>>
> My point of view here is that as a developer I never know how I want
> to categorize my log during development. Some people use class name as
> a hack for doing this. What about functional programs that don't use
> class/objects? What about logical component/classes that span multiple
> classes? I always found class based grouping for logging as a hack. D
> made the observation that classes are not always the best abstraction
> unit so it introduced modules. std.log filters based on modules
> (actually source files to be exact but if D had __MODULE__, std.log
> would use that instead.)

I wasn't speaking so much about filtering logging based on classes (and  
BTW, most logging libraries use hierarchical symbols to denote logging  
instances, they don't necessarily have to follow class names), but simply  
being able to have multiple log instances.  That is, instead of having one  
global instance of log, what about multiple instances (I don't care if  
they are named or not).  They don't have to be based on some sort of name,  
just another place you can configure independently of the rest of the  
application.  For example, I may want a log instance in my library that  
logs to some library log file independent of the full application log.

-Steve