Skip to content

Conversation

@MadLittleMods
Copy link
Contributor

@MadLittleMods MadLittleMods commented Nov 12, 2025

Restore printing sentinel for log_record.request

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
  2. Start Synapse: poetry run synapse_homeserver --config-path homeserver.yaml
  3. Notice the asyncio - 64 - DEBUG - - Using selector: EpollSelector log line (notice empty string - -)
  4. With this PR, the log line will be asyncio - 64 - DEBUG - sentinel - Using selector: EpollSelector (notice sentinel)

Pull Request Checklist

  • Pull request is based on the develop branch
  • Pull request includes a changelog file. The entry should:
    • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
    • Use markdown where necessary, mostly for code blocks.
    • End with either a period (.) or an exclamation mark (!).
    • Start with a capital letter.
    • Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
  • Code style is correct (run the linters)

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

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.

@MadLittleMods MadLittleMods changed the title Print sentinel for log_record.request Restore sentinel for log_record.request Nov 12, 2025
@MadLittleMods MadLittleMods changed the title Restore sentinel for log_record.request Restore printing sentinel for log_record.request Nov 12, 2025
@MadLittleMods MadLittleMods marked this pull request as ready for review November 12, 2025 22:35
@MadLittleMods MadLittleMods requested a review from a team as a code owner November 12, 2025 22:35
@MadLittleMods MadLittleMods merged commit b9dda0f into develop Nov 13, 2025
78 of 80 checks passed
@MadLittleMods MadLittleMods deleted the madlittlemods/print-sentinel branch November 13, 2025 15:57
@MadLittleMods
Copy link
Contributor Author

Thanks for the review @reivilibre 🐆

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants