-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Add lazyDebugf method to process the arguments lazily #7158
Conversation
@@ -79,6 +79,34 @@ func (pl *PrefixLogger) Debugf(format string, args ...any) { | |||
|
|||
} | |||
|
|||
// LazyDebugf does info logging at verbose level 2, only | |||
// evaluating arguments if logging is enabled. | |||
func (pl *PrefixLogger) LazyDebugf(format string, args ...func() any) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this worth it?
This makes the call site:
logger.LazyDebugf("blah %s %s %v", func() { blah1() }, func() { blah2() }, func() { blah3() })
vs.
if logger.V(2) {
logger.Infof("blah %s %s %v", blah1(), blah2(), blah3())
}
Yes it's "3 lines instead of 1 line" but:
- it's not obvious that Debugf is checking V(2), and
- the usage with Lazy is really hard to read and very long because of all the extraneous closures.
I think I'd rather just delete Debugf
instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How many occurrences of Debugf do we have?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
17 i believe
Taking pause on this sometime to prioritize other issues. Will update this PR based on this comment |
Let's close this PR for now and re-open when we are actively working on it |
Fixes #7154
RELEASE NOTES: