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

Adds option to use non-global verbosity per logger. #26

Merged
merged 1 commit into from
Jul 14, 2022

Conversation

iamcalledrob
Copy link
Contributor

Other loggers included with logr already support this (e.g. testr). A pointer is used for Verbosity in order to provide differentiation of the zero value of Verbosity from log level 0.

Usage example:

level := 10
l := stdr.NewWithOptions(log.Default(), stdr.Options {
  Verbosity: &level,
})

@thockin
Copy link
Collaborator

thockin commented May 7, 2022

We alreayd have the global verbosity - what is the use-case for this? Can you show me what you are trying to solve or express?

@thockin thockin self-assigned this May 7, 2022
@thockin
Copy link
Collaborator

thockin commented Jul 6, 2022

Let me know if you want to come back to this

@thockin thockin closed this Jul 6, 2022
@iamcalledrob
Copy link
Contributor Author

Hey, sorry about the slow follow-up—I missed the original notification.

This introduces non-global verbosity, so I can have multiple instances of stdr in my project that log with different configurations. Unless I'm misunderstanding something, currently the only way to change the verbosity is at a global level.

This means that if I can't reduce the volume of one package's logging vs. another if I use stdr. For example:

// By default, we want to process all logs <= level 10
stdr.SetVerbosity(10)

normalService.logger = stdr.NewWithOptions(log.Default(), stdr.Options {})

// noisyService logs a lot of unimportant stuff at levels > 5, filter those out
// **Without this PR, it's not possible to do this.**
lvl := 5
noisyService.logger = stdr.NewWithOptions(log.Default(), stdr.Options {
  Verbosity: &lvl,
})

I can already do this with testr, as follows:

normalService.logger = testr.NewWithOptions(t, testr.Options {
  Verbosity: 10
})

normalService.logger = testr.NewWithOptions(t, testr.Options {
  Verbosity: 5
})

This PR brings the functionality of stdr in-line with what testr (and others) can do, without introducing a breaking API change (hence the use of the pointer)

@thockin
Copy link
Collaborator

thockin commented Jul 9, 2022

I see. I am not necessarily AGAINST this, but I think there's another way.

// By default, we want to process all logs <= level 10
stdr.SetVerbosity(10)

normalService.logger = stdr.NewWithOptions(log.Default(), stdr.Options {})

// noisyService logs a lot of unimportant stuff at levels > 5, filter those out
// **Without this PR, it's not possible to do this.**
lvl := 5
noisyService.logger = normalService.logger.V(5)

Does that work for you?

@thockin
Copy link
Collaborator

thockin commented Jul 14, 2022

ping again, ICYMI - I know I miss a lot of github emails :)

@iamcalledrob
Copy link
Contributor Author

I think it's similar, but not quite the same (unless I'm misunderstanding—very possible!)

noisyService.logger = normalService.logger.V(5)

^ this has the effect of "everything that noisyService logs will become less important"

lvl := 5
noisyService.logger = stdr.NewWithOptions(log.Default(), stdr.Options {
  Verbosity: &lvl,
})

^ whereas this has the effect of "everything that noisyService logs > lvl5 will be filtered out"

My particular use-case is to define a few log level constants (e.g. Default, Debug, Trace), and then use the Verbosity setting to decide what the cut-off point is for a given service.

For example, I might be debugging an issue and want to enable Debug logs everywhere except for noisyService, which is just really spammy at that level. If I were to increase the V-level of the logger, then everything noisyService logs will be made less important, including perhaps the important logs that were at V0 before.

There's a good chance this is me attempting some kind of anti-pattern with logr, so let me know if that's the case :)

@thockin
Copy link
Collaborator

thockin commented Jul 14, 2022

I see. You really want per-logger verbosity. It seems very tedious to manage in a real application - if you have more than two or three of these you have to go an update them all. That said, I'll reconsider it.

@thockin thockin reopened this Jul 14, 2022
@thockin thockin merged commit 96bad1d into go-logr:master Jul 14, 2022
@richardfuchs
Copy link

Will there be a new release that includes this feature?

@thockin
Copy link
Collaborator

thockin commented Dec 20, 2022 via email

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 this pull request may close these issues.

3 participants