Review: std.logger

linkrope via Digitalmars-d digitalmars-d at puremagic.com
Sun Jul 20 09:15:52 PDT 2014


Pros
----
The lighning talk about the std.logger proposal at DConf 2014 had 
a positive impact.
We were able to change the "Current D Use" entry of our company 
from
     "Uses D2 / Phobos, Tango (log, xml)"
to
     "Uses D2 / Phobos, Tango (xml)".
(We got rid of tango.util.log; we still rely on the fast 
tango.text.xml.)

Cons
----
1. I am not happy with the (reverse) hungarian-style naming

At least in the code of our company, logging a formatted string 
is the basic use case.
The function for the basic use case should not require a suffix 
letter.
The consistency argument, that 'infof' is like 'writef', does not 
fully apply:
neither 'infoln' nor 'infofln' make sense.
(In my opinion, "half consistent" is inconsistent.)

Currently, suffix 'c' is used for conditional logging.
But, how will future extensions like glog's LOG_EVERY_N or 
LOG_FIRST_N be named?
With suffix 'e'? Suffix 'f' is already assigned!
The suffix letter sequence seems to be the road to confusion.

I would prefer the explicit naming of the previous std.log 
proposal:
     log.when(condition)(...)
However, there is only a small advantage over
     if (condition) log(...)

2. No support for 24/7 (server) applications

In my opinion, I really need logging for applications that 
possibly run forever.
With the FileLogger, the file will grow forever.
That's why most other frameworks provide something like a 
RollingFileLogger or some "logrotate-aware" FileLogger.

By the way: Instead of what I really need, I get a NullLogger.
I have no clue, why I never ever missed such an oddity.

3. Implementation is hidden behind 'mixin' expressions

When I tried to look at the implementation, I found long 
sequences of lines like this:
     mixin(buildLogFunction(true, false, false, LogLevel.info));

Nowadays, this changed into:
     mixin(freeLog.format(
         "info", "info", "info", "info",
         "info", "info", "info", "info",
         "info", "info", "info", "info",
         "info", "info", "info", "info"));

This is much better, but I still think, it's a complicated 
solution for a simple problem.
And it would be a shame for D, if there is no simple solution.

Small stuff
-----------
4. FileLogger needs flush

It's annoying when the events that caused a crash have been 
logged, but they never have been written to the file.

5. Suspect use of '__gshared'

The FileLogger has a field
     private __gshared File file_;

In this case, "__gshared is equivalent to static".
This means that all FileLogger instances share the same file!

6. Bad naming of "StdIOLogger"

Like 'std.stdio.StdioException', the 'io' should be lower case.
If the 'StdIOLogger' logs to 'stdout', 'StdoutLogger' would be 
preferable.

7. No need for StdIOLogger

'stdout' (and 'stderr') are Files, so a FileLogger should be able 
to handle them.
A second constructor should do the trick.

8. Log levels

Many frameworks mix the types "log level" and "set of log levels" 
(for filtering).
While 'trace', ..., 'fatal' are log levels, 'all' and 'off' 
(better: 'none'?) are sets of log levels.
(I have no idea about the type of 'unspecific'.)

A clean separation would avoid confusion:
why is there 'info(...)' but not 'all(...)'?

Also, it would be easier to log for example 'trace' and 'info' to 
'stdout'.

9. Bad naming of "std.logger"

The focus of this proposal is on the log/logging API; the loggers 
are only examples.

The recommended use should be
     import log = std.logger;

Then, the name "std.log" (of the previous proposal) would be more 
appropriate.

Counter Proposal
----------------
As a consequence of these issues, I once decided to spend a 
weekend to prepare a counter proposal:

http://code.dlang.org/packages/log

The design goal was simplicity. So:
- conditional logging is not supported
- no suffix letter sequences
- there is no NullLogger
- there is no MultiLogger (this functionality is implicit)
- there is no need to provide a name for a logger

I prefer 'alias' over 'mixin':
'info' is just an alias for 'log(arg)' as well as for 'log(fmt, 
args)' at log level 'info'.

Sets of log levels are implemented as (bit) sets of log levels.
A helper function lets you select the traditional >= filtering:
     LogLevel.info.orHigher

For convenience, 'stdoutLogger' and 'stderrLogger' are factory 
functions that create 'FileLogger' instances.

Of course, a RollingFileLogger as well a a "RotatingFileLogger" 
(that reopens the log file on SIGHUP) are provided.

By now, this simple solution is in use in tens of thousands lines 
of commercial code.
(Where it outperforms the previously used tango.util.log 
implementation.)


More information about the Digitalmars-d mailing list