Skip to content

Conversation

MadLittleMods
Copy link
Contributor

@MadLittleMods MadLittleMods commented Sep 19, 2025

Introduce Clock.add_system_event_trigger(...) to wrap system event callback code in a logcontext, ensuring we can identify which server generated the logs.

Background:

Ideally, nothing from the Synapse homeserver would be logged against the `sentinel`
logcontext as we want to know which server the logs came from. In practice, this is not
always the case yet especially outside of request handling.
Global things outside of Synapse (e.g. Twisted reactor code) should run in the
`sentinel` logcontext. It's only when it calls into application code that a logcontext
gets activated. This means the reactor should be started in the `sentinel` logcontext,
and any time an awaitable yields control back to the reactor, it should reset the
logcontext to be the `sentinel` logcontext. This is important to avoid leaking the
current logcontext to the reactor (which would then get picked up and associated with
the next thing the reactor does).

Also adds a lint to prefer Clock.add_system_event_trigger(...) over reactor.addSystemEventTrigger(...)

Part of #18905

Dev notes

Adding logcontext to looping_call and call_later in #18907

Add logcontext to call_when_running in #18944

Mypy lint pattern came from #18828

Todo

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)

@MadLittleMods MadLittleMods marked this pull request as ready for review September 19, 2025 23:09
@MadLittleMods MadLittleMods requested a review from a team as a code owner September 19, 2025 23:09
Copy link
Contributor

@reivilibre reivilibre left a comment

Choose a reason for hiding this comment

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

nice approach

Base automatically changed from madlittlemods/hs-clock-utils-logcontext to develop September 22, 2025 15:28
…add-system-event-trigger

Conflicts:
	scripts-dev/mypy_synapse_plugin.py
	synapse/util/clock.py
@MadLittleMods MadLittleMods merged commit d05f44a into develop Sep 22, 2025
76 of 78 checks passed
@MadLittleMods MadLittleMods deleted the madlittlemods/hs-clock-utils-log-context-add-system-event-trigger branch September 22, 2025 16:47
@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.

2 participants