Round of a formal review before proceeding to voting. Subject for Phobos inclusion : http://wiki.dlang.org/Review/std.logger authored by Robert Schadek.

Code:
    https://github.com/D-Programming-Language/phobos/pull/1500
Documentation:
    http://burner.github.io/phobos/phobos-prerelease/std_logger_core.html
    http://burner.github.io/phobos/phobos-prerelease/std_logger_stdiologger.html
    http://burner.github.io/phobos/phobos-prerelease/std_logger_filelogger.html
    http://burner.github.io/phobos/phobos-prerelease/std_logger_multilogger.html
    http://burner.github.io/phobos/phobos-prerelease/std_logger_nulllogger.html
    http://burner.github.io/phobos/phobos-prerelease/std_logger_templatelogger.html
DUB package:
    http://code.dlang.org/packages/logger

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

======================================================================

Summary of changes since last discussion (by Robert):


* logger is now a package of multiple files
* a lot more documentation
* log and logf now behave as their write and writef counterparts
* for logging with an explicit LogLevel call logl loglf
* for logging with an explicit condition call logc logcf
* for logging with an explicit LogLevel and explicit condition call
loglc loglcf
* the log function with an explicit LogLevel like info, warning, ... can
be extended into c, f or cf and therefore require a condition and/or are
printf functions
* unittest have been updated
* dub package for easy testing

======================================================================

This is one of long sanding Phobos candidates Robert has put some great effort into. As far as I know it is already used in several D projects and it is a good moment to make it official.

Review goals, as usual : verify that API is robust enough to build more complicated logging systems on top of it, verify Phobos style compatibility, check if provided documentation is complete and friendly enough.

Base for review process is defined by http://wiki.dlang.org/Review/Process

As usual, this review round will last for 2 weeks unless someone asks for delay.

Please share link to this thread via twitter, reddit, G+ and whatever else may be used out there.

On Friday, 11 July 2014 at 14:36:34 UTC, Dicebot wrote:
> Round of a formal review before proceeding to voting. Subject for Phobos inclusion : http://wiki.dlang.org/Review/std.logger authored by Robert Schadek.

Is this for std.* or std.experimental.*?

David

On Friday, 11 July 2014 at 14:39:09 UTC, David Nadlinger wrote:
> On Friday, 11 July 2014 at 14:36:34 UTC, Dicebot wrote:
>> Round of a formal review before proceeding to voting. Subject for Phobos inclusion : http://wiki.dlang.org/Review/std.logger authored by Robert Schadek.
>
> Is this for std.* or std.experimental.*?
>
> David

Deciding this is subject of this review/voting iteration too - it is mostly matter of API stability, how much of a trust reviewers are ready to put into existing API.

Personally I believe that for something like logging library stabilization period of one release cycle in std.experimental is desirable because wider usage is very likely to result in breaking change suggestions.

The API seems nice and simple while providing enough flexibility,
and I think it would work quite well for my purposes. Some
comments:

API:
Not clear what the difference between globalLogLevel and a log
level on each individual logger is. Does the global one only
affect the global logger? What happens if you change the
globalLogLevel and the logLevel on the logger set as the global
one?

Documentation:
Logger Constructor: "The fatal handler will throw, and Error if a
log call is made with a LogLevel LogLevel.fatal." - Should be
"will throw an error if".
LogManager class: "It also handels the defaultLogger which is
used" - Should be handles.
LogManager class: "also allows to retrieve Logger by there name"
- Should be their.
LogManager class: "The static LogManager handles the creation,
and the release" - The comma shouldn't be there.
defaultLogger: "that means it can be assigend" - assigned
defaultLogger: "The Logger is returned as a reference that means
it can be assigend, thus changing the defaultLogger." - Wording
is a bit odd right now, perhaps something like "The Logger is
returned as a reference, meaning it can be assigned to change the
defaultLogger.".

On Friday, 11 July 2014 at 14:38:19 UTC, Dicebot wrote:
> As usual, this review round will last for 2 weeks unless someone asks for delay.
>
> Please share link to this thread via twitter, reddit, G+ and whatever else may be used out there.

On lines 606 and 729 in the comments:

/** This class is the base of every logger. In order to create a new kind of
logger a derivating class needs to implementation the method $(D writeLogMsg).
*/

I think "deriving" would be a better word to use.

Am Fri, 11 Jul 2014 14:36:30 +0000
schrieb "Dicebot" <public@dicebot.lv>:

> Round of a formal review before proceeding to voting. Subject for Phobos inclusion : http://wiki.dlang.org/Review/std.logger authored by Robert Schadek.
> 
> Code:
>      https://github.com/D-Programming-Language/phobos/pull/1500

https://github.com/D-Programming-Language/phobos/pull/1500/files#diff-165d04f1fd1a1db2c37af18580f41fb4R421
The 'frontend' log methods call format and the pass the formatted string
to the Logger implementations.

I still wonder if we can do better, and if we need to modify the API for that.

I've got three ideas:

* Using some sort of static buffer in the log* functions. I don't like
  this that much cause it will limit the msg string length
* Use malloc: This way no GC allocations, but still dynamic memory
  allocation which might not be necessary if you log e.g. to the
  console or a file.

These two options have the benefit that the API can remain unchanged and they could be implemented after the review.



The third option requires changes to the API. We overload
------------------
writeLogMsg(ref LoggerPayload payload);
with
writeLogHeader(ref LoggerPayload header);
------------------

And add another function:
------------------
void put(/*scope*/ const(char)[] msg);
------------------

writeLogHeader would receive all data as usual, except for the message string. Then after calling writeLogHeader, the (formatted) message string will be passed in any number of calls to put.

I used put here to make this output-range compatible. This way it plugs directly into formattedWrite.


An logger can still receive the complete string by appending the chunks received in put but we might need some 'finish' function to signal the end of the message. I guess not all loggers can fit into this interface, so we should try to make this optional. But simple loggers (file, console) which write the header first and the message last could work without any dynamic memory allocation. (formattedWrite probably uses an fixed-size buffer on the stack, but that's fine)

On Fri, Jul 11, 2014 at 02:59:43PM +0000, Dicebot via Digitalmars-d wrote:
> On Friday, 11 July 2014 at 14:39:09 UTC, David Nadlinger wrote:
> >On Friday, 11 July 2014 at 14:36:34 UTC, Dicebot wrote:
> >>Round of a formal review before proceeding to voting. Subject for Phobos inclusion : http://wiki.dlang.org/Review/std.logger authored by Robert Schadek.
> >
> >Is this for std.* or std.experimental.*?
> >
> >David
> 
> Deciding this is subject of this review/voting iteration too - it is mostly matter of API stability, how much of a trust reviewers are ready to put into existing API.
> 
> Personally I believe that for something like logging library stabilization period of one release cycle in std.experimental is desirable because wider usage is very likely to result in breaking change suggestions.

I vote for std.experimental. We keep talking about it, but never do anything in that direction. Let's start. If it works out poorly, we can always scrap the idea later. But we'll never know if we never do it.

(In contrast, putting it directly in std risks the necessity of breaking changes later, which is a Bad Thing. Putting it in std.experimental now does no harm whatsoever -- the worst that can happen is that it's delayed entering std. The best is that breaking changes will not annoy users. So we have nothing to lose.)


T

-- 
"Computer Science is no more about computers than astronomy is about telescopes." -- E.W. Dijkstra

Haven't had a chance to look closely at the new version yet, but looks pretty good.  Couple initial comments:

* Definite vote for std.experimental.  Should get a bunch of folks to bang on it before the API has to be locked down.  Having it as a dub package first has made it much easier to pick up and use, feel like this is a good path for all std packages to take - get it out there and iterate with user input before pulling in to std.

* The 'Tracer' doesn't feel like it belongs here, and if I understand things correctly won't actually work properly anyway, so should probably be removed.

* What others have said about string formatting.  I poked around with this a bit before, but wasn't able to get a clean solution, should probably try again before complaining...





On Fri, Jul 11, 2014 at 9:27 AM, H. S. Teoh via Digitalmars-d < digitalmars-d@puremagic.com> wrote:

> On Fri, Jul 11, 2014 at 02:59:43PM +0000, Dicebot via Digitalmars-d wrote:
> > On Friday, 11 July 2014 at 14:39:09 UTC, David Nadlinger wrote:
> > >On Friday, 11 July 2014 at 14:36:34 UTC, Dicebot wrote:
> > >>Round of a formal review before proceeding to voting. Subject for Phobos inclusion : http://wiki.dlang.org/Review/std.logger authored by Robert Schadek.
> > >
> > >Is this for std.* or std.experimental.*?
> > >
> > >David
> >
> > Deciding this is subject of this review/voting iteration too - it is mostly matter of API stability, how much of a trust reviewers are ready to put into existing API.
> >
> > Personally I believe that for something like logging library stabilization period of one release cycle in std.experimental is desirable because wider usage is very likely to result in breaking change suggestions.
>
> I vote for std.experimental. We keep talking about it, but never do anything in that direction. Let's start. If it works out poorly, we can always scrap the idea later. But we'll never know if we never do it.
>
> (In contrast, putting it directly in std risks the necessity of breaking changes later, which is a Bad Thing. Putting it in std.experimental now does no harm whatsoever -- the worst that can happen is that it's delayed entering std. The best is that breaking changes will not annoy users. So we have nothing to lose.)
>
>
> T
>
> --
> "Computer Science is no more about computers than astronomy is about telescopes." -- E.W. Dijkstra
>

Some logging backends (e.g. systemd journal) support structured logging. Should support for this be included (as a subclass, presumably)?