Skip to content

Commit b9dda0f

Browse files
Restore printing sentinel for log_record.request (#19172)
This was unintentionally changed in #19068. There is no real bug here. Without this PR, we just printed an empty string for the `sentinel` logcontext whereas the prior art behavior was to print `sentinel` which this PR restores. Found while staring at the logs in #19165 ### Reproduction strategy 1. Configure Synapse with [logging](https://github.com/element-hq/synapse/blob/df802882bb2ec7d52d5c064c20531fe5a5b263b1/docs/sample_log_config.yaml) 1. Start Synapse: `poetry run synapse_homeserver --config-path homeserver.yaml` 1. Notice the `asyncio - 64 - DEBUG - - Using selector: EpollSelector` log line (notice empty string `- -`) 1. With this PR, the log line will be `asyncio - 64 - DEBUG - sentinel - Using selector: EpollSelector` (notice `sentinel`)
1 parent 938c974 commit b9dda0f

2 files changed

Lines changed: 18 additions & 12 deletions

File tree

changelog.d/19172.misc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Restore printing `sentinel` for the log record `request` when no logcontext is active.

synapse/logging/context.py

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -619,19 +619,24 @@ def filter(self, record: logging.LogRecord) -> Literal[True]:
619619
True to include the record in the log output.
620620
"""
621621
context = current_context()
622-
record.request = self._default_request
623-
624-
# Avoid overwriting an existing `server_name` on the record. This is running in
625-
# the context of a global log record filter so there may be 3rd-party code that
626-
# adds their own `server_name` and we don't want to interfere with that
627-
# (clobber).
628-
if not hasattr(record, "server_name"):
629-
record.server_name = "unknown_server_from_no_logcontext"
630-
631-
# context should never be None, but if it somehow ends up being, then
632-
# we end up in a death spiral of infinite loops, so let's check, for
622+
# type-ignore: `context` should never be `None`, but if it somehow ends up
623+
# being, then we end up in a death spiral of infinite loops, so let's check, for
633624
# robustness' sake.
634-
if context is not None:
625+
#
626+
# Add some default values to avoid log formatting errors.
627+
if context is None:
628+
record.request = self._default_request # type: ignore[unreachable]
629+
630+
# Avoid overwriting an existing `server_name` on the record. This is running in
631+
# the context of a global log record filter so there may be 3rd-party code that
632+
# adds their own `server_name` and we don't want to interfere with that
633+
# (clobber).
634+
if not hasattr(record, "server_name"):
635+
record.server_name = "unknown_server_from_no_logcontext"
636+
637+
# Otherwise, in the normal, expected case, fill in the log record attributes
638+
# from the logcontext.
639+
else:
635640

636641
def safe_set(attr: str, value: Any) -> None:
637642
"""

0 commit comments

Comments
 (0)