Disable pylint check logging-fstring-interpolation globally

[leandro-lucarella-frequenz]

pylint warns against using f-strings in logging functions because the built-in interpolation provides an optimization: it does not do the interpolation if the message is not emitted. This is inconvenient because f-strings have been the recommended way to do interpolation for a long time now, and having to resort to the old interpolation style means developers need to know 2 ways to interpolate strings, which are very different.

Doing a quick search, it looks like the performance gain is really negligible, so I propose to disable the pylint check logging-fstring-interpolation globally in the pyproject.toml config and use f-strings instead for logging.

For hot paths that need logging (which in general will be a bad idea anyway) we can either use the old interpolation, or better, use if logger.isEnabledFor(logging.DEBUG): to avoid the whole call to the logger if it's not going to be logged.

Just for completeness, here is a comment mentioning a couple of reasons why using logging interpolation could be good regardless of the performance issue:

• If interpolation fails, it will be logged instead of raising an exception
• Things like Sentry can aggregate log messages. If you interpolate before logging, Sentry would see it as one event [0]

[shsms]

What do you think about keeping these in the sdk, but disabling them in external actors/application code by default.

Then users get the freedom to log as they prefer, and don't have to be affected by out decisions. Is it really a big inconvenience to do old style logging?

[leandro-lucarella-frequenz]

I do think using different interpolation formats is an unnecessary burden (again for me the ideal would be that there is an option to make logging use the new syntax, but on a google search nobody seems to be even talking about this, so I don't see happening any time soon).

I do agree we shouldn't sacrifice performance in the SDK if possible. I was just thinking that in debug messages (which are usually disabled), it might also be common to print something that generates a lot of output, like maybe a whole file or whatever. That would be bad.

In those cases we can use this:

For hot paths that need logging (which in general will be a bad idea anyway) we can either use the old interpolation, or better, use if logger.isEnabledFor(logging.DEBUG): to avoid the whole call to the logger if it's not going to be logged.

We'll lose some linting because a linter can't tell when something could be expensive (and unlikely to be logged, and logged many times), but we don't necessarily have to sacrifice (too much) performance if we go this route.

But yeah, now I'm undecided. 🤔

[matthias-wende-frequenz]

I'd vote for keeping the old style logging in loggers, at least in the sdk, too. I don't feel it's inconvenient and we have linters inside our text editors as immediate reminders. The only disadvantage I see, is that people ask questions about why this linter rule exists.

[leandro-lucarella-frequenz]

2.5 against 0.5...

[christianparpart]

NB: I was just running into this in an actor code, where I inherently disagreed with that warning. I am generally for globally disabling this logging-fstring-interpolation :)

[leandro-lucarella-frequenz]

We are still 2.5 against 1.5 😆

[mathias-baumann-frequenz]

I thought pressing the upwards-arrow would count my vote, but I guess I have to explicitly write a comment then :P

[mathias-baumann-frequenz]

(I mean, I count three up-votes on your post, so no idea why you say 0.5 or 1.5)

[leandro-lucarella-frequenz]

Yeah, no, I didn't see it. It is a tie now then 2.5 vs 2.5.

But I think now I'm more leaning towards leaving it. For me the case is debug messages, the most likely to to do expensive interpolation and to never been shown could impact performance inadvertently.

For actors/apps, specially ones that are not CPU intensive, I think I'm fine with this though.

[mathias-baumann-frequenz]

I suppose this part here is an argument against f-strings in logs: http://tohyongcheng.github.io/python/2016/02/20/testing-your-logger-in-python-unit-tests.html