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

Reorder logging levels to reduce debug noise #96

Open
cbovis opened this issue Jul 30, 2019 · 10 comments · May be fixed by #97 or #105
Open

Reorder logging levels to reduce debug noise #96

cbovis opened this issue Jul 30, 2019 · 10 comments · May be fixed by #97 or #105

Comments

@cbovis
Copy link

cbovis commented Jul 30, 2019

Is your feature request related to a problem? Please describe.
Currently I find it difficult to effectively reduce noise in my signale logging due to how the logging levels are set up.

Traditionally with logging libraries I've used, the debug level would be higher in the chain than info. This allows me to set logging level as info and ignore all debug information. From what I can see, this isn't possible in signale because setting log level to info will include debug (and setting to debug will exclude info).

Describe the solution you'd like
Follow the conventional logging levels used by libraries such as winston (https://github.com/winstonjs/winston#logging-levels), with the priority order as follows:

  • error
  • warn
  • info
  • timer
  • debug

Alternatively provide an array based API, allowing specific levels to be given (e.g. { logLevel: ['error', 'info', 'timer'] }). This would help avoid breaking changes but would be a slightly more complex solution.

@kke
Copy link

kke commented Aug 22, 2019

I agree, the documented order (btw it's documented twice in the README, the latter one makes more sense) is very unuseful.

I would expect logLevel: "info" to output everything that is info or above, not debug. I would expect logLevel: "debug" to output everything.

Anyway, I don't see the logLevel actually making any difference. Changing the logLevel to any of the documented levels, I still get messages of all levels in the output.

@toverux
Copy link

toverux commented Sep 9, 2019

Yeah, migrating to signale, I've immediately stumbled upon this!
I'm surprised there is only this issue with so little reactions.
Also, a verbose level is a must have. I had to replace my .verbose() calls from winston with .info(), too.

@cbovis
Copy link
Author

cbovis commented Sep 10, 2019

Any thoughts on this @klaussinani? I'm considering migration to a different logging library because of this limitation.

@b4nst
Copy link

b4nst commented Sep 11, 2019

I agree that this order make no sense and is very limiting when using this lib. I would add that the success level should appear under info :

level types expected to output
error fatal + error
warn above + warn
info above + info + success
timer above + time + timeEnd
debug above + debug

@b4nst b4nst linked a pull request Sep 11, 2019 that will close this issue
@cspotcode
Copy link

Agreed; signale's ordering seems wrong.

The log levels are listed here: https://github.com/klaussinani/signale/blob/master/src/signale.js#L90-L98

This is contrary to, for example, https://github.com/winstonjs/winston#logging-levels, which is apparently referring to an RFC. Regardless, Winston's ordering is more intuitive to me.

semantic-release seems to workaround this by re-declaring the types to a simpler subset, and perhaps avoiding log levels altogether? (I haven't thoroughly read their code; maybe I'm wrong about this)

IMO the best workaround is to subclass Signale and use that instead. You'll need to:
a) override the _logLevels getter to return your custom level ordering.
b) override scope() to return new MySubclassOfSignale()
c) If you've added or renamed log levels, re-declare the types to match the new names. You can start with require('signale/types') and modify as needed.

If we devise a nice preset, we can publish it as a library.

@rafaelrpinto
Copy link

I was in the middle of the refactoring from winston to this library when I hit this issue and had to rollback.
Unfortunately the current log levels make it impossible for us to use this :(

@cspotcode
Copy link

@rafaelrpinto since you made it halfway through the refactor, did you try the subclassing solution I proposed?

@rafaelrpinto
Copy link

@cspotcode

since you made it halfway through the refactor, did you try the subclassing solution I proposed?

No as we avoid depending on workarounds as much as possible.

But thanks for sharing your solution.

@cspotcode
Copy link

That makes sense. They should add that mechanism to their official API so it's not a workaround.

anru added a commit to anru/signales that referenced this issue Apr 8, 2020
What's inside:

1. Added the "alert" logger, it is intended for messages that must be seen, for example, to manually catch an error.
2. The "debug" level now has a low priority and is not so visually highlighted: it is intended for unimportant messages that are usually not needed, but which would be useful to have in the system if something goes wrong.
3. In case someone wants to inherit from the "Signale" class, the "clone" method now correctly creates an instance of the current class with which it was created.
4. Added the ability to set your own log levels, without having to inherit to override the "_logLevels" getter.

Fixes klaudiosinani#96
@anru anru linked a pull request Apr 8, 2020 that will close this issue
anru added a commit to anru/signales that referenced this issue Apr 8, 2020
What's inside:

1. Added the "alert" logger, it is intended for messages that must be seen, for example, to manually catch an error.
2. The "debug" level now has a low priority and is not so visually highlighted: it is intended for unimportant messages that are usually not needed, but which would be useful to have in the system if something goes wrong.
3. In case someone wants to inherit from the "Signale" class, the "scope" method now correctly creates an instance of the current class with which it was created.
4. Added the ability to set your own log levels, without having to inherit to override the "_logLevels" getter.

Fixes klaudiosinani#96
anru added a commit to anru/signales that referenced this issue Apr 21, 2020
What's inside:

1. Added the "alert" logger, it is intended for messages that must be seen, for example, to manually catch an error.
2. The "debug" level now has a low priority and is not so visually highlighted: it is intended for unimportant messages that are usually not needed, but which would be useful to have in the system if something goes wrong.
3. In case someone wants to inherit from the "Signale" class, the "scope" method now correctly creates an instance of the current class with which it was created.
4. Added the ability to set your own log levels, without having to inherit to override the "_logLevels" getter.

Fixes klaudiosinani#96
@anru
Copy link

anru commented Apr 21, 2020

Ok, if someone wanted this issue fixed right now I made a fork and published npm package signales with #105 PR merged into it.

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

Successfully merging a pull request may close this issue.

7 participants