Skip to content

Restore printing sentinel for log_record.request #19172

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 13, 2025
Merged

Restore printing sentinel for log_record.request #19172

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
450273c
Print `sentinel` for `log_record.request`
MadLittleMods Nov 12, 2025
70d9285
Add changelog
MadLittleMods Nov 12, 2025
21bab18
Fix lints
MadLittleMods Nov 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/19172.misc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Restore printing `sentinel` for the log record `request` when no logcontext is active.
29 changes: 17 additions & 12 deletions synapse/logging/context.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -619,19 +619,24 @@ def filter(self, record: logging.LogRecord) -> Literal[True]:
True to include the record in the log output.
"""
context = current_context()
record.request = self._default_request
Copy link
Contributor Author

@MadLittleMods MadLittleMods Nov 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To explain the bug, we would default record.request to an empty string here, and then when we later used safe_set("request", str(context)) below, it would see our default value and not clobber it to sentinel.

This only results in an empty string getting printed whereas the prior art was to print sentinel which this PR restores.


# 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
# type-ignore: `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:
#
# Add some default values to avoid log formatting errors.
if context is None:
record.request = self._default_request # type: ignore[unreachable]

# 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"

# Otherwise, in the normal, expected case, fill in the log record attributes
# from the logcontext.
else:

def safe_set(attr: str, value: Any) -> None:
"""
Expand Down