-
Notifications
You must be signed in to change notification settings - Fork 387
Description
Remove sentinel
logcontext where we log in Synapse
Since I plan to tackle this in a piece-meal fashion, this issue serves as a central place to link back to and organize the work.
Why
Lines 71 to 81 in 9cc4001
Ideally, nothing from the Synapse homeserver would be logged against the `sentinel` | |
logcontext as we want to know which server the logs came from. In practice, this is not | |
always the case yet especially outside of request handling. | |
Global things outside of Synapse (e.g. Twisted reactor code) should run in the | |
`sentinel` logcontext. It's only when it calls into application code that a logcontext | |
gets activated. This means the reactor should be started in the `sentinel` logcontext, | |
and any time an awaitable yields control back to the reactor, it should reset the | |
logcontext to be the `sentinel` logcontext. This is important to avoid leaking the | |
current logcontext to the reactor (which would then get picked up and associated with | |
the next thing the reactor does). |
(docs updated in #18900)
Why are we doing this now?
This is spawning from #18868
As part of Element's plan to support a light form of vhosting (virtual host) (multiple instances of Synapse in the same Python process), we're currently diving into the details and implications of running multiple instances of Synapse in the same Python process.
Removing our sentinel
logcontext usage is a prerequisite for #18868. Logging with the sentinel
logcontext means we won't know which server the log came from.
"Per-tenant logging" tracked internally by https://github.com/element-hq/synapse-small-hosts/issues/48
Dev notes
Places to look out for
- Wherever we have a callback from the reactor, replace with
Clock
utility that manages the logcontext (and add lints) with PreserveLoggingContext():
which the default here is using thesentinel
logcontext- Ensure usages of
run_as_background_process
usemake_deferred_yieldable
if they wait on the result -> #18938 - Ensure usages of
run_in_background
usemake_deferred_yieldable
if they wait on the result -> #18937
PR list
- Remove
sentinel
logcontext where we log insetup
,start
and exit #18870 - Wrap the Rust HTTP client with
make_deferred_yieldable
#18903 - Remove
sentinel
logcontext inClock
utilities (looping_call
,looping_call_now
,call_later
) #18907 - Fix
run_in_background
not be awaited properly causingLoggingContext
problems #18937 - Fix
run_as_background_process
not be awaited properly causingLoggingContext
problems #18938 - Introduce
Clock.call_when_running(...)
to include logcontext by default #18944 - Introduce
Clock.add_system_event_trigger(...)
to include logcontext by default #18945 - Fix
run_coroutine_in_background(...)
incorrectly handling logcontext #18964
Historical related PR's before this issue: