Ilan:
Technically it may be true (although frankly I don’t know for sure), but
practically at warn level any extra work isn’t worth the confusion.
What do I mean by that? Well, we built up a huge debt with all sorts of debug
and info and even trace messages that either concatenated strings or called
methods or… See LUCENE-7788 for the amount of code that had to be changed.
So when I wrote the checker, I decided to flag all the concatenations in the
code reasoning that for warn level messages, any extra work was so trivial that
you couldn’t really measure it overall, and it’d be worth it to keep from
“broken window" issues. I.e. someone sees log.warn(“whatever” + “whenever”) and
thinks it’s OK to use that pattern for debug, trace or info.
I don’t particularly care if warn messages are a little inefficient on the
theory that there should be so few of them that it’s not measurable.
All that said, this is absolutely a judgement call that I made trying to
balance the technical with the practical. Given the number of people who
contribute to the code, I think it’s worthwhile to keep certain patterns out of
the code. Especially given how obscure logging costs are. The difference
between 'log.trace(“message {}”, object.toString())’ and 'log.trace(“message
{}”, object)’ for instance is unknown to a _lot_ of developers. Including me
before I started looking at logging in general ;)
Best,
Erick
> On May 20, 2020, at 1:05 PM, Ilan Ginzburg <[email protected]> wrote:
>
> This might have been discussed previously but since I'm seeing this
> behavior...
>
> Gradle precommit check does not allow code such as:
> log.warn("Only in tree one: " + t1);
>
> And forces changing it into:
> log.warn("Only in tree one: {}", t1);
>
> I do understand such constraints for debug level logs to not pay the
> concatenation cost if the log is not output (unless the log is inside an if
> block testing debug level), but for logs generally turned on (which I assume
> warn logs are), this is counter productive: the second form is less efficient
> during execution than the first one.
>
> Ilan
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
