As `std.logger` is still marked as "work in progress" this thread is less formal that typical pre-voting review. Goal is to provide as much input about desirable `std.logger` functionality and current state and let module author to use this information for preparing proposal for actual review / voting.

Lets unleash the forces of constructive destruction.

===== Meta =====

Code: https://github.com/D-Programming-Language/phobos/pull/1500/files
Docs: http://burner.github.io/phobos/phobos-prerelease/std_logger.html

First announcement / discussion thread : http://forum.dlang.org/post/mailman.313.1377180809.1719.digitalmars-d@puremagic.com

Am 14.10.2013 13:39, schrieb Dicebot:
> As `std.logger` is still marked as "work in progress" this thread is
> less formal that typical pre-voting review. Goal is to provide as much
> input about desirable `std.logger` functionality and current state and
> let module author to use this information for preparing proposal for
> actual review / voting.
>
> Lets unleash the forces of constructive destruction.
>
> ===== Meta =====
>
> Code: https://github.com/D-Programming-Language/phobos/pull/1500/files
> Docs: http://burner.github.io/phobos/phobos-prerelease/std_logger.html
>
> First announcement / discussion thread :
> http://forum.dlang.org/post/mailman.313.1377180809.1719.digitalmars-d@puremagic.com
>

Sorry in advance for the long list of issues. I think the general approach is fine, but over the years I've grown some preferences for this stuff. Generally, I think we should make sure that such a module is flexible enough to fulfill most people's needs, or it will probably fail the widespread adoption that is desired to actually improve interoperability.

 - LogLevel: enum values should start lower case according to the
   Phobos conventions.
 - The static methods in LogManager should be made global and the class
   be removed. It's not for objects so it shouldn't be a class.
 - For me this logger is completely worthless without any debug log
   levels. The last std.log entry had at least anonymous verbosity
   levels, but I'd prefer something like I did in vibe.d [1], where
   each level has a defined role. This should especially improve the
   situation when multiple libraries are involved.
 - My experience tells me that logging formatted messages by default
   (or even drop the non-formatted version) is perfectly fine, but
   others may differ of course and an additional "f" is not that bad.
 - Similarly, I've never felt the need for conditional logging, but
   without it being lazy evaluated what's the use for it, actually?
 - I really think there should be shortcuts for the different log
   levels. Typing out "LogLevel.xxx" each time looks tedious for
   something that is used in so many places.
 - There should be some kind of MultiLogger so that multiple log
   destinations can be configured (e.g. console + file). Or, instead of
   a single default logger, there could be a list of loggers.
 - Logger.logMessage: Exchanging the bunch of parameters with a single
   struct that is passed by reference makes the API much more
   flexible/future-proof and is more efficient when the data needs to
   be passed on to other functions.
 - "shared" - probably best to leave this out until we have a verified
   design for that - but in theory the loggers should probably be
   shared.
 - On the same topic, if I'm right and the default logger is stored as
   __gshared, it should be documented that Loggers need to be
   thread-safe.
 - GC allocations for each log message _must_ be avoided at all costs.
   Using formattedWrite() instead of format() on either a temporary
   buffer that is reused, or, better, on some kind of output range
   interface of the Logger would solve this.
 - A note on DDOC comments: The first paragraph of a doc comment is
   treated as a short description according to the DDOC spec. It should
   be kept to around a single sentence, followed by a more detailed
   paragraph. While this doesn't matter much with the current HTML doc
   layout, the short description is used in the single-page
   documentation inside of overview tables [2].

[1]: http://vibed.org/api/vibe.core.log/LogLevel
[2]: http://vibed.org/temp/d-programming-language.org/phobos/std/ascii.html

On Monday, 14 October 2013 at 11:39:52 UTC, Dicebot wrote:
> As `std.logger` is still marked as "work in progress" this thread is less formal that typical pre-voting review. Goal is to provide as much input about desirable `std.logger` functionality and current state and let module author to use this information for preparing proposal for actual review / voting.

Would be nice if either FileLogger, or a "proxy" logger available to users, would add timestamps. Timestamps are very useful for log files, as they allow to correlate logged events with other events on the system (e.g. other applications' log files or file modification times), so I think they should be there by default.

My personal preference of timestamp format is "[YYYY-MM-DD HH:MM:SS.FFF] " (though it needs to be either UTC or additionally specify the timezone, to disambiguate overlapping local time when DST ends).

My own few comments from quick overview of the documentation (must admit I did not pay much attention to previous logger discussion threads).

=== critical ===

1)

I don't like forced verbosity of mentioning logging levels. In my opinion logging is one place where laconic syntax elegance really helps. Would have been nice to have `error(...)` as an alias to `log(LogLevel.Error, ...)` etc.

2)

I don't like that default `log` function outputs conditionally. It is rather unexpected and less frequent use case. I think current `logf` should become `log` and current `log` turn into `logIf` (with matching `errorIf`, `debugIf` etc.)

3)

Simple way to define multiple loggers as default ones is missing. As far as I understand, currently it can be done by creating own Logger which embeds multiple simple ones and registering it as default in LogManager. I think such common task should have built-in solution.

=== possible features? ===

One interesting addition would be to embed module name into logger call and provide facilities to filter output on per-module basis (similar to log level). D introspection tools should allow to do that with no observable API complication.

Having skimmed through the docs I noticed that there are three features missing that I use and would like to see in standard logger. First is the ability to write to several loggers at once, the second is optional formatting of log output and the third is an option to tell logger to log only one concrete log level.

All these can be accomplished by writing my own logger that would do this, but it would be nice to have some batteries included, e.g.:

1) MultiLogger class that takes references to other loggers and just forwards the call to the log function.

2) Optional string parameter that describes the desired log output (i.e. format and position of timestamp, line, file, message, ...) for different needs, e.g. machine vs. human read log.

2.1) Allow more than one string to be logged. For example I want to add component name, task identifier, etc., but I want to let correct formatting on the logger.

***
Example:

auto machineLogger = new FileLogger(format = "%ts;%f;%l;{1};{2};{3};");
auto humanLogger = new FileLogger(format = "[%ts] :: %f:%l\nComponent:
{1}\nTask:{2}\n{3});

machineLogger.log("Some component", "Some task", "Everything is well");

output =>
2013-10-14 14:28:05;file.d;54;Some component;Some task;Everything is well;

humanLogger.log("Some component", "Some task", "Everything is well");

output =>
[2013-10-14 14:28:05] :: file.d:54
Component: Some component
Task: Some task
Everything is well
***

Regards,
Martin

On Monday, 14 October 2013 at 11:39:52 UTC, Dicebot wrote:
> As `std.logger` is still marked as "work in progress" this thread is less formal that typical pre-voting review. Goal is to provide as much input about desirable `std.logger` functionality and current state and let module author to use this information for preparing proposal for actual review / voting.
>
> Lets unleash the forces of constructive destruction.
>
> ===== Meta =====
>
> Code: https://github.com/D-Programming-Language/phobos/pull/1500/files
> Docs: http://burner.github.io/phobos/phobos-prerelease/std_logger.html
>
> First announcement / discussion thread : http://forum.dlang.org/post/mailman.313.1377180809.1719.digitalmars-d@puremagic.com

Just for comparison, on Android you can write something like:
FileLogger.w(...)  instead of FileLogger.log(LogLevel.Warning...)

(and there's a "wtf" loglevel for "temporary" debugging)

On Monday, 14 October 2013 at 12:45:06 UTC, Dicebot wrote:
> === possible features? ===
>
> One interesting addition would be to embed module name into logger call and provide facilities to filter output on per-module basis (similar to log level). D introspection tools should allow to do that with no observable API complication.

As a random idea this can be possibly done by providing default ModuleLogger in a similar way to default global logger, so that one can easily use both from the module context (i.e. logging most errors globally but making rest suppressible by module-specific settings)

On 10/14/2013 02:39 PM, Sönke Ludwig wrote:
> Am 14.10.2013 13:39, schrieb Dicebot:
>> As `std.logger` is still marked as "work in progress" this thread is less formal that typical pre-voting review. Goal is to provide as much input about desirable `std.logger` functionality and current state and let module author to use this information for preparing proposal for actual review / voting.
>>
>> Lets unleash the forces of constructive destruction.
>>
>> ===== Meta =====
>>
>> Code: https://github.com/D-Programming-Language/phobos/pull/1500/files Docs: http://burner.github.io/phobos/phobos-prerelease/std_logger.html
>>
>> First announcement / discussion thread : http://forum.dlang.org/post/mailman.313.1377180809.1719.digitalmars-d@puremagic.com
>>
>>
>
> Sorry in advance for the long list of issues. I think the general approach is fine, but over the years I've grown some preferences for this stuff. Generally, I think we should make sure that such a module is flexible enough to fulfill most people's needs, or it will probably fail the widespread adoption that is desired to actually improve interoperability.
>
>  - LogLevel: enum values should start lower case according to the
>    Phobos conventions.
will be fixed
>  - The static methods in LogManager should be made global and the class
>    be removed. It's not for objects so it shouldn't be a class.
LogManager also stores the global log level. Sure I can make another static global function storing this log level, but I would like to keep them together as they belong together IMO.
>  - For me this logger is completely worthless without any debug log
>    levels. The last std.log entry had at least anonymous verbosity
>    levels, but I'd prefer something like I did in vibe.d [1], where
>    each level has a defined role. This should especially improve the
>    situation when multiple libraries are involved.
Logger.log(LogLevel.(d|D)ebug, "Your message");
>  - Similarly, I've never felt the need for conditional logging, but
>    without it being lazy evaluated what's the use for it, actually?
The conditional logging part is totally transparent.
>  - I really think there should be shortcuts for the different log
>    levels. Typing out "LogLevel.xxx" each time looks tedious for
>    something that is used in so many places.
One could argue that writting logger.logDebug("...") is more tedious
than writing,
logger.logLevel = LogLevel.xxx;
logger.log("...");
logger.log("...");
...

This has been argued in the last logger discussion to some extend and it looked to me like this is the mostly preferred version.
>  - There should be some kind of MultiLogger so that multiple log
>    destinations can be configured (e.g. console + file). Or, instead of
>    a single default logger, there could be a list of loggers.
there is one default logger. I will create a MultiLogger, good point. I'm currently sure how to store the multi logger (LL or Array or ... )
>  - Logger.logMessage: Exchanging the bunch of parameters with a single
>    struct that is passed by reference makes the API much more
>    flexible/future-proof and is more efficient when the data needs to
>    be passed on to other functions.
good point
>  - "shared" - probably best to leave this out until we have a verified
>    design for that - but in theory the loggers should probably be
>    shared.
my thoughts exactly
>  - On the same topic, if I'm right and the default logger is stored as
>    __gshared, it should be documented that Loggers need to be
>    thread-safe.
It is not stored __gshared, but If, you're right.
>  - GC allocations for each log message _must_ be avoided at all costs.
>    Using formattedWrite() instead of format() on either a temporary
>    buffer that is reused, or, better, on some kind of output range
>    interface of the Logger would solve this.
This was proposed in the last thread. A fixed size buffer would scream bufferoverflow, a dynamic buffer not but both would raise the question of thread safety.
>  - A note on DDOC comments: The first paragraph of a doc comment is
>    treated as a short description according to the DDOC spec. It should
>    be kept to around a single sentence, followed by a more detailed
>    paragraph. While this doesn't matter much with the current HTML doc
>    layout, the short description is used in the single-page
>    documentation inside of overview tables [2].
Will be fixed

Awesome comments, thanks

On 10/14/2013 02:42 PM, Vladimir Panteleev wrote:
> Would be nice if either FileLogger, or a "proxy" logger available to users, would add timestamps. Timestamps are very useful for log files, as they allow to correlate logged events with other events on the system (e.g. other applications' log files or file modification times), so I think they should be there by default.
Good point, a timestamp will be part of the future struct passed down to logMessage.

On 10/14/2013 02:32 PM, Martin Drasar wrote:
> Having skimmed through the docs I noticed that there are three features missing that I use and would like to see in standard logger. First is the ability to write to several loggers at once, the second is optional formatting of log output and the third is an option to tell logger to log only one concrete log level.
>
> All these can be accomplished by writing my own logger that would do this, but it would be nice to have some batteries included, e.g.:
>
> 1) MultiLogger class that takes references to other loggers and just forwards the call to the log function.
will be done, see the reply to Sönke's post.
>
> 2) Optional string parameter that describes the desired log output (i.e. format and position of timestamp, line, file, message, ...) for different needs, e.g. machine vs. human read log.
I thought about that, but I'm not sure if that won't the logger to complex.  IMO it is better to have some reasonable hard-coded default than a log format parser. If the user needs something else, subclassing a logger and changing the format is a 7 liner. And this is the design Idea behind the logger.
>
> 2.1) Allow more than one string to be logged. For example I want to add component name, task identifier, etc., but I want to let correct formatting on the logger.
logf("name %s, taskId %s", name, taskId); already works

Thanks, keep it coming