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

Introduce Mute/None log level #729

Open
kriskowal opened this issue Jul 9, 2019 · 5 comments
Open

Introduce Mute/None log level #729

kriskowal opened this issue Jul 9, 2019 · 5 comments

Comments

@kriskowal
Copy link

YARPC observability middleware allow users to specify a log level for (inbound, outbound) x (success, failure, application error) traffic edges as of yarpc/yarpc-go#1763. Users would like to be able to mute certain edges, such that they can see their own debug messages without seeing YARPC’s.

Currently, the best advice we can provide is to raise the development log level to "info" to suppress the YARPC debug logs, then raise application log messages to "info" during development, requiring the application debug log level to be configurable. This is acceptable but unpalatable.

To this end, would it be wise to introduce a new Mute or None debug level with numeric value -2? If so, I can initiate a change.

@prashantv
Copy link
Collaborator

Some related issues: #680 & #713

I think this is a slightly different request -- it's asking for a level that will never be logged, which allows for easily disabling logging via level. If we do this:

  • we should probably use a very negative number in case we want to add more levels (like trace) in future. So something like -1000
  • I'd prefer None as the name, as the level is used both for logs but also for configuring minimum logger levels. A minimum of Mute seems confusing, whereas a minimum level of None makes more sense
  • This only helps if the caller always uses Check for logging, as it's the only way to pass a level parameter.
  • We will need to make sure the sampler and other similar components handle this level fine

This makes sense at a high level to me, we should get another opinion too.

@abhinav
Copy link
Collaborator

abhinav commented Jul 10, 2019

A None level (with a large negative value) sounds reasonable to me.

@jcorbin
Copy link
Contributor

jcorbin commented Jul 14, 2019

A zap.NoneLevel = -1000 (or so) seems reasonable to me; in addition to sampler (as mentioned by prashant) I suppose we should add guards to all care implementations that zap ships, e.g. zapcore.ioCore.Write, zapcore.multiCore.Write, etc).

@LeviMatus
Copy link

If this is still something that is still desired then I'd like to take it up.

@abhinav
Copy link
Collaborator

abhinav commented Feb 18, 2021

Since adding a new level would be a breaking change, we'd like to avoid doing so in 1.0.

We should defer this to a 2.0, whenever that happens. (#388)

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

No branches or pull requests

6 participants