Syslog Levels

[vmadman]

In all other configs the numerical order is from lowest to highest (e.g. 'npm' has silly=0,error=5). However, the syslog levels were from highest to lowest (debug=7,emerg=0). The CLI config also had a similar order as the NPM config.

I've corrected this file to follow the same pattern. Its so obvious and fundamental that I know I am probably breaking something that has been the source of long debates or something, but I cannot think of why it is the way it is.

The main reason this change is needed is because of the behavior of the 'level' param when adding transports. Before this change, if I set the level of my console transport to 'notice', then it would listen for: debug, info, and notice. Thus, the setting would imply "Listen for this level and all levels of lower priority" (i.e. MaxLevel)

Limiting by 'Maximum Level' is counter-intuitive. More often than otherwise I would want to prevent lesser priorities from being logged.. not higher priorities. In fact, this setup would make it impossible to disable debug logging. Instead, level should refer to 'Minimum Level', enabling this setup:

    - Debug > Console
    - Info, Notice, Warning, Error, Crit, Alert, Emerg > Log File
    - Notice, Warning, Error, Crit, Alert, Emerg > Logging System
    - Crit, Alert, Emerg > Cell Phone

I also added some additional docs for levels.

[gdw2]

I ran into this bug today. It seems weird that I can see 'debug' messages, but not 'emerg' messages.

[gdw2]

likely duplicate: #290

[indexzero]

This is better than #349, but we should look at #290 as it seems to be much more extensive. Probably some combination of the two will get merged in.

[vmadman]

It seems to me that this pull request and #290 are mutually exclusive. Whereas I took the approach of reverting Syslog levels back to match the order of the other built-in levels, #290 took the approach of modifying all of the other levels to match Syslog's order. It then updated the gt and lt logic to indicate that 0 (zero) is considered to be a higher priority than 1 (one).

I think both approaches achieve the same result, but are not compatible. The only reason my PR is currently a bit better is because #290 is mysteriously failing the unit tests.

If and when that is fixed, though, I think #290 is the better route because the severity levels in RFC 5424 go from highest priority (0=emerg) to lowest (7=debug).

From: http://tools.ietf.org/html/rfc5424#page-11

           Numerical         Severity
             Code

              0       Emergency: system is unusable
              1       Alert: action must be taken immediately
              2       Critical: critical conditions
              3       Error: error conditions
              4       Warning: warning conditions
              5       Notice: normal but significant condition
              6       Informational: informational messages
              7       Debug: debug-level messages

Although there may be other documents that I am not aware of, it does not appear that NPM offers an opinion on the matter, as it does not provide numerical equivalents for its string levels. (see: https://www.npmjs.org/doc/misc/npm-config.html#loglevel). So, I do not think there is a compelling reason to go against the order defined in RFC 5424, which my PR does.

Therefore, I'd suggest that this PR should ultimately be rejected in favor of #290.

[indexzero]

This is very interesting @vmadman. Will consider this when merging. We are leaning towards your work, but #290 has a lot more tests. The breaking change in 651b13e is really that we changed the order of the levels without changing the underlying logic in the logger.

Seems like the correct breaking change for winston@1.0.0 here is to keep the levels the way they are and change the <= to a >= so we conform to the RFC.

[vmadman]

Yeah, it seems to me that this PR is better for a pre-1.0.0 merge, if you plan to have any more, since it is non-breaking. I mean, it is impactful and may cause some breakage, but only in places where implementers have created workarounds, because syslog levels are currently not usable as intended.

However, #290 is better for 1.0.0 since breaking changes will be expected, to some degree, and it is better overall because it better conforms to RFC 5424. That is, assuming you're ok with Syslog being the principal driver of Winston's priority conventions.

Also, #290 does offer unit tests, and this one does not, but this PR has less of a need for them as it does not introduce any radical changes. (which is not to say that it shouldn't have them anyway)

Sorry, not trying to tell you guys what to do, I just wanted to offer my thoughts on it in case they are useful.

[vmadman]

I guess I could have just said that this PR is basically a hot fix, and #290 is an actual improvement ;)

[vmadman]

Sorry, last point..

Seems like the correct breaking change for winston@1.0.0 here is to keep the levels the way they are and change the <= to a >= so we conform to the RFC.

I may be misunderstanding you, but based on your statement I worry that the main problem has not been properly illustrated. So just in case, for clarity:

The root of the problem is that the syslog levels and all of the other levels are numbered using opposite conventions, since 651b13e. Winston's NPM config does it one way (5=error, highest), and it's Syslog config does it another (0=emerg, highest).

I only wanted to make sure that's clear because it negates the option of being able to "keep the levels the way they are and change the <= to a >=". One way or another, priority levels will need to be changed.

[jmcbee]

So you have the levels but how do you output them to the console in syslog format?

[patrick-webs]

Just found this bug report after debugging. please fix this in winston, it is rather surprising behavior.

[indexzero]

Thanks again for your contribution. This has been resolved in the 2.x branch. We will be releasing winston@2.0.0 shortly with this fix.