Port LogContext to Rust#19979
Draft
erikjohnston wants to merge 5 commits into
Draft
Conversation
Pin the Python-observable behaviour of the logcontext machinery ahead of porting it to Rust: - LogContextErrorMessageTestCase pins the exact logcontext_error message wording, argument order and the conditions that trigger each abuse warning. These are load-bearing: tests and downstream log scraping match on them. Messages interpolating a context via %r embed the object's repr (id/address), so expected strings are reconstructed from the same live objects rather than hard-coded. - LoggingContextFilterTestCase pins LoggingContextFilter — the entire observable surface for logging: record attributes are filled from a real logcontext, and (crucially for 3rd-party code) under the sentinel only absent attributes get defaults. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JFbRtswu7rsHrttJFauUUb
The first slice of moving the logcontext machinery into the Rust extension: ContextResourceUsage becomes a native class in the new synapse.synapse_rust.logcontext module (rust/src/logging/context.rs), re-exported from synapse.logging.context so callers are unchanged. The public attribute surface, operators and repr are a compatibility contract with the Python callers (Measure, request/background-process metrics, the task scheduler, ...). Keeping the tracker native lets the upcoming switch machinery do its rusage accounting without allocating a Python object per operation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JFbRtswu7rsHrttJFauUUb
The "current logcontext" slot moves from a Python threading.local into the Rust extension, together with a native port of LoggingContext. A Python thread-local is invisible to Rust: each tokio worker thread would see its own slot, permanently at the sentinel, so logging emitted from Rust could not be attributed to the request that caused it. This lays the storage groundwork; a follow-up change gives tokio tasks a task-scoped capture (see the module-doc TODO). Design notes, for review: - The slot is typed: Option<Py<LoggingContext>>, with None representing the sentinel. The _Sentinel class and SENTINEL_CONTEXT singleton stay pure Python, unchanged; thin wrappers on current_context and set_current_context in synapse.logging.context convert between the singleton and None at the boundary, so no Rust code ever sees or produces the sentinel object. pyo3's extraction enforces the type: anything that is not a LoggingContext (or subclass) or None raises TypeError. - The accounting policy is native too: set_current_context reads the thread rusage once via libc (no per-switch struct_rusage allocation) and runs the stop/start bookkeeping inline for base LoggingContexts, only dispatching through Python for subclasses (BackgroundProcessLoggingContext) so their overrides run. start()/stop() now take an Optional[tuple[float, float]] instead of a struct_rusage, the get_thread_resource_usage/is_thread_resource_usage_supported/ get_thread_id module helpers are gone, LoggingContext.previous_context is now Optional[LoggingContext] (None where it used to hold SENTINEL_CONTEXT), and the nominally-private _resource_usage attribute is no longer exposed (nothing read it; use get_resource_usage()) — worth an upgrade note when this is released, as out-of-tree code may rely on the old shapes. - The switch path avoids per-operation allocation and Python round-trips: names are stored as Py<PyString> (LoggingContextFilter reads server_name and str(context) per log record process-wide, now INCREF-only), error messages materialise the context name only in the cold branches, and the thread id is read via PyThread_get_thread_ident (the exact value threading.get_ident() returns) rather than by calling into Python. - The attribute surface, method set and error-message wording are a compatibility contract, pinned by the characterization tests (which now exercise the tuple-based start/stop API). - The opt-in synapse.logging.context.debug switch traces are emitted from Rust via pyo3-log, whose level cache only refreshes on reset_logging_config(); docs/log_contexts.md documents the manhole procedure. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JFbRtswu7rsHrttJFauUUb
Give tokio tasks a captured logcontext, resolving the module-doc TODO: - LogContextHandle is a cheap, clone-able, GIL-free handle in the same Option<Py<LoggingContext>> representation the storage slots use. create_deferred captures the caller's context at the FFI boundary and scopes it onto the spawned task via a tokio task-local, which rides with the task across .await points. current_context() gives the task-local read precedence, so log records emitted while a task is polled — via LoggingContextFilter and pyo3-log — are attributed to the captured context with no per-record stamping. - The switch primitive is only ever driven on reactor/threadpool threads, never during a tokio-scoped poll (where the write would be invisible to reads); swap_current_context enforces that invariant with an error log rather than trusting it. - run_python_awaitable restores the captured context on the reactor thread before driving the awaitable, so Python called back from Rust (e.g. DatabasePool.runInteraction from the Rust /versions handler) runs in — and accounts its DB usage against — the right request. The restore protocol lives in a new with_logcontext helper (the Rust equivalent of `with PreserveLoggingContext(...)`): an error cannot skip the restore (which would leak the context onto the reactor thread permanently), and a context that has already finished is not re-started (create_deferred does not propagate cancellation, so a task can outlive its request; see the TODO) — such work runs in the sentinel instead. - tests/synapse_rust/test_logcontext.py exercises both guarantees through real production code paths: reqwest's log records carry the caller's request id, and the /versions handler's DB transaction lands on the caller's usage accounting. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JFbRtswu7rsHrttJFauUUb
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JFbRtswu7rsHrttJFauUUb
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ports the logcontext classes to Rust, and gives tokio tasks a captured logcontext so that work running in (or spawned from) Rust is attributed to the request that caused it.
Best reviewed commit by commit:
logcontext_errormessage shapes, the abuse-detection code paths andLoggingContextFilter's observable behaviour, against the existing Python implementation (this commit is green on its own). These are the behavioural contract the port has to satisfy.ContextResourceUsageto a Rust pyclass — self-contained: the newsynapse_rust.logcontextmodule, the class, its stub and the re-export.LoggingContextto Rust — the core change; the commit message carries detailed design notes. Highlights:Option<Py<LoggingContext>>, withNonerepresenting the sentinel._Sentinel/SENTINEL_CONTEXTstay pure Python (unchanged); thin wrappers oncurrent_context/set_current_contextconvert at the boundary, and pyo3's extraction enforces the type (TypeErrorotherwise).getrusage(RUSAGE_THREAD)read per switch via libc, inlinestop/startbookkeeping for baseLoggingContexts, Python dispatch only for subclasses (BackgroundProcessLoggingContext) so their overrides run. The thread id comes fromPyThread_get_thread_ident(the exactthreading.get_ident()value) without calling into Python.
Py<PyString>(the per-log-recordstr(context)/server_namereads are INCREF-only), error branches materialise strings only when hit.create_deferredcaptures the caller's context and scopes it onto the spawned task via a tokio task-local (LogContextHandle);current_context()gives the task-local read precedence, soLoggingContextFilter/pyo3-logresolve the right context on workerthreads with no per-record stamping.
run_python_awaitablerestores the captured context (via awith_logcontexthelper, the RustPreserveLoggingContext) around Python called back from Rust, so e.g.runInteractionfrom the Rust/versionshandler accounts its DB usage against the right request. Integration tests exercise both guarantees through real production code paths.Follow-up work on top of this (separate PR): porting
BackgroundProcessLoggingContextnatively and removing furtherPy<_>indirections.