Be mindful of other logging context filters in 3rd-party code (#19068)
Be mindful that Synapse can be run alongside other code in the same Python process. We shouldn't overwrite fields on given log record unless we know it's relevant to Synapse. (no clobber) ### Background 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. "Per-tenant logging" tracked internally by https://github.com/element-hq/synapse-small-hosts/issues/48
This commit is contained in:
1
changelog.d/19068.misc
Normal file
1
changelog.d/19068.misc
Normal file
@@ -0,0 +1 @@
|
||||
Be mindful of other logging context filters in 3rd-party code and avoid overwriting log record fields unless we know the log record is relevant to Synapse.
|
||||
@@ -604,25 +604,57 @@ class LoggingContextFilter(logging.Filter):
|
||||
self._default_request = request
|
||||
|
||||
def filter(self, record: logging.LogRecord) -> Literal[True]:
|
||||
"""Add each fields from the logging contexts to the record.
|
||||
"""
|
||||
Add each field from the logging context to the record.
|
||||
|
||||
Please be mindful of 3rd-party code outside of Synapse (like in the case of
|
||||
Synapse Pro for small hosts) as this is running as a global log record filter.
|
||||
Other code may have set their own attributes on the record and the log record
|
||||
may not be relevant to Synapse at all so we should not mangle it.
|
||||
|
||||
We can have some defaults but we should avoid overwriting existing attributes on
|
||||
any log record unless we actually have a Synapse logcontext (not just the
|
||||
default sentinel logcontext).
|
||||
|
||||
Returns:
|
||||
True to include the record in the log output.
|
||||
"""
|
||||
context = current_context()
|
||||
record.request = self._default_request
|
||||
record.server_name = "unknown_server_from_no_context"
|
||||
|
||||
# Avoid overwriting an existing `server_name` on the record. This is running in
|
||||
# the context of a global log record filter so there may be 3rd-party code that
|
||||
# adds their own `server_name` and we don't want to interfere with that
|
||||
# (clobber).
|
||||
if not hasattr(record, "server_name"):
|
||||
record.server_name = "unknown_server_from_no_logcontext"
|
||||
|
||||
# context should never be None, but if it somehow ends up being, then
|
||||
# we end up in a death spiral of infinite loops, so let's check, for
|
||||
# robustness' sake.
|
||||
if context is not None:
|
||||
record.server_name = context.server_name
|
||||
|
||||
def safe_set(attr: str, value: Any) -> None:
|
||||
"""
|
||||
Only write the attribute if it hasn't already been set or we actually have
|
||||
a Synapse logcontext (indicating that this log record is relevant to
|
||||
Synapse).
|
||||
"""
|
||||
if context is not SENTINEL_CONTEXT or not hasattr(record, attr):
|
||||
setattr(record, attr, value)
|
||||
|
||||
safe_set("server_name", context.server_name)
|
||||
|
||||
# Logging is interested in the request ID. Note that for backwards
|
||||
# compatibility this is stored as the "request" on the record.
|
||||
record.request = str(context)
|
||||
safe_set("request", str(context))
|
||||
|
||||
# Add some data from the HTTP request.
|
||||
request = context.request
|
||||
# The sentinel logcontext has no request so if we get past this point, we
|
||||
# know we have some actual Synapse logcontext and don't need to worry about
|
||||
# using `safe_set`. We'll consider this an optimization since this is a
|
||||
# pretty hot-path.
|
||||
if request is None:
|
||||
return True
|
||||
|
||||
|
||||
Reference in New Issue
Block a user