Missing 'log' method to serve as a drop-in replacement for 'console'

sompylasar commented 9 years ago

I'd like to replace all console.* calls to loglevel.* calls but I discovered that there is no log method which is widely used with console. The info method does not fit well because the browser console highlights the info message with an icon making these messages somewhat more specific than default logging.

Any chance adding log?

pimterry commented 9 years ago

I'd suggest using debug instead, for starters, since most browsers treat that as the standard default unmessed-around version.

Anyway, I'm a bit averse to adding an actually adding a new case for .log(), because it doesn't fit well within the range of levels. Every single other logging tool I'm aware of uses the same set (trace/debug/info/warn/error), and the consistency there is useful. In addition, I think because it's inconsistent, it's not easy to tell how that fits in the scale. Does it log when you're at error level, or at info level, or what? Not clear to be, and certainly not likely to be clear to other library users.

I see your point about easy migration though. Would adding loglevel.log as an alias for loglevel.debug solve your problem?

sompylasar commented 9 years ago

No, it does not. The browser console outputs the log and debug messages differently.

mroderick commented 9 years ago

Anyway, I'm a bit averse to adding an actually adding a new case for .log(), because it doesn't fit well within the range of levels. Every single other logging tool I'm aware of uses the same set (trace/debug/info/warn/error), and the consistency there is useful. In addition, I think because it's inconsistent, it's not easy to tell how that fits in the scale. Does it log when you're at error level, or at info level, or what? Not clear to be, and certainly not likely to be clear to other library users.

For me it is important that loglevel's api looks familiar to developers from non-JavaScript platforms.

@sompylasar you can always decorate or wrap loglevel after loading it, or even create a plugin if you want.

I use these two methods in a current project, so that including loglevel on a webpage is optional.

function info() {
    if (log) {
        log.info.apply(null, arguments);
    }
}

function error() {
    if (log) {
        log.error.apply(null, arguments);
    }
}

sompylasar commented 9 years ago

If non-JS developers are the target audience of this library, this should likely be stated. For me this library is a wrapper around the browser console which handles all of its inconsistencies in a small amount of code.

Decorating loglevel won't add a loglevel.log wrapper around console.log hence this issue. There is no public API to wrap any method of the browser console object. Moreover, a decorator won't prevent loglevel from touching cookies and localStorage (that's another issue).

The browser console renders console.info output with an (i) icon next to the log message making the message somewhat special (compared to a plain log message without any color or icon).

pimterry commented 9 years ago

Which browser are you using that renders .debug and .log differently? In both Chrome and Firefox locally I see exactly the same thing. Chrome's own docs actually describe it as identical to console.log. Is there another browser that's not matching up with that?

Non-JS developers aren't explicitly the target of this library, but you can't ignore context. Pretty much every other logging library everywhere, JS and otherwise, uses these levels. They're the standard log levels and I'm not going to deviate from that standard, because it's definitely going to be confusing to a substantial part of the userbase (anybody who has used any other logging libraries).

I am quite happy to add an explicit alias, which it sounds like would solve the practical element of your problem (being able to quickly swap console for loglevel), if we can work out what level is suitable to alias to.

I still don't think it's a good idea to actively use that though, except just to get started easily. This library isn't just a wrapper for console.log, it also provides the basic level of log level management that you need in any serious logging environment, and unfilterable logging (which I think is what you're suggesting) does not fit in that model (and is generally not a good idea in any meaningful codebase).

I'm also curious why you don't want to persist the configured log level locally in your cookies/localStorage. Any particular reason for that? I'd originally added it because otherwise it's very fiddly to get log output for your app startup code without changing the live codebase. Is there a downside to this that you're hitting?

sompylasar commented 9 years ago

I use the latest Google Chrome, the devtools console renders debug in blue color, log in black color, info in black color with a blue info icon on the left.

I needed a tiny library that copes with console inconsistencies and is browserifiable/webpackable. For my case I forked and patched loglevel to remove persistence and add log as a second method on the INFO level (see my fork: https://github.com/sompylasar/loglevel/blob/master/lib/loglevel.js#L77 ). I am not suggesting to use the same level for all logging, I am suggesting to wrap every logging method the browser provides.

I understand your thoughts on persistence: you want to visit your app in production, tamper with your own storage and read the logs. It's a good idea. But if the persistence is hardcoded into the logging library, it touches the environment without express permission from the app author. I see this as putting the persistence part into a separate optional module which lets configure the cookie and the storage or opt out of persistence, and using loglevel.setLevel(loglevelPersistence.getLogLevel()) once in the app's entry point. No further code changes are needed.

ramumb commented 8 years ago

I'd like to +1 adding a "log" method. I just ran into this same issue while integrating loglevel. It's such a common method that it needs to be in the lib; I'd dare say that it's more often used the "info".

tauren commented 7 years ago

I'll throw my two cents into the mix:

Problems

`console.trace` sucks for trace level logging

I avoid using console.trace unless I want to actually see a stack trace. This is because Chrome shows the stack trace expanded, which clutters up the console. I don't feel it is a good method to use for the trace level of a logging tool.

However, by not using log.trace with loglevel, I've limited myself to only 4 levels (error, warn, info, and debug). This is annoying because I often would like to add trace level logs for things that are deeper than debug. But I do not want to see stack traces for them.

Note that console.warn and console.error in Chrome also include stack traces, but they are much more usable since the traces are collapsed. I only have an issue with console.trace.

Not taking full advantage of browser log styling

It is important to consider that Chrome styles console.log, console.debug, console.info, console.warn, and console.error differently. However, loglevel doesn't take full advantage of this by not supporting console.log. IMO, this is a major downside of loglevel at this time.

Making log.log an alias for some other loglevel will not solve this. We really need a new loglevel for console.log.

App environment is affected by `loglevel`

I agree with @sompylasar that many app developers don't want their logging tool to impact the app environment without permission. However, the local storage feature is useful and think it makes sense to continue to support it.

Proposed Solution

I propose the following enhancements be made to loglevel:

add a verbose log level between debug andtrace
add a log.verbose method that maps to console.log
add a log.log method that maps to console.log
add a configuration option that disables/enables local storage features

I disagree with @mroderick regarding solving this by decorating log methods or creating a plugin. As soon as any wrapper is used around loglevel, the browser will report logs as coming from the wrapping code's file and line number instead of the actual file and line of the log call within your application.

I am willing to make these improvements and create a PR, but I'd like to hear from @pimterry and others that this proposal would be acceptable.

olegstepura commented 7 years ago

Very good point @tauren :+1: I've just discovered loglevel and tried to use it. I have a lot of server-side coding experience in Scala and PHP together with client-side scripting and I totally agree with you. I cannot accept blue text color for debug messages in console. +1 for PR

One extra thing I did not find in documentation is if I can reset (persisted) log level after debugging session finished. There seem to be no log.reset* methods.

The rest is very good (the right source of each message is a greatest pros of this library), ability to persist log level is also very cool.

victornpb commented 7 years ago

+1 to points made in @tauren reply

I added a new level by log.verbose = log.methodFactory('log', .5);

shellscape commented 7 years ago

piling on here, noting early comments in this thread it's important to point out that console.debug has since been deprecated and is currently treated as a noop in the latest versions of modern browsers.

victornpb commented 7 years ago

@shellscape the recommendation on MDN is to use console.log() instead of console.debug().

Following the pattern it would become log.log(), looks kind of weird.

shellscape commented 7 years ago

@victornpb I've implemented logging wrappers in the past, and we've always defaulted to the "plain" old log, equivalent to console.log was log(...). Wouldn't be unreasonable that the function by default logged. Appearances aside, given that this is a browser-first and browser-oriented script, it would make more sense to follow browser convention.

A further little tidbit, console.debug in node has also been relegated to a noop and it's not even listed in the documentation for Console https://nodejs.org/api/console.html.

The argument for log.debug was probably sound 2.5 years ago, but times have changed.

pimterry commented 7 years ago

I'd rather not have a log.log method. As in my first posts way above, it's not a common log level in most other libraries, it's not obvious where it fits in the semantic order of levels, and it would be a breaking change anyway, which I'd really like to avoid.

I do totally agree we need to find a solution now that console.debug seems to have been quietly killed though. I need to think about it further, but in principle I think I'd be happy to make log.debug start using console.log under the hood, instead of console.debug. How would people feel about that?

olegstepura commented 7 years ago

Since browser logging methods do not fit the old good logging levels which every programmer (who programmed more than for the browser) used to, I would suggest to mimic those logging levels with the existing browser methods as close as possible. But there should exist all 6 levels.

I'd be happy to make log.debug start using console.log under the hood, instead of console.debug

+1

tauren commented 7 years ago

I think I'd be happy to make log.debug start using console.log under the hood, instead of console.debug. How would people feel about that?

This seems like a good solution to me as well. Because console.trace has a different behavior than the other log methods, I personally don't use it as "standard" loglevel in my apps. And with console.debug being a noop, the log methods available for my apps has shrunk to just log.info, log.warn, and log.error. Making log.debug work again by mapping to console.log would certainly help!

I'd rather not have a log.log method. As in my first posts way above, it's not a common log level in most other libraries, it's not obvious where it fits in the semantic order of levels, and it would be a breaking change anyway, which I'd really like to avoid.

Why not just have log.log be an alias for log.debug? It wouldn't add another loglevel, but it would be available for consistency for those who wish their logging tool to have the same API as browser logging. I personally would use log.debug over log.log, but I wouldn't mind the alias existing. And doing this would squash many of the complaints people have.

Revised Proposal

Now that console.debug is no longer a thing, I would revise my proposal (see post above) to the following:

change the log.debug method to map to console.log
add a log.log method that maps to console.log (as an alias, no additional loglevel)
add a configuration option that disables/enables local storage features

I've removed log.verbose because there isn't anything for it to map to.

pimterry commented 7 years ago

add a configuration option that disables/enables local storage features

Optional persistence has actually already been added, since the initial discussion on this issue. See https://github.com/pimterry/loglevel/pull/72. As far as I'm aware loglevel will now never initially set anything in your storage, and .setLevel has an optional 2nd argument which stops it persisting the new set level if you change it: log.setLevel('WARN', false). As long as you always call that with false you'll never store anything.

add a log.log method that maps to console.log (as an alias, no additional loglevel)

Sure, that's fine by me. I'd quite like to make sure it's clear in the docs that this isn't part of the recommended API, and it's just an bonus convenient alias, to keep the core API focused on the existing levels. I don't have any problem with it quietly existing though.

I'll get around to doing all this and shipping a new release in the next week or two. If anybody wants it more urgently, or is feeling particularly keen, I'd welcome PRs in the meantime. I'd also welcome any more feedback: if anybody else has strong opinions on this, now's the time.

victornpb commented 7 years ago

I aprove the proposal apart from the verbose level. It's quite useful from having something above trace.

pimterry commented 7 years ago

Both the proposed changes have now been made, and released in 1.5.0:

log.debug now uses console.log under the hood, so will work in Chrome versions that make console.debug a noop.
log.log now exists, as a simple alias to log.debug

pimterry / loglevel