bpo-38762: Fix edge case where logging records an incorrect processNa…

[iritkatriel]

…me by removing the flaky assumption that if multiprocessing is not in sys.modules then it must be the main process.

I rewrote the multiprocessing test because it was not checking processName at all off the main process. Now it checks with and without multiprocessing in sys.modules, and repeats that on and off the main process.

https://bugs.python.org/issue38762

[iritkatriel]

It's not great that the logger always imports multiprocessing, even for a program that only ever has one process. The only way, currently, to turn it off is to set logMultiprocessing to False. I wonder if there is another way to make it avoid the import in the majority of cases.

In fact the processName field would not even be used in such a case (by default the process name is not displayed, right?). So how about making processName a property, so that it's only calculated when it's accessed? We would need to capture the pid (with os.getpid) at the time that the LogRecord is created, so that processName can be calculated from it (this can happen in a different process).

[iritkatriel]

We could do this instead: revert back to not importing multiprocessing if it's not there.

If it's there we use it, but if it's not we set processName to something like f"pid={os.getpid()}" instead of "MainThread".

1. os is already imported in logging.

2. In most cases where multiprocessing is not imported it's because there is only one process, so processName will not likely be used as all.

3. In the edge case of shutdown or someone clearing sys.modules the processName will not be pretty, but it will at least be correct.

[bedevere-bot]

🤖 New build scheduled with the buildbot fleet by @vsajip for commit 1db80631531f0628bc015fb45e0776ee84e4e876 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

[vsajip]

It's not great that the logger always imports multiprocessing

IIRC that's why the original code was the way it was, and why I consider the case where sys.modules[multiprocessing] "disappears" should be dealt with by whoever runs into the unusual use case.

In most cases where multiprocessing is not imported it's because there is only one process, so processName will not likely be used as all.

If a user is that concerned about performance in a single-process context, they can set logging.logMultiprocessing to False, in which case processName will be set to None. I've just noticed that this optimising information appears to missing from the documentation, but that can easily be rectified.

[iritkatriel]

I think this PR needs a minor revision to the doc because after this change the Current Process Name will be added to the log always (and not only when there are multiple processes). I will push another commit soon.

[iritkatriel]

I did some bad pull/rebase and pushed other stuff into this branch. Let me sort it out.

[iritkatriel]

Sorry about that, it's fixed now. It added a bunch of reviewers to the PR - please ignore, it was an accident.

[iritkatriel]

I don't know what the win64 asyncio test failure is about. Will try to close and reopen to get a new test run.

[iritkatriel]

I've created PR 22142 with an alternative solution that doesn't import multiprocessing, and instead just doesn't calculate prcocessName if multiprocessing is not imported.

I'm leaning towards the solution of PR 22142, let's discuss.