diff --git a/rust/src/logging/context.rs b/rust/src/logging/context.rs index c962596dbd..28861fe858 100644 --- a/rust/src/logging/context.rs +++ b/rust/src/logging/context.rs @@ -54,13 +54,73 @@ use pyo3::prelude::*; use pyo3::types::{PyDict, PyTuple}; use pyo3::{PyTraverseError, PyVisit}; -/// The Python sentinel logcontext (`synapse.logging.context.SENTINEL_CONTEXT`). -/// -/// Pushed in from Python at import time via [`register_sentinel`] rather than -/// imported here, to avoid a circular import at module-registration time (Rust -/// must not import `synapse.logging.context`; see [`crate::deferred`]). +/// The sentinel logcontext singleton (`synapse.logging.context.SENTINEL_CONTEXT`), +/// created lazily by [`sentinel`]. Owned natively: Rust defines the [`Sentinel`] +/// type *and* holds the one instance, so there is no Python-side bootstrap and no +/// import of `synapse.logging.context` at registration time (which would be a +/// circular import; see [`crate::deferred`]). static SENTINEL: OnceCell> = OnceCell::new(); +/// The root "no logcontext" marker (`synapse.logging.context.SENTINEL_CONTEXT`). +/// +/// A drop-in for the former Python `_Sentinel`: a singleton whose fields are inert +/// defaults and whose methods are no-ops, and which is *falsy* so callers can test +/// `if not current_context()` to detect "no logcontext". [`switch_context`] +/// special-cases it by identity, so its `start`/`stop` never run on the hot path; +/// the other no-op methods exist only so code holding a `LoggingContextOrSentinel` +/// can call them without first checking the concrete type. +#[pyclass(name = "_Sentinel", get_all, set_all)] +pub struct Sentinel { + previous_context: Option>, + finished: bool, + scope: Option>, + server_name: String, + request: Option>, + tag: Option>, +} + +impl Sentinel { + /// The singleton's initial state (mirrors the former Python `_Sentinel.__init__`). + fn instance() -> Self { + Sentinel { + previous_context: None, + finished: false, + scope: None, + server_name: "unknown_server_from_sentinel_context".to_owned(), + request: None, + tag: None, + } + } +} + +#[pymethods] +impl Sentinel { + fn __str__(&self) -> &'static str { + "sentinel" + } + + /// No-op: the sentinel is never actually running, so there is nothing to + /// account. + fn start(&self, _rusage: Option<(f64, f64)>) {} + + /// No-op counterpart to [`Self::start`]. + fn stop(&self, _rusage: Option<(f64, f64)>) {} + + /// No-op: work done under the sentinel is attributed to no context. + fn add_database_transaction(&self, _duration_sec: f64) {} + + /// No-op counterpart to [`Self::add_database_transaction`]. + fn add_database_scheduled(&self, _sched_sec: f64) {} + + /// No-op: event fetches under the sentinel are attributed to no context. + fn record_event_fetch(&self, _event_count: i64) {} + + /// The sentinel is falsy, matching the former Python `_Sentinel.__bool__`. + fn __bool__(&self) -> bool { + false + } +} + /// Name of the opt-in logger for logcontext switch tracing. /// /// This is the single source of truth for the logger name: it is used as the @@ -830,26 +890,18 @@ pub fn set_current_context(py: Python<'_>, context: Bound<'_, PyAny>) -> PyResul Ok(current) } -/// Register the Python sentinel logcontext. +/// Get a reference to the sentinel logcontext singleton, creating it on first use. /// -/// Called once from `synapse.logging.context` at import time. Registering twice -/// is a no-op (the first registration wins); this keeps the identity of the -/// sentinel object we return from [`current_context`] equal to Python's -/// `SENTINEL_CONTEXT` singleton, preserving `context is SENTINEL_CONTEXT` and -/// `bool(context)` semantics. -#[pyfunction] -pub fn register_sentinel(sentinel: Py) { - let _ = SENTINEL.set(sentinel); -} - -/// Get a fresh reference to the sentinel logcontext. +/// The instance is owned here (not pushed in from Python), so its identity is +/// stable and equal to the `SENTINEL_CONTEXT` exported by [`register_module`], +/// preserving `context is SENTINEL_CONTEXT` and `bool(context)` semantics. fn sentinel(py: Python<'_>) -> Py { SENTINEL - .get() - .expect( - "synapse.logging.context sentinel not registered with the Rust logcontext slot; \ - synapse.logging.context must call register_sentinel() at import", - ) + .get_or_init(|| { + Py::new(py, Sentinel::instance()) + .expect("failed to create the sentinel logcontext") + .into_any() + }) .clone_ref(py) } @@ -895,11 +947,14 @@ pub fn register_module(py: Python<'_>, m: &Bound<'_, PyModule>) -> PyResult<()> let child_module: Bound<'_, PyModule> = PyModule::new(py, "logcontext")?; child_module.add_class::()?; child_module.add_class::()?; + child_module.add_class::()?; child_module.add_function(wrap_pyfunction!(current_context, &child_module)?)?; child_module.add_function(wrap_pyfunction!(swap_current_context, &child_module)?)?; child_module.add_function(wrap_pyfunction!(set_current_context, &child_module)?)?; - child_module.add_function(wrap_pyfunction!(register_sentinel, &child_module)?)?; child_module.add("DEBUG_LOGGER_NAME", DEBUG_LOGGER_NAME)?; + // The sentinel singleton is owned by Rust; export the one instance so Python's + // `SENTINEL_CONTEXT` is that exact object (identity preserved). + child_module.add("SENTINEL_CONTEXT", sentinel(py))?; m.add_submodule(&child_module)?; @@ -920,11 +975,9 @@ mod tests { use super::*; - /// Register a sentinel exactly once (the `OnceCell` keeps the first) and - /// return whichever object is actually registered, so identity assertions - /// hold regardless of which test ran first. + /// The native sentinel singleton (created lazily on first use), used for + /// identity assertions. fn registered_sentinel(py: Python<'_>) -> Py { - register_sentinel(PyString::new(py, "SENTINEL").into_any().unbind()); sentinel(py) } diff --git a/synapse/logging/context.py b/synapse/logging/context.py index 23b0c2bee5..a15cb5c210 100644 --- a/synapse/logging/context.py +++ b/synapse/logging/context.py @@ -53,10 +53,11 @@ from twisted.python.threadpool import ThreadPool from synapse.logging.loggers import ExplicitlyConfiguredLogger from synapse.synapse_rust.logcontext import ( DEBUG_LOGGER_NAME, + SENTINEL_CONTEXT as SENTINEL_CONTEXT, ContextResourceUsage as ContextResourceUsage, LoggingContext as LoggingContext, + _Sentinel as _Sentinel, current_context as current_context, - register_sentinel, set_current_context as set_current_context, ) from synapse.util.stringutils import random_string_insecure_fast @@ -109,69 +110,13 @@ class ContextRequest: user_agent: str -LoggingContextOrSentinel = Union["LoggingContext", "_Sentinel"] +LoggingContextOrSentinel = Union[LoggingContext, _Sentinel] - -class _Sentinel: - """ - Sentinel to represent the root context - - This should only be used for tasks outside of Synapse like when we yield control - back to the Twisted reactor (event loop) so we don't leak the current logging - context to other tasks that are scheduled next in the event loop. - - Nothing from the Synapse homeserver should be logged with the sentinel context. i.e. - we should always know which server the logs are coming from. - """ - - __slots__ = [ - "previous_context", - "finished", - "scope", - "server_name", - "request", - "tag", - ] - - def __init__(self) -> None: - # Minimal set for compatibility with LoggingContext - self.previous_context = None - self.finished = False - self.server_name = "unknown_server_from_sentinel_context" - self.request = None - self.scope = None - self.tag = None - - def __str__(self) -> str: - return "sentinel" - - def start(self, rusage: "tuple[float, float] | None") -> None: - pass - - def stop(self, rusage: "tuple[float, float] | None") -> None: - pass - - def add_database_transaction(self, duration_sec: float) -> None: - pass - - def add_database_scheduled(self, sched_sec: float) -> None: - pass - - def record_event_fetch(self, event_count: int) -> None: - pass - - def __bool__(self) -> Literal[False]: - return False - - -SENTINEL_CONTEXT = _Sentinel() - -# Hand the sentinel to the Rust logcontext slot, which owns the "current context" -# storage (see `synapse.synapse_rust.logcontext` / `rust/src/logcontext.rs`). Rust -# returns this exact object when no context is set, so `context is SENTINEL_CONTEXT` -# identity and `bool(context)` semantics are preserved. We push it in from here -# rather than have Rust import this module, to avoid a circular import. -register_sentinel(SENTINEL_CONTEXT) +# `_Sentinel` (the root "no logcontext" marker) and its singleton `SENTINEL_CONTEXT` +# are now defined and owned by the Rust logcontext module, which holds the "current +# context" storage (see `synapse.synapse_rust.logcontext` / `rust/src/logging/context.rs`). +# Rust returns this exact object when no context is set, so `context is SENTINEL_CONTEXT` +# identity and `bool(context)` semantics are preserved without a Python-side bootstrap. class LoggingContextFilter(logging.Filter): diff --git a/synapse/synapse_rust/logcontext.pyi b/synapse/synapse_rust/logcontext.pyi index 6bf2e1b0ed..ba58c9b13c 100644 --- a/synapse/synapse_rust/logcontext.pyi +++ b/synapse/synapse_rust/logcontext.pyi @@ -11,7 +11,7 @@ # . from types import TracebackType -from typing import TYPE_CHECKING, Optional +from typing import TYPE_CHECKING, Literal, Optional from synapse.logging.context import ContextRequest, LoggingContextOrSentinel @@ -86,6 +86,27 @@ class LoggingContext: def add_database_scheduled(self, sched_sec: float) -> None: ... def record_event_fetch(self, event_count: int) -> None: ... +class _Sentinel: + """The root "no logcontext" marker. A falsy singleton (see `SENTINEL_CONTEXT`) + whose fields are inert defaults and whose methods are no-ops.""" + + previous_context: None + finished: bool + scope: None + server_name: str + request: None + tag: None + + def __str__(self) -> str: ... + def start(self, rusage: "Optional[tuple[float, float]]") -> None: ... + def stop(self, rusage: "Optional[tuple[float, float]]") -> None: ... + def add_database_transaction(self, duration_sec: float) -> None: ... + def add_database_scheduled(self, sched_sec: float) -> None: ... + def record_event_fetch(self, event_count: int) -> None: ... + def __bool__(self) -> Literal[False]: ... + +SENTINEL_CONTEXT: _Sentinel + def current_context() -> LoggingContextOrSentinel: """Get the current logging context. @@ -114,11 +135,3 @@ def set_current_context( Reads the thread CPU usage once via `getrusage(RUSAGE_THREAD)` and does the `stop`/`start` accounting natively; raises `TypeError` if `context` is `None`. """ - -def register_sentinel(sentinel: LoggingContextOrSentinel) -> None: - """Register the Python sentinel logcontext with the Rust slot. - - Called once from `synapse.logging.context` at import time so that the object - returned by `current_context()` when no context is set is Python's - `SENTINEL_CONTEXT` singleton (preserving identity and `bool()` semantics). - """