Resolve breaking change to run_as_background_process in module API (#18737)

Fix #18735

In #18670, we updated
`run_as_background_process` to add a `server_name` argument. Because
this function is directly exported from the Synapse module API, this is
a breaking change to any downstream Synapse modules that use
`run_as_background_process`.

This PR shims and deprecates the existing
`run_as_background_process(...)` for modules by providing a stub
`server_name` value and introduces a new
`ModuleApi.run_as_background_process(...)` that covers the `server_name`
logic automagically.

Author: MadLittleMods

changelog.d/18737.removal

@@ -0,0 +1 @@
+Deprecate `run_as_background_process` exported as part of the module API interface in favor of `ModuleApi.run_as_background_process`. See the relevant section in the upgrade notes for more information.

docs/upgrade.md

@@ -119,6 +119,35 @@ stacking them up. You can monitor the currently running background updates with
 
 # Upgrading to v1.136.0
 
+## Deprecate `run_as_background_process` exported as part of the module API interface in favor of `ModuleApi.run_as_background_process`
+
+The `run_as_background_process` function is now a method of the `ModuleApi` class. If
+you were using the function directly from the module API, it will continue to work fine
+but the background process metrics will not include an accurate `server_name` label.
+This kind of metric labeling isn't relevant for many use cases and is used to
+differentiate Synapse instances running in the same Python process (relevant to Synapse
+Pro: Small Hosts). We recommend updating your usage to use the new
+`ModuleApi.run_as_background_process` method to stay on top of future changes.
+
+<details>
+<summary>Example <code>run_as_background_process</code> upgrade</summary>
+
+Before:
+```python
+class MyModule:
+    def __init__(self, module_api: ModuleApi) -> None:
+        run_as_background_process(__name__ + ":setup_database", self.setup_database)
+```
+
+After:
+```python
+class MyModule:
+    def __init__(self, module_api: ModuleApi) -> None:
+        module_api.run_as_background_process(__name__ + ":setup_database", self.setup_database)
+```
+
+</details>
+
 ## Metric labels have changed on `synapse_federation_last_received_pdu_time` and `synapse_federation_last_sent_pdu_time`
 
 Previously, the `synapse_federation_last_received_pdu_time` and
@@ -136,6 +165,7 @@ this change but you will need to manually update your own existing Grafana dashb
 using these metrics.
 
 
+
 # Upgrading to v1.135.0
 
 ## `on_user_registration` module API callback may now run on any worker

synapse/events/auto_accept_invites.py

@@ -114,7 +114,6 @@ async def on_new_event(self, event: EventBase, *args: Any) -> None:
         # that occurs when responding to invites over federation (see https://github.com/matrix-org/synapse-auto-accept-invite/issues/12)
         run_as_background_process(
             "retry_make_join",
-            self.server_name,
             self._retry_make_join,
             event.state_key,
             event.state_key,

synapse/module_api/__init__.py

@@ -23,6 +23,7 @@
 from typing import (
     TYPE_CHECKING,
     Any,
+    Awaitable,
     Callable,
     Collection,
     Dict,
@@ -80,7 +81,9 @@
     make_deferred_yieldable,
     run_in_background,
 )
-from synapse.metrics.background_process_metrics import run_as_background_process
+from synapse.metrics.background_process_metrics import (
+    run_as_background_process as _run_as_background_process,
+)
 from synapse.module_api.callbacks.account_validity_callbacks import (
     IS_USER_EXPIRED_CALLBACK,
     ON_LEGACY_ADMIN_REQUEST,
@@ -158,6 +161,9 @@
 from synapse.util.frozenutils import freeze
 
 if TYPE_CHECKING:
+    # Old versions don't have `LiteralString`
+    from typing_extensions import LiteralString
+
     from synapse.app.generic_worker import GenericWorkerStore
     from synapse.server import HomeServer
 
@@ -216,6 +222,65 @@ class UserIpAndAgent:
     last_seen: int
 
 
+def run_as_background_process(
+    desc: "LiteralString",
+    func: Callable[..., Awaitable[Optional[T]]],
+    *args: Any,
+    bg_start_span: bool = True,
+    **kwargs: Any,
+) -> "defer.Deferred[Optional[T]]":
+    """
+    XXX: Deprecated: use `ModuleApi.run_as_background_process` instead.
+
+    Run the given function in its own logcontext, with resource metrics
+
+    This should be used to wrap processes which are fired off to run in the
+    background, instead of being associated with a particular request.
+
+    It returns a Deferred which completes when the function completes, but it doesn't
+    follow the synapse logcontext rules, which makes it appropriate for passing to
+    clock.looping_call and friends (or for firing-and-forgetting in the middle of a
+    normal synapse async function).
+
+    Args:
+        desc: a description for this background process type
+        server_name: The homeserver name that this background process is being run for
+            (this should be `hs.hostname`).
+        func: a function, which may return a Deferred or a coroutine
+        bg_start_span: Whether to start an opentracing span. Defaults to True.
+            Should only be disabled for processes that will not log to or tag
+            a span.
+        args: positional args for func
+        kwargs: keyword args for func
+
+    Returns:
+        Deferred which returns the result of func, or `None` if func raises.
+        Note that the returned Deferred does not follow the synapse logcontext
+        rules.
+    """
+
+    logger.warning(
+        "Using deprecated `run_as_background_process` that's exported from the Module API. "
+        "Prefer `ModuleApi.run_as_background_process` instead.",
+    )
+
+    # Historically, since this function is exported from the module API, we can't just
+    # change the signature to require a `server_name` argument. Since
+    # `run_as_background_process` internally in Synapse requires `server_name` now, we
+    # just have to stub this out with a placeholder value and tell people to use the new
+    # function instead.
+    stub_server_name = "synapse_module_running_from_unknown_server"
+
+    return _run_as_background_process(
+        desc,
+        stub_server_name,
+        func,
+        *args,
+        bg_start_span=bg_start_span,
+        **kwargs,
+    )
+
+
 def cached(
     *,
     max_entries: int = 1000,
@@ -1323,10 +1388,9 @@ def looping_background_call(
 
         if self._hs.config.worker.run_background_tasks or run_on_all_instances:
             self._clock.looping_call(
-                run_as_background_process,
+                self.run_as_background_process,
                 msec,
                 desc,
-                self.server_name,
                 lambda: maybe_awaitable(f(*args, **kwargs)),
             )
         else:
@@ -1382,9 +1446,8 @@ def delayed_background_call(
         return self._clock.call_later(
             # convert ms to seconds as needed by call_later.
             msec * 0.001,
-            run_as_background_process,
+            self.run_as_background_process,
             desc,
-            self.server_name,
             lambda: maybe_awaitable(f(*args, **kwargs)),
         )
 
@@ -1590,6 +1653,44 @@ async def get_room_state(
 
         return {key: state_events[event_id] for key, event_id in state_ids.items()}
 
+    def run_as_background_process(
+        self,
+        desc: "LiteralString",
+        func: Callable[..., Awaitable[Optional[T]]],
+        *args: Any,
+        bg_start_span: bool = True,
+        **kwargs: Any,
+    ) -> "defer.Deferred[Optional[T]]":
+        """Run the given function in its own logcontext, with resource metrics
+
+        This should be used to wrap processes which are fired off to run in the
+        background, instead of being associated with a particular request.
+
+        It returns a Deferred which completes when the function completes, but it doesn't
+        follow the synapse logcontext rules, which makes it appropriate for passing to
+        clock.looping_call and friends (or for firing-and-forgetting in the middle of a
+        normal synapse async function).
+
+        Args:
+            desc: a description for this background process type
+            server_name: The homeserver name that this background process is being run for
+                (this should be `hs.hostname`).
+            func: a function, which may return a Deferred or a coroutine
+            bg_start_span: Whether to start an opentracing span. Defaults to True.
+                Should only be disabled for processes that will not log to or tag
+                a span.
+            args: positional args for func
+            kwargs: keyword args for func
+
+        Returns:
+            Deferred which returns the result of func, or `None` if func raises.
+            Note that the returned Deferred does not follow the synapse logcontext
+            rules.
+        """
+        return _run_as_background_process(
+            desc, self.server_name, func, *args, bg_start_span=bg_start_span, **kwargs
+        )
+
     async def defer_to_thread(
         self,
         f: Callable[P, T],