www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Review: std.logger

reply "Dicebot" <public dicebot.lv> writes:
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
Jul 11 2014
next sibling parent reply "Dicebot" <public dicebot.lv> writes:
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.
Jul 11 2014
parent "Meta" <jared771 gmail.com> writes:
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.
Jul 11 2014
prev sibling next sibling parent reply "David Nadlinger" <code klickverbot.at> writes:
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
Jul 11 2014
next sibling parent reply "Dicebot" <public dicebot.lv> writes:
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.
Jul 11 2014
next sibling parent "H. S. Teoh via Digitalmars-d" <digitalmars-d puremagic.com> writes:
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
Jul 11 2014
prev sibling parent Jeremy Powers via Digitalmars-d <digitalmars-d puremagic.com> writes:
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
Jul 11 2014
prev sibling next sibling parent "Robert burner Schadek" <rburners gmail.com> writes:
currently the import reads std.logger; Actually I don't care, as 
long as
I get more comments and tester.

OT to David: after rereading the std.experimental thread, I'm 
currently
collecting ideas for a DIP about it.
Jul 11 2014
prev sibling parent reply "Jesse Phillips" <Jesse.K.Phillips+D gmail.com> writes:
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


I'm of the opinion we should review for std.* and vote for 
std.experimental.*

Prior to or during beta we vote to move into std.*
Jul 11 2014
parent reply "Dicebot" <public dicebot.lv> writes:
On Saturday, 12 July 2014 at 03:22:10 UTC, Jesse Phillips 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


 I'm of the opinion we should review for std.* and vote for 
 std.experimental.*

 Prior to or during beta we vote to move into std.*


Agreed.
Jul 12 2014
parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 7/12/14, 4:04 AM, Dicebot wrote:

 On Saturday, 12 July 2014 at 03:22:10 UTC, Jesse Phillips 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


 I'm of the opinion we should review for std.* and vote for
 std.experimental.*

 Prior to or during beta we vote to move into std.*


 Agreed.


That's nice. Doing laps in std.experimental for one release cycle can 
only improve form. Let's. -- Andrei
Jul 12 2014
prev sibling next sibling parent "Kapps" <opantm2+spam gmail.com> writes:
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.".
Jul 11 2014
prev sibling next sibling parent reply Johannes Pfau <nospam example.com> writes:
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)
Jul 11 2014
parent reply "Robert burner Schadek" <rburners gmail.com> writes:
On Friday, 11 July 2014 at 15:20:51 UTC, Johannes Pfau wrote:

 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)


The api for none printf like logging has changed into something
like
write. So put properly needs to become a template. Any I'm not
sure if
there is a nice way around the template/inheritance problematic.
Other
than options one and two.
Jul 11 2014
parent Johannes Pfau <nospam example.com> writes:
Am Fri, 11 Jul 2014 21:14:36 +0000
schrieb "Robert burner Schadek" <rburners gmail.com>:


 On Friday, 11 July 2014 at 15:20:51 UTC, Johannes Pfau wrote:

 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)


 
 The api for none printf like logging has changed into something
 like
 write. So put properly needs to become a template. Any I'm not
 sure if
 there is a nice way around the template/inheritance problematic.
 Other
 than options one and two.


Last time we discussed this I also thought we'd need to make put a
template. But we overlooked the obvious solution:

Type->string formatting stays part of the 'frontend' functions. We only
pass strings to the backend. But instead of insisting that message is
one string, we allow to pass the msg as many strings. This means that
formatting can go in a fixed size stack buffer and still support
arbitrary length strings.

Here's a pull request to implement this idea:
https://github.com/burner/logger/pull/9
Jul 12 2014
prev sibling next sibling parent reply "Marc =?UTF-8?B?U2Now7x0eiI=?= <schuetzm gmx.net> writes:
Some logging backends (e.g. systemd journal) support structured 
logging. Should support for this be included (as a subclass, 
presumably)?
Jul 11 2014
parent reply "Robert burner Schadek" <rburners gmail.com> writes:
On Friday, 11 July 2014 at 18:02:58 UTC, Marc Schütz wrote:

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


Well, the idea behind std.logger is that I can not and should not 
even try to give you implementations for all your backend needs. 
It will just fall short and you will tell me that is it bad and 
should not go into phobos. And even if I manage it is going to be 
curl all over again.

So subclass Logger, implement writeLogMsg to your needs and be 
happy.
Jul 11 2014
parent reply Johannes Pfau <nospam example.com> writes:
Am Fri, 11 Jul 2014 21:26:25 +0000
schrieb "Robert burner Schadek" <rburners gmail.com>:


 On Friday, 11 July 2014 at 18:02:58 UTC, Marc Sch=C3=BCtz wrote:

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


=20
 Well, the idea behind std.logger is that I can not and should not=20
 even try to give you implementations for all your backend needs.=20
 It will just fall short and you will tell me that is it bad and=20
 should not go into phobos. And even if I manage it is going to be=20
 curl all over again.
=20
 So subclass Logger, implement writeLogMsg to your needs and be=20
 happy.


Yes, but for structured loggers the frontend log* functions do not
provide enough information to the backend. Of course you can use the
systemd journal as a normal backend, but you lose features.


I think we can provide structured logging support as a non-breaking API
extension, so we should not make this part of this review. But here's
how I'd imagine such an API to work:

Frontend:

* log* get new overloads which accept (T...) as the last parameter (or
  if T... is already the last parameter that's fine).

* Add a new struct to logger.core: struct MsgID which is just a
  strong typedef for UUID
* Add a templated type, KeyValue, which can be used like this:
  KeyValue("user", "nobody") //string key / string value
  KeyValue("age", 42); //string key / T value
  KeyValue("msg", "Hello %s! %s", "World", 42); //string key/fmt val
* KeyValue stores it's parameters, no string processing yet
* Multivalue parameters handled by many KeyValue with same key? Might
  complicate backend. Or don't support multivalue at all? Or
  KeyValue("key", MultiValue(a, ,b, c)) (MultiValue =3D=3D Tuple?)
* Structured loggers do not use msg, instead they use a KeyValue with
  "msg" key. This is cause you usually want different messages with
  structured loggers. We still keep everything in one function, so the
  user doesn't have to do "if(structuredlogger) logstruct() else log()"
  for every log message.
* MsgID marks the end of normal format parameters. See example below.
  This is also the reason why we can't use UUID directly

Usage:
string error;
logf("Something bad happened: %s", error,
     MsgID("abcd-valid-uuid"),       //MsgID--> end of fmt params
     KeyValue("msg", "Something bad happend"),
     KeyValue("error-code", error));

output:
normal backend: test.d:42 Something bad happened: out of memory
structured backend: (only example, exact format backend specific)
{
  "msg": "Something bad happened",
  "error": "out of memory",
  "file": "test.d",
  "line": 42
}




The next part is an efficient Backend Layer:

class StructuredLogger : Logger
{
    logHeader;
    writeLogMsg; //Not used
    finishLogMsg;

    void logKey(string key);
    void valuePart(const(char)[] part);
    void finishValue(bool last); //Last only if we support multivalue
}

Usage:

auto slog =3D new StructuredLogger();
slog.logHeader(...);
foreach(KeyValue kv; T...)
{
    slog.logKey(kv.key);
    //Need slog -> outputrange adapter: map put<>valuePart
    //see https://github.com/burner/logger/pull/9
    formattedWrite(wrap(slog), kv.formatstring, kv.args);
    slog.finishValue(true);
}
finishLogMsg();
Jul 12 2014
parent reply "Robert burner Schadek" <rburners gmail.com> writes:
On Saturday, 12 July 2014 at 09:19:47 UTC, Johannes Pfau wrote:


 I think we can provide structured logging support as a 
 non-breaking API
 extension, so we should not make this part of this review. But 
 here's
 how I'd imagine such an API to work:



I look at the journald spec and LoggerPayload already contains 
most of the key value pairs. MSG_ID seams to be the last 
important one missing inside LoggerPayload.

Thank you for taking the time to create a PR put, IMO that 
approach is to complicated. Anyway, if there are performance 
problems down the road this will be the first approach to try.
Jul 12 2014
parent reply Johannes Pfau <nospam example.com> writes:
Am Sat, 12 Jul 2014 13:09:37 +0000
schrieb "Robert burner Schadek" <rburners gmail.com>:


 On Saturday, 12 July 2014 at 09:19:47 UTC, Johannes Pfau wrote:
 

 I think we can provide structured logging support as a 
 non-breaking API
 extension, so we should not make this part of this review. But 
 here's
 how I'd imagine such an API to work:


 
 
 I look at the journald spec and LoggerPayload already contains 
 most of the key value pairs. MSG_ID seams to be the last 
 important one missing inside LoggerPayload.
 
 Thank you for taking the time to create a PR put, IMO that 
 approach is to complicated. Anyway, if there are performance 
 problems down the road this will be the first approach to try.


The whole point of journald is that the user can define arbitrary
key/value pairs. you're not limited to the special key/value pairs.
http://0pointer.de/blog/projects/journal-submit.html

Also giving log messages uuids is an important property of structured
logging.
Jul 12 2014
parent "Robert burner Schadek" <rburners gmail.com> writes:
On Saturday, 12 July 2014 at 14:04:00 UTC, Johannes Pfau wrote:


 The whole point of journald is that the user can define 
 arbitrary
 key/value pairs. you're not limited to the special key/value 
 pairs.
 http://0pointer.de/blog/projects/journal-submit.html

 Also giving log messages uuids is an important property of 
 structured
 logging.


I will take a third look, I might have an idea solving the fixed 
size buffer and structured logging approach together.
Jul 13 2014
prev sibling next sibling parent reply "NCrashed" <NCrashed gmail.com> writes:
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.

 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


Mistype at std.logger.core.Tracer: "Tracer generates trace calls 
to the passed logger wenn the Tracer struc" wenn should be "when".

The API is well done. The only thing I want to have in the 
package is a delayed logger: delayed logger wraps another logger, 
accumulates messages and writes to inner logger by explicit call.

That is useful while testing. Major part of diagnostic 
information is needed only when test is failed:
```
unittest
{
     auto logger = new DelayedLogger(myCustomLogger);
     scope(failure) logger.flush;

     // some activity that can throw
}
```
Jul 11 2014
parent "Robert burner Schadek" <rburners gmail.com> writes:
Yes, Tracer is doomed. It is gone go. scope(exit) is way smarter
Jul 11 2014
prev sibling next sibling parent "Brian Schott" <briancschott gmail.com> writes:
Some static analysis warnings:

std/logger/core.d(808:18)[warn]: Parameter cond is never used
std/logger/core.d(1069:10)[warn]: Variable ll is never used
std/logger/core.d(1106:14)[warn]: Variable tracer is never used
std/logger/core.d(1161:9)[warn]: Variable nLineNumber is never 
used
std/logger/core.d(1263:12)[warn]: Variable name is never used
std/logger/core.d(1300:12)[warn]: Variable name is never used
std/logger/core.d(1323:10)[warn]: Variable readLine is never used

Several of the functions in the API are missing "Params:" 
sections.
Jul 11 2014
prev sibling next sibling parent reply Xavier Bigand <flamaros.xavier gmail.com> writes:
Le 11/07/2014 16:36, Dicebot a écrit :

 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


In the documentation you tell about informations added by the module 
(filename, linenumber,...) but you don't put sample of the resulting output.
IMO it's important to provide a standard format of the output then D's 
IDE would be able to parse it to create links to the code or colorize it,...

Does the output contain a message counter? This help to distinguish 
message duplicates, on some console when prints are fast we don't see 
lines added if their are all the same.

Is it too late for an integration in std.expirimental with dmd 2.66?
Jul 11 2014
parent reply "Robert burner Schadek" <rburners gmail.com> writes:
On Friday, 11 July 2014 at 20:57:06 UTC, Xavier Bigand wrote:

 In the documentation you tell about informations added by the 
 module (filename, linenumber,...) but you don't put sample of 
 the resulting output.


I will add one.

 IMO it's important to provide a standard format of the output 
 then D's IDE would be able to parse it to create links to the 
 code or colorize it,...


I see your point, but people will change the output, we have 
multiple IDE's who do stuff different, so creating a framework 
for that is properly out of scope. But in the templatelogger 
module there is a function I would currently consider the 
default. I'll think about adding a very simple regex parser that 
returns a LoggerPayload.

 Does the output contain a message counter? This help to 
 distinguish message duplicates, on some console when prints are 
 fast we don't see lines added if their are all the same.


It does not. Adding that to the LoggerPayload will be no problem.

 Is it too late for an integration in std.expirimental with dmd 
 2.66?


ask Andrew, but I'm 99.99999% sure it is to late
Jul 11 2014
next sibling parent Jeremy Powers via Digitalmars-d <digitalmars-d puremagic.com> writes:
 Is it too late for an integration in std.expirimental with dmd 2.66?


 ask Andrew, but I'm 99.99999% sure it is to late


Thanks to dub, the need for std.experimental is significantly reduced.  You
can just pull in the package and use it right now:

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



 Jul 11 2014



prev sibling  parent  "Dicebot" <public dicebot.lv>  writes:


On Friday, 11 July 2014 at 21:34:37 UTC, Robert burner Schadek 
wrote:

 Is it too late for an integration in std.expirimental with dmd 
 2.66?


 ask Andrew, but I'm 99.99999% sure it is to late


Despite how reasonable it may seem I believe that being stupid 
formal about release process is the Right Thing -> it is too late.



 Jul 11 2014



prev sibling next sibling parent reply "Brian Schott" <briancschott gmail.com>  writes:


https://github.com/burner/logger/pull/8

I've fixed some of the typos pointed out in this thread.



 Jul 11 2014



  parent  "Robert burner Schadek" <rburners gmail.com>  writes:


On Friday, 11 July 2014 at 21:09:10 UTC, Brian Schott wrote:

 https://github.com/burner/logger/pull/8

 I've fixed some of the typos pointed out in this thread.


I like your style, thank you, "Talk is cheap ..."



 Jul 11 2014



prev sibling next sibling parent reply Johannes Pfau <nospam example.com>  writes:


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
 Documentation:


Overall design & API looks good.

Some detail comments and / or questions:

logger.core:
* For some people the EBNF might be useful, but it looks kinda scary to
  me if I open the documentation ;-)
* The docs do not mention thread safety at all. Can I change the
  default logger while other threads are logging?
* The docs should clearly mention the default logger (this is kinda
  mentioned, but I'd make it more explicit) and the default loglevel.
* Can the logger be influenced by command line flags / environment
  variables? I guess not, but if it can it should be documented.
* I think an example how to change the default logger should be at the
  top of the module. (I think it's quite common people would want to
  log to files as well)
* Why does LogManager.defaultLogger return by ref instead of just using
  a setter?
* The logger.core documentation should have an overview of available
  logger types and link to them. Probably a table with
  Logger      |     module             | Description
  ----------------------------------------------------
  StdIOLogger | std.logger.stdiologger | log to standard output(console)


logger.multilogger:
* Should mention thread safety. If other threads log to the multilogger
  can I simply add/remove loggers?



 Jul 12 2014



  parent reply "Robert burner Schadek" <rburners gmail.com>  writes:


On Saturday, 12 July 2014 at 12:19:03 UTC, Johannes Pfau wrote:

 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
 Documentation:


 Overall design & API looks good.

 Some detail comments and / or questions:

 logger.core:
 * For some people the EBNF might be useful, but it looks kinda 
 scary to
   me if I open the documentation ;-)


I know, I will make it less scary.


 * The docs do not mention thread safety at all. Can I change the
   default logger while other threads are logging?


That is already on my todo list.

 * The docs should clearly mention the default logger (this is 
 kinda
   mentioned, but I'd make it more explicit) and the default 
 loglevel.


ok

 * Can the logger be influenced by command line flags / 
 environment
   variables? I guess not, but if it can it should be documented.


I thought not writing about it makes it clear that you can not.


 * I think an example how to change the default logger should be 
 at the
   top of the module. (I think it's quite common people would 
 want to
   log to files as well)


Will do

 * Why does LogManager.defaultLogger return by ref instead of 
 just using
   a setter?


Because I use the defaultLogger at quite a lot of places and 
treating it as an property just felt right.

 * The logger.core documentation should have an overview of 
 available
   logger types and link to them. Probably a table with
   Logger      |     module             | Description
   ----------------------------------------------------
   StdIOLogger | std.logger.stdiologger | log to standard 
 output(console)


Will do

 logger.multilogger:
 * Should mention thread safety. If other threads log to the 
 multilogger


I know, threads

   can I simply add/remove loggers?


yes: new WhatEverLoggerYouWant("some_good_name", LogLevel.xxxx);



 Jul 12 2014



  parent  Johannes Pfau <nospam example.com>  writes:


Am Sat, 12 Jul 2014 13:18:29 +0000
schrieb "Robert burner Schadek" <rburners gmail.com>:


 * Can the logger be influenced by command line flags / 
 environment
   variables? I guess not, but if it can it should be documented.  


 I thought not writing about it makes it clear that you can not.
 


Yes, the documentation is OK in that regard, that was more a question
than a criticism about the docs ;-)



 * Why does LogManager.defaultLogger return by ref instead of 
 just using
   a setter?  


 Because I use the defaultLogger at quite a lot of places and 
 treating it as an property just felt right.


Yes, it's perfectly fine as a property. But why not:

static  property  trusted Logger defaultLogger() //getter
{
    return ...;
}

static  property  trusted void defaultLogger(Logger value) //setter
{
    myLogger = value;
}

Usage of the property is exactly the same, but an explicit setter is
more powerful and afaik we usually use explicit setters.



 Jul 12 2014



prev sibling next sibling parent reply =?UTF-8?B?U8O2bmtlIEx1ZHdpZw==?= <sludwig rejectedsoftware.com>  writes:


Overall looks good to me. Some points that haven't been mentioned so far 
in this review round:

  - Using a class with static members doesn't seem to be very idiomatic. 
It seems like the three member properties can simply be made global and 
everything should be fine - the "LogManager." prefix doesn't really add 
information. This has been mentioned in the last review round and it's 
not a very important point in this particular instance, but we should 
really make a decision here that will also decide how future modules go 
about this.

  - Personally, I really find additional verbose log levels useful. 
Currently there is only "trace". Previous proposals had a generic 
"verbose(N)" set of levels, but I think it's important for the proper 
interaction of multiple libraries to define a semantic set of verbose 
levels. A proposal, which has worked very well me (from low to high):

   trace:
      as now, used for low level tracing of program flow
   debugv:
     verbose debug messages, may flood the log
   debug:
     normal, low frequency debug messages, useful for the developer
   diagnostic:
     diagnostic output also potentially useful for the user
     this is what you typically would get with the -v command line switch

  - There is a "formatString" string mixin that includes a lot of 
if-else cases that seem to do exactly what formattedWrite would do 
anyway, is there something that it actually does in addition to that? 
Also, why is it a string mixin in contrast to a simple (template) 
function? It may be a matter of taste (but also of compile time/memory), 
but I'd almost always prefer a non-string-mixin solution.

  - Even if it may be more typing (or maybe not?), actually writing out 
the different signatures for each log level and the c/f suffixes would 
be very advantageous for the documentation and for code completion. It 
would also make the EBNF unnecessary, which I agree with Johannes looks 
a little scary. All of the functions could be based on the generic 
logl/loglf functions, so that there wouldn't be much redundancy.

  - I'm still really not sure if the "c" versions of the log functions 
pull their own weight. They seem to be in line with trivial convenience 
functions, which are generally discouraged in Phobos (with the same 
issues, such as additional cognitive load and bigger API size).

  - The functions error(), info(), fatal(), etc. don't follow the usual 
rule for functions to start with a verb. The question is if saving three 
characters over logError() is worth making the code more ambiguous for 
the outside reader (e.g. "does error() throw an exception? or set some 
internal error state?" "does fatal() terminate the process?")



 Jul 12 2014



 next sibling parent reply "H. S. Teoh via Digitalmars-d" <digitalmars-d puremagic.com>  writes:


On Sat, Jul 12, 2014 at 06:13:28PM +0200, Sönke Ludwig via Digitalmars-d wrote:

 Overall looks good to me. Some points that haven't been mentioned so far in
 this review round:
 
  - Using a class with static members doesn't seem to be very
 idiomatic. It seems like the three member properties can simply be
 made global and everything should be fine - the "LogManager." prefix
 doesn't really add information. This has been mentioned in the last
 review round and it's not a very important point in this particular
 instance, but we should really make a decision here that will also
 decide how future modules go about this.


[...]

+1. If something is a global, just call it a global. There's no need to
be ashamed of that. Classes with only static members sound to me like
forcing round pegs into square holes.


T

-- 
What doesn't kill me makes me stranger.



 Jul 12 2014



  parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org>  writes:


On 7/12/14, 10:19 AM, H. S. Teoh via Digitalmars-d wrote:

 On Sat, Jul 12, 2014 at 06:13:28PM +0200, Sönke Ludwig via Digitalmars-d wrote:

 Overall looks good to me. Some points that haven't been mentioned so far in
 this review round:

   - Using a class with static members doesn't seem to be very
 idiomatic. It seems like the three member properties can simply be
 made global and everything should be fine - the "LogManager." prefix
 doesn't really add information. This has been mentioned in the last
 review round and it's not a very important point in this particular
 instance, but we should really make a decision here that will also
 decide how future modules go about this.


 [...]

 +1. If something is a global, just call it a global. There's no need to
 be ashamed of that. Classes with only static members sound to me like
 forcing round pegs into square holes.


I agree. This is not Java. That idiom must go. -- Andrei



 Jul 12 2014



  parent  "Robert burner Schadek" <rburners gmail.com>  writes:


On Saturday, 12 July 2014 at 21:23:28 UTC, Andrei Alexandrescu
wrote:


 I agree. This is not Java. That idiom must go. -- Andrei


I will fix this today



 Jul 13 2014



prev sibling next sibling parent reply Jeremy Powers via Digitalmars-d <digitalmars-d puremagic.com>  writes:


  ... I'd almost always prefer a non-string-mixin solution.

  ... actually writing out the different signatures ...


+1


From a maintenance/sanity standpoint, I'd much rather have a little bit of


duplication over one large if/else'd string mixin.  It is much easier to
figure out exactly what is going on, and what the intent is, without having
to pragma(msg) everything first.



 Jul 13 2014



  parent reply "Dragos Carp" <dragoscarp gmail.com>  writes:


On Sunday, 13 July 2014 at 07:14:50 UTC, Jeremy Powers via
Digitalmars-d wrote:

  ... I'd almost always prefer a non-string-mixin solution.

  ... actually writing out the different signatures ...


 +1



I also think that the usage of mixins in std.log is an "unforced
error" and non-idiomatic D code.

A nice alternative is http://code.dlang.org/packages/log



 Jul 13 2014



  parent reply "Robert burner Schadek" <rburners gmail.com>  writes:


On Sunday, 13 July 2014 at 11:19:20 UTC, Dragos Carp wrote:

 On Sunday, 13 July 2014 at 07:14:50 UTC, Jeremy Powers via
 Digitalmars-d wrote:

 ... I'd almost always prefer a non-string-mixin solution.

 ... actually writing out the different signatures ...


 +1



 I also think that the usage of mixins in std.log is an "unforced
 error" and non-idiomatic D code.


But creating x , necessary, function with only slightly different 
names is also very non idiomatic D code. Anyway I'm working on 
that.



 Jul 13 2014



  parent reply "Dragos Carp" <dragoscarp gmail.com>  writes:


 But creating x , necessary, function with only slightly 
 different names is also very non idiomatic D code. Anyway I'm 
 working on that.


For the conditional variants, wouldn't be enough an overload with
the condition on the first position?



 Jul 13 2014



  parent reply "Robert burner Schadek" <rburners gmail.com>  writes:


On Sunday, 13 July 2014 at 13:30:20 UTC, Dragos Carp wrote:

 But creating x , necessary, function with only slightly 
 different names is also very non idiomatic D code. Anyway I'm 
 working on that.


 For the conditional variants, wouldn't be enough an overload 
 with
 the condition on the first position?


people hated it and writeln like logging makes this impossible 
anyway



 Jul 13 2014



  parent reply "Dragos Carp" <dragoscarp gmail.com>  writes:


On Sunday, 13 July 2014 at 13:37:13 UTC, Robert burner Schadek
wrote:

 On Sunday, 13 July 2014 at 13:30:20 UTC, Dragos Carp wrote:

 But creating x , necessary, function with only slightly 
 different names is also very non idiomatic D code. Anyway I'm 
 working on that.


 For the conditional variants, wouldn't be enough an overload 
 with
 the condition on the first position?


 people hated it and writeln like logging makes this impossible 
 anyway


write/writeln are generic functions that are used for producing
plain-text output in line oriented format (.csv, etc.) or not
line oriented format (json, xml, etc.). This text is human
readable, but not necessary user friendly.

A logging facility need to produce both human readable and user
friendly messages.

I would prefer:
    info("%s is the answer", true);
instead of
    info(true, " is the answer");

The variant without format specifier also makes the
internationalization of the log entries almost impossible.


IMHO these are some good reasons why the logging facility should
not try to support the variants that the std.stdio with
write/f/ln provides.



 Jul 13 2014



  parent reply "Robert burner Schadek" <rburners gmail.com>  writes:


On Sunday, 13 July 2014 at 14:25:42 UTC, Dragos Carp wrote:

 write/writeln are generic functions that are used for producing
 plain-text output in line oriented format (.csv, etc.) or not
 line oriented format (json, xml, etc.). This text is human
 readable, but not necessary user friendly.

 A logging facility need to produce both human readable and user
 friendly messages.

 I would prefer:
    info("%s is the answer", true);


write infof("%b is the answer", true); and it works write now


 instead of
    info(true, " is the answer");

 The variant without format specifier also makes the
 internationalization of the log entries almost impossible.


 IMHO these are some good reasons why the logging facility should
 not try to support the variants that the std.stdio with
 write/f/ln provides.


there also good points for it.
* people expect it after using write
* sometimes it is less work typing
* there is no reason to limit it arbitrarily
* ...



 Jul 13 2014



 next sibling parent reply "Dragos Carp" <dragoscarp gmail.com>  writes:


 write infof("%b is the answer", true); and it works write now


This is clear... the argument was against "info" (without format
specifier).



 there also good points for it.
 * people expect it after using write


This is a different module. Btw. std.file.write looks completely
different :)


 * sometimes it is less work typing


With more work, if you later need to internationalize your
application.


 * there is no reason to limit it arbitrarily


Simple is better than complex.



 Jul 13 2014



  parent reply "Robert burner Schadek" <rburners gmail.com>  writes:


On Sunday, 13 July 2014 at 14:53:20 UTC, Dragos Carp wrote:

 write infof("%b is the answer", true); and it works write now


 This is clear... the argument was against "info" (without format
 specifier).


then I did not get it.

 there also good points for it.
 * people expect it after using write


 This is a different module. Btw. std.file.write looks completely
 different :)


 * sometimes it is less work typing


 With more work, if you later need to internationalize your
 application.


std.string.translate and map are already in place.

 * there is no reason to limit it arbitrarily


 Simple is better than complex.


it is hardly complex



 Jul 13 2014



  parent reply "Dragos Carp" <dragoscarp gmail.com>  writes:


 * there is no reason to limit it arbitrarily


 Simple is better than complex.


 it is hardly complex


Of course it is... wasn't this one of the main reasons you chose
the mixin solution?



 Jul 13 2014



  parent reply "Robert burner Schadek" <rburners gmail.com>  writes:


On Sunday, 13 July 2014 at 15:12:19 UTC, Dragos Carp wrote:

 * there is no reason to limit it arbitrarily


 Simple is better than complex.


 it is hardly complex


 Of course it is... wasn't this one of the main reasons you chose
 the mixin solution?


no, I find typing nearly the same function over and over stupidly 
boring and error prone



 Jul 13 2014



  parent  "Dicebot" <public dicebot.lv>  writes:


On Sunday, 13 July 2014 at 15:19:19 UTC, Robert burner Schadek 
wrote:

 On Sunday, 13 July 2014 at 15:12:19 UTC, Dragos Carp wrote:

 * there is no reason to limit it arbitrarily


 Simple is better than complex.


 it is hardly complex


 Of course it is... wasn't this one of the main reasons you 
 chose
 the mixin solution?


 no, I find typing nearly the same function over and over 
 stupidly boring and error prone


When it comes to public API having perfect documentation is much 
more important in my opinion - one can still mixin the bodies but 
repeating signatures is lesser of evils.



 Jul 13 2014



prev sibling  parent reply "Dicebot" <public dicebot.lv>  writes:


On Sunday, 13 July 2014 at 14:37:15 UTC, Robert burner Schadek 
wrote:

 I would prefer:
   info("%s is the answer", true);


 write infof("%b is the answer", true); and it works write now 
 arbitrarily


I am kind of fine both ways, it is not that big deal but I 
remember being very surprised that most common use case is called 
`logf` and much less common - `log`. For `write` function family 
common usage pattern is a bit different.

In the end it boils down to "consistency" vs "convenience" 
argument and I don't think either has fundamental advantage over 
the other. Sticking to whatever Robert prefers is reasonable way 
to avoid hundreds post long bike-shedding ;)



 Jul 13 2014



  parent reply "Dragos Carp" <dragoscarp gmail.com>  writes:


 I am kind of fine both ways, it is not that big deal but I 
 remember being very surprised that most common use case is 
 called `logf` and much less common - `log`. For `write` 
 function family common usage pattern is a bit different.


If we get rid of the write style variant, then you need to
remember just one name with 4 overloads:

log(string message);
log(string format, Args...);
log(bool condition, string message);
log(bool condition, string format, Args...);

This is a library: you can always add names, but it is very hard
to remove them. If next version of std.logger should support
something like logFirstN or logEveryN (ideas from Google log
library). How this should look like? logfnf, logenf...?



 Jul 13 2014



 next sibling parent reply "Robert burner Schadek" <rburners gmail.com>  writes:


On Sunday, 13 July 2014 at 16:02:05 UTC, Dragos Carp wrote:

 I am kind of fine both ways, it is not that big deal but I 
 remember being very surprised that most common use case is 
 called `logf` and much less common - `log`. For `write` 
 function family common usage pattern is a bit different.


 If we get rid of the write style variant, then you need to
 remember just one name with 4 overloads:

 log(string message);
 log(string format, Args...);
 log(bool condition, string message);
 log(bool condition, string format, Args...);


It had that, it was to little for users.


 This is a library: you can always add names, but it is very hard
 to remove them. If next version of std.logger should support
 something like logFirstN or logEveryN (ideas from Google log
 library). How this should look like? logfnf, logenf...?


c++ does not have foreach(i; 1 .. n)



 Jul 13 2014



  parent reply "Dragos Carp" <dragoscarp gmail.com>  writes:


 This is a library: you can always add names, but it is very 
 hard
 to remove them. If next version of std.logger should support
 something like logFirstN or logEveryN (ideas from Google log
 library). How this should look like? logfnf, logenf...?


 c++ does not have foreach(i; 1 .. n)


Sorry that I didn't described how logFirstN and logEveryN work.

Let say you output a log in a loop with 100 iterations.
logFirstN(10, ...) outputs the log message just for the first 10
iterations, and logEveryN(10, ...)  outputs only for 1st, 11th,
21st, .. and 91st iterations.



 Jul 13 2014



 next sibling parent  "David Nadlinger" <code klickverbot.at>  writes:


On Sunday, 13 July 2014 at 16:44:21 UTC, Dragos Carp wrote:

 Let say you output a log in a loop with 100 iterations.
 logFirstN(10, ...) outputs the log message just for the first 10
 iterations, and logEveryN(10, ...)  outputs only for 1st, 11th,
 21st, .. and 91st iterations.


In D, we can easily make this syntax every!(10, log)(…) or 
something along the lines, sidestepping any naming problems and 
also allowing for more flexibility at the same time.

David



 Jul 13 2014



prev sibling  parent  "Robert burner Schadek" <rburners gmail.com>  writes:


On Sunday, 13 July 2014 at 16:44:21 UTC, Dragos Carp wrote:




 Sorry that I didn't described how logFirstN and logEveryN work.

 Let say you output a log in a loop with 100 iterations.
 logFirstN(10, ...) outputs the log message just for the first 10
 iterations, and logEveryN(10, ...)  outputs only for 1st, 11th,
 21st, .. and 91st iterations.


I know how they work. apart from my foreach brain frat. But what 
do you think is logc for.



 Jul 13 2014



prev sibling next sibling parent reply Jeremy Powers via Digitalmars-d <digitalmars-d puremagic.com>  writes:


 If we get rid of the write style variant, then you need to
 remember just one name with 4 overloads:

 log(string message);
 log(string format, Args...);
 log(bool condition, string message);
 log(bool condition, string format, Args...);


But you would also have to remember that log(...) behaves like writef(...)
and not write(...)

Having consistency with the already existing write methods is a good thing.
 And having both options is useful, as some prefer one over the other (and
you can do more interesting logging things with a list of items to log when
you aren't restricted by a format string).

On the conditional log methods: +1 to not having them, I believe that they
are more complicating than useful.  They muddy the responsibility of
logging, putting program logic along with log level configuration to decide
what to log.  They are an easy thing to add as an extension to the logging
library, no need to include them in the base logging API.



 Jul 13 2014



  parent reply "Dragos Carp" <dragoscarp gmail.com>  writes:


 Having consistency with the already existing write methods is a 
 good thing.
  And having both options is useful, as some prefer one over the 
 other (and
 you can do more interesting logging things with a list of items 
 to log when
 you aren't restricted by a format string).


Could you provide an example?


 On the conditional log methods: +1 to not having them, I 
 believe that they
 are more complicating than useful.


+1 for not having the conditional log



 Jul 13 2014



 next sibling parent reply Jeremy Powers via Digitalmars-d <digitalmars-d puremagic.com>  writes:


 Could you provide an example?



If you decouple the things being logged from the actual logged string, you
can do stuff like have different log sinks format things differently, or
even filter what appears in the log line as needed.

For example, you could have a setup with logging going to two separate
files.  One used for auditing, which only contains the bare timestamp and
log message, and the other for debugging that contains all the things
verbosely.  Or filter the output to any log sink based on circumstances
(prod vs. dev, etc).  You write your code to give all the context to the
logger, and let the logging figure out what belongs where.  This can be
especially important when the log output has to conform to existing
systems, and you don't want to change every logging call.

Basically, limiting to string format logging limits to thinking of logging
as purely strings.  Using an API of 'list of items' decouples it to the
more abstract 'logging stuff', with the actual string only mattering for
final output.


Logically, I look at the formatting version as a convenience version of a
one-argument call to the non-formatting version.  So:

logf("some %s other %s", foo, bar)

is like

log(format("some %s other %s", foo, bar), <other stuff>)

where the 'other stuff' may be formatted as required by logging
configuration, and is not possible with the logf version.

logf(string msg, ...stuff to put in message)
log(string msg, ...stuff to log with message)



 Jul 13 2014



  parent reply "Dragos Carp" <dragoscarp gmail.com>  writes:


 Basically, limiting to string format logging limits to thinking 
 of logging
 as purely strings.  Using an API of 'list of items' decouples 
 it to the
 more abstract 'logging stuff', with the actual string only 
 mattering for
 final output.


This is really valid point.
There is still a problem: the analogy, that log/logf is similar
to write/writef, is not right anymore. Please correct me if I'm
wrong... but AFAIK write converts every argument to string and
the output device has no mean to choose a preferred
representation of the arguments.

It will be a surprise if I replace
    log(a, b, c);
with
    logf("%s%s%s", a, b, c);
and possibly get different results.



 Jul 13 2014



  parent  Jeremy Powers via Digitalmars-d <digitalmars-d puremagic.com>  writes:


 There is still a problem: the analogy, that log/logf is similar
 to write/writef, is not right anymore. Please correct me if I'm
 wrong... but AFAIK write converts every argument to string and
 the output device has no mean to choose a preferred
 representation of the arguments.

 It will be a surprise if I replace
    log(a, b, c);
 with
    logf("%s%s%s", a, b, c);
 and possibly get different results.



If you are just using default logging, they are the same.  Point is that it
allows for different results based on logging configuration/implementation.
 The logf version says the logging output has to be exactly that string, in
all instances.  The other says here is some things I want logged, and the
logging can decide how to output them.



 Jul 14 2014



prev sibling  parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org>  writes:


On 7/13/14, 11:45 AM, Dragos Carp wrote:

 On the conditional log methods: +1 to not having them, I believe that
 they
 are more complicating than useful.


 +1 for not having the conditional log


Not so sure about that. -- Andrei



 Jul 14 2014



  parent reply "Dragos Carp" <dragoscarp gmail.com>  writes:


 +1 for not having the conditional log


 Not so sure about that. -- Andrei


I agree that the conditional could be useful. But I think that 
the current API definition where we already have some strange 
letter combinations ('l', 'c', 'f') is an one-way street, that 
throws away some good possibilities to extend it later. Should we 
keep adding cryptic letters? That is the reason why I think it is 
better to let out the conditional log for now.

errorIf - looks D-like
errorc  - is at most C-like

The argument that writef/-ln also contain some cryptic letters 
have historical reasons and should not motivate us to add some 
more.



 Jul 15 2014



  parent reply "Robert burner Schadek" <rburners gmail.com>  writes:


On Tuesday, 15 July 2014 at 07:31:39 UTC, Dragos Carp wrote:

 +1 for not having the conditional log


 Not so sure about that. -- Andrei


 I agree that the conditional could be useful. But I think that 
 the current API definition where we already have some strange 
 letter combinations ('l', 'c', 'f') is an one-way street, that 
 throws away some good possibilities to extend it later. Should 
 we keep adding cryptic letters? That is the reason why I think 
 it is better to let out the conditional log for now.

 errorIf - looks D-like
 errorc  - is at most C-like

 The argument that writef/-ln also contain some cryptic letters 
 have historical reasons and should not motivate us to add some 
 more.


I wouldn't call them cryptic, they are in fact very easy.

l = LogLevel
c = conditional
f = printf

what is so difficult about them. Sure errorIf looks easier, but 
what
about errorLogLevelIfPrintfFormat in comparison to errorlcf



 Jul 15 2014



  parent reply "linkrope" <linkrope github.com>  writes:


What does 'errorlcf' mean?

You specify log level 'error' and then you specify another log 
level ('l):
which log level wins?

The order of these modifier letters has to be 'lcf', not 'cfl', 
...?

On Tuesday, 15 July 2014 at 08:01:12 UTC, Robert burner Schadek 
wrote:

 I wouldn't call them cryptic, they are in fact very easy.

 l = LogLevel
 c = conditional
 f = printf

 what is so difficult about them. Sure errorIf looks easier, but 
 what
 about errorLogLevelIfPrintfFormat in comparison to errorlcf





 Jul 15 2014



  parent  "Robert burner Schadek" <rburners gmail.com>  writes:


On Tuesday, 15 July 2014 at 10:33:24 UTC, linkrope wrote:

 What does 'errorlcf' mean?

 You specify log level 'error' and then you specify another log 
 level ('l):
 which log level wins?

 The order of these modifier letters has to be 'lcf', not 'cfl', 
 ...?


good point one l to much its errorcf

now I feel stupid



 Jul 15 2014



prev sibling  parent  Andrei Alexandrescu <SeeWebsiteForEmail erdani.org>  writes:


On 7/13/14, 9:02 AM, Dragos Carp wrote:

 This is a library: you can always add names, but it is very hard
 to remove them. If next version of std.logger should support
 something like logFirstN or logEveryN (ideas from Google log
 library). How this should look like? logfnf, logenf...?


There were some nice ideas about that in the previous proposal. -- Andrei



 Jul 14 2014



prev sibling next sibling parent reply "Robert burner Schadek" <rburners gmail.com>  writes:


On Saturday, 12 July 2014 at 16:13:22 UTC, Sönke Ludwig wrote:

 Overall looks good to me. Some points that haven't been 
 mentioned so far in this review round:

  - Using a class with static members doesn't seem to be very 
 idiomatic. It seems like the three member properties can simply 
 be made global and everything should be fine - the 
 "LogManager." prefix doesn't really add information. This has 
 been mentioned in the last review round and it's not a very 
 important point in this particular instance, but we should 
 really make a decision here that will also decide how future 
 modules go about this.

  - Personally, I really find additional verbose log levels 
 useful. Currently there is only "trace". Previous proposals had 
 a generic "verbose(N)" set of levels, but I think it's 
 important for the proper interaction of multiple libraries to 
 define a semantic set of verbose levels. A proposal, which has 
 worked very well me (from low to high):

   trace:
      as now, used for low level tracing of program flow
   debugv:
     verbose debug messages, may flood the log
   debug:
     normal, low frequency debug messages, useful for the 
 developer
   diagnostic:
     diagnostic output also potentially useful for the user
     this is what you typically would get with the -v command 
 line switch


the LogLevel enum has quite a lot of free number in between trace 
and info and so forth. In combination with a MultiLogger it is 
very easy to build verbose logging special to your needs.


  - There is a "formatString" string mixin that includes a lot 
 of if-else cases that seem to do exactly what formattedWrite 
 would do anyway, is there something that it actually does in 
 addition to that? Also, why is it a string mixin in contrast to 
 a simple (template) function? It may be a matter of taste (but 
 also of compile time/memory), but I'd almost always prefer a 
 non-string-mixin solution.

  - Even if it may be more typing (or maybe not?), actually 
 writing out the different signatures for each log level and the 
 c/f suffixes would be very advantageous for the documentation 
 and for code completion. It would also make the EBNF 
 unnecessary, which I agree with Johannes looks a little scary. 
 All of the functions could be based on the generic logl/loglf 
 functions, so that there wouldn't be much redundancy.


I'm about to change the codegen.


  - I'm still really not sure if the "c" versions of the log 
 functions pull their own weight. They seem to be in line with 
 trivial convenience functions, which are generally discouraged 
 in Phobos (with the same issues, such as additional cognitive 
 load and bigger API size).


Anyone else?


  - The functions error(), info(), fatal(), etc. don't follow 
 the usual rule for functions to start with a verb. The question 
 is if saving three characters over logError() is worth making 
 the code more ambiguous for the outside reader (e.g. "does 
 error() throw an exception? or set some internal error state?" 
 "does fatal() terminate the process?")


if I change it back, people will argue that that is redundant and 
unintuitive. Than I will change it back again and the discussion 
starts again.



 Jul 13 2014



 next sibling parent reply "Dicebot" <public dicebot.lv>  writes:


On Sunday, 13 July 2014 at 11:57:26 UTC, Robert burner Schadek 
wrote:

 - The functions error(), info(), fatal(), etc. don't follow 
 the usual rule for functions to start with a verb. The 
 question is if saving three characters over logError() is 
 worth making the code more ambiguous for the outside reader 
 (e.g. "does error() throw an exception? or set some internal 
 error state?" "does fatal() terminate the process?")


 if I change it back, people will argue that that is redundant 
 and unintuitive. Than I will change it back again and the 
 discussion starts again.


I think those functions are used so often that can be worth an 
exception.



 Jul 13 2014



  parent  "Marc =?UTF-8?B?U2Now7x0eiI=?= <schuetzm gmx.net>  writes:


On Sunday, 13 July 2014 at 12:12:46 UTC, Dicebot wrote:

 On Sunday, 13 July 2014 at 11:57:26 UTC, Robert burner Schadek 
 wrote:

 - The functions error(), info(), fatal(), etc. don't follow 
 the usual rule for functions to start with a verb. The 
 question is if saving three characters over logError() is 
 worth making the code more ambiguous for the outside reader 
 (e.g. "does error() throw an exception? or set some internal 
 error state?" "does fatal() terminate the process?")


 if I change it back, people will argue that that is redundant 
 and unintuitive. Than I will change it back again and the 
 discussion starts again.


 I think those functions are used so often that can be worth an 
 exception.


+1

And it has precursors in other logging APIs.



 Jul 13 2014



prev sibling  parent reply Jacob Carlborg <doob me.com>  writes:


On 2014-07-13 13:57, Robert burner Schadek wrote:


 Anyone else?


I agree with Sönke.


 if I change it back, people will argue that that is redundant and
 unintuitive. Than I will change it back again and the discussion starts
 again.


"logError" is a lot more clear and descriptive. I think that's 
important. If people don't like that they can use an alias.

-- 
/Jacob Carlborg



 Jul 13 2014



 next sibling parent reply "Dicebot" <public dicebot.lv>  writes:


On Sunday, 13 July 2014 at 12:18:40 UTC, Jacob Carlborg wrote:

 "logError" is a lot more clear and descriptive. I think that's 
 important. If people don't like that they can use an alias.


It is effectively same as using static namespace class but 
C-style - mangling namespace into function name. `log.error` 
looks descriptive enough to me.



 Jul 13 2014



  parent  Jacob Carlborg <doob me.com>  writes:


On 2014-07-13 14:21, Dicebot wrote:


 It is effectively same as using static namespace class but C-style -
 mangling namespace into function name. `log.error` looks descriptive
 enough to me.


I think most usage would _not_ look like that because of the free functions.

-- 
/Jacob Carlborg



 Jul 13 2014



prev sibling  parent reply "ponce" <contact gam3sfrommars.fr>  writes:


On Sunday, 13 July 2014 at 12:18:40 UTC, Jacob Carlborg wrote:

 On 2014-07-13 13:57, Robert burner Schadek wrote:


 Anyone else?


 I agree with Sönke.


 if I change it back, people will argue that that is redundant 
 and
 unintuitive. Than I will change it back again and the 
 discussion starts
 again.


 "logError" is a lot more clear and descriptive. I think that's 
 important. If people don't like that they can use an alias.


I see a lot of people pushing for opposite changes that would 
make Robert make full circle:
- info/warning/error cs logInfo/logWarning/logError
- infof/warnf/errorf vs info/warn/error (formatted the default)
- removing conditionals overloads vs having conditional overloads 
with own name vs having conditional overloads with same name

Not everyone will ever get satisfied.



 Jul 14 2014



 next sibling parent  Robert burner Schadek via Digitalmars-d <digitalmars-d puremagic.com>  writes:


On 07/14/2014 01:45 PM, ponce via Digitalmars-d wrote:

 Not everyone will ever get satisfied.


Indeed. I can only compromise so much, until it becomes compromised.



 Jul 14 2014



prev sibling  parent reply Jeremy Powers via Digitalmars-d <digitalmars-d puremagic.com>  writes:


 Not everyone will ever get satisfied.


 Indeed. I can only compromise so much, until it becomes compromised.


Naming is one of the two hard problems (along with cache invalidation and
off-by-one errors).

We can argue about these things forever, but ultimately all that really
matters is: does it do what we need?  As long as the API works, and lets
one build fancier functionality underneath, it is good.  (Though, in my
experience, any API written will be wrong the first try)


That said, my bikeshed preferences are:
 - namespaced log.info, log.warning, etc
 - info(f) matching write(f)
 - conditionals as an extension/wrapper, not part of base



 Jul 14 2014



  parent  "ponce" <contact gam3sfrommars.fr>  writes:


On Monday, 14 July 2014 at 17:18:05 UTC, Jeremy Powers via 
Digitalmars-d wrote:

 We can argue about these things forever, but ultimately all 
 that really
 matters is: does it do what we need?  As long as the API works, 
 and lets
 one build fancier functionality underneath, it is good.  
 (Though, in my
 experience, any API written will be wrong the first try)


 That said, my bikeshed preferences are:
  - namespaced log.info, log.warning, etc
  - info(f) matching write(f)
  - conditionals as an extension/wrapper, not part of base


My vote goes for:

- vibe.d names (logDebug/logInfo)
- info(f) matching write(f)
- conditionals as an extension/wrapper, not part of base



 Jul 14 2014



prev sibling  parent reply "sigod" <sigod.mail gmail.com>  writes:


On Saturday, 12 July 2014 at 16:13:22 UTC, Sönke Ludwig wrote:

  - The functions error(), info(), fatal(), etc. don't follow 
 the usual rule for functions to start with a verb. The question 
 is if saving three characters over logError() is worth making 
 the code more ambiguous for the outside reader (e.g. "does 
 error() throw an exception? or set some internal error state?" 
 "does fatal() terminate the process?")


I use my own log module like this:
```
import log = util.log;

log.info(...);
log.debug(...);
// etc
```



 Jul 13 2014



  parent reply "Dicebot" <public dicebot.lv>  writes:


On Sunday, 13 July 2014 at 12:41:59 UTC, sigod wrote:

 I use my own log module like this:
 ```
 import log = util.log;

 log.info(...);
 log.debug(...);
 // etc
 ```


Exactly. I think this should be popularized as default style via 
std.logger documentation.



 Jul 13 2014



 next sibling parent reply Jacob Carlborg <doob me.com>  writes:


On 2014-07-13 14:45, Dicebot wrote:


 Exactly. I think this should be popularized as default style via
 std.logger documentation.


If that's the style everyone is encouraged to use, why not force it then?

-- 
/Jacob Carlborg



 Jul 13 2014



 next sibling parent  "Robert burner Schadek" <rburners gmail.com>  writes:


On Sunday, 13 July 2014 at 14:12:56 UTC, Jacob Carlborg wrote:

 On 2014-07-13 14:45, Dicebot wrote:


 Exactly. I think this should be popularized as default style 
 via
 std.logger documentation.


 If that's the style everyone is encouraged to use, why not 
 force it then?


because if you force it, you can not choose to do it differently



 Jul 13 2014



prev sibling  parent reply "Dicebot" <public dicebot.lv>  writes:


On Sunday, 13 July 2014 at 14:12:56 UTC, Jacob Carlborg wrote:

 On 2014-07-13 14:45, Dicebot wrote:


 Exactly. I think this should be popularized as default style 
 via
 std.logger documentation.


 If that's the style everyone is encouraged to use, why not 
 force it then?


Because starting with documentation in Phobos and than proceeding 
with convincing Walter to add built-in support for such idiom is 
much more realistic way than the other way around ;)



 Jul 13 2014



  parent reply Jacob Carlborg <doob me.com>  writes:


On 13/07/14 17:16, Dicebot wrote:


 Because starting with documentation in Phobos and than proceeding with
 convincing Walter to add built-in support for such idiom is much more
 realistic way than the other way around ;)


"built-in" as in built-in to the language?

-- 
/Jacob Carlborg



 Jul 14 2014



  parent reply "Dicebot" <public dicebot.lv>  writes:


On Monday, 14 July 2014 at 09:10:36 UTC, Jacob Carlborg wrote:

 On 13/07/14 17:16, Dicebot wrote:


 Because starting with documentation in Phobos and than 
 proceeding with
 convincing Walter to add built-in support for such idiom is 
 much more
 realistic way than the other way around ;)


 "built-in" as in built-in to the language?


Yes, something like separate "partially static" import type.



 Jul 14 2014



  parent reply Jacob Carlborg <doob me.com>  writes:


On 14/07/14 11:35, Dicebot wrote:


 Yes, something like separate "partially static" import type.


I doesn't need language support. Just have a single function, "log", 
which returns a struct. The struct have all the "error", "warning" and 
so on, functions.

-- 
/Jacob Carlborg



 Jul 14 2014



  parent reply "Dicebot" <public dicebot.lv>  writes:


On Monday, 14 July 2014 at 14:24:39 UTC, Jacob Carlborg wrote:

 On 14/07/14 11:35, Dicebot wrote:


 Yes, something like separate "partially static" import type.


 I doesn't need language support. Just have a single function, 
 "log", which returns a struct. The struct have all the "error", 
 "warning" and so on, functions.


It is exactly what I don't want to see, it is an aberration. 
Never ever.

D unit of encapsulation is module. Any other "namespace" 
solutions must die after proper torture. However we do miss sane 
built-in compromise between overly verbose static imports and 
overly clashing normal imports - this need to be solved via 
language feature in the long term because it is fundamental 
module system use case.



 Jul 14 2014



 next sibling parent  "Dicebot" <public dicebot.lv>  writes:


On Monday, 14 July 2014 at 15:34:12 UTC, Dicebot wrote:

 On Monday, 14 July 2014 at 14:24:39 UTC, Jacob Carlborg wrote:

 On 14/07/14 11:35, Dicebot wrote:


 Yes, something like separate "partially static" import type.


 I doesn't need language support. Just have a single function, 
 "log", which returns a struct. The struct have all the 
 "error", "warning" and so on, functions.


 It is exactly what I don't want to see, it is an aberration. 
 Never ever.

 D unit of encapsulation is module. Any other "namespace" 
 solutions must die after proper torture. However we do miss 
 sane built-in compromise between overly verbose static imports 
 and overly clashing normal imports - this need to be solved via 
 language feature in the long term because it is fundamental 
 module system use case.


One option could be to relax `static import` to allow 
module-qualified usage too. Until then `import mod = pkg.mod;` is 
a good simple idiom that fits existing module system.



 Jul 14 2014



prev sibling  parent reply "Daniel Murphy" <yebbliesnospam gmail.com>  writes:


"Dicebot"  wrote in message news:veqlcdugugxpffajprsj forum.dlang.org... 


 D unit of encapsulation is module. Any other "namespace" 
 solutions must die after proper torture.


Just use this in the next release!

extern(C++, log)
{
extern(D):
   ... functions ...
}



 Jul 14 2014



  parent reply "Dicebot" <public dicebot.lv>  writes:


On Monday, 14 July 2014 at 15:43:33 UTC, Daniel Murphy wrote:

 "Dicebot"  wrote in message 
 news:veqlcdugugxpffajprsj forum.dlang.org...


 D unit of encapsulation is module. Any other "namespace" 
 solutions must die after proper torture.


 Just use this in the next release!

 extern(C++, log)
 {
 extern(D):
   ... functions ...
 }


Not helping :grumpy:



 Jul 14 2014



  parent reply Jeremy Powers via Digitalmars-d <digitalmars-d puremagic.com>  writes:


 D unit of encapsulation is module. Any other "namespace" solutions must

 die after proper torture.


 Just use this in the next release!

 extern(C++, log)
 {
 extern(D):
   ... functions ...
 }


 Not helping :grumpy:


Is this where I talk about hierarchical logging?

There was some talk in the original review thread about having each log
import create or tag the log with the module, so one could refer to and
filter by modules.  In java land you get a logger instance per class, in a
tree based on package hierarchy, and can then configure output levels
differently for various places in the hierarchy - very useful (esp. with
runtime configuration).

While discussing the proper way to namespace/import logger, can you make it
such that doing so gives hierarchical logging?



 Jul 14 2014



  parent reply "Dicebot" <public dicebot.lv>  writes:


On Monday, 14 July 2014 at 17:44:37 UTC, Jeremy Powers via 
Digitalmars-d wrote:

 Is this where I talk about hierarchical logging?

 There was some talk in the original review thread about having 
 each log
 import create or tag the log with the module, so one could 
 refer to and
 filter by modules.  In java land you get a logger instance per 
 class, in a
 tree based on package hierarchy, and can then configure output 
 levels
 differently for various places in the hierarchy - very useful 
 (esp. with
 runtime configuration).

 While discussing the proper way to namespace/import logger, can 
 you make it
 such that doing so gives hierarchical logging?


Replacing `import log = std.logger` with something like this:

```
import std.logger.modulelog;
auto log = new ModuleLogger;
```

should do the trick as far as I understand the current design - 
this assumes that module/package name filters are implemented as 
static ModuleLogger fields.



 Jul 14 2014



  parent reply "MrSmith" <mrsmith33 yandex.ru>  writes:


While trying to use logger i've found that this doesn't work. The 
execution stalls and thread is not executed, while eith writeln 
it works fine.

import std.logger;
import std.parallelism;

void worker()
{
	log("in worker");
}

void main()
{
	auto pool = taskPool;
	pool.put(task!worker);
}



 Jul 14 2014



  parent reply "Robert burner Schadek" <rburners gmail.com>  writes:


On Monday, 14 July 2014 at 18:12:44 UTC, MrSmith wrote:

 While trying to use logger i've found that this doesn't work. 
 The execution stalls and thread is not executed, while eith 
 writeln it works fine.

 import std.logger;
 import std.parallelism;

 void worker()
 {
 	log("in worker");
 }

 void main()
 {
 	auto pool = taskPool;
 	pool.put(task!worker);
 }


can you make an issue out of it here 
https://github.com/burner/logger, so we
don't lose track



 Jul 14 2014



  parent reply "MrSmith" <mrsmith33 yandex.ru>  writes:


On Monday, 14 July 2014 at 18:54:35 UTC, Robert burner Schadek 
wrote:

 On Monday, 14 July 2014 at 18:12:44 UTC, MrSmith wrote:

 While trying to use logger i've found that this doesn't work. 
 The execution stalls and thread is not executed, while eith 
 writeln it works fine.

 import std.logger;
 import std.parallelism;

 void worker()
 {
 	log("in worker");
 }

 void main()
 {
 	auto pool = taskPool;
 	pool.put(task!worker);
 }


 can you make an issue out of it here 
 https://github.com/burner/logger, so we
 don't lose track


Here it is https://github.com/burner/logger/issues/10



 Jul 14 2014



  parent  "Robert burner Schadek" <rburners gmail.com>  writes:


On Monday, 14 July 2014 at 18:59:45 UTC, MrSmith wrote:

 can you make an issue out of it here 
 https://github.com/burner/logger, so we
 don't lose track


 Here it is https://github.com/burner/logger/issues/10


fixed



 Jul 14 2014



prev sibling  parent reply =?UTF-8?B?U8O2bmtlIEx1ZHdpZw==?= <sludwig rejectedsoftware.com>  writes:


Am 13.07.2014 14:45, schrieb Dicebot:

 On Sunday, 13 July 2014 at 12:41:59 UTC, sigod wrote:

 I use my own log module like this:
 ```
 import log = util.log;

 log.info(...);
 log.debug(...);
 // etc
 ```


 Exactly. I think this should be popularized as default style via
 std.logger documentation.


Yeah, I guess that should be fine, too.



 Jul 13 2014



  parent  "Robert burner Schadek" <rburners gmail.com>  writes:


On Sunday, 13 July 2014 at 14:32:40 UTC, Sönke Ludwig wrote:

 Am 13.07.2014 14:45, schrieb Dicebot:

 On Sunday, 13 July 2014 at 12:41:59 UTC, sigod wrote:

 I use my own log module like this:
 ```
 import log = util.log;

 log.info(...);
 log.debug(...);
 // etc
 ```


 Exactly. I think this should be popularized as default style 
 via
 std.logger documentation.


 Yeah, I guess that should be fine, too.


I guess it is settled then



 Jul 13 2014



prev sibling next sibling parent reply Jacob Carlborg <doob me.com>  writes:


On 2014-07-11 16:36, 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.


* The free functions are not documented
* The API of the free functions look complicated and have cryptic names
* What's the EBNF for? Seems complicated as well

-- 
/Jacob Carlborg



 Jul 13 2014



  parent reply "Robert burner Schadek" <rburners gmail.com>  writes:


On Sunday, 13 July 2014 at 11:25:51 UTC, Jacob Carlborg wrote:

 On 2014-07-11 16:36, 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.


 * The free functions are not documented


The compiler does not create ddoc for mixined source


 * The API of the free functions look complicated and have 
 cryptic names
 * What's the EBNF for? Seems complicated as well


The names are quite easy if you look at the bnf.



 Jul 13 2014



  parent  Jacob Carlborg <doob me.com>  writes:


On 2014-07-13 14:01, Robert burner Schadek wrote:


 The compiler does not create ddoc for mixined source


Hasn't that been fixed recently? If not, then I don't think you should 
use mixins. We can't have undocumented functions.


 The names are quite easy if you look at the bnf.


If you don't use abbreviations there's no need for a EBNF.

-- 
/Jacob Carlborg



 Jul 13 2014



prev sibling next sibling parent reply "Ola Fosheim =?UTF-8?B?R3LDuHN0YWQi?=  writes:


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


I guess this is possible with the proposal, but I'd like to see 
structured logging in the runtime and in a way that is compatible 
with existing services, so that all libraries use the same 
logging infrastructure and such  a way that it can be redirected 
easily without rewriting any logging calls.

E.g. when building a service on AppEngine you log to a buffer of 
1+GB for all your servers implemented in various languages and 
can do structured searching by type: (debug, info, warning, 
error, critical), time:real representing seconds since epoch, and 
message: string. However, since system level "debug" is higher 
level than language level "debug" there should be several levels 
below "debug" used in libraries and frameworks that is kept 
in-memory only to avoid spamming the global application/system 
level debug-logging.

Having a flexible language level logging mechanism is good, but 
making sure it fits into existing logging-frameworks (that cannot 
be modified) is more important and what it should be evaluated 
against.



 Jul 14 2014



  parent reply Robert burner Schadek via Digitalmars-d <digitalmars-d puremagic.com>  writes:


On 07/14/2014 01:07 PM, via Digitalmars-d 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


 I guess this is possible with the proposal, but I'd like to see
 structured logging in the runtime and in a way that is compatible with
 existing services, so that all libraries use the same logging
 infrastructure and such  a way that it can be redirected easily
 without rewriting any logging calls.

 E.g. when building a service on AppEngine you log to a buffer of 1+GB
 for all your servers implemented in various languages and can do
 structured searching by type: (debug, info, warning, error, critical),
 time:real representing seconds since epoch, and message: string.
 However, since system level "debug" is higher level than language
 level "debug" there should be several levels below "debug" used in
 libraries and frameworks that is kept in-memory only to avoid spamming
 the global application/system level debug-logging.

 Having a flexible language level logging mechanism is good, but making
 sure it fits into existing logging-frameworks (that cannot be
 modified) is more important and what it should be evaluated against.


? could you rephrase, I can not grasp your point (points)



 Jul 14 2014



  parent reply "Ola Fosheim =?UTF-8?B?R3LDuHN0YWQi?=  writes:


On Monday, 14 July 2014 at 11:35:16 UTC, Robert burner Schadek 
via Digitalmars-d wrote:

 ? could you rephrase, I can not grasp your point (points)


The point is that the most crucial aspect of logging is being 
able to filter or put triggers on the logs in pre-existing 
external logging-solutions without having to modify the 
frameworks you use. Therefore a standardized structure that fits 
common patterns in online services would be beneficial.

E.g.:

If I use vibe.d on AppEngine then I want to redirect the vibe.d 
logging to the AppEngine logging-service without requiring vibe.d 
authors to know AppEngine or me to change vibe.d.

I'd like to plug in a "logging redirection handler" in the D 
runtime with the ability to filter/translate the logging severity 
level (at least corresponding to the severity levels: 
library/internal, debug, info, warning, error, critical) and the 
source.

So in the logging-handler you need to the following information:
1. source (e.g. vibe.d)
2. severity level
3. message (just a string)
4. time (but that can be inferred by the logging handler)

Not particularly complicated, I think, but the logging 
functionality should match up to the requirements (for ease of 
use, ability to scale etc)



 Jul 14 2014



  parent reply Jeremy Powers via Digitalmars-d <digitalmars-d puremagic.com>  writes:


 The point is that the most crucial aspect of logging is being able to
 filter or put triggers on the logs in pre-existing external
 logging-solutions without having to modify the frameworks you use.



Important, but I'd hesitate to call it the most crucial aspect...  that
honour probably goes to 'write messages to a sink'

The stated goal of this log library is to provide a base with common useful
functionality, in a way that can be extended to fit more complicated use
cases.  I'd argue that what you are talking about fits into the 'more
complicated' - the log API should definitely allow this, in a way that it
can be plugged in without log using code having to change, but I don't
believe the functionality needs to be in the base library (at this time).



 Jul 14 2014



  parent reply "Ola Fosheim =?UTF-8?B?R3LDuHN0YWQi?=  writes:


On Monday, 14 July 2014 at 17:02:59 UTC, Jeremy Powers via 
Digitalmars-d wrote:

 The stated goal of this log library is to provide a base with 
 common useful
 functionality, in a way that can be extended to fit more 
 complicated use
 cases.  I'd argue that what you are talking about fits into the 
 'more
 complicated' - the log API should definitely allow this, in a 
 way that it
 can be plugged in without log using code having to change, but 
 I don't
 believe the functionality needs to be in the base library (at 
 this time).


Getting things changed is more difficult than getting it right 
from the start. As far as I am concerned it is crucial in the 
sense that if it does not support severity levels and easy 
integration with existing logging services then I probably won't 
use it.



 Jul 14 2014



  parent reply Jeremy Powers via Digitalmars-d <digitalmars-d puremagic.com>  writes:


 Getting things changed is more difficult than getting it right from the
 start.



Very true.  The logging API needs to be right before it goes into std and
has to be locked down.  But then, no API is ever right the first time,
needs banging on to expose the weaknesses so they can be fixed.




 As far as I am concerned it is crucial in the sense that if it does not
 support severity levels and easy integration with existing logging services
 then I probably won't use it.


The logging API in the standard library needs to be able to support this
kind of thing.  Doesn't mean it actually needs to be included in the base
implementation.  I would recommend trying to see if you can implement what
you want given the existing framework - and if not, yell so it can be
changed.  What is missing from the existing stuff that keeps this from
working as an extension?



 Jul 14 2014



  parent reply "Ola Fosheim =?UTF-8?B?R3LDuHN0YWQi?=  writes:


On Monday, 14 July 2014 at 20:45:29 UTC, Jeremy Powers via 
Digitalmars-d wrote:

 The logging API in the standard library needs to be able to 
 support this
 kind of thing.  Doesn't mean it actually needs to be included 
 in the base
 implementation.


Will you then be able to get fully inlined low overhead 
ringbuffer logging throughout the application and used 
frameworks? E.g. do low level logging to a ringbuffer that is 
only saved/mailed upon fatal crashes. This is useful for online 
services.



 Jul 14 2014



  parent reply "Robert burner Schadek" <rburners gmail.com>  writes:


On Tuesday, 15 July 2014 at 03:22:46 UTC, Ola Fosheim Grøstad 
wrote:

 On Monday, 14 July 2014 at 20:45:29 UTC, Jeremy Powers via 
 Digitalmars-d wrote:

 The logging API in the standard library needs to be able to 
 support this
 kind of thing.  Doesn't mean it actually needs to be included 
 in the base
 implementation.


 Will you then be able to get fully inlined low overhead 
 ringbuffer logging throughout the application and used 
 frameworks? E.g. do low level logging to a ringbuffer that is 
 only saved/mailed upon fatal crashes. This is useful for online 
 services.


Yes, apart from the inlined part, I'm not sure what the compiler 
does with virtual functions.

I would you can do that in <=15 lines of code



 Jul 15 2014



  parent reply Jeremy Powers via Digitalmars-d <digitalmars-d puremagic.com>  writes:


 Will you then be able to get fully inlined low overhead ringbuffer logging

 throughout the application and used frameworks? E.g. do low level logging
 to a ringbuffer that is only saved/mailed upon fatal crashes. This is
 useful for online services.


 Yes, apart from the inlined part, I'm not sure what the compiler does with
 virtual functions.


But can you do it without modifying the user code?

Ideally, one could control how logging happens without having to change the
code that requests the logging.  This is important for cases like when you
have a library that uses logging, and you need to make it conform without
digging into its guts.

I believe to do this with the current setup, you would need some standard
'dispatcher' logger, and have every user of logging use it.  Then could
configure the dispatcher as needed for your case without modifying all the
code to use a different logger.

Ideally this standard setup would be the default/recommended way to use the
logging library - any thoughts on how to do this properly (and maybe get
inlining/templating)?



 Jul 15 2014



 next sibling parent  "Ola Fosheim =?UTF-8?B?R3LDuHN0YWQi?=  writes:


On Tuesday, 15 July 2014 at 17:41:09 UTC, Jeremy Powers via 
Digitalmars-d wrote:

 I believe to do this with the current setup, you would need 
 some standard
 'dispatcher' logger, and have every user of logging use it.  
 Then could
 configure the dispatcher as needed for your case without 
 modifying all the
 code to use a different logger.


If there is a standard dispatcher with calling conventions that 
all libraries use (e.g. logging.debug(), logging.info(), 
logging.error() etc) then I guess I could use textual search and 
replace to retrofit the standard API with my own inlined logger. 
Customization of the logging API or too many variations could 
make that difficult or impossible.

Virtual functions, locking etc should be avoided so that you can 
do very low overhead full logging on a live server that is under 
attack. Basically just writing some bytes to a set of thread 
local ring-buffers with timing information, then collect and 
merge them on a crash for analysis so you can get the gory 
details that lead to the crash and do emergency patching asap.



 Jul 15 2014



prev sibling  parent  "Robert burner Schadek" <rburners gmail.com>  writes:


On Tuesday, 15 July 2014 at 17:41:09 UTC, Jeremy Powers via 
Digitalmars-d wrote:

 Will you then be able to get fully inlined low overhead 
 ringbuffer logging

 throughout the application and used frameworks? E.g. do low 
 level logging
 to a ringbuffer that is only saved/mailed upon fatal crashes. 
 This is
 useful for online services.


 Yes, apart from the inlined part, I'm not sure what the 
 compiler does with
 virtual functions.


 But can you do it without modifying the user code?

 Ideally, one could control how logging happens without having 
 to change the
 code that requests the logging.  This is important for cases 
 like when you
 have a library that uses logging, and you need to make it 
 conform without
 digging into its guts.


If the user uses the defaultLogger it is trivial. Otherwise if 
you get the logger by calling a factory function you maintain. It 
is also trival.


 I believe to do this with the current setup, you would need 
 some standard
 'dispatcher' logger, and have every user of logging use it.  
 Then could
 configure the dispatcher as needed for your case without 
 modifying all the
 code to use a different logger.


Sounds like a MultiLogger as defaultLogger

 Ideally this standard setup would be the default/recommended 
 way to use the
 logging library - any thoughts on how to do this properly (and 
 maybe get
 inlining/templating)?


I think the default way depends on your use case. If it is forced 
its properly to flexible enough



 Jul 15 2014



prev sibling next sibling parent  "Robert burner Schadek" <rburners gmail.com>  writes:


I just pushed a new version to dub and the PR.

Highlights:

* LogManager is gone. ( s/LogManager\.//g should fix all user 
code)
* The codegen was replaced by some format token strings
* log(lcf) and trace(cf) now present documentation
   * trace(cf) stands as an example for info, warning ... and so 
forth
* StdIOLogger is now thread safe
* StdIOLogger docu tells you that it is thread safe

I also updated the html documents so you don't have to read 
through the source, but you are of course welcome to do so.



 Jul 16 2014



prev sibling next sibling parent reply "linkrope" <linkrope github.com>  writes:


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.)



 Jul 20 2014



 next sibling parent reply "Robert burner Schadek" <rburners gmail.com>  writes:


On Sunday, 20 July 2014 at 16:15:53 UTC, linkrope wrote:

 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.)


I didn't expect to hear from you about this, after you did not 
reply to my email about this topic.

If xml is problem for you, where is the PR?


 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.)


so we have gone full circle now ....


 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?


That's an easy one.

```
auto a = LOG_FIRST_N(1337);

logc(a, "Hello world");

auto b = WHAT_EVERY_THE(....);
logc(b, "Hello world again");
```


 With suffix 'e'? Suffix 'f' is already assigned!


what is 'e'?


 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.


That was a user request, through github. Where I asked you to 
submit PRs and issues.

Have you tried subclassing Logger? I asked for PRs in the email I 
wrote to you at least twice.


 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.


Yes please, suggestions?


 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.


I will fix that in the next session.


 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!


I missed that, thank you


 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.


easy fix


 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.


It is a special file, I wanted to have that clear. two different 
classes does the trick for me.


 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(...)'?


unspecific is about to be removed, all and off are pretty easy to 
understand but than ....


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


not if you want to have that logged somewhere else.


 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;


you got that wrong, you can do it like that, nobody will force 
you and properly people will do it different.
You can also create a module wide global logger and use that

Again, std.logger is not the solution that works for every 
special case anybody comes up with out of the box. It is a set of 
ideas that enable you to have the logging tailored to your needs 
easily. On top of that, it allows you very fast access to basic 
logging that can be extend later on easily and seamlessly.


 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


So you deleted code and functionality, hm?


 - there is no NullLogger


Same point


 - there is no MultiLogger (this functionality is implicit)


It is not, you can't remove Loggers individual and you can't 
build trees.


 - there is no need to provide a name for a logger


Because, you have no MultiLogger


 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


So I need nine LogLevels, how do I add one between info and warn?


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


Object.factory


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


Subclassing Logger should get the job done in under 30 lines.


 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.)


So what are the numbers for std.logger and where is the benchmark 
file to test it?

 From the source it looks like you kept parts of the design of 
std.logger and pulled out everything you didn't agree with. Also 
you only have global logging, to cite Brian: "Why?"

You added the parts, that you described are missing in 
std.logger. Of course your counter proposal will meet your needs 
then. I mean what would be the point of coding it up anyway else?



 Jul 21 2014



  parent reply "linkrope" <linkrope github.com>  writes:


Sorry, but at the first contact with the implementation (the one 
with 'genDocComment' and 'buildLogFunction' from months ago) I 
was scared away. (It's better now.)

I feared that if I criticize the 'mixin' sequences you would ask 
something like "suggestions?" ;-)
So, I made this experiment to provide an answer. Now, I can 
suggest: try something like this:

     alias trace = log!(LogLevel.trace);
     alias info = log!(LogLevel.info);
     alias warn = log!(LogLevel.warn);
     alias error = log!(LogLevel.error);
     alias fatal = log!(LogLevel.fatal);

(see https://github.com/linkrope/log/blob/master/src/log.d)

Controversial conditional logging
----------------------------
The only advantage of
     tracec(condition, "passed");
over
     if (condition) trace("passed");
would be, that a costly evaluation of the condition is omitted 
when there is no trace logger.
That's why the std.log proposal had 'when(lazy bool now)'.

First, I was puzzled about your argument that LOG_FIRST_N or 
LOG_EVERY_N would be no problem with the '...c' functions. But a 
look at the implementation confirmed that the std.logger has no 
lazy evaluation of the condition; discarding the only advantage.

Sets of log levels
--------------
No!
Of course, I can log (trace | info) to stdout, warn.orHigher to 
stderr, and for instance info.orHigher to some file.

Simplicity
--------
"The simplest way to achieve simplicity is through thoughtful 
reduction."

We started with tango.util.log (best described as log4j for D). 
We are happier now with a lot less functionality, but on the 
other hand with the simplest API that works.

On Monday, 21 July 2014 at 22:53:27 UTC, Robert burner Schadek 
wrote:

 On Sunday, 20 July 2014 at 16:15:53 UTC, linkrope wrote:

 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.)


 I didn't expect to hear from you about this, after you did not 
 reply to my email about this topic.

 If xml is problem for you, where is the PR?


 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.)


 so we have gone full circle now ....


 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?


 That's an easy one.

 ```
 auto a = LOG_FIRST_N(1337);

 logc(a, "Hello world");

 auto b = WHAT_EVERY_THE(....);
 logc(b, "Hello world again");
 ```


 With suffix 'e'? Suffix 'f' is already assigned!


 what is 'e'?


 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.


 That was a user request, through github. Where I asked you to 
 submit PRs and issues.

 Have you tried subclassing Logger? I asked for PRs in the email 
 I wrote to you at least twice.


 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.


 Yes please, suggestions?


 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.


 I will fix that in the next session.


 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!


 I missed that, thank you


 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.


 easy fix


 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.


 It is a special file, I wanted to have that clear. two 
 different classes does the trick for me.


 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(...)'?


 unspecific is about to be removed, all and off are pretty easy 
 to understand but than ....


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


 not if you want to have that logged somewhere else.


 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;


 you got that wrong, you can do it like that, nobody will force 
 you and properly people will do it different.
 You can also create a module wide global logger and use that

 Again, std.logger is not the solution that works for every 
 special case anybody comes up with out of the box. It is a set 
 of ideas that enable you to have the logging tailored to your 
 needs easily. On top of that, it allows you very fast access to 
 basic logging that can be extend later on easily and seamlessly.


 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


 So you deleted code and functionality, hm?


 - there is no NullLogger


 Same point


 - there is no MultiLogger (this functionality is implicit)


 It is not, you can't remove Loggers individual and you can't 
 build trees.


 - there is no need to provide a name for a logger


 Because, you have no MultiLogger


 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


 So I need nine LogLevels, how do I add one between info and 
 warn?


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


 Object.factory


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


 Subclassing Logger should get the job done in under 30 lines.


 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.)


 So what are the numbers for std.logger and where is the 
 benchmark file to test it?

 From the source it looks like you kept parts of the design of 
 std.logger and pulled out everything you didn't agree with. 
 Also you only have global logging, to cite Brian: "Why?"

 You added the parts, that you described are missing in 
 std.logger. Of course your counter proposal will meet your 
 needs then. I mean what would be the point of coding it up 
 anyway else?





 Jul 22 2014



  parent reply "Robert burner Schadek" <rburners gmail.com>  writes:


On Tuesday, 22 July 2014 at 21:52:09 UTC, linkrope wrote:

 Sorry, but at the first contact with the implementation (the 
 one with 'genDocComment' and 'buildLogFunction' from months 
 ago) I was scared away. (It's better now.)


There was no need, even read that, for implementing a 
FileRotateLogger


 I feared that if I criticize the 'mixin' sequences you would 
 ask something like "suggestions?" ;-)
 So, I made this experiment to provide an answer. Now, I can 
 suggest: try something like this:

     alias trace = log!(LogLevel.trace);
     alias info = log!(LogLevel.info);
     alias warn = log!(LogLevel.warn);
     alias error = log!(LogLevel.error);
     alias fatal = log!(LogLevel.fatal);

 (see https://github.com/linkrope/log/blob/master/src/log.d)


I did and I had to mod it quite a bit to make it work with 
default logging instance logging and an more elaborate api


 Controversial conditional logging
 ----------------------------
 The only advantage of
     tracec(condition, "passed");
 over
     if (condition) trace("passed");
 would be, that a costly evaluation of the condition is omitted 
 when there is no trace logger.
 That's why the std.log proposal had 'when(lazy bool now)'.

 First, I was puzzled about your argument that LOG_FIRST_N or 
 LOG_EVERY_N would be no problem with the '...c' functions. But 
 a look at the implementation confirmed that the std.logger has 
 no lazy evaluation of the condition; discarding the only 
 advantage.


passing a bool as a delegate that is the first thing that gets 
evaluated does not sound right.


 Sets of log levels
 --------------
 No!
 Of course, I can log (trace | info) to stdout, warn.orHigher to 
 stderr, and for instance info.orHigher to some file.

 Simplicity
 --------
 "The simplest way to achieve simplicity is through thoughtful 
 reduction."

 We started with tango.util.log (best described as log4j for D). 
 We are happier now with a lot less functionality, but on the 
 other hand with the simplest API that works.


In your last post you said that you wrote it on a weekend, which 
is it?

That api may work for your requirements, but this should go into 
phobos and allow anybody to get satisfied.



 Jul 22 2014



 next sibling parent reply "Ola Fosheim =?UTF-8?B?R3LDuHN0YWQi?=  writes:


 That api may work for your requirements, but this should go 
 into phobos and allow anybody to get satisfied.


I think a hardcoded fast version is preferable, then you simply 
use a modified version of phobos/runtime if you want low overhead 
in-memory logging.

The most important thing, besides being fast, is that everybody 
use the same syntax, making it grep'able.



 Jul 22 2014



  parent reply "Robert burner Schadek" <rburners gmail.com>  writes:


On Wednesday, 23 July 2014 at 06:10:22 UTC, Ola Fosheim Grøstad 
wrote:

 The most important thing, besides being fast, is that everybody 
 use the same syntax, making it grep'able.


It is easily grepable



 Jul 23 2014



  parent reply "Ola Fosheim =?UTF-8?B?R3LDuHN0YWQi?=  writes:


On Wednesday, 23 July 2014 at 09:15:26 UTC, Robert burner Schadek 
wrote:

 On Wednesday, 23 July 2014 at 06:10:22 UTC, Ola Fosheim Grøstad 
 wrote:

 The most important thing, besides being fast, is that 
 everybody use the same syntax, making it grep'able.


 It is easily grepable


I mean in the sense that you can write a regexp that catch all 
cases for automated search and replace.



 Jul 23 2014



  parent  "Robert burner Schadek" <rburners gmail.com>  writes:


On Wednesday, 23 July 2014 at 22:07:16 UTC, Ola Fosheim Grøstad 
wrote:

 It is easily grepable


 I mean in the sense that you can write a regexp that catch all 
 cases for automated search and replace.


that is no problem.



 Jul 23 2014



prev sibling  parent reply "linkrope" <linkrope github.com>  writes:


On Tuesday, 22 July 2014 at 23:43:59 UTC, Robert burner Schadek 
wrote:

 On Tuesday, 22 July 2014 at 21:52:09 UTC, linkrope wrote:

 Controversial conditional logging
 ----------------------------
 The only advantage of
    tracec(condition, "passed");
 over
    if (condition) trace("passed");
 would be, that a costly evaluation of the condition is omitted 
 when there is no trace logger.
 That's why the std.log proposal had 'when(lazy bool now)'.

 First, I was puzzled about your argument that LOG_FIRST_N or 
 LOG_EVERY_N would be no problem with the '...c' functions. But 
 a look at the implementation confirmed that the std.logger has 
 no lazy evaluation of the condition; discarding the only 
 advantage.


 passing a bool as a delegate that is the first thing that gets 
 evaluated does not sound right.


Indeed: that's why the lazy condition should be evaluated last!

Your "will log" condition is very simple and efficient: only >= 
and !=
But you cannot know, how long the evaluation of the user-provided 
condition will take.

BTW: with 'globalLogLevel', 'defaultLogger.logLevel'and 
'LogLevel.off' the usual 'willLog' predicate will come in handy 
to avoid code duplication.

While the lazy evaluation of the condition would be the only 
advantage over

     if (condtion) log(...);

I haven't encountered a single opportunity for conditional 
logging in the code of our company.

'if'/'log' is always followed by 'return', 'break', 'continue', 
...

     if (condition)
     {
         log("will do something else because condition passed");
         return;
     }



 Jul 23 2014



  parent  "Robert burner Schadek" <rburners gmail.com>  writes:


On Wednesday, 23 July 2014 at 17:56:52 UTC, linkrope wrote:

 Indeed: that's why the lazy condition should be evaluated last!

 Your "will log" condition is very simple and efficient: only >= 
 and !=
 But you cannot know, how long the evaluation of the 
 user-provided condition will take.


I believe this is a matter of opinion. From what I have seen the 
condition is usually very easy to computer and does not justify 
turning it into a delegate.


 BTW: with 'globalLogLevel', 'defaultLogger.logLevel'and 
 'LogLevel.off' the usual 'willLog' predicate will come in handy 
 to avoid code duplication.


Why didn't you start your review with something useful like this?


 While the lazy evaluation of the condition would be the only 
 advantage over

     if (condtion) log(...);

 I haven't encountered a single opportunity for conditional 
 logging in the code of our company.


All swans are white!



 Jul 24 2014



prev sibling  parent reply "ponce" <contact gam3sfrommars.fr>  writes:


On Sunday, 20 July 2014 at 16:15:53 UTC, linkrope wrote:

 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.


I'm in 100% disagreement. If you don't add the f suffix, users 
will write:

info(<user provided string that could contain format>);

This is a crash if the user provided string happens to contains 
"%s".
I wouldn't use such an API which makes format bugs hard to find.




 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.


Do you realize rolling loggers are not there because they are 
supposed to be in another layer as subclasses of Logger?



 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.


I asked for it. And I use it, because I write libraries that log 
warnings but don't forcefully require the users to provide a 
Logger if they don't want to.
And that way, I can still write "logger.warningf" without "if" 
everywhere.



 Jul 22 2014



 next sibling parent reply "Dragos Carp" <dragoscarp gmail.com>  writes:


 I'm in 100% disagreement. If you don't add the f suffix, users 
 will write:

 info(<user provided string that could contain format>);

 This is a crash if the user provided string happens to contains 
 "%s".
 I wouldn't use such an API which makes format bugs hard to find.


It does the right thing... there is an overload: without 
additional arguments, the first argument (not necessary a string) 
is not a format string.


 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.


 Do you realize rolling loggers are not there because they are 
 supposed to be in another layer as subclasses of Logger?


Contrary to the NullLogger, writing a rolling logger is a 
non-trivial task.


 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.


 I asked for it. And I use it, because I write libraries that 
 log warnings but don't forcefully require the users to provide 
 a Logger if they don't want to.
 And that way, I can still write "logger.warningf" without "if" 
 everywhere.





 Jul 22 2014



 next sibling parent  "Robert burner Schadek" <rburners gmail.com>  writes:


On Tuesday, 22 July 2014 at 07:55:04 UTC, Dragos Carp wrote:


 Contrary to the NullLogger, writing a rolling logger is a 
 non-trivial task.


It is an easy ten line function and one additional if.



 Jul 22 2014



prev sibling  parent reply "ponce" <contact gam3sfrommars.fr>  writes:


On Tuesday, 22 July 2014 at 07:55:04 UTC, Dragos Carp wrote:

 I'm in 100% disagreement. If you don't add the f suffix, users 
 will write:

 info(<user provided string that could contain format>);

 This is a crash if the user provided string happens to 
 contains "%s".
 I wouldn't use such an API which makes format bugs hard to 
 find.


 It does the right thing... there is an overload: without 
 additional arguments, the first argument (not necessary a 
 string) is not a format string.


Now if write a bug:
warning("there was an error because the file %s is missing!");

and I forgot the argument, no problem, it will run without 
errrors despite I made one. This is not an improvement over 
writef.

(this particular bug came up in DUB among others, and my last 
example was a real case too in production software where the 
users would call things with "%" in the names)

Two different operations conceptually => two names.


 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.


 Do you realize rolling loggers are not there because they are 
 supposed to be in another layer as subclasses of Logger?


 Contrary to the NullLogger, writing a rolling logger is a 
 non-trivial task.


NullLogger is there precisely because it's trivial and needed.
Of course a rolling logger is not that trivial, but std.logger is 
there to be orthogonal and a foundation not providing everything 
non-trivial.



 Jul 22 2014



 next sibling parent  "Dragos Carp" <dragoscarp gmail.com>  writes:


 It does the right thing... there is an overload: without 
 additional arguments, the first argument (not necessary a 
 string) is not a format string.


 Now if write a bug:
 warning("there was an error because the file %s is missing!");

 and I forgot the argument, no problem, it will run without 
 errrors despite I made one. This is not an improvement over 
 writef.

 (this particular bug came up in DUB among others, and my last 
 example was a real case too in production software where the 
 users would call things with "%" in the names)


It is easy to make mistakes while producing text.

One nasty example:

   writef("On %s the exchange rate raised with more than 5%%", 
date);

later changed to:

   if (date == today())
     write("Today the exchange rate raised with more than 5%%");
   else
     writef("On %s the exchange rate raised with more than 5%%", 
date);


 Two different operations conceptually => two names.


The logging facility is a high-level utility library. I think, 
its main
design goal should be: the usage is a no-brainer, it stays out of
the flow of thoughts while using it. The decision whether a 
message is
a warning, an error, or just an info, it's enough of a burden for 
the
user already, who anyhow takes a lot of decisions while writing 
the
actual code.

It is a matter of opinion if warning and warningf are different
operations, both produce a message and warningf with one argument 
will
be inefficient in best case, or an error, as you described above.

We already have in standard library examples of overloaded 
functions
for "different operations".

For example "find" in std.algorithm (low-level library): it has an
overload which takes an element as argument and another one which 
takes
ranges. Some may say that this is conceptually not different, but 
then
if we look at the predicate of find, we see that they are quite
different: the first overload  matches the argument in the input 
range,
the second matches successive elements of the argument against
successive elements in the input range. You can say these are
conceptualy the same, but you can also say it is different 
(probably
this is the reason why findSplit* didn't bother to implement the
Element overload).

As I already said, I think that the analogy with write/writef is 
wrong
because of at least 2 reasons:
  1. write is a low level function which is part of the flow of 
thoughts
     for resolving the problem at hand
  2. Contrary to the log functions, write is used sometimes to 
produce
     non human-readable output, thus write(arg1, arg2, arg3) being 
a
     valid usage of it.


 Do you realize rolling loggers are not there because they are 
 supposed to be in another layer as subclasses of Logger?


 Contrary to the NullLogger, writing a rolling logger is a 
 non-trivial task.


 NullLogger is there precisely because it's trivial and needed.
 Of course a rolling logger is not that trivial, but std.logger 
 is there to be orthogonal and a foundation not providing 
 everything non-trivial.


Probably you need a NullLogger, if you use the MultiLogger.
For a big application I think it is better to have a big log file,
and filter afterwards based on module, than the other way around:
you have 3 files and need to merge/correlate them.

Regarding having different log levels per module: ex. the 
application
has log level warning and a selected module log level trace, I 
think
there is a simpler solution. Instead of having different loggers 
for
each module, it would be easier to have an list, similar to an 
ACL,
generated at compile time, containg the log level of the selected
module(s). This list will be consulted by the log function in 
addition
to normal log level check.

This will avoid code like (again a decision to make):
   logFoo.warning("Log in my module");
   log.warning("Log in application log");



 Jul 24 2014



prev sibling  parent reply Jacob Carlborg <doob me.com>  writes:


On 22/07/14 11:43, ponce wrote:


 NullLogger is there precisely because it's trivial and needed.


If it's so trivial then the users can implement that themselves. A 
standard library isn't about implementing what's trivial, it's about 
implementing what's most useful to most people.

-- 
/Jacob Carlborg



 Jul 25 2014



 next sibling parent reply "Brian Schott" <briancschott gmail.com>  writes:


On Friday, 25 July 2014 at 07:11:06 UTC, Jacob Carlborg wrote:

 A standard library isn't about implementing what's trivial, 
 it's about implementing what's most useful to most people.


On the other hand, not having to re-implement trivial functions 
that you need all the time is a form of usefulness.



 Jul 25 2014



  parent  Jacob Carlborg <doob me.com>  writes:


On 25/07/14 09:37, Brian Schott wrote:


 On the other hand, not having to re-implement trivial functions that you
 need all the time is a form of usefulness.


I'm arguing that NullLogger is not something that's needed all the time.

-- 
/Jacob Carlborg



 Jul 25 2014



prev sibling next sibling parent reply "Dicebot" <public dicebot.lv>  writes:


On Friday, 25 July 2014 at 07:11:06 UTC, Jacob Carlborg wrote:

 On 22/07/14 11:43, ponce wrote:


 NullLogger is there precisely because it's trivial and needed.


 If it's so trivial then the users can implement that 
 themselves. A standard library isn't about implementing what's 
 trivial, it's about implementing what's most useful to most 
 people.


That is interesting clash of attitude to standard library :) In 
my opinion it is quite the opposite - standard library is here to 
ensure primarily that all trivial things are done in a same way 
in all projects. Anything more complicated can be packaged as a 
separate library but trivialities are exactly the things that set 
up common ground and ensure good library interoperation.



 Jul 25 2014



  parent  Jacob Carlborg <doob me.com>  writes:


On 2014-07-25 14:52, Dicebot wrote:


 That is interesting clash of attitude to standard library :) In my
 opinion it is quite the opposite - standard library is here to ensure
 primarily that all trivial things are done in a same way in all
 projects. Anything more complicated can be packaged as a separate
 library but trivialities are exactly the things that set up common
 ground and ensure good library interoperation.


Do you think std.datetime is easy? I heard it was quite hard to 
implement correctly. Taken into account daylight savings and what not. 
Phobos is full of things like that. But it also contains trivial 
functionality. And we should not even start talking about druntime.

-- 
/Jacob Carlborg



 Jul 25 2014



prev sibling  parent reply "francesco cattoglio" <francesco.cattoglio gmail.com>  writes:


On Friday, 25 July 2014 at 07:11:06 UTC, Jacob Carlborg wrote:

 On 22/07/14 11:43, ponce wrote:


 NullLogger is there precisely because it's trivial and needed.


 If it's so trivial then the users can implement that 
 themselves. A standard library isn't about implementing what's 
 trivial, it's about implementing what's most useful to most 
 people.




"fmin" is trivial as well, is not used all the time, yet is in 
the standard library. Sometime trivial stuff is still good to 
have.



 Jul 25 2014



 next sibling parent  "eles" <eles eles.com>  writes:


On Friday, 25 July 2014 at 13:01:29 UTC, francesco cattoglio 
wrote:

 On Friday, 25 July 2014 at 07:11:06 UTC, Jacob Carlborg wrote:

 On 22/07/14 11:43, ponce wrote:





 "fmin" is trivial as well, is not used all the time, yet is in 
 the standard library. Sometime trivial stuff is still good to 
 have.


I think one good reason to put some things into the standard 
library is to allow and enforce them to become a... standard.

That way, probably everybody would do that thing in a unique way, 
the... standard way.

Optimization of standard features also propagates everywhere.



 Jul 25 2014



prev sibling  parent  Jacob Carlborg <doob me.com>  writes:


On 2014-07-25 15:01, francesco cattoglio wrote:


 "fmin" is trivial as well, is not used all the time, yet is in the
 standard library. Sometime trivial stuff is still good to have.


I'm not saying it bad to have trivial stuff in the standard library. I 
just don't see how NullLogger is useful. But I can see that a rolling 
logger is very useful. Or a systemd logger.

-- 
/Jacob Carlborg



 Jul 25 2014



prev sibling  parent reply "linkrope" <linkrope github.com>  writes:


On Tuesday, 22 July 2014 at 07:27:38 UTC, ponce wrote:

 On Sunday, 20 July 2014 at 16:15:53 UTC, linkrope wrote:

 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.


 I asked for it. And I use it, because I write libraries that 
 log warnings but don't forcefully require the users to provide 
 a Logger if they don't want to.
 And that way, I can still write "logger.warningf" without "if" 
 everywhere.


But then it's better to provide no logger (or at least no logger 
for level warning) than an artificial NullLogger.



 Jul 22 2014



  parent reply "ponce" <contact gam3sfrommars.fr>  writes:


On Tuesday, 22 July 2014 at 08:44:06 UTC, linkrope wrote:

 On Tuesday, 22 July 2014 at 07:27:38 UTC, ponce wrote:

 On Sunday, 20 July 2014 at 16:15:53 UTC, linkrope wrote:

 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.


 I asked for it. And I use it, because I write libraries that 
 log warnings but don't forcefully require the users to provide 
 a Logger if they don't want to.
 And that way, I can still write "logger.warningf" without "if" 
 everywhere.


 But then it's better to provide no logger (or at least no 
 logger for level warning) than an artificial NullLogger.


My need is not "artificial", at least in my view.
Your opinion is different from mine. That's fine.
That's why we need someone to try to reconcile the many, many 
opinions about this.



 Jul 22 2014



  parent reply "linkrope" <linkrope github.com>  writes:


On Tuesday, 22 July 2014 at 09:51:24 UTC, ponce wrote:

 On Tuesday, 22 July 2014 at 08:44:06 UTC, linkrope wrote:

 On Tuesday, 22 July 2014 at 07:27:38 UTC, ponce wrote:

 On Sunday, 20 July 2014 at 16:15:53 UTC, linkrope wrote:

 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.


 I asked for it. And I use it, because I write libraries that 
 log warnings but don't forcefully require the users to 
 provide a Logger if they don't want to.
 And that way, I can still write "logger.warningf" without 
 "if" everywhere.


 But then it's better to provide no logger (or at least no 
 logger for level warning) than an artificial NullLogger.


 My need is not "artificial", at least in my view.
 Your opinion is different from mine. That's fine.
 That's why we need someone to try to reconcile the many, many 
 opinions about this.


Not the need is artificial. For example, I have the need to 
measure the performance of an application with and without 
logging.

I think, the solution is artificial. The obvious solution would 
be to register no logger at all.



 Jul 22 2014



 next sibling parent  "Robert burner Schadek" <rburners gmail.com>  writes:


On Tuesday, 22 July 2014 at 10:24:33 UTC, linkrope wrote:


 Not the need is artificial. For example, I have the need to 
 measure the performance of an application with and without 
 logging.

 I think, the solution is artificial. The obvious solution would 
 be to register no logger at all.


that solution can also be unpractical and unworkable



 Jul 22 2014



prev sibling  parent  "ponce" <contact gam3sfrommars.fr>  writes:


On Tuesday, 22 July 2014 at 10:24:33 UTC, linkrope wrote:

 On Tuesday, 22 July 2014 at 09:51:24 UTC, ponce wrote:

 On Tuesday, 22 July 2014 at 08:44:06 UTC, linkrope wrote:

 On Tuesday, 22 July 2014 at 07:27:38 UTC, ponce wrote:

 On Sunday, 20 July 2014 at 16:15:53 UTC, linkrope wrote:

 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.


 I asked for it. And I use it, because I write libraries that 
 log warnings but don't forcefully require the users to 
 provide a Logger if they don't want to.
 And that way, I can still write "logger.warningf" without 
 "if" everywhere.


 But then it's better to provide no logger (or at least no 
 logger for level warning) than an artificial NullLogger.


 My need is not "artificial", at least in my view.
 Your opinion is different from mine. That's fine.
 That's why we need someone to try to reconcile the many, many 
 opinions about this.


 Not the need is artificial. For example, I have the need to 
 measure the performance of an application with and without 
 logging.

 I think, the solution is artificial. The obvious solution would 
 be to register no logger at all.


This only works if you have a global set of registered loggers 
(like your solution 
https://github.com/linkrope/log/blob/master/src/log.d#L133).
The obvious flaw is that prevents to have a different logging 
sink for different parts of a program. Now is it ever needed? 
Yes, for topical logging (eg. "I want only log messages from 
subsystem A", again real use case).

std.logger use the Composite pattern 
http://en.wikipedia.org/wiki/Composite_pattern instead to allow 
for a hierarchy.



 Jul 22 2014



prev sibling next sibling parent  "Dicebot" <public dicebot.lv>  writes:


Initial review period is closing soon. I will make a summary and 
start a voting thread this weekend.



 Jul 24 2014



prev sibling next sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org>  writes:


On 7/11/14, 7:36 AM, 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.


Preface: (1) We already use the std.logger candidate at Facebook; (2) I 
really want to get this in, it's been past time already.

Overall: I think std.logger has a passable overall design having 
simplicity and no-nonsense as compelling advantages. However, there's 
some serious tactical work needed.

0. There's no way to set the minimal logging level statically, except 
for an on/off switch. There must be a way to define e.g. 
-version=logLevel=trace that sets the minimum logging level actually 
performed. Any logging below that level is a no-op. Which segues into 
the next problem:

1. There's a bunch of code still generated even with logging statically 
disabled (per http://d.godbolt.org). I could not find a way to 
completely eliminate generated code. Unused lazy parameters MUST 
generate no code at the front end level. This is a performance bug in 
the D front end that blocks acceptance of this proposal.

2. The three one-letter suffixes don't seem to scale well. They optimize 
for brevity but at the cost of clarity. Also they establish a precedent 
- are we sure we recommend future D code in the standard library and 
beyond to mangle names using one-letter conventions? I think we must 
find a way to solve things via overloads.

Currently we have: log, logc, logf, logl, logcf, loglf, loglc, loglcf. 
One would almost expect logwtf to be somewhere in there.

I think an overloading-based solution would be a better choice here. 
First, I think the logging level must guide all overloads. True, we'd 
sometimes like to log the logging level itself, but that's easily done 
by using a formatting function (as shown below). So:

log(A...)(lazy A stuff); // just log whatevs
log(A...)(LogLevel lvl, lazy A stuff); // log at specified level
log(A...)(bool c, LogLevel lvl, lazy A stuff); // conditionally log at 
specified level

Then three more logf with similar signatures, using the precedent set by 
write/writef. And we're done.

To log the logging level itself, just use logf(someLevel, "%s", 
someLevel) and be done with it.

There's no conditional logging without specifying a level, which should 
be fine seeing as conditional logging is not that frequent to start with.

There should be some shortcuts for logging levels such that one can 
write log(info, "crap") instead of log(LogLevel.info, "crap").

3. I'm not sure I like the design using defaultLogger in conjunction 
with free functions using it. It clearly makes for comfortable casual 
logging by just calling log(whatevs) and it uses a precedent set by 
stdout. But I wonder if it would be cleaner to just give it a shorter 
name "log" and then have it have a "write" method:

log("crap");
-> becomes ->
log.write("crap");

Also there'd be log.writef("%s", "crap") etc.

4. Conditional logging must be justified. In my mind the justification 
is that statically setting the log level makes the code disappear 
without the condition being ever evaluated, but the current design 
doesn't allow setting the log level!

5. There was some nice stuff in the previous std.logger work by me and 
later Jose (I think), which allowed logging every n times/milliseconds 
so as to allow nice throttling. That's nice to omit/defer for 
simplification purposes, but I noticed that log noise is a serious matter.

6. The current backend design requires use of classes and references, 
i.e. garbage collection. Amid the current tendency to make std work 
without requiring GC, I think a design based on RefCounted would be 
recommended here.

7. The current backend design fills a struct with data then passes it to 
the implementation. But if the implementation doesn't use e.g. the 
timestamp then that work has been wasted. Maybe offer the fields as 
properties instead, with caching upon first use?

8. Documentation needs work as it has disfluencies and typos.

9. I've also posted a bunch of comments to the code at 
https://github.com/D-Programming-Language/phobos/pull/1500/files


Andrei



 Jul 24 2014



 next sibling parent  "Dicebot" <public dicebot.lv>  writes:


On Thursday, 24 July 2014 at 18:51:03 UTC, Andrei Alexandrescu 
wrote:

 3. I'm not sure I like the design using defaultLogger in 
 conjunction with free functions using it. It clearly makes for 
 comfortable casual logging by just calling log(whatevs) and it 
 uses a precedent set by stdout. But I wonder if it would be 
 cleaner to just give it a shorter name "log" and then have it 
 have a "write" method:

 log("crap");
 -> becomes ->
 log.write("crap");

 Also there'd be log.writef("%s", "crap") etc.


It is amusing to see that you don't like D module system too :)

(and no, "write" it is not cleaner at all being different from 
pretty much any logging system out there)



 Jul 24 2014



prev sibling next sibling parent reply "Robert burner Schadek" <rburners gmail.com>  writes:


On Thursday, 24 July 2014 at 18:51:03 UTC, Andrei Alexandrescu 
wrote:

 0. There's no way to set the minimal logging level statically, 
 except for an on/off switch. There must be a way to define e.g. 
 -version=logLevel=trace that sets the minimum logging level 
 actually performed. Any logging below that level is a no-op. 
 Which segues into the next problem:


I'm currently working on this.


 1. There's a bunch of code still generated even with logging 
 statically disabled (per http://d.godbolt.org). I could not 
 find a way to completely eliminate generated code. Unused lazy 
 parameters MUST generate no code at the front end level. This 
 is a performance bug in the D front end that blocks acceptance 
 of this proposal.


that is part of 0. work


 2. The three one-letter suffixes don't seem to scale well. They 
 optimize for brevity but at the cost of clarity. Also they 
 establish a precedent - are we sure we recommend future D code 
 in the standard library and beyond to mangle names using 
 one-letter conventions? I think we must find a way to solve 
 things via overloads.

 Currently we have: log, logc, logf, logl, logcf, loglf, loglc, 
 loglcf. One would almost expect logwtf to be somewhere in there.

 I think an overloading-based solution would be a better choice 
 here. First, I think the logging level must guide all 
 overloads. True, we'd sometimes like to log the logging level 
 itself, but that's easily done by using a formatting function 
 (as shown below). So:

 log(A...)(lazy A stuff); // just log whatevs
 log(A...)(LogLevel lvl, lazy A stuff); // log at specified level
 log(A...)(bool c, LogLevel lvl, lazy A stuff); // conditionally 
 log at specified level


log(A...)(lazy A stuff) matches the next two sigs as well. I 
tried that.
Maybe some overloading inside the function body may work, but 
that is just a mess IMo.


 Then three more logf with similar signatures, using the 
 precedent set by write/writef. And we're done.

 To log the logging level itself, just use logf(someLevel, "%s", 
 someLevel) and be done with it.

 There's no conditional logging without specifying a level, 
 which should be fine seeing as conditional logging is not that 
 frequent to start with.


there is. tracec, infoc, ....


 There should be some shortcuts for logging levels such that one 
 can write log(info, "crap") instead of log(LogLevel.info, 
 "crap").



there is trace%s, info%s, warning%s .... c|f


 3. I'm not sure I like the design using defaultLogger in 
 conjunction with free functions using it. It clearly makes for 
 comfortable casual logging by just calling log(whatevs) and it 
 uses a precedent set by stdout. But I wonder if it would be 
 cleaner to just give it a shorter name "log" and then have it 
 have a "write" method:

 log("crap");
 -> becomes ->
 log.write("crap");

 Also there'd be log.writef("%s", "crap") etc.


well, this is by design. I wanted to provide very easy simple 
looging for hacking a small script. If you want more, you 
properly want to handle Loggers as variables.


 4. Conditional logging must be justified. In my mind the 
 justification is that statically setting the log level makes 
 the code disappear without the condition being ever evaluated, 
 but the current design doesn't allow setting the log level!

 5. There was some nice stuff in the previous std.logger work by 
 me and later Jose (I think), which allowed logging every n 
 times/milliseconds so as to allow nice throttling. That's nice 
 to omit/defer for simplification purposes, but I noticed that 
 log noise is a serious matter.


I could start std.logger.condition


 6. The current backend design requires use of classes and 
 references, i.e. garbage collection. Amid the current tendency 
 to make std work without requiring GC, I think a design based 
 on RefCounted would be recommended here.


Maybe I'm wrong, but RefCounted does not support polymorphism and 
that would break not only the MultiLogger and the defaultLogger. 
I think this is a legitimate use of classes, as Logger properly 
stay alive the complete run of the program.


 7. The current backend design fills a struct with data then 
 passes it to the implementation. But if the implementation 
 doesn't use e.g. the timestamp then that work has been wasted. 
 Maybe offer the fields as properties instead, with caching upon 
 first use?


hm, but taking the timestamp after the log call seams wrong. 
Again, I think this is by design from using polymorphism.


 8. Documentation needs work as it has disfluencies and typos.


If have already worked in all of JakovOvrum and you fixes.


 9. I've also posted a bunch of comments to the code at 
 https://github.com/D-Programming-Language/phobos/pull/1500/files


 Andrei





 Jul 24 2014



  parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org>  writes:


On 7/24/14, 12:23 PM, Robert burner Schadek wrote:

 On Thursday, 24 July 2014 at 18:51:03 UTC, Andrei Alexandrescu wrote:

 log(A...)(lazy A stuff); // just log whatevs
 log(A...)(LogLevel lvl, lazy A stuff); // log at specified level
 log(A...)(bool c, LogLevel lvl, lazy A stuff); // conditionally log at
 specified level


 log(A...)(lazy A stuff) matches the next two sigs as well. I tried that.
 Maybe some overloading inside the function body may work, but that is
 just a mess IMo.


Use template constraints.

I realized after posting that the log level should also be a static 
parameter so the logging framework can choose to disable it entirely:

log!Loglevel.info("crap");

But wait there are the explicit functions trace, error etc. so... those 
do implicitly choose the level statically. Noice.


 There's no conditional logging without specifying a level, which
 should be fine seeing as conditional logging is not that frequent to
 start with.


 there is. tracec, infoc, ....


I meant in the proposed design.


 There should be some shortcuts for logging levels such that one can
 write log(info, "crap") instead of log(LogLevel.info, "crap").



 there is trace%s, info%s, warning%s .... c|f


Okay.


 3. I'm not sure I like the design using defaultLogger in conjunction
 with free functions using it. It clearly makes for comfortable casual
 logging by just calling log(whatevs) and it uses a precedent set by
 stdout. But I wonder if it would be cleaner to just give it a shorter
 name "log" and then have it have a "write" method:

 log("crap");
 -> becomes ->
 log.write("crap");

 Also there'd be log.writef("%s", "crap") etc.


 well, this is by design. I wanted to provide very easy simple looging
 for hacking a small script. If you want more, you properly want to
 handle Loggers as variables.


I think I'm fine with that after all.


 5. There was some nice stuff in the previous std.logger work by me and
 later Jose (I think), which allowed logging every n times/milliseconds
 so as to allow nice throttling. That's nice to omit/defer for
 simplification purposes, but I noticed that log noise is a serious
 matter.


 I could start std.logger.condition


Thing is you must make sure you integrate with statically setting the 
logging level. Throttling is _especially_ used/useful in intensive 
loops. Being able to rebuild a large app with a different logging level 
to debug a pernicious condition at the cost of some speed is pretty 
awesome. It also makes verbose logging psychologically "free" the same 
way assert is.

In fact you may want to define an extra logging level e.g. "verbose" or 
"yap" which is by default disabled and can be enabled explicitly. It 
would be hierarchically below normal logging.


 6. The current backend design requires use of classes and references,
 i.e. garbage collection. Amid the current tendency to make std work
 without requiring GC, I think a design based on RefCounted would be
 recommended here.


 Maybe I'm wrong, but RefCounted does not support polymorphism and that
 would break not only the MultiLogger and the defaultLogger. I think this
 is a legitimate use of classes, as Logger properly stay alive the
 complete run of the program.


The use of polymorphism is legit. RefCounted either works with classes 
or must be made to work with classes.


 7. The current backend design fills a struct with data then passes it
 to the implementation. But if the implementation doesn't use e.g. the
 timestamp then that work has been wasted. Maybe offer the fields as
 properties instead, with caching upon first use?


 hm, but taking the timestamp after the log call seams wrong. Again, I
 think this is by design from using polymorphism.


Yah, it'd be a tad later but I guess it shouldn't be a large problem. 
Same goes about the thread id.


 8. Documentation needs work as it has disfluencies and typos.


 If have already worked in all of JakovOvrum and you fixes.


Cool, thanks.



Andrei



 Jul 24 2014



  parent  Jeremy Powers via Digitalmars-d <digitalmars-d puremagic.com>  writes:


 In fact you may want to define an extra logging level e.g. "verbose" or
 "yap" which is by default disabled and can be enabled explicitly. It would
 be hierarchically below normal logging.



Isn't this what 'trace' level is for?



 Jul 24 2014



prev sibling next sibling parent  Johannes Pfau <nospam example.com>  writes:


Am Thu, 24 Jul 2014 11:50:54 -0700
schrieb Andrei Alexandrescu <SeeWebsiteForEmail erdani.org>:


 6. The current backend design requires use of classes and references, 
 i.e. garbage collection. Amid the current tendency to make std work 
 without requiring GC, I think a design based on RefCounted would be 
 recommended here.


Classes could be backed by other allocators, but this requires some
thought. For example a 'dispose' like method could be added. Or maybe
RefCounted works with classes? But IIRC RefCounted actually uses the GC
for some allocations, it only provides deterministic destruction.

I think the bigger problem is string formatting, which currently
produces one garbage collected string for every log call. Has this been
fixed in the mean time?

It can be fixed but it requires changes in the backend class
interface. I even opened a pull request to fix it. As long as this is
not fixed I can't vote for this proposal, I think it's a serious issue.



 Jul 25 2014



prev sibling  parent  "Paolo Invernizzi" <paolo.invernizzi no.address>  writes:


On Thursday, 24 July 2014 at 18:51:03 UTC, Andrei Alexandrescu 
wrote:

 0. There's no way to set the minimal logging level statically, 
 except for an on/off switch. There must be a way to define e.g. 
 -version=logLevel=trace that sets the minimum logging level 
 actually performed. Any logging below that level is a no-op. 
 Which segues into the next problem:


Internally in SRLabs we are using a logging library with static 
logging level and static tagging of the log lines, something like:

   logdbg!(GL, SUR, REND, DBG_AA)("initialising the surface and 
the texture data");

Where 'logdbg' is just an helper for having the right log level 
template params.
The tags are defined and reserved in a common module in a tuple, 
and commented/decommented by the single developer by needs (well, 
actually we are mixing an external not committed file):

   ...
   CAVS,  /// cavs machinery.
   //GL,      /// flavour OpenGL.
   //PAR,    /// descriptors parser.
   REND, /// global renderer threaded loop.
   //SUR,    /// generic mixed library HL surface handlers.

   // ... reserved to developers ...
   //DBG_AA, /// Aaron is interested in that for his debugging 
purpose.
   DBG_PI,  /// Paolo is interested in that for his debugging 
purpose.


So, we have a very nice granularity and a compile-time 
guillotine, and we can cherry-pick log emissions in a very 
pragmatic way in different context (bugs hunting, production, 
test environ, etc).

The tag approach is strange, and for what I know, it's something 
that I've never seen around, but everyone here is liking it a lot.


 1. There's a bunch of code still generated even with logging 
 statically disabled (per http://d.godbolt.org). I could not 
 find a way to completely eliminate generated code. Unused lazy 
 parameters MUST generate no code at the front end level. This 
 is a performance bug in the D front end that blocks acceptance 
 of this proposal.


That's the big problem: we were able to completely obliterated 
log lines that don't use lazy parameters, but the lazy is just 
unavoidable in a log library: I'm very interested in a solution 
for this issue.

---
Paolo



 Jul 26 2014



prev sibling  parent reply "Jakob Ovrum" <jakobovrum gmail.com>  writes:


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.


First I want to say that I want this to be *the* logging library, 
just as I always want a D library to be the best library in the 
world at what it does, as I believe D allows us to do that. I'm 
confident std.logger can become that, so thank you Robert for 
your work.

As for using class-based polymorphism; this doesn't have to mean 
GC memory. The only part of the library code that should have a 
need to allocate a logger class is the lazy initialization in 
`defaultLogger`. If we change this to allocate in global memory, 
then it's entirely up to the user how to allocate logger 
instances. Giving up classes here because of GC means giving up 
classes in no-GC code entirely, which I think is a serious 
overreaction. I think there are reasons to question the 
class-based polymorphism, on grounds such as - do we require that 
`writeLogMsg` is  nogc so we can log in  nogc code? What about 
nothrow? When it comes to performance and the indirect call 
involved, I don't think there's a way around that.

When it comes to GC memory, MultiLogger using an associative 
array is a blocker. However, I don't think it can be elegantly 
fixed as we don't have pluggable allocators for any of our 
standard containers yet. Maybe using an array (perhaps sorted) is 
an acceptable compromise measure if  nogc is deemed a priority.

One thing that really bugs me though, is that `Logger` is an 
abstract class and not an interface. An abstract class should 
only be needed when it has both pure virtual functions as well as 
default functionality that can be conditionally overridden, so 
`Logger` should not be a candidate. It should be rewritten in 
terms of an interface, which enables users to implement logger 
functionality in any of their classes, instead of having to 
dedicate a new class type just to override one virtual function.

I much prefer overloading over the mess of suffixes for the same 
reasons Andrei mentioned.

The library's stance on thread safety needs to be clearly 
defined. Currently, `defaultLogger` is process-wide, which can 
only mean logger instances must be thread-safe. Yet there is no 
mention in the documentation that loggers must be thread-safe, 
and indeed I think most of the default-provided loggers are 
written with no concern for thread safety.

I suggest one of two approaches: 1) make `defaultLogger` TLS and 
rework the documentation so it's clear that each thread must 
manage their own logger, or 2) make it clear that `defaultLogger` 
must be thread-safe, and take extreme care in the 
default-provided loggers that they are indeed thread-safe. Maybe 
a templated base logger class `LockingLogger` or something could 
help here.

The documentation needs a lot of work, but I think anyone can 
help with that. I intend to file a pull request to Robert's fork 
with fixes I could spot; it seems more efficient for both of us 
than posting an endless stream of line comments.



 Jul 24 2014



  parent reply "Robert burner Schadek" <rburners gmail.com>  writes:


On Thursday, 24 July 2014 at 22:27:34 UTC, Jakob Ovrum 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.


 First I want to say that I want this to be *the* logging 
 library, just as I always want a D library to be the best 
 library in the world at what it does, as I believe D allows us 
 to do that. I'm confident std.logger can become that, so thank 
 you Robert for your work.


That is music in my ears...


 As for using class-based polymorphism; this doesn't have to 
 mean GC memory. The only part of the library code that should 
 have a need to allocate a logger class is the lazy 
 initialization in `defaultLogger`. If we change this to 
 allocate in global memory, then it's entirely up to the user 
 how to allocate logger instances. Giving up classes here 
 because of GC means giving up classes in no-GC code entirely, 
 which I think is a serious overreaction. I think there are 
 reasons to question the class-based polymorphism, on grounds 
 such as - do we require that `writeLogMsg` is  nogc so we can 
 log in  nogc code? What about nothrow? When it comes to 
 performance and the indirect call involved, I don't think 
 there's a way around that.


I do this lazily in a function, because having it global froze 
std.concurrency and std.process unittest. I couldn't figure out 
why.
As said earlier, I think GC and Logger is a none issue. I mean 
how often has anyone seen a Logger created in a loop over and 
over again.
nothrow will be hard as std.logger uses format, same for nogc


 When it comes to GC memory, MultiLogger using an associative 
 array is a blocker. However, I don't think it can be elegantly 
 fixed as we don't have pluggable allocators for any of our 
 standard containers yet. Maybe using an array (perhaps sorted) 
 is an acceptable compromise measure if  nogc is deemed a 
 priority.


So you're thinking of a stack array?


 One thing that really bugs me though, is that `Logger` is an 
 abstract class and not an interface. An abstract class should 
 only be needed when it has both pure virtual functions as well 
 as default functionality that can be conditionally overridden, 
 so `Logger` should not be a candidate. It should be rewritten 
 in terms of an interface, which enables users to implement 
 logger functionality in any of their classes, instead of having 
 to dedicate a new class type just to override one virtual 
 function.


What about the log functions and there implementation as well as 
the Logger specific LogLevel and name?

 I much prefer overloading over the mess of suffixes for the 
 same reasons Andrei mentioned.


I'm working on that.


 The library's stance on thread safety needs to be clearly 
 defined. Currently, `defaultLogger` is process-wide, which can 
 only mean logger instances must be thread-safe. Yet there is no 
 mention in the documentation that loggers must be thread-safe, 
 and indeed I think most of the default-provided loggers are 
 written with no concern for thread safety.

 I suggest one of two approaches: 1) make `defaultLogger` TLS 
 and rework the documentation so it's clear that each thread 
 must manage their own logger, or 2) make it clear that 
 `defaultLogger` must be thread-safe, and take extreme care in 
 the default-provided loggers that they are indeed thread-safe. 
 Maybe a templated base logger class `LockingLogger` or 
 something could help here.


yeah, apart from StdIOLogger everything is thread unsafe. I like 
the template base class LockingLogger idea. I will see what I can 
do.


 The documentation needs a lot of work, but I think anyone can 
 help with that. I intend to file a pull request to Robert's 
 fork with fixes I could spot; it seems more efficient for both 
 of us than posting an endless stream of line comments.


+1



 Jul 24 2014



  parent reply "Jakob Ovrum" <jakobovrum gmail.com>  writes:


On Thursday, 24 July 2014 at 23:01:56 UTC, Robert burner Schadek 
wrote:

 I do this lazily in a function, because having it global froze 
 std.concurrency and std.process unittest. I couldn't figure out 
 why.


It could still be initialized lazily, using `emplace`, ala

---
private static __gshared Logger _defaultLogger;

Logger defaultLogger()  safe  nogc
{
     static __gshared ubyte[__traits(classInstanceSize, 
StdoutLogger)] buffer;

     if(!_defaultLogger) // TODO: thread safety
         _defaultLogger = ()  trusted { return 
emplace!StdoutLogger(buffer); }();

     return _defaultLogger;
}

void defaultLogger(Logger newDefaultLogger)  safe  nogc
{
     _defaultLogger = newDefaultLogger;
}
---


 As said earlier, I think GC and Logger is a none issue. I mean 
 how often has anyone seen a Logger created in a loop over and 
 over again.


Some programs want to forego having a GC-heap entirely. That 
means any GC allocation is a no-go.

Class instances don't have to be GC-allocated, so it's not an 
issue to use classes.


 nothrow will be hard as std.logger uses format, same for nogc


How often have you seen a formatted log message logged in a loop? 
I'd wager that happens quite often. Using `format` is a no-go as 
it completely thrashes the GC, it needs to use `formattedWrite` 
to write directly to the underlying buffer (such as a file).

Even using `formattedWrite` though, `nothrow` is still a problem, 
and since exceptions are still GC-allocated, it doesn't help with 
 nogc either. The latter is something we can fix in the future 
though.


 So you're thinking of a stack array?


No, MultiLogger could manage a non-GC yet still heap-allocated 
array using std.container.array.Array. It uses the C heap 
internally, i.e. malloc, realloc and free. Sortedness can be used 
for searching by name in logarithmic time if desired.


 What about the log functions and there implementation as well 
 as the Logger specific LogLevel and name?


The log functions don't need to be virtual, only the 
`writeLogMsg` function does, so these implementations can either 
be final member functions of the interface, or UFCS functions as 
I suggested in the corresponding line comment.

The Logger-specific LogLevel and the name do not have to be 
implemented by Logger. Leave it to concrete classes to implement 
those. As an internal aid, an abstract GenericLogger base class 
that manages these properties as member variables (as Logger 
currently does) can help.



 Jul 24 2014



 next sibling parent reply "Jakob Ovrum" <jakobovrum gmail.com>  writes:


On Thursday, 24 July 2014 at 23:40:56 UTC, Jakob Ovrum wrote:

 How often have you seen a formatted log message logged in a 
 loop? I'd wager that happens quite often. Using `format` is a 
 no-go as it completely thrashes the GC, it needs to use 
 `formattedWrite` to write directly to the underlying buffer 
 (such as a file).


To eloborate on this: using `format` like std.logger currently 
does, for short-lived strings that are consumed and discared 
almost immediately in a higher stack frame, is extremely 
inefficient as every string needs at least one heap memory 
allocation (though `format` can easily do *multiple* in one 
call). It's a good way to quickly thrash and fragment the GC 
heap, putting an extreme amount of stress on the collector.

Even C has fprintf. If we don't provide efficient formatted 
writing, people will not use our abstractions.

The solution is to use `formattedWrite` for custom allocation 
behaviour. When I've used `formattedWrite` for this kind of 
problem before, it has generally come down to the following 
patterns; let's assume the underlying sink is a file:

  * One solution is to use `formattedWrite` to write to the file 
directly. Of course, Logger doesn't provide an interface for 
writing partial log messages, or writing log messages in chunks, 
so this would require modifying the basic API. Also, 
`formattedWrite` doesn't guarantee any minimum chunk size, so in 
the worst case, it might result in one write operation per 
character, which is very inefficient.

* Another solution is to use `formattedWrite` to write to a 
stack-allocated character buffer, then subsequently pass it to 
`writeLogMsg`. This is a leaky abstraction because it puts an 
arbitrary upper limit on how long log entries can be. A practical 
compromise is to revert to a heap allocation for entries that 
don't fit in the stack buffer, but we can do better;

* The best solution is a hybrid one. Use `formattedWrite` to 
write to a stack-allocated buffer, then whenever it's full, write 
the contents of the buffer to the underlying sink (this is easily 
doable by passing a delegate to `formattedWrite` that sees the 
stack buffer as an upvalue). Yes, this does require modifying the 
basic logger API to support partial writes, but it gives us 
optimal performance.

The last solution gives us no dynamic memory allocations unless 
an exception is thrown, while still minimizing the number of 
writes to the underlying sink, which is important when writes can 
be expensive, such as writes to a file or a socket.



 Jul 24 2014



  parent  Johannes Pfau <nospam example.com>  writes:


Am Fri, 25 Jul 2014 01:23:21 +0000
schrieb "Jakob Ovrum" <jakobovrum gmail.com>:


 On Thursday, 24 July 2014 at 23:40:56 UTC, Jakob Ovrum wrote:

 How often have you seen a formatted log message logged in a 
 loop? I'd wager that happens quite often. Using `format` is a 
 no-go as it completely thrashes the GC, it needs to use 
 `formattedWrite` to write directly to the underlying buffer 
 (such as a file).


 
 To eloborate on this: using `format` like std.logger currently 
 does, for short-lived strings that are consumed and discared 
 almost immediately in a higher stack frame, is extremely 
 inefficient as every string needs at least one heap memory 
 allocation (though `format` can easily do *multiple* in one 
 call). It's a good way to quickly thrash and fragment the GC 
 heap, putting an extreme amount of stress on the collector.
 
 Even C has fprintf. If we don't provide efficient formatted 
 writing, people will not use our abstractions.
 
 The solution is to use `formattedWrite` for custom allocation 
 behaviour. When I've used `formattedWrite` for this kind of 
 problem before, it has generally come down to the following 
 patterns; let's assume the underlying sink is a file:
 
   * One solution is to use `formattedWrite` to write to the file 
 directly. Of course, Logger doesn't provide an interface for 
 writing partial log messages, or writing log messages in chunks, 
 so this would require modifying the basic API. Also, 
 `formattedWrite` doesn't guarantee any minimum chunk size, so in 
 the worst case, it might result in one write operation per 
 character, which is very inefficient.
 
 * Another solution is to use `formattedWrite` to write to a 
 stack-allocated character buffer, then subsequently pass it to 
 `writeLogMsg`. This is a leaky abstraction because it puts an 
 arbitrary upper limit on how long log entries can be. A practical 
 compromise is to revert to a heap allocation for entries that 
 don't fit in the stack buffer, but we can do better;
 
 * The best solution is a hybrid one. Use `formattedWrite` to 
 write to a stack-allocated buffer, then whenever it's full, write 
 the contents of the buffer to the underlying sink (this is easily 
 doable by passing a delegate to `formattedWrite` that sees the 
 stack buffer as an upvalue). Yes, this does require modifying the 
 basic logger API to support partial writes, but it gives us 
 optimal performance.
 
 The last solution gives us no dynamic memory allocations unless 
 an exception is thrown, while still minimizing the number of 
 writes to the underlying sink, which is important when writes can 
 be expensive, such as writes to a file or a socket.


https://github.com/burner/logger/pull/9



 Jul 25 2014



prev sibling  parent  "Robert burner Schadek" <rburners gmail.com>  writes:


On Thursday, 24 July 2014 at 23:40:56 UTC, Jakob Ovrum wrote:

 On Thursday, 24 July 2014 at 23:01:56 UTC, Robert burner 
 Schadek wrote:

 I do this lazily in a function, because having it global froze 
 std.concurrency and std.process unittest. I couldn't figure 
 out why.


 It could still be initialized lazily, using `emplace`, ala

 ---
 private static __gshared Logger _defaultLogger;

 Logger defaultLogger()  safe  nogc
 {
     static __gshared ubyte[__traits(classInstanceSize, 
 StdoutLogger)] buffer;

     if(!_defaultLogger) // TODO: thread safety
         _defaultLogger = ()  trusted { return 
 emplace!StdoutLogger(buffer); }();

     return _defaultLogger;
 }

 void defaultLogger(Logger newDefaultLogger)  safe  nogc
 {
     _defaultLogger = newDefaultLogger;
 }
 ---


 As said earlier, I think GC and Logger is a none issue. I mean 
 how often has anyone seen a Logger created in a loop over and 
 over again.


 Some programs want to forego having a GC-heap entirely. That 
 means any GC allocation is a no-go.

 Class instances don't have to be GC-allocated, so it's not an 
 issue to use classes.


 nothrow will be hard as std.logger uses format, same for nogc


 How often have you seen a formatted log message logged in a 
 loop? I'd wager that happens quite often. Using `format` is a 
 no-go as it completely thrashes the GC, it needs to use 
 `formattedWrite` to write directly to the underlying buffer 
 (such as a file).

 Even using `formattedWrite` though, `nothrow` is still a 
 problem, and since exceptions are still GC-allocated, it 
 doesn't help with  nogc either. The latter is something we can 
 fix in the future though.


 So you're thinking of a stack array?


 No, MultiLogger could manage a non-GC yet still heap-allocated 
 array using std.container.array.Array. It uses the C heap 
 internally, i.e. malloc, realloc and free. Sortedness can be 
 used for searching by name in logarithmic time if desired.


Andrei asked for a simple array like multilogger. If it uses an 
array sorting can be toggled by an template parameter


 What about the log functions and there implementation as well 
 as the Logger specific LogLevel and name?


 The log functions don't need to be virtual, only the 
 `writeLogMsg` function does, so these implementations can 
 either be final member functions of the interface, or UFCS 
 functions as I suggested in the corresponding line comment.


all log functions are templates.


 The Logger-specific LogLevel and the name do not have to be 
 implemented by Logger. Leave it to concrete classes to 
 implement those. As an internal aid, an abstract GenericLogger 
 base class that manages these properties as member variables 
 (as Logger currently does) can help.


that seams manageable



 Jul 25 2014