View mode: basic / threaded / horizontal-split · Log in · Help
February 17, 2012
Re: Review of Jose Armando Garcia Sancio's std.log
> I think this is way too many levels. Why not just define few levels (around three) and let the user define new levels when needed.

I agree this is too many level, though I think letting the user define
their own levels is a needless complication.
>From experience, I would recommend:
- CRIT - the application cannot continue
- ERROR - the application cannot carry out the requested task
- WARN - the requested task cannot be completed normally, but we can
work around it
- INFO - FYI. Things sysadmins would like to know.
- DEBUG - For developers debugging.
- TRACE (maybe) - also sometimes called fine or verbose - for
developers debugging.

If an application needs separate channels of logging, I would using a
separate logger instance for different logging categories.
February 17, 2012
Re: Review of Jose Armando Garcia Sancio's std.log
On Thu, Feb 16, 2012 at 06:49:41PM -0700, Jonathan Stephens wrote:
> > I think this is way too many levels. Why not just define few levels
> > (around three) and let the user define new levels when needed.
> 
> I agree this is too many level, though I think letting the user define
> their own levels is a needless complication.
> >From experience, I would recommend:
>  - CRIT - the application cannot continue
>  - ERROR - the application cannot carry out the requested task
>  - WARN - the requested task cannot be completed normally, but we can
> work around it
>  - INFO - FYI. Things sysadmins would like to know.
>  - DEBUG - For developers debugging.
>  - TRACE (maybe) - also sometimes called fine or verbose - for
> developers debugging.
[...]

>From my experience, such a proliferation of log levels is rarely used
correctly. Most people, sadly to say, can't be bothered to read the
definitions of what each level is supposed to be, and will just
copy-n-paste the first instance they find. Of course, copying CRIT or
ERROR will quickly get a high-priority bug filed against it, but besides
CRIT and ERROR, the rest of the levels are rarely used properly.

I've seen people use WARN just because they think it's appropriate for
"warning" the developer that a certain condition has happened that might
trigger the bug they're hunting for. Then afterwards they forget to turn
that off, and the log fills up with irrelevant messages.

Similarly, INFO tends to be interpreted as "garbage dump for anything I
might want to know while writing this code, who cares what happens to
those messages afterwards". DEBUG and TRACE are rarely ever used after 4
or 5 developers have had their hands on it, because turning those on
will usually flood the logs with reams and reams of irrelevant messages
from unrelated modules, so most people would stop using it after the
first time.

Bottomline: reduce the number of channels, and let the programmer define
their own channels if they need to. Module-specific channels are the
best, so that you don't get reams of messages from completely unrelated
code. But only the user knows what modules they have, so let them make
that decision.


T

-- 
Those who don't understand Unix are condemned to reinvent it, poorly.
February 17, 2012
Re: Review of Jose Armando Garcia Sancio's std.log
Am 17.02.2012 02:49, schrieb Jonathan Stephens:
>> I think this is way too many levels. Why not just define few levels (around three) and let the user define new levels when needed.
>
> I agree this is too many level, though I think letting the user define
> their own levels is a needless complication.
>> From experience, I would recommend:
>   - CRIT - the application cannot continue
>   - ERROR - the application cannot carry out the requested task
>   - WARN - the requested task cannot be completed normally, but we can
> work around it
>   - INFO - FYI. Things sysadmins would like to know.
>   - DEBUG - For developers debugging.
>   - TRACE (maybe) - also sometimes called fine or verbose - for
> developers debugging.
>

Thats also exactly my experience (and what I use).
February 17, 2012
Re: Review of Jose Armando Garcia Sancio's std.log
On Thu, Feb 16, 2012 at 7:21 AM, Jonathan M Davis <jmdavisProg@gmx.com> wrote:
> On Monday, February 13, 2012 16:50:04 David Nadlinger wrote:
>> Please post all feedback in this thread, and remember: Although
>> comprehensive reviews are obviously appreciated, short comments are very
>> welcome as well!
>
> Why does vlog even exist? It's a needless complication IMHO. Let the log
> levels manage what does and doesn't get logged. I see no reason to add the
> concept of verbosity on top of that. It's a needless complication.
>

Needless? Man, you really make some "strange" comments. I believe that
this is better served by you asking for the motivation for vlog
instead of make a useless comment like the one above. The reason why
vlog exist is to provide users with the ability to enable/disable
logging at a package or module level. Lets say that for a particular
load you are seeing that module X is not behaving correctly. The next
time you run your application (or you can do this at runtime if you
have coded your application correctly) you can enable verbose logging
in this module by using the --v and --vmodule command line options. I
have tried to explain this in the properties
Configuration.maxVerboseLevel and Configuration.verboseFilter. Let me
know if it still no clear and if I should expand the documentation.

> Also, _please_ add a debug level. Personally, I'd argue for simply copying
> syslog's levels and matching them, since ideally any logging on Linux would be
> going to syslog anyway. But there are good reasons to have messages beyond
> info. I sure wouldn't want _all_ messages which don't indicate a problem in
> the app to be marked as info. For instance, what if I want to have info
> displayed in release mode but want greater verbosity in debug mode?

debug info("More information in debug mode");

I think it is very helpful that instead of just suggestion more level
we try to see how we can use this module and the power of D to do what
you want. If we find that it is not possible or clunky then we can
talk about adding more functionality.

> I'd need
> another log level which isn't there. Using the concept of verbosity to try and
> handle this is a needless complication. syslog has
>
> #define LOG_EMERG       0       /* system is unusable */
> #define LOG_ALERT       1       /* action must be taken immediately */
> #define LOG_CRIT        2       /* critical conditions */
> #define LOG_ERR         3       /* error conditions */
> #define LOG_WARNING     4       /* warning conditions */
> #define LOG_NOTICE      5       /* normal but significant condition */
> #define LOG_INFO        6       /* informational */
> #define LOG_DEBUG       7       /* debug-level messages */
>
> And I'd like to at least see notice and debug added.
>
> While we're at it, what's the point of dfatal? Why on earth would a _fatal_
> condition not be fatal if it were in release mode if it were fatal in debug
> mode? Is it fatal or not? It seems to me like another needless complication.
>

Barely a complication. dfatal looks as follow:

debug alias log!(Severity.fatal) dfatal; /// ditto
else alias log!(Severity.critical) dfatal; /// ditto

> If you're going to have write, then have writef, not format. Then it's
> actually consistent with our normal I/O functions.

Good suggestion. Will do.

> Also, do writef and format
> automatically append a newline? If so, then they should be writeln and
> writefln.
>

Technically, no. The module was abstracted with a frontend and a
backend. At a very high-level the front does filtering based on
compile time and run time configuration options. The backend is
basically responsible for persisting messages. It just happens that
the only backend implementation that we have writes to a human
consumable file hence the newline but it is possible that users are
going to want to store this data in persistent storage where each
logging event is not separated by a newline.

> Rich is a horrible name IMHO. It says nothing about what it actually is or
> does. I'm not sure what a good name would be (BoolMessage?, LogResult?), but
> Rich by itself is very confusing and utterly uninformative.
>
Yeah. I don't like Rich. Let me think about a better name. Thanks for
the suggestions!

> And why does Configuration's logger property throw if you set it after a
> logging call has been made. Is it really that inconceivable that someone would
> swap out loggers at runtime? Or is the idea that you'd swap out the
> Configuration?
>
The idea is not to swap out the Configuration but to instead be able
to reset each property. There is no technical reason why you can't
replace the Logger. I just didn't think this is something the users
would want to do in practice. Again this goes beyond technical reason
is more of the operational consequence if you allow this. For example:

1. You started logging to /tmp/application/...log...
2. You swapped the logger at runtime to start logging to syslog

You have a bunch of important data in /tmp/application/...log... what
are you going to do with it? Think of this like a stream flowing into
a lake and at some point you want to route the stream to the ocean.
What is the state of the lake after the routing change?

I would also like to add that this is a restriction that we can remove
in the future. I am honestly a little hesitant to remove it now
without giving it a little bit of more thought. Thoughts?

Thanks!
-Jose

> - Jonathan M Davis
February 17, 2012
Re: Review of Jose Armando Garcia Sancio's std.log
On Thu, Feb 16, 2012 at 7:57 AM, Sönke Ludwig
<ludwig@informatik.uni-luebeck.de> wrote:
> Am 16.02.2012 10:21, schrieb Jonathan M Davis:
>>
>> On Monday, February 13, 2012 16:50:04 David Nadlinger wrote:
>>>
>>> Please post all feedback in this thread, and remember: Although
>>> comprehensive reviews are obviously appreciated, short comments are very
>>> welcome as well!
>>
>>
>> Why does vlog even exist? It's a needless complication IMHO. Let the log
>> levels manage what does and doesn't get logged. I see no reason to add the
>> concept of verbosity on top of that. It's a needless complication.
>>
>> Also, _please_ add a debug level. Personally, I'd argue for simply copying
>> syslog's levels and matching them, since ideally any logging on Linux
>> would be
>> going to syslog anyway. But there are good reasons to have messages beyond
>> info. I sure wouldn't want _all_ messages which don't indicate a problem
>> in
>> the app to be marked as info. For instance, what if I want to have info
>> displayed in release mode but want greater verbosity in debug mode? I'd
>> need
>> another log level which isn't there. Using the concept of verbosity to try
>> and
>> handle this is a needless complication. syslog has
>>
>> #define LOG_EMERG       0       /* system is unusable */
>> #define LOG_ALERT       1       /* action must be taken immediately */
>> #define LOG_CRIT        2       /* critical conditions */
>> #define LOG_ERR         3       /* error conditions */
>> #define LOG_WARNING     4       /* warning conditions */
>> #define LOG_NOTICE      5       /* normal but significant condition */
>> #define LOG_INFO        6       /* informational */
>> #define LOG_DEBUG       7       /* debug-level messages */
>>
>> And I'd like to at least see notice and debug added.
>>
>
> Well in addition to Debug I would also like to see Trace but it's f. ex.
> hard for me to tell the difference between Info and Notice and their names
> do not imply that certain severity order IMO.

Yes. I would really like to avoid adding more severities because it
complicates the decision the developer needs to make when deciding at
which log level to log. This reminds of a situation that came up at a
previous employer. A bunch of smart people tried to come up with a
policy for deciding at which log level to log a particular message.
Let just say that the discussion lasted days and no decision was made.
I believe that the only reason why this subject even came up at my
previous job is because most logging framework provide hard-coded
levels that don't have any semantic difference between them.
Interesting talk on the subject:
http://www.ted.com/talks/barry_schwartz_on_the_paradox_of_choice.html

> So I see a point in the
> argument that vlog() allows everyone to be happy without endless numbers of
> predefined log levels.. however I'm also not quite convinced.
>
>
>> While we're at it, what's the point of dfatal? Why on earth would a
>> _fatal_
>> condition not be fatal if it were in release mode if it were fatal in
>> debug
>> mode? Is it fatal or not? It seems to me like another needless
>> complication.
>>
>> If you're going to have write, then have writef, not format. Then it's
>> actually consistent with our normal I/O functions. Also, do writef and
>> format
>> automatically append a newline? If so, then they should be writeln and
>> writefln.
>>
>
> I think the names should be as short as possible for the common 99% case. As
> this is not a general purpose stream, I think it is fine to drop the 'ln'.
> And the current version that defines info("") as the version that can format
> and info.write("") as the plain string version seems to be quite optimal in
> this regard.
>
> In my optinion, more descriptive names would just impair readability here
> instead of helping. They will be written endless number of times but do not
> influence the program flow and should immediately understandable by anyone
> who sees them. But something like log.warn/logf.warn or log.warn/log.warnf
> might also work if you really want the consistency...
>
As of right now (or in the immediate future) we have:
info("%s message", Severity.info);
info.write(Severity.info, " message);
info.writef("%s message", Severity.info);

Thanks!
-Jose

>
>> Rich is a horrible name IMHO. It says nothing about what it actually is or
>> does. I'm not sure what a good name would be (BoolMessage?, LogResult?),
>> but
>> Rich by itself is very confusing and utterly uninformative.
>>
>> And why does Configuration's logger property throw if you set it after a
>> logging call has been made. Is it really that inconceivable that someone
>> would
>> swap out loggers at runtime? Or is the idea that you'd swap out the
>> Configuration?
>>
>> - Jonathan M Davis
>
>
February 17, 2012
Re: Review of Jose Armando Garcia Sancio's std.log
On 2/17/12 12:06 PM, Jose Armando Garcia wrote:
> info("%s message", Severity.info);

I think defaulting to formatted stuff may be confusing.

Andrei
February 17, 2012
Re: Review of Jose Armando Garcia Sancio's std.log
On 2/13/2012 7:50 AM, David Nadlinger wrote:
> Please post all feedback in this thread, and remember: Although comprehensive
> reviews are obviously appreciated, short comments are very welcome as well!

This is a general comment, not specific to std.log:

All new library submissions need to be reviewed for applicability of:

@safe
const
pure
nothrow

for all user-facing API functions.
February 17, 2012
Re: Review of Jose Armando Garcia Sancio's std.log
On Fri, Feb 17, 2012 at 6:48 PM, Andrei Alexandrescu
<SeeWebsiteForEmail@erdani.org> wrote:
> On 2/17/12 12:06 PM, Jose Armando Garcia wrote:
>>
>> info("%s message", Severity.info);
>
>
> I think defaulting to formatted stuff may be confusing.
>
Even if we document this? I mean:

info("Info message");

Just works. The one place where is wont just work is with:

info("This is an ", Severity.info, " message);

But do we think users will do with this without reading one bit of the
documentation? And even then we can say RTFM ;).

I don't have a strong argument or preference for one over the other.
>From my experience I tend to write or log using a format string mainly
because it is more readable to me and plays nicer with localization
frameworks like gettext.

Thoughts?
-Jose

> Andrei
February 17, 2012
Re: Review of Jose Armando Garcia Sancio's std.log
On Fri, Feb 17, 2012 at 7:12 PM, Walter Bright
<newshound2@digitalmars.com> wrote:
> On 2/13/2012 7:50 AM, David Nadlinger wrote:
>>
>> Please post all feedback in this thread, and remember: Although
>> comprehensive
>> reviews are obviously appreciated, short comments are very welcome as
>> well!
>
>
> This is a general comment, not specific to std.log:
>
> All new library submissions need to be reviewed for applicability of:
>
> @safe
> const
> pure
> nothrow
>
> for all user-facing API functions.

Yep. I have been following that thread and your commits. I have a TODO for this.

Thanks.
-Jose
February 18, 2012
Re: Review of Jose Armando Garcia Sancio's std.log
On Friday, 17 February 2012 at 06:39:29 UTC, H. S. Teoh wrote:
> […] DEBUG and TRACE are rarely ever used after 4
> or 5 developers have had their hands on it, because turning 
> those on
> will usually flood the logs with reams and reams of irrelevant 
> messages
> from unrelated modules, so most people would stop using it 
> after the
> first time.

I think avoiding this problem is one of the major benefits of the 
proposed std.log design (because vlog output can be filtered on a 
per-module basis).

David
2 3 4 5 6 7 8 9 10
Top | Discussion index | About this forum | D home