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

Debug levels and scoped to modules unreliably works #30

Open
toddb opened this issue Jun 27, 2020 · 14 comments
Open

Debug levels and scoped to modules unreliably works #30

toddb opened this issue Jun 27, 2020 · 14 comments

Comments

@toddb
Copy link

toddb commented Jun 27, 2020

I wondering if it is just me. I want to change (and also have set) log levels through local storage (in Chrome). It is described here.

On occasion that I have managed to get levels and scoping to work, I have to set BOTH the log level and the scoping. This isn't documented.

For example, I have a module 'Loader' that I want to exclude:

eg I set these values in Chrome > Dev tools > Application > Local Storage < [site]:
key | value
log | debug
debug | *,-Loader

Does this resonate with anyone? Cheers in advance

ps I load ulog via anylogger

@Download
Copy link
Owner

Hi @toddb

Please get my phone number (see your other issue) so we can talk directly via What's App (or at least, so you will be able to ping me when I don't respond) because I have dozens of repos here on github and many times I just don't notice new issues and they just sit here gathering dust... I get so many notifications from Github that I cannot see the forest from the trees.

ulog is important to me and you are (for me) an important user because you are actually reporting issues and providing valuable feedback. So for you I am willing to go the extra mile, like you have for this project.

That being said I want to close this issue. Because v2 is... well just broken atm. I have been completely rewriting it and the current state is a mess. So for the short term, I would suggest to use ulog v1, or v2-beta.7 (8 once you do your thing) but without relying on the new features.

Once v2 is finished, it will work like this, given modules my:lib-a, my:lib-b, my:ignored-lib:

DEBUG=my:*,-my:ignored-lib

will force the log level to debug for all modules starting with my:, except for my:ignored-lib
This is basically exactly what debug does.

LOG=my:*,-my:ignored-lib=info; another=lib=warn

will set the log level to info for the same set of modules and set it to warn for another-lib. So it basically works almost exactly the same as the config for debug, except that you can specify a level value and include multiple configs by separating them with semicolons.

The combination of the two:

DEBUG=my:*,-my:ignored-lib
LOG=my:*,-my:ignored-lib=info; another=lib=warn

will force the log level to debug for all modules starting with my:, except for my:ignored-lib (so overriding the level configured in the LOG setting) and still set the level to warn for another-lib.

There will be a setting to configure formatters as well and it will work in the same way as LOG setting.

However, as it stands this functionality is not (completely) working as expected in the betas. And I just want to get v2 out based on anylogger so I don't want to spend any time on it now. So I want to close it for now, but if you still find such issues once v2 is out, please do report them (and ping me if I don't respond) and I will fix them all. Promised!

@Download
Copy link
Owner

Should work as described now.

@toddb
Copy link
Author

toddb commented Apr 27, 2021

@Download I have tried and tried and I still don't get this functionality working. Do you have it working and perhaps in a pen? This is the killer feature I need (oh, as well as typescript support #37 :-) )

@Download
Copy link
Owner

Download commented May 5, 2021

@toddb Have you checked out the tutorial?
Or, maybe the other way around, have you got a pen or repo for me where I can see what you are doing so I can maybe find the bug in ulog, or where you are using it wrong?

@Download Download reopened this May 5, 2021
@toddb
Copy link
Author

toddb commented May 5, 2021

I hadn't seen that tutorial before but apart from the lazy loading, it all looks familiar. It could be my build chain and bundling.

@Download
Copy link
Owner

Download commented May 5, 2021

If you share a project / repo / codepen, I can have a look.

It would be better if it was so easy you would not need my guidance, but I did not succeed in making it easy enough.
Ulog v2 has been in beta for 2 years now (!) but I don't care if it stays in beta for another 2 years, as long as I keep getting valuable feedback like this so I can actually improve it to the point where you can just pick it up and use it and it all works as you expect. It's clearly not there yet. But I'm stubborn and will keep going at this 😄

@Download
Copy link
Owner

Download commented May 5, 2021

Oh and if you are building with webpack it may help to look at ulogs webpack config file, because that's basically the build for the tutorial project.

@toddb
Copy link
Author

toddb commented May 9, 2021

I am going to recommend that this ticket is closed. The main issue was a library that had earlier versions of ulog and anylogger. Once I upgraded everything, I managed to get it to work. However, I wouldn't say seamlessly. There still needs typings for ulog (if you use typescript) and to include it in bundling if you dependency manage it.

Here's my context to replicate the issue. I created a brand new Vue project and added a logging message. I do this to create anylogger as a dependency unlike the samples where it is directly loaded in the browser and globally available.

Based on the sample README, only a reference to anylogger is required.

import anylogger from 'anylogger';

const log = anylogger('Logging');
log.debug('I will not log unless log level is reduced');
log.error('I should log even in default settings');

// result nothing logs

Add ulog import, still doesn't work (under my build system-let's assume "tree shaking" broadly)

import anylogger from 'anylogger';
import ulog from 'ulog';

const log = anylogger('Logging');
log.debug('I will not log unless log level is reduced');
log.error('I should log even in default settings');

// result still nothing logs (as ulog still isn't there)

Of course, the logging library is actually required to implement the interface. Without typings, it all gets hard and picked something random to log that ensures the library is bundled.

import anylogger from 'anylogger';
import ulog from 'ulog';

const log = anylogger('Logging');
// need a reference to include library and ensure no linting errors
log.debug(ulog.levels);  // ah, need typings so added `declare module 'ulog'` (or could use `ulog.d.ts`)
log.debug('I will not log unless log level is reduced');
log.error('I should log even in default settings');

// now I log

@toddb toddb closed this as completed May 9, 2021
@toddb
Copy link
Author

toddb commented May 9, 2021

Documentation says that caps DEBUG can be used to set the log levels. In local storage, keys are case sensitive.

Note: in the log form log level is case insensitive.

This issue would also relate to the complete list log, warn, info, error.

Expected Behaviour

Logging based on levels changes when local storage switches are changed

Actual Behaviour

In Chrome/Firefox/Safari, only lower case key (eg debug) is respected.

Steps to Reproduce

  1. Add to local storage case insensitive key/value pairs for logging configuration
  2. Refresh browser and see expected log messages

Solutions

Change:

  • the documentation
    • note case sensitivity
    • add a BNF
    • step out behaviours separately from configuration (eg log versus loglevel forms)
    • there is no documentation on the order of precedence in the rules.
      • Looks to be that the 'lowest' level rules (eg if I add a rule for debug and error on a module, debug always wins)
      • who wins between url and local storage?
  • the behaviour
    • check case insensitive which probably include title case

@toddb toddb reopened this May 9, 2021
@Download
Copy link
Owner

Download commented May 10, 2021

@toddb

the logging library is actually required to implement the interface. [..] picked something random to log that ensures the library is bundled.

Yes. At one single point, you should import/require ulog. The current recommendation is to do this as the very first line in your application, what Webpack et all call the 'entry point':

Add to entry point

In the entry point of your application import ulog:

index.js

import "ulog"

But maybe this recommendation is not clear enough?

Documentation says that caps DEBUG can be used to set the log levels.

Yes, the example shows it like that. And that works, but only when setting an environment variable like in the example. It does not work for localStorage and the querystring. That's a bit fuzzy I admit. The thing is that environment variables are case insensitive on Windows so cross-platform apps are sort of forced to support that. I don't think making the querystring and localStorage settings case insensitive is a good idea. These are normally case-sensitive and because it's a cross platform web standard it would be strange if they were suddenly case insensitive here. I think maybe it's better to just change the example and maybe tell people about the exception for env vars explicitly in the docs.

What do you think?

I am going to recommend that this ticket is closed.

Yes, I agree, but maybe we should change the docs? Let me keep it open just a bit longer while I wait for your recommendation on this. If we need to change the docs, I will keep this issue open as a reminder until I got around to doing that, if that's ok with you.

@toddb
Copy link
Author

toddb commented May 10, 2021

@Download I recommended and closed and then re-opened once I realised that I still had issues from the original.

  • I wouldn't close until you resolve at least the documentation

(note: the import issue is another ticket although it is related—I have updated my example to show how the documentation might be misleading)

@Download
Copy link
Owner

Download commented May 12, 2021

@toddb Yes I will try to update the docs based on your feedback. I don't think accepting case-insensitive keys on localStorage is a good idea. Honestly, I wish env vars would also simply be case sensitive as it's much simpler (especially as, afaik, there is no easy way to access env vars in a case insensitive manner on Node JS when running on Linux, because you access it like process.env['some_string'], where 'some-string' is de-facto case sensitive... But probably explaining this better in the docs can help. About the BNF, I'm not exactly sure what you would like to see there... Can you elaborate?

Of course a PR would be super helpful here, but I understand if you don't have the time for that.

the import issue is another ticket

Do you mean literally? And if so, which ticket?

@toddb
Copy link
Author

toddb commented May 12, 2021

@Download

I don't think accepting case-insensitive keys on localStorage is a good idea.

My point is that current behaviour should be documented as clearly as possible for library users. Clean and clear interfaces.

About the BNF, I'm not exactly sure what you would like to see there... Can you elaborate?

I put a link there. The configuration has a syntax which can be systematically documented. Inside it there are enumerations, defaults and optionals. You explanation is in natural language and has a particular logic. I would also like something concise and systematic.

Do you mean literally? And if so, which ticket?

I'll create one so that future users can resolve the issue until the documentation is resolved.

@Download
Copy link
Owner

About the BNF
I would also like something concise and systematic.

Ah yes agreed. But frankly it seems very daunting to me to write such a grammar.
The configuration syntax is basically an expansion of that of debug, which also doesn't have a BNF I think. Otherwise we could maybe take that and expand on it. If you would like to try writing a BNF I would happily help you get a PR merged.

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

2 participants