October 14, 2013
On 10/14/2013 05:27 PM, ilya-stromberg wrote:
>
> Add e-mail logger (useful for critical errors) for example via
> `std.net.curl.SMTP`.
And than at a email format config parser that fulfills everyones wishes, would bet the idea of the design. And I though people where trying to remove curl.
October 14, 2013
On 10/14/2013 04:44 PM, Sönke Ludwig wrote:
> Am 14.10.2013 15:12, schrieb Robert Schadek:
>> On 10/14/2013 02:39 PM, Sönke Ludwig wrote:
>>>  - The static methods in LogManager should be made global and the class
>>>    be removed. It's not for objects so it shouldn't be a class.
>> LogManager also stores the global log level. Sure I can make another static global function storing this log level, but I would like to keep them together as they belong together IMO.
> The same could be said about the global "log" functions, which are tightly coupled to that state. I think this is already nicely grouped together by the logger module itself, since there is not much else in it.
>
> Basically, I just wouldn't consider this style to be particularly idiomatic D code, but of course that's just personal perception/preference (there is also some precedence using "struct" instead of "class" in Druntime). However, if it ends up like this in the final version, it should get a "@disable this();" to prevent misuse.
It is for ment for phobos not druntime. Anyway structs would mean all
templates and people will scream template bloat. And this would break
the design, which I find to be a valid use of classes and
polymorphisms.  The StdIOLogger can have its default constructor called IMO.
>
>>>  - For me this logger is completely worthless without any debug log
>>>    levels. The last std.log entry had at least anonymous verbosity
>>>    levels, but I'd prefer something like I did in vibe.d [1], where
>>>    each level has a defined role. This should especially improve the
>>>    situation when multiple libraries are involved.
>> Logger.log(LogLevel.(d|D)ebug, "Your message");
> That would be my idea. Having at least two (diagnostic output for the user and debug output for the developer), but better all four debug levels can be very useful, though.
Maybe I miscommunicated what I want to show by that example. The (d|D)
part is the rename to enum lower case.
The debug log level is given through the LogLevel.Debug, which will be
renamed to LogLevel.debug. I would call the developer the user of the
logger. Maybe log messages can be communicated to the user of the
applicaiton and the developer of the application through a MultiLogger
class.
>>>  - Similarly, I've never felt the need for conditional logging, but
>>>    without it being lazy evaluated what's the use for it, actually?
>> The conditional logging part is totally transparent.
> But is there a compelling use case? It's transparent, but still increases the size/complexity of the API, not much, but there should be a reason for that IMO.
I had the case that a custom class made problems in some special case. And IMO it was just prettier to have the condition if it should be printed passed as first argument rather than adding a if branch in my top level code.

log(cls.canBePrinted(), "%s", cls.toString());
is IMO prettier than

if(cls.canBePrinted()) {
    log("%s", cls.toString());
}

>
>>>  - I really think there should be shortcuts for the different log
>>>    levels. Typing out "LogLevel.xxx" each time looks tedious for
>>>    something that is used in so many places.
>> One could argue that writting logger.logDebug("...") is more tedious
>> than writing,
>> logger.logLevel = LogLevel.xxx;
>> logger.log("...");
>> logger.log("...");
>> ...
>>
>> This has been argued in the last logger discussion to some extend and it looked to me like this is the mostly preferred version.
> The last discussion resulted (if I remember right) in something like "log.info(...)". This would be fine, too, although I think just "logInfo" is slightly preferable due to its length and the principle of least surprise.
At least jmdavis had a very strong argument against it. Something like function should be verbs ... Maybe something like logCritical, logInfo would be a compromise, but I think everybody here has a different idea of what is correct.
>>>  - There should be some kind of MultiLogger so that multiple log
>>>    destinations can be configured (e.g. console + file). Or, instead of
>>>    a single default logger, there could be a list of loggers.
>> there is one default logger. I will create a MultiLogger, good point. I'm currently sure how to store the multi logger (LL or Array or ... )
> I'd say a dynamic array is fine because the list of loggers will rarely change and this would be most efficient for iteration.
Properly yes, at least will that be my first try.
>
>>>  - On the same topic, if I'm right and the default logger is stored as
>>>    __gshared, it should be documented that Loggers need to be
>>>    thread-safe.
>> It is not stored __gshared, but If, you're right.
> So the defaultLogger is per-thread? That may result in unexpected log output if you just do simple code like "spawn({ log("Hello, World!"); });" and have a custom logger set up during application initialization. Anyway, let's just say the threading behavior in general should be clearly documented here as this is critical for both log users and Logger implementors.
Yes it is per thread. I concur that this needs to be documented well.
>>>  - GC allocations for each log message _must_ be avoided at all costs.
>>>    Using formattedWrite() instead of format() on either a temporary
>>>    buffer that is reused, or, better, on some kind of output range
>>>    interface of the Logger would solve this.
>> This was proposed in the last thread. A fixed size buffer would scream bufferoverflow, a dynamic buffer not but both would raise the question of thread safety.
> Something like a thread local buffer that grows when needed, or small fixed buffer + a scoped heap allocation for large messages should be fine. Using an output range interface could of course avoid buffering each message altogether. That would just open up the question of a nice API design for such.
Maybe, but I bet somebody else will bring up Allocator and Ref Counting in that context.
>
> One last thing just occurred to me, in the default logger in vibe.d I've made it so (thanks to Jordi Sayol for requesting this) that the "info" level gets output to stdout and all other levels to stderr. Either that or always outputting to stderr would be conforming to the idea of stdout/stderr and would avoid that some library with logging calls interferes with the output of an application (when piping process output in a shell script).
First, the default logger is assignable so maybe we should talk about the StdIOLogger which prints to StdIO or maybe StdErr. IMHO { I hate when that happens, I hate writing redirects to the cmd. } But this is properly a task issue.
>
> Thanks for bringing this forward!

To whom is this directed?
October 14, 2013
On Monday, 14 October 2013 at 18:00:12 UTC, Robert Schadek wrote:
> On 10/14/2013 05:27 PM, ilya-stromberg wrote:
>>
>> Add e-mail logger (useful for critical errors) for example via
>> `std.net.curl.SMTP`.
> And than at a email format config parser that fulfills everyones wishes,
> would bet the idea of the design. And I though people where trying to
> remove curl.

I said: "for example via `std.net.curl.SMTP`". If you don't want to use curl, you shouldn't. But yes, I think that e-mail logger can be useful. For example Linux use it (and bash can send errors via e-mail).

About "email format config parser that fulfills everyones wishes": you shouldn't do it, a simple string with error message should be enough: we should inform admins about the problem, not provide a marketing e-mail.

If you disagree, please tell why.
October 14, 2013
It's weird that LogManager.defaultLogger exists as a read-only property, and if I want to change it I have to use the global static "log" variable.

I think this was mentioned elsewhere, but is the global logger thread-local, shared, or __gshared?  Or is this something we can control?  I can see maybe wanting to have a separate logger per thread in some cases, for example.

I kind of wanted channels per Boost::Log, but simply creating multiple loggers that I can reference by name seems reasonable.  Channels add a ton of complexity anyway.

We really need timestamps, and the timestamp format should be configurable.  One thing I like about Boost::Log is that I can statically specify the format of log lines.  This would be particularly useful when trying to integrate a D app into a setup that already expects a particular log line format.

This is an implementation detail, but I imagine the buffer these lines are written into is a static buffer that grows as needed and is just reused for each log line written?  I understand why a custom logger would receive only the generated log string, but if this is allocating unnecessarily in the background it could be a problem.  Maybe we could get some way to specify the initial size of this internal buffer as well, or limit the total buffer size to some value?  In some cases, you don't want to log more than N bytes per entry even if it means truncating the message.

Formatted logging should be the default.  I almost never want to just long a string with no parameters.

I'm not entirely happy with the conditional just sitting in the parameter list.  Maybe if the command were "logIf" instead of just "log" or there were some way to mark up the conditional in code as that it determines whether the log line is written...  I think this goes with my general aversion to using magic numbers as function parameters.  I either like the function call to read line a sentence or if that isn't reasonable, for there to be some way to indicate what a particular parameter is for.

For what it's worth, I think the conditional is a reasonable substitute for not having the LogLevel be a bitmask.

For the rest, I think the current API is sufficient to make most use cases work.  I might sometimes create a proxy logger than fans out to multiple discrete loggers underneath, for example.  But that seems straightforward.
October 14, 2013
On Monday, 14 October 2013 at 12:40:16 UTC, Sönke Ludwig wrote:
> Am 14.10.2013 13:39, schrieb Dicebot:
>  - For me this logger is completely worthless without any debug log levels. The last std.log entry had at least anonymous verbosity
> levels, but I'd prefer something like I did in vibe.d [1], where each level has a defined role. This should especially improve the situation when multiple libraries are involved.

Orthogonal to log levels one idea could be to define a namespace for a logger instance.

* The namespace could be in the form of a prefix (useful for grepping and simple to implement)

logger.setPrefix("myApi__");
logger.warn("a warning"); // would output "myApi__a warning"

* The namespace could be deduced from the module name and filtered at runtime by a flag 'à la' gtest ( https://code.google.com/p/googletest/wiki/AdvancedGuide#Running_a_Subset_of_the_Tests ). This would be helpful when displaying fine grained log for debugging purpose without being flooded by other libraries logs.

>./foo Has no flag, and thus output all the logs.
>./foo --log_namespace=myLyb.* Output everything in myLyb module.

This option requires initializing the test library in main and would be more costly because of the namespace matching ( maybe the initialization could trigger a different implementation at runtime or we enable this feature at compile time through a version )

October 14, 2013
On 2013-10-14 20:24, Robert Schadek wrote:

> At least jmdavis had a very strong argument against it. Something like
> function should be verbs ... Maybe something like logCritical, logInfo
> would be a compromise, but I think everybody here has a different idea
> of what is correct.

If "log.info" is used it minimizes the module level functions.

-- 
/Jacob Carlborg
October 14, 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.

Thanks for taking care of that.
My logger class logs warning in yellow and errors in red ;),
but honestly I will take _anything_ since the lack of std.logger is a big problem right now for library writers.
I cannot write a nice wrapper for a C library without forcing my pet logger implementation [that no one wants].
October 14, 2013
> Orthogonal to log levels one idea could be to define a namespace for a
logger instance.
Take a look at the way log4j does it, with logger hierarchy:
http://logging.apache.org/log4j/1.2/manual.html

This is incredibly useful.


October 14, 2013
On 10/14/2013 08:29 PM, ilya-stromberg wrote:
>
> I said: "for example via `std.net.curl.SMTP`". If you don't want to use curl, you shouldn't. But yes, I think that e-mail logger can be useful. For example Linux use it (and bash can send errors via e-mail).
>
> About "email format config parser that fulfills everyones wishes": you shouldn't do it, a simple string with error message should be enough: we should inform admins about the problem, not provide a marketing e-mail.
>
> If you disagree, please tell why.
I disagree on having a simple email layer among the default logger, because I feel that having this "special" logger in would water the design of the logging module. Maybe you are happy with a simple string message mail with fixed subject and sender, the next guy will not. And he will be asking for it and (we || I ) have to tell him, why the simple one is in and his version is not. I would rather have a minimal std(io|err), file logger version which is pure and easy to mod.
October 14, 2013
On 10/14/2013 08:55 PM, Sean Kelly wrote:
> It's weird that LogManager.defaultLogger exists as a read-only property, and if I want to change it I have to use the global static "log" variable.
this will be properly be fixed in the next incarnation (tonight, no
promises)
>
> I think this was mentioned elsewhere, but is the global logger thread-local, shared, or __gshared?  Or is this something we can control?  I can see maybe wanting to have a separate logger per thread in some cases, for example.
>
currently all are thread local. the default logger will properly be gshared. the others, I wish I could predict the shared discussion outcome.
> I kind of wanted channels per Boost::Log, but simply creating multiple loggers that I can reference by name seems reasonable.  Channels add a ton of complexity anyway.
>
> We really need timestamps, and the timestamp format should be configurable.  One thing I like about Boost::Log is that I can statically specify the format of log lines.  This would be particularly useful when trying to integrate a D app into a setup that already expects a particular log line format.
timestamps will be in
>
> This is an implementation detail, but I imagine the buffer these lines are written into is a static buffer that grows as needed and is just reused for each log line written?  I understand why a custom logger would receive only the generated log string, but if this is allocating unnecessarily in the background it could be a problem.  Maybe we could get some way to specify the initial size of this internal buffer as well, or limit the total buffer size to some value?  In some cases, you don't want to log more than N bytes per entry even if it means truncating the message.
>
> Formatted logging should be the default.  I almost never want to just long a string with no parameters.
>
> I'm not entirely happy with the conditional just sitting in the parameter list.  Maybe if the command were "logIf" instead of just "log" or there were some way to mark up the conditional in code as that it determines whether the log line is written...  I think this goes with my general aversion to using magic numbers as function parameters.  I either like the function call to read line a sentence or if that isn't reasonable, for there to be some way to indicate what a particular parameter is for.
I'm not sure what is the best, all fitting way here.
>
> For what it's worth, I think the conditional is a reasonable substitute for not having the LogLevel be a bitmask.
>
> For the rest, I think the current API is sufficient to make most use cases work.  I might sometimes create a proxy logger than fans out to multiple discrete loggers underneath, for example.  But that seems straightforward.