log,lint: help avoid unintended allocations using log.V and its friends

[ajwerner]

Is your feature request related to a problem? Please describe.

The purpose of log.V and, to a lesser extent (maybe), log.VEventf, is to avoid the performance overhead of very verbose logging when it isn't requested. In practice, because of the way go escape analysis works, we often find these calls leading to variables escaping to the heap because of calls in branches that are almost never taken. The best tactic in these cases is likely to copy the struct prior to logging it; this generally works though is onerous to the point that if there isn't a linter it's probably not going to happen until somebody looks at a profile. It's also important that a pointer to the object get passed in common cases to ensure that redaction happens properly. This issue is for us to build the nudge in the way that makes the most sense.

Describe the solution you'd like

One option is to build an analysis linter that checks if we're in a log.V branch and then checks to see if non-pointer types are getting passed to a logging function, particularly by reference. This linter may also give some warnings for cases where things are being passed by value and the reference to that type implements some handy logging interfaces related to redaction.

There's a hazard that this linter is too annoying in two dimensions:

• If the object already escapes, we don't really care. Unfortunately this seems hard to know. The analysis framework doesn't have tools to tell us (currently, I'm trying to engage this point with the go community). Also, even if it did, we'd probably need to go back to compiling the whole compilation unit to find the answer and that wasn't really okay.
• There are pieces of performance insensitive code that really don't care.

Describe alternatives you've considered

We could do something that just looks at the gcflags output on the whole program instead and then cross reference with the AST somehow.

Jira issue: CRDB-3073

[github-actions[bot]]

We have marked this issue as stale because it has been inactive for 18 months. If this issue is still relevant, removing the stale label or adding a comment will keep it active. Otherwise, we'll close it in 10 days to keep the issue queue tidy. Thank you for your contribution to CockroachDB!