October 21, 2013
On Thursday, 17 October 2013 at 07:34:28 UTC, qznc wrote:
> On Thursday, 17 October 2013 at 02:13:12 UTC, Eric Anderton wrote:
>> The strength of this is that it would allow us to freely integrate D libraries that use std.logger, yet filter their log output from *outside* the library through the std.logger API.
>
> This is one of the most important aspects in my opinion.
> Std.logger should be easy to use, so library writers are
> encouraged to use it. Compare this with the "unittest" keyword,
> which makes it easy to write some simple tests. Of course,
> flexibility to use complex machinery for using the messages/tests
> is necessary. Just like we can hook up more advanced unit testing
> frameworks, we should be able to hook up more advanced logging
> machinery. The advanced stuff is not for Phobos though. Advanced
> stuff for unittests is for example, parallel execution and
> graphical reports. Advanced stuff for logging is for example log
> rotation and networking.

There is no contradiction. Complex log libraries become (relatively) complex when one wants to use their advanced features, but are as simple as the others when one wants to use them simply.
That's why in the Java world nearly everyone uses Log4j instead of the official JEE API.

In practive, you really want a powerful logging facility.
Another feature I used once in a project, was to log to RAM. We decided to log TRACE logs in production in order to catch a rare bug, but of course, it would output too many logs. So we decided to log everything in RAM and keep the last 1000 logs. Whenever there would be an exception or a CRITICAL log, the whole 1000 logs would be dumped on disk. That feature proved very useful.
October 21, 2013
On Monday, 21 October 2013 at 06:27:39 UTC, SomeDude wrote:
> On Thursday, 17 October 2013 at 07:34:28 UTC, qznc wrote:
>> On Thursday, 17 October 2013 at 02:13:12 UTC, Eric Anderton wrote:
>>> The strength of this is that it would allow us to freely integrate D libraries that use std.logger, yet filter their log output from *outside* the library through the std.logger API.
>>
>> This is one of the most important aspects in my opinion.
>> Std.logger should be easy to use, so library writers are
>> encouraged to use it. Compare this with the "unittest" keyword,
>> which makes it easy to write some simple tests. Of course,
>> flexibility to use complex machinery for using the messages/tests
>> is necessary. Just like we can hook up more advanced unit testing
>> frameworks, we should be able to hook up more advanced logging
>> machinery. The advanced stuff is not for Phobos though. Advanced
>> stuff for unittests is for example, parallel execution and
>> graphical reports. Advanced stuff for logging is for example log
>> rotation and networking.
>
> There is no contradiction. Complex log libraries become (relatively) complex when one wants to use their advanced features, but are as simple as the others when one wants to use them simply.
> That's why in the Java world nearly everyone uses Log4j instead of the official JEE API.
>
> In practive, you really want a powerful logging facility.
> Another feature I used once in a project, was to log to RAM. We decided to log TRACE logs in production in order to catch a rare bug, but of course, it would output too many logs. So we decided to log everything in RAM and keep the last 1000 logs. Whenever there would be an exception or a CRITICAL log, the whole 1000 logs would be dumped on disk. That feature proved very useful.

+1
Also, it would be useful to use buffered asynchronous logger (mentioned above).
It will help to trace and save all logs, but should be fast for production usage.
October 21, 2013
On 10/21/2013 06:19 AM, SomeDude wrote:
> On Tuesday, 15 October 2013 at 08:47:00 UTC, Robert Schadek wrote:
>> On 10/15/2013 09:32 AM, Sönke Ludwig wrote:
>>> Am 15.10.2013 09:08, schrieb Jacob Carlborg:
>>>> On 2013-10-14 23:22, Dicebot wrote:
>>>>
>>>>> If we need to care about that, D module system is a failure. But I don't think it is a valid concern.
>>>>
>>>> People already complain about conflict function names in Phobos.
>>>>
>>>
>>> And I'd agree with them. At least inside of a library, care IMO should be taken to minimize overlap (of course functionally equivalent ones in different overload sets are fine, though). But in case of "logXXX" this seems to be very unlikely, much in contrast to "log" (std.math.log).
>> yes and no. Of course does logXXX create less conflict, but I like to simply write log and don't care about the LogLevel. So again pros and cons
>
> I for once have never seen any log API with
> log.level = INFO;
> Logger.log("Here be dragons");
>
> And this I believe for a good reason: in 99% of production code I've seen, several log levels are mixed, i.e INFO, CRITICAL and DEBUG for instance, so the case where a single log level is used, even in the same method, just never happens. The proposed solution looks extremely inconvenient to me as it will almost always necessit two lines of code instead of one.
How good than, that you can pass the LogLevel as first argument to the log function.
>
> I am with Sönke on this one, as well as the need for multi logger output. That's the absolute minimum requirement. If this doesn't exist, what will happen is, someone will make something better.
I added a MultiLogger five days ago...
October 21, 2013
On Monday, 21 October 2013 at 08:37:43 UTC, Robert Schadek wrote:
>> I for once have never seen any log API with
>> log.level = INFO;
>> Logger.log("Here be dragons");
>>
>> And this I believe for a good reason: in 99% of production code I've
>> seen, several log levels are mixed, i.e INFO, CRITICAL and DEBUG for
>> instance, so the case where a single log level is used, even in the
>> same method, just never happens. The proposed solution looks extremely
>> inconvenient to me as it will almost always necessit two lines of code
>> instead of one.
> How good than, that you can pass the LogLevel as first argument to the
> log function.

We repeat many times: you can add
Logger.logInfo("Here be dragons");
syntax. It's much better than
Logger.log(LogLevel.Info, "Here be dragons");
October 21, 2013
On 10/21/2013 08:27 AM, SomeDude wrote:
>
> In practive, you really want a powerful logging facility.
> Another feature I used once in a project, was to log to RAM. We
> decided to log TRACE logs in production in order to catch a rare bug,
> but of course, it would output too many logs. So we decided to log
> everything in RAM and keep the last 1000 logs. Whenever there would be
> an exception or a CRITICAL log, the whole 1000 logs would be dumped on
> disk. That feature proved very useful.
That was a feature you added or was it part of the logging library?
October 21, 2013
On Mon, 14 Oct 2013 13:39:51 +0200, Dicebot wrote:

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

std.logger is a good start, but I must admit I do not like the way it is architectured. I use Java's Logging and Log4J for years so naturally I compare std.logger with these two frameworks (that are heavily used in production).

std.logger is a bad name, IMHO - it should be std.logging as there will be many Logger implementations there I presume...
October 21, 2013
On 10/21/2013 10:19 PM, Dejan Lekic wrote:
> On Mon, 14 Oct 2013 13:39:51 +0200, Dicebot wrote:
>
>> As `std.logger` is still marked as "work in progress" this thread is less formal that typical pre-voting review. Goal is to provide as much input about desirable `std.logger` functionality and current state and let module author to use this information for preparing proposal for actual review / voting.
>>
>> Lets unleash the forces of constructive destruction.
>>
>> ===== Meta =====
>>
>> Code: https://github.com/D-Programming-Language/phobos/pull/1500/files Docs: http://burner.github.io/phobos/phobos-prerelease/std_logger.html
>>
>> First announcement / discussion thread : http://forum.dlang.org/post/mailman.313.1377180809.1719.digitalmars-
> d@puremagic.com
>
> std.logger is a good start, but I must admit I do not like the way it is architectured. I use Java's Logging and Log4J for years so naturally I compare std.logger with these two frameworks (that are heavily used in production).
unfamiliar or bad (details please).

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

Ok, finally making some conclusions.

===================
As a review manager
===================

This what needs to be done before moving to next review stage:

1.

Judging by review comments it is clear that some more efforts need to be put into demonstrating how existing platform can be used as a standard API for more complex logging functionality. Addition of `MultiLogger` is good thing here, but providing either separate article on topic or extra module that demonstrates enhancing existing system feels like a good way to avoid confusion.

That said, I do agree that std.logger itself should not be "all batteries included" module as it does contradict overall Phobos style. It may be extended in future once we start using sub-packages much more but right now having good standard API is much more important.

2.

Standard logger must be made thread-safe by default. Out of the box safety is very important for standard library.

3.

Making default `log` a conditional logger does not seem to be gladly accepted decision. I'd recommend to swap it with `logf` and rename to `logIf`.

Naming overall should be reviewed to make sure there is no word duplication in typical usage pattern - Phobos does favor plain procedural/functional approach as default and D module system makes it unnecessary to encode namespaces into names.

====================
Personal preferences
====================

It is a matter of personal taste but any single thing in that list will make me vote "No" on logger proposal:

1.

API it too verbose.
I want stuff as short and simple as this:
```
import std.logger;
warn("1");
inform("2");
die("3");
```
I don't want to refer logger class instances explicitly for any typical task.

Stuff like `auto logger = new MultiLogger(); logger.insertLogger(someOtherLogger);` is redundant beyond absurd - just how many times one may insert word "logger" in a single sentence? :)

2.

Logging payload needs to include fully qualified module name. This is necessary for enabling module-local output.

3.

I am not sure if proper global vs local log separation can be done without providing two distinct default instances in std.logger (as I have already said, I don't consider solutions which imply tracking class instance manually).

4.

Static "namespace" classes are redundant and should be replaced with module-scope globals.
November 04, 2013
At any moment when you feel you are ready to convince everyone that the proposal should be accepted as is - just let me know and a more formal review will be scheduled ;)
November 04, 2013
On 11/04/2013 02:46 PM, Dicebot wrote:
> On Monday, 14 October 2013 at 11:39:52 UTC, Dicebot wrote:
>> As `std.logger` is still marked as "work in progress" this thread is less formal that typical pre-voting review. Goal is to provide as much input about desirable `std.logger` functionality and current state and let module author to use this information for preparing proposal for actual review / voting.
>>
>> Lets unleash the forces of constructive destruction.
>>
>> ===== Meta =====
>>
>> Code: https://github.com/D-Programming-Language/phobos/pull/1500/files Docs: http://burner.github.io/phobos/phobos-prerelease/std_logger.html
>>
>> First announcement / discussion thread : http://forum.dlang.org/post/mailman.313.1377180809.1719.digitalmars-d@puremagic.com
>
> Ok, finally making some conclusions.
>
> ===================
> As a review manager
> ===================
>
> This what needs to be done before moving to next review stage:
>
> 1.
>
> Judging by review comments it is clear that some more efforts need to be put into demonstrating how existing platform can be used as a standard API for more complex logging functionality. Addition of `MultiLogger` is good thing here, but providing either separate article on topic or extra module that demonstrates enhancing existing system feels like a good way to avoid confusion.
>
> That said, I do agree that std.logger itself should not be "all batteries included" module as it does contradict overall Phobos style. It may be extended in future once we start using sub-packages much more but right now having good standard API is much more important.
I looked at journalctl, it looks promising any small enough to fit in the docu as an example.
>
> 2.
>
> Standard logger must be made thread-safe by default. Out of the box safety is very important for standard library.
>
on my todo list: My first idea is to make the global logger shared and StdIoLogger and FileLogger synchronized in the right places.
> 3.
>
> Making default `log` a conditional logger does not seem to be gladly accepted decision. I'd recommend to swap it with `logf` and rename to `logIf`.
I don't see the fuss. the bool in front is just a transparent. But If people really want to write the 2 extra chars, be my guest.
>
> Naming overall should be reviewed to make sure there is no word duplication in typical usage pattern - Phobos does favor plain procedural/functional approach as default and D module system makes it unnecessary to encode namespaces into names.
>
> ====================
> Personal preferences
> ====================
>
> It is a matter of personal taste but any single thing in that list will make me vote "No" on logger proposal:
>
> 1.
>
> API it too verbose.
> I want stuff as short and simple as this:
> ```
> import std.logger;
> warn("1");
> inform("2");
> die("3");
> ```
> I don't want to refer logger class instances explicitly for any
> typical task.
>
> Stuff like `auto logger = new MultiLogger();
> logger.insertLogger(someOtherLogger);` is redundant beyond absurd -
> just how many times one may insert word "logger" in a single sentence? :)
>
no and yes, the warn inform, maybe.
The MultiLogger part ... it needs a better constructor. Put it on my
todo list
> 2.
>
> Logging payload needs to include fully qualified module name. This is necessary for enabling module-local output.
Easy fix, put it on my todo list.
>
> 3.
>
> I am not sure if proper global vs local log separation can be done without providing two distinct default instances in std.logger (as I have already said, I don't consider solutions which imply tracking class instance manually).
I'm not sure as well
>
> 4.
>
> Static "namespace" classes are redundant and should be replaced with module-scope globals.
not sure either


Thank you for taking effort to work on this "soon to be" review. Robert