Configurable persistence

[xmedeko]

getPersistedLevel is called for each logger. I do not need that. I think the persistence should be configurable same way as methodFactory for plugins - i.e. it should be possible to switch it off, use only cookies, etc.

[pimterry]

Hi @xmedeko. Could you tell me more about why you need to disable this?

Right now, persistence is fully configurable at level configuration time - you can pass an option to setLevel to control this. If you never call that with persistence enabled, then nothing will ever be stored, and getPersistedLevel will never do anything, and has no impact. Is there a reason that doesn't work for you?

Unfortunately, if it is a problem, it would be difficult to disable persistence at load time. By design loglevel is immediately initialized and available when imported, with zero options or construction required, to make it as easy as possible to quickly use. That means that getPersistedLevel is already called before the point where you could configure it.

[xmedeko]

I would like to disable it because it's useless for our Electron.js app. It's a very small slow down for every getLogger() call (and I use getLogger() often.)

May be you think - hey, the slow down is negligible, why do you care? My reply is: why not to care? Why to have some code reading localStore and cookie which I do not need? IMO a good logging library should do just logging and everything else should be optional (or when default, then possible to opt-out.) And loglevel is almost like that except the persistence 😀

I understand it's not possible to change it for the current API. So I suggest:

• What about to have a possibility to disable/change it for the getLogger() calls at least?
• Think of it when doing any Loglevel 2.0 Loglevel 2.0 #119

[pimterry]

It's not called every time you call getLogger, it's called once for each different logger that you use. If you pass the same name twice, it's only called the first time around:

loglevel/lib/loglevel.js

Lines 247 to 252 in 342f103

	var logger = _loggersByName[name];
	if (!logger) {
	logger = _loggersByName[name] = new Logger(
	name, defaultLogger.getLevel(), defaultLogger.methodFactory);
	}
	return logger;

I've done a little digging, and as far as I can tell in practice you'd have to instantiate many thousands of distinct logger instances before the local storage check has even 1ms total impact. If you're doing that much logging frequently enough that this has a measurable impact (e.g. creating 1000 loggers every second, each with distinct keys) then it's likely that you're generating huge amounts of logs, and that will be significantly more expensive than any localstorage lookups.

That said, if you're committed to trying to avoid this regardless:

• It is possible to disable persistence in all subloggers today, by using Symbol keys for your logger names instead of strings. Symbol keys by definition aren't comparable between page loads, so they can never be meaningfully persisted, and all persistence is skipped:

  loglevel/lib/loglevel.js

  Lines 125 to 127 in 342f103

  	} else if (typeof name === "symbol") {
  	storageKey = undefined;
  	}

• Alternatively, I'd be open to providing options to getLogger somehow, e.g. with a { persistence: false } option. I don't agree that this is sensible for anybody from a performance perspective, but it seems like that might be a useful feature for other use cases anyway, so if you're keen on it you're welcome to investigate. Tricky question though: what happens if you call getLogger twice, with different options? If logger caching needs to deep compare the configured options objects then this is plausibly going to make getLogger more expensive for almost all use cases, not cheaper. Open to ideas there.

Honestly though, I don't think you should do any of this. LocalStorage is only expensive in comparison to bare object property lookups, loglevel is not optimized for performance to that degree at all, and unless you're working in some very specific high-performance environments the rest of your JS application is not either. There will be other much easier and more effective performance improvements in your code elsewhere!

If you are doing a quantity of logging where this truly matters, and you're really really sure that this practically affects your performance, imo you're better off rolling your own microlibrary entirely. This is a tiny lib and it has lots of small UX & cross-browser compatibility details you could skip entirely for a custom case. There's probably many other micro-optimizations and simplifications here that would save >1ms in such an application, e.g. if you're calling getLogger billions of times then I suspect removing the argument validation checks in getLogger that happen on every call would make a bigger practical difference (but changes like that doesn't make sense for loglevel itself, since there's a UX tradeoff).

[xmedeko]

I just repeat my previous comment:

May be you think - hey, the slow down is negligible, why do you care? My reply is: why not to care? Why to have some code reading localStore and cookie which I do not need?

Instead of getLogger({ persistence: false }) I would welcome some configuration like methodFactory - e.g.:

require('loglevel').persistence = {
    getLevel(loggerName) { ... },
    setLevel(loggerName, levelName) { ... }
}

Which would be more versatile.

This is not a blocker for me, just IMO such things should be configurable. And, of course, I can always do copy&paste a remove that code.