From 41a2762e588013887682cdd5a997f6d5b8aa6c2b Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 31 Oct 2025 10:12:05 -0500 Subject: [PATCH] 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 --- changelog.d/19068.misc | 1 + synapse/logging/context.py | 40 ++++++++++++++++++++++++++++++++++---- 2 files changed, 37 insertions(+), 4 deletions(-) create mode 100644 changelog.d/19068.misc diff --git a/changelog.d/19068.misc b/changelog.d/19068.misc new file mode 100644 index 000000000..9e5c34b60 --- /dev/null +++ b/changelog.d/19068.misc @@ -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. diff --git a/synapse/logging/context.py b/synapse/logging/context.py index 86e994cbb..919493d1a 100644 --- a/synapse/logging/context.py +++ b/synapse/logging/context.py @@ -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