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

Feature request; GetV() function #149

Closed
uablrek opened this issue Aug 30, 2022 · 3 comments · Fixed by #163
Closed

Feature request; GetV() function #149

uablrek opened this issue Aug 30, 2022 · 3 comments · Fixed by #163

Comments

@uablrek
Copy link

uablrek commented Aug 30, 2022

The glog.V() function allows;

if glog.V(2) { glog.Info("log this") }

This is the reason;

The second form is shorter but the first is cheaper if logging is off because it does not evaluate its arguments.

To not evaluate the arguments can be very important for instance if a function is called that searches 100,000 objects for some crucial trace info. You do not want to to that if not on trace level, like V(2). Even though the glog implementation is pretty cool I realize it may not be feasable for logr, but a possibility to read the V-level should be provided.

@pohly
Copy link
Contributor

pohly commented Aug 30, 2022

The same works with logr:

   if loggerV := logger.V(5); loggerV.Enabled() {
       // Do something expensive.
      loggerV.Info("here's an expensive log message", ...)
   }

Note that the log call reuses the result of logger.V(5). That ensures that logger implementations which record the log level pick the right one.

@uablrek
Copy link
Author

uablrek commented Aug 30, 2022

Thanks! But this is not really intuitive (at least not for me) so IMHO it should be documented with an example.

@uablrek uablrek closed this as completed Aug 30, 2022
@pohly
Copy link
Contributor

pohly commented Aug 30, 2022

Feel free to add it somewhere, PR welcome.

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.

2 participants