Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow setting logLevel based on env variable #124

Open
zanona opened this issue Oct 9, 2018 · 7 comments
Open

allow setting logLevel based on env variable #124

zanona opened this issue Oct 9, 2018 · 7 comments

Comments

@zanona
Copy link

zanona commented Oct 9, 2018

It would be nice if we could have something like LOG_LEVEL=1 node log-test.js where if LOG_LEVEL (or any defined var name) could have an impact on the log level used rather than having to declare:

import log from 'loglevel';

log.setLevel(parseInt(process.env.LOG_LEVEL, 10));
log.warn('hello world');

This would be great for CI, in case we wanted to switch log levels without affecting the codebase.

@pimterry
Copy link
Owner

This is a good idea, I like it. My main concern is that it would be a breaking change, since users might be using that env var for something else, so this would suddenly change how their logging worked. We could somewhat mitigate this with a different variable name (e.g. LOGLEVEL_LEVEL), but it's still not truly 100% non-breaking, and most more specific names are a bit inconvenient.

Happy to take suggestions for how to avoid making this change require a major bump, but otherwise I'll definitely consider it for the future, and we should absolutely include it if/when we end up publishing loglevel 2.0.

@zanona
Copy link
Author

zanona commented Oct 10, 2018

I am in favour of having LOG_LEVEL in the next major release, although I'm afraid that also wouldn't prevent users from making use of that variable for other systems/programs and inheriting side effects. But I believe that's the reason the env var LOG_LEVEL exists, so many programs run with the same log level, where the user expects to see logs for only that specific type i.e: warn, info, debug, etc.

Therefore, I agree with the point of not introducing this without some disclaimer as a breaking change.

For the time being, I believe it's perfectly fine to have an alternative name as LOGLEVEL_LEVEL which was a great suggestion, btw. 👏

Later on, with the 2.0 migration notes, alert users of a possible name change for the env variable, if necessary.

@asasine
Copy link

asasine commented Oct 10, 2018

Is this similar to what this plugin does? https://github.com/vectrlabs/loglevel-debug

As another suggestion, why not make the user of the environment variable opt-in? Users would have to configure the logger to use the environment variable, and could be configured on a module-wide setting (similar to log.setDefaultLeve(...)). [Existing] users who have not configured it would not experience breaking changes, and users who want it can enable it with a simple flag. It would require some explicit "enable this feature" call/parameter similar to @zanona's code snippet but perhaps it's a good compromise between breaking and a valuable new feature.

@pimterry
Copy link
Owner

Yes, very similar - that plugin looks like a great solution to this.

Other than that, adding opt-in support in the short term isn't super useful imo. We could add a method to enable it, but it's super easy to opt-in by writing it from scratch anyway. The simplest fix is just:

if (process.env.LOG_LEVEL != null) log.setLevel(process.env.LOG_LEVEL);

That supports error/warn/info/etc values, or you could change it to log.setLevel(parseInt(process.env.LOG_LEVEL, 10)) if you want to support numeric levels instead.

We could create a log.readFromEnvironment() method so you can opt-in, but it's only marginally shorter than the above, and less clear & flexible, so I don't think it's that useful.

Automatically picking the level from env vars would be definitely useful, but is a breaking change.

For now I think I'm going to sit on this until I get around to the next breaking release, but it's definitely good fodder for that.

@xmedeko
Copy link
Contributor

xmedeko commented May 21, 2020

Whatever name for env variable you choose, it should be possible to change the name of the variable (and disable it) so a user may avoid name conflicts is necessary.

The name of the variable should not be DEBUG since it's used already by very used https://github.com/visionmedia/debug project and two libs with both logging may meet in one project.

However, it would require to implement some name hierarchy, e.g. dot mymodule.myclass.myfeature to be able to switch on/off whole module. And level assignment, e.g. LOG_LEVEL=mymodule.myclass:warn or LOG_LEVEL=mymodule:info,mymodule.mychattyclass:warn.

Note: debug use semicolon and a star * wildcards. I have experience with logging in different languages - Java log4j, logback, Python logging, PHP log4php, C# log4net, NLog and they use dot notation and no star wildcard. Enabling a module should enable all submodules, too. Disabling a module should disable all submodules, too.

@AmreeshTyagi
Copy link

AmreeshTyagi commented Jun 20, 2020

what about LL_LOG_LEVEL? Seems unique to me.

@xmedeko
Copy link
Contributor

xmedeko commented Apr 21, 2021

ulog is configurable from env var and it allow to set log levels, too, see ulog Log configuration syntax

E.g. this test=debug;my:*,-my:lib=info means "set the logger test to debug and my:* loggers except for my:lib to info.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants