From ec6758d47253fd6f0cee567f38c8996e35a4de7a Mon Sep 17 00:00:00 2001 From: Dirk Klimpel <5740567+dklimpel@users.noreply.github.com> Date: Wed, 3 Aug 2022 10:41:57 +0200 Subject: [PATCH 01/10] Fix wrong headline for `url_preview_accept_language` in docs (#13437) Fixes: #13433 --- changelog.d/13437.doc | 1 + docs/usage/configuration/config_documentation.md | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 changelog.d/13437.doc diff --git a/changelog.d/13437.doc b/changelog.d/13437.doc new file mode 100644 index 0000000000..fb772b24dc --- /dev/null +++ b/changelog.d/13437.doc @@ -0,0 +1 @@ +Fix wrong headline for `url_preview_accept_language` in documentation. diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index 6f8d7b7d26..d072240abf 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -1859,7 +1859,7 @@ Example configuration: max_spider_size: 8M ``` --- -### `url_preview_language` +### `url_preview_accept_language` A list of values for the Accept-Language HTTP header used when downloading webpages during URL preview generation. This allows From 5eccfdfafdf957739697d667c05fde829cc0f8bc Mon Sep 17 00:00:00 2001 From: Dirk Klimpel <5740567+dklimpel@users.noreply.github.com> Date: Wed, 3 Aug 2022 11:19:20 +0200 Subject: [PATCH 02/10] Remove 'Contents' section from the Configuration Manual (#13438) Fixes: #13053 --- changelog.d/13438.doc | 1 + .../configuration/config_documentation.md | 43 ------------------- 2 files changed, 1 insertion(+), 43 deletions(-) create mode 100644 changelog.d/13438.doc diff --git a/changelog.d/13438.doc b/changelog.d/13438.doc new file mode 100644 index 0000000000..163b63ffc6 --- /dev/null +++ b/changelog.d/13438.doc @@ -0,0 +1 @@ +Remove redundant 'Contents' section from the Configuration Manual. Contributed by @dklimpel. diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index d072240abf..2e2e59195b 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -72,49 +72,6 @@ apply if you want your config file to be read properly. A few helpful things to In addition, each setting has an example of its usage, with the proper indentation shown. -## Contents -[Modules](#modules) - -[Server](#server) - -[Homeserver Blocking](#homeserver-blocking) - -[TLS](#tls) - -[Federation](#federation) - -[Caching](#caching) - -[Database](#database) - -[Logging](#logging) - -[Ratelimiting](#ratelimiting) - -[Media Store](#media-store) - -[Captcha](#captcha) - -[TURN](#turn) - -[Registration](#registration) - -[API Configuration](#api-configuration) - -[Signing Keys](#signing-keys) - -[Single Sign On Integration](#single-sign-on-integration) - -[Push](#push) - -[Rooms](#rooms) - -[Opentracing](#opentracing) - -[Workers](#workers) - -[Background Updates](#background-updates) - ## Modules Server admins can expand Synapse's functionality with external modules. From 570bf32bbb9b4bd31a9783746187769613591499 Mon Sep 17 00:00:00 2001 From: Matt C <96466754+buffless-matt@users.noreply.github.com> Date: Wed, 3 Aug 2022 19:25:36 +1000 Subject: [PATCH 03/10] Add module API method to resolve a room alias to a room ID (#13428) Co-authored-by: MattC Co-authored-by: Brendan Abolivier --- changelog.d/13428.feature | 1 + synapse/module_api/__init__.py | 24 ++++++++++++++++++++++++ tests/module_api/test_api.py | 19 +++++++++++++++++++ 3 files changed, 44 insertions(+) create mode 100644 changelog.d/13428.feature diff --git a/changelog.d/13428.feature b/changelog.d/13428.feature new file mode 100644 index 0000000000..085b61483f --- /dev/null +++ b/changelog.d/13428.feature @@ -0,0 +1 @@ +Add a module API method to translate a room alias into a room ID. diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index 6d8bf54083..18d6d1058a 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -1452,6 +1452,30 @@ class ModuleApi: start_timestamp, end_timestamp ) + async def lookup_room_alias(self, room_alias: str) -> Tuple[str, List[str]]: + """ + Get the room ID associated with a room alias. + + Added in Synapse v1.65.0. + + Args: + room_alias: The alias to look up. + + Returns: + A tuple of: + The room ID (str). + Hosts likely to be participating in the room ([str]). + + Raises: + SynapseError if room alias is invalid or could not be found. + """ + alias = RoomAlias.from_string(room_alias) + (room_id, hosts) = await self._hs.get_room_member_handler().lookup_room_alias( + alias + ) + + return room_id.to_string(), hosts + class PublicRoomListManager: """Contains methods for adding to, removing from and querying whether a room diff --git a/tests/module_api/test_api.py b/tests/module_api/test_api.py index 169e29b590..8e05590230 100644 --- a/tests/module_api/test_api.py +++ b/tests/module_api/test_api.py @@ -635,6 +635,25 @@ class ModuleApiTestCase(HomeserverTestCase): [{"set_tweak": "sound", "value": "default"}] ) + def test_lookup_room_alias(self) -> None: + """Test that modules can resolve a room alias to a room ID.""" + password = "password" + user_id = self.register_user("user", password) + access_token = self.login(user_id, password) + room_alias = "my-alias" + reference_room_id = self.helper.create_room_as( + tok=access_token, extra_content={"room_alias_name": room_alias} + ) + self.assertIsNotNone(reference_room_id) + + (room_id, _) = self.get_success( + self.module_api.lookup_room_alias( + f"#{room_alias}:{self.module_api.server_name}" + ) + ) + + self.assertEqual(room_id, reference_room_id) + class ModuleApiWorkerTestCase(BaseMultiWorkerStreamTestCase): """For testing ModuleApi functionality in a multi-worker setup""" From d6e94ad9d9b22efe0f5eb64d946c10a542068f7c Mon Sep 17 00:00:00 2001 From: Dirk Klimpel <5740567+dklimpel@users.noreply.github.com> Date: Wed, 3 Aug 2022 11:40:20 +0200 Subject: [PATCH 04/10] Rename `RateLimitConfig` to `RatelimitSettings` (#13442) --- changelog.d/13442.misc | 1 + synapse/api/ratelimiting.py | 6 ++--- synapse/config/ratelimiting.py | 42 ++++++++++++++++----------------- synapse/rest/client/register.py | 4 ++-- synapse/util/ratelimitutils.py | 6 ++--- 5 files changed, 30 insertions(+), 29 deletions(-) create mode 100644 changelog.d/13442.misc diff --git a/changelog.d/13442.misc b/changelog.d/13442.misc new file mode 100644 index 0000000000..f503bc79d3 --- /dev/null +++ b/changelog.d/13442.misc @@ -0,0 +1 @@ +Rename class `RateLimitConfig` to `RatelimitSettings` and `FederationRateLimitConfig` to `FederationRatelimitSettings`. \ No newline at end of file diff --git a/synapse/api/ratelimiting.py b/synapse/api/ratelimiting.py index f43965c1c8..044c7d4926 100644 --- a/synapse/api/ratelimiting.py +++ b/synapse/api/ratelimiting.py @@ -17,7 +17,7 @@ from collections import OrderedDict from typing import Hashable, Optional, Tuple from synapse.api.errors import LimitExceededError -from synapse.config.ratelimiting import RateLimitConfig +from synapse.config.ratelimiting import RatelimitSettings from synapse.storage.databases.main import DataStore from synapse.types import Requester from synapse.util import Clock @@ -314,8 +314,8 @@ class RequestRatelimiter: self, store: DataStore, clock: Clock, - rc_message: RateLimitConfig, - rc_admin_redaction: Optional[RateLimitConfig], + rc_message: RatelimitSettings, + rc_admin_redaction: Optional[RatelimitSettings], ): self.store = store self.clock = clock diff --git a/synapse/config/ratelimiting.py b/synapse/config/ratelimiting.py index 5a91917b4a..1ed001e105 100644 --- a/synapse/config/ratelimiting.py +++ b/synapse/config/ratelimiting.py @@ -21,7 +21,7 @@ from synapse.types import JsonDict from ._base import Config -class RateLimitConfig: +class RatelimitSettings: def __init__( self, config: Dict[str, float], @@ -34,7 +34,7 @@ class RateLimitConfig: @attr.s(auto_attribs=True) -class FederationRateLimitConfig: +class FederationRatelimitSettings: window_size: int = 1000 sleep_limit: int = 10 sleep_delay: int = 500 @@ -50,11 +50,11 @@ class RatelimitConfig(Config): # Load the new-style messages config if it exists. Otherwise fall back # to the old method. if "rc_message" in config: - self.rc_message = RateLimitConfig( + self.rc_message = RatelimitSettings( config["rc_message"], defaults={"per_second": 0.2, "burst_count": 10.0} ) else: - self.rc_message = RateLimitConfig( + self.rc_message = RatelimitSettings( { "per_second": config.get("rc_messages_per_second", 0.2), "burst_count": config.get("rc_message_burst_count", 10.0), @@ -64,9 +64,9 @@ class RatelimitConfig(Config): # Load the new-style federation config, if it exists. Otherwise, fall # back to the old method. if "rc_federation" in config: - self.rc_federation = FederationRateLimitConfig(**config["rc_federation"]) + self.rc_federation = FederationRatelimitSettings(**config["rc_federation"]) else: - self.rc_federation = FederationRateLimitConfig( + self.rc_federation = FederationRatelimitSettings( **{ k: v for k, v in { @@ -80,17 +80,17 @@ class RatelimitConfig(Config): } ) - self.rc_registration = RateLimitConfig(config.get("rc_registration", {})) + self.rc_registration = RatelimitSettings(config.get("rc_registration", {})) - self.rc_registration_token_validity = RateLimitConfig( + self.rc_registration_token_validity = RatelimitSettings( config.get("rc_registration_token_validity", {}), defaults={"per_second": 0.1, "burst_count": 5}, ) rc_login_config = config.get("rc_login", {}) - self.rc_login_address = RateLimitConfig(rc_login_config.get("address", {})) - self.rc_login_account = RateLimitConfig(rc_login_config.get("account", {})) - self.rc_login_failed_attempts = RateLimitConfig( + self.rc_login_address = RatelimitSettings(rc_login_config.get("address", {})) + self.rc_login_account = RatelimitSettings(rc_login_config.get("account", {})) + self.rc_login_failed_attempts = RatelimitSettings( rc_login_config.get("failed_attempts", {}) ) @@ -101,20 +101,20 @@ class RatelimitConfig(Config): rc_admin_redaction = config.get("rc_admin_redaction") self.rc_admin_redaction = None if rc_admin_redaction: - self.rc_admin_redaction = RateLimitConfig(rc_admin_redaction) + self.rc_admin_redaction = RatelimitSettings(rc_admin_redaction) - self.rc_joins_local = RateLimitConfig( + self.rc_joins_local = RatelimitSettings( config.get("rc_joins", {}).get("local", {}), defaults={"per_second": 0.1, "burst_count": 10}, ) - self.rc_joins_remote = RateLimitConfig( + self.rc_joins_remote = RatelimitSettings( config.get("rc_joins", {}).get("remote", {}), defaults={"per_second": 0.01, "burst_count": 10}, ) # Track the rate of joins to a given room. If there are too many, temporarily # prevent local joins and remote joins via this server. - self.rc_joins_per_room = RateLimitConfig( + self.rc_joins_per_room = RatelimitSettings( config.get("rc_joins_per_room", {}), defaults={"per_second": 1, "burst_count": 10}, ) @@ -124,31 +124,31 @@ class RatelimitConfig(Config): # * For requests received over federation this is keyed by the origin. # # Note that this isn't exposed in the configuration as it is obscure. - self.rc_key_requests = RateLimitConfig( + self.rc_key_requests = RatelimitSettings( config.get("rc_key_requests", {}), defaults={"per_second": 20, "burst_count": 100}, ) - self.rc_3pid_validation = RateLimitConfig( + self.rc_3pid_validation = RatelimitSettings( config.get("rc_3pid_validation") or {}, defaults={"per_second": 0.003, "burst_count": 5}, ) - self.rc_invites_per_room = RateLimitConfig( + self.rc_invites_per_room = RatelimitSettings( config.get("rc_invites", {}).get("per_room", {}), defaults={"per_second": 0.3, "burst_count": 10}, ) - self.rc_invites_per_user = RateLimitConfig( + self.rc_invites_per_user = RatelimitSettings( config.get("rc_invites", {}).get("per_user", {}), defaults={"per_second": 0.003, "burst_count": 5}, ) - self.rc_invites_per_issuer = RateLimitConfig( + self.rc_invites_per_issuer = RatelimitSettings( config.get("rc_invites", {}).get("per_issuer", {}), defaults={"per_second": 0.3, "burst_count": 10}, ) - self.rc_third_party_invite = RateLimitConfig( + self.rc_third_party_invite = RatelimitSettings( config.get("rc_third_party_invite", {}), defaults={ "per_second": self.rc_message.per_second, diff --git a/synapse/rest/client/register.py b/synapse/rest/client/register.py index b7ab090bbd..956c45e60a 100644 --- a/synapse/rest/client/register.py +++ b/synapse/rest/client/register.py @@ -33,7 +33,7 @@ from synapse.api.ratelimiting import Ratelimiter from synapse.config import ConfigError from synapse.config.emailconfig import ThreepidBehaviour from synapse.config.homeserver import HomeServerConfig -from synapse.config.ratelimiting import FederationRateLimitConfig +from synapse.config.ratelimiting import FederationRatelimitSettings from synapse.config.server import is_threepid_reserved from synapse.handlers.auth import AuthHandler from synapse.handlers.ui_auth import UIAuthSessionDataConstants @@ -325,7 +325,7 @@ class UsernameAvailabilityRestServlet(RestServlet): self.registration_handler = hs.get_registration_handler() self.ratelimiter = FederationRateLimiter( hs.get_clock(), - FederationRateLimitConfig( + FederationRatelimitSettings( # Time window of 2s window_size=2000, # Artificially delay requests if rate > sleep_limit/window_size diff --git a/synapse/util/ratelimitutils.py b/synapse/util/ratelimitutils.py index dfe628c97e..6394cc39ac 100644 --- a/synapse/util/ratelimitutils.py +++ b/synapse/util/ratelimitutils.py @@ -21,7 +21,7 @@ from typing import Any, DefaultDict, Iterator, List, Set from twisted.internet import defer from synapse.api.errors import LimitExceededError -from synapse.config.ratelimiting import FederationRateLimitConfig +from synapse.config.ratelimiting import FederationRatelimitSettings from synapse.logging.context import ( PreserveLoggingContext, make_deferred_yieldable, @@ -36,7 +36,7 @@ logger = logging.getLogger(__name__) class FederationRateLimiter: - def __init__(self, clock: Clock, config: FederationRateLimitConfig): + def __init__(self, clock: Clock, config: FederationRatelimitSettings): def new_limiter() -> "_PerHostRatelimiter": return _PerHostRatelimiter(clock=clock, config=config) @@ -63,7 +63,7 @@ class FederationRateLimiter: class _PerHostRatelimiter: - def __init__(self, clock: Clock, config: FederationRateLimitConfig): + def __init__(self, clock: Clock, config: FederationRatelimitSettings): """ Args: clock From fb7a2cc4cc7f0b49cabeec08d4ceb2dd2350e945 Mon Sep 17 00:00:00 2001 From: Dirk Klimpel <5740567+dklimpel@users.noreply.github.com> Date: Wed, 3 Aug 2022 11:41:19 +0200 Subject: [PATCH 05/10] Update doc for setting `macaroon_secret_key` (#13443) * Update doc for setting `macaroon_secret_key` * newsfile --- changelog.d/13443.doc | 1 + docs/usage/configuration/config_documentation.md | 10 +++++++--- 2 files changed, 8 insertions(+), 3 deletions(-) create mode 100644 changelog.d/13443.doc diff --git a/changelog.d/13443.doc b/changelog.d/13443.doc new file mode 100644 index 0000000000..0db5d1b3b4 --- /dev/null +++ b/changelog.d/13443.doc @@ -0,0 +1 @@ +Update documentation for config setting `macaroon_secret_key`. \ No newline at end of file diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index 2e2e59195b..3a9466a837 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -2495,9 +2495,13 @@ track_appservice_user_ips: true --- ### `macaroon_secret_key` -A secret which is used to sign access tokens. If none is specified, -the `registration_shared_secret` is used, if one is given; otherwise, -a secret key is derived from the signing key. +A secret which is used to sign +- access token for guest users, +- short-term login token used during SSO logins (OIDC or SAML2) and +- token used for unsubscribing from email notifications. + +If none is specified, the `registration_shared_secret` is used, if one is given; +otherwise, a secret key is derived from the signing key. Example configuration: ```yaml From 668597214f0cfeb9d7a642ef0564e73198b7bc82 Mon Sep 17 00:00:00 2001 From: jejo86 <28619134+jejo86@users.noreply.github.com> Date: Wed, 3 Aug 2022 12:15:23 +0200 Subject: [PATCH 06/10] Improve documentation on becoming server admin (#13230) * Improved section regarding server admin Added steps describing how to elevate an existing user to administrator by manipulating a `postgres` database. Signed-off-by: jejo86 28619134+jejo86@users.noreply.github.com * Improved section regarding server admin * Reference database settings Add instructions to check database settings to find out the database name, instead of listing all available PostgreSQL databases. * Add suggestions from PR conversation Replace config filename `homeserver.yaml`. with "config file". Remove instructions to switch to `postgres` user. Add instructions how to connect to SQLite database. * Update changelog.d/13230.doc Co-authored-by: reivilibre --- changelog.d/13230.doc | 1 + docs/usage/administration/admin_api/README.md | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) create mode 100644 changelog.d/13230.doc diff --git a/changelog.d/13230.doc b/changelog.d/13230.doc new file mode 100644 index 0000000000..dce7be2425 --- /dev/null +++ b/changelog.d/13230.doc @@ -0,0 +1 @@ +Add steps describing how to elevate an existing user to administrator by manipulating the database. diff --git a/docs/usage/administration/admin_api/README.md b/docs/usage/administration/admin_api/README.md index c60b6da0de..f11e0b19a6 100644 --- a/docs/usage/administration/admin_api/README.md +++ b/docs/usage/administration/admin_api/README.md @@ -5,8 +5,9 @@ Many of the API calls in the admin api will require an `access_token` for a server admin. (Note that a server admin is distinct from a room admin.) -A user can be marked as a server admin by updating the database directly, e.g.: +An existing user can be marked as a server admin by updating the database directly. +Check your [database settings](config_documentation.md#database) in the configuration file, connect to the correct database using either `psql [database name]` (if using PostgreSQL) or `sqlite3 path/to/your/database.db` (if using SQLite) and elevate the user `@foo:bar.com` to administrator. ```sql UPDATE users SET admin = 1 WHERE name = '@foo:bar.com'; ``` From 503a95804e61be692abb2c872e03284efc541afb Mon Sep 17 00:00:00 2001 From: Jasper Spaans Date: Wed, 3 Aug 2022 12:16:32 +0200 Subject: [PATCH 07/10] Install cryptography build dependencies in requirements image. (#13372) --- changelog.d/13372.docker | 1 + docker/Dockerfile | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) create mode 100644 changelog.d/13372.docker diff --git a/changelog.d/13372.docker b/changelog.d/13372.docker new file mode 100644 index 0000000000..238c78de09 --- /dev/null +++ b/changelog.d/13372.docker @@ -0,0 +1 @@ +Make docker images build on armv7 by installing cryptography dependencies in the "requirements" stage. Contributed by Jasper Spaans. \ No newline at end of file diff --git a/docker/Dockerfile b/docker/Dockerfile index 97bb03b08f..fa58ae3acb 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -40,7 +40,8 @@ FROM docker.io/python:${PYTHON_VERSION}-slim as requirements RUN \ --mount=type=cache,target=/var/cache/apt,sharing=locked \ --mount=type=cache,target=/var/lib/apt,sharing=locked \ - apt-get update -qq && apt-get install -yqq git \ + apt-get update -qq && apt-get install -yqq \ + build-essential cargo git libffi-dev libssl-dev \ && rm -rf /var/lib/apt/lists/* # We install poetry in its own build stage to avoid its dependencies conflicting with From 78a3111c41bf93fd52774965af50d62b74d937de Mon Sep 17 00:00:00 2001 From: andrew do Date: Wed, 3 Aug 2022 05:26:31 -0700 Subject: [PATCH 08/10] Return 404 or member list when getting joined_members after leaving (#13374) Signed-off-by: Andrew Doh Co-authored-by: Patrick Cloke Co-authored-by: Andrew Morgan Co-authored-by: Brendan Abolivier --- changelog.d/13374.bugfix | 1 + synapse/handlers/message.py | 6 ++++-- tests/rest/admin/test_room.py | 15 +++++++++++++++ 3 files changed, 20 insertions(+), 2 deletions(-) create mode 100644 changelog.d/13374.bugfix diff --git a/changelog.d/13374.bugfix b/changelog.d/13374.bugfix new file mode 100644 index 0000000000..1c5bd1b363 --- /dev/null +++ b/changelog.d/13374.bugfix @@ -0,0 +1 @@ +Fix a bug introduced in Synapse 0.24.0 that would respond with the wrong error status code to `/joined_members` requests when the requester is not a current member of the room. Contributed by @andrewdoh. \ No newline at end of file diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index e85b540451..ee0773988e 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -324,8 +324,10 @@ class MessageHandler: room_id, user_id, allow_departed_users=True ) if membership != Membership.JOIN: - raise NotImplementedError( - "Getting joined members after leaving is not implemented" + raise SynapseError( + code=403, + errcode=Codes.FORBIDDEN, + msg="Getting joined members while not being a current member of the room is forbidden.", ) users_with_profile = await self.store.get_users_in_room_with_profiles(room_id) diff --git a/tests/rest/admin/test_room.py b/tests/rest/admin/test_room.py index 623883b53c..989cbdb5e2 100644 --- a/tests/rest/admin/test_room.py +++ b/tests/rest/admin/test_room.py @@ -1772,6 +1772,21 @@ class RoomTestCase(unittest.HomeserverTestCase): tok=admin_user_tok, ) + def test_get_joined_members_after_leave_room(self) -> None: + """Test that requesting room members after leaving the room raises a 403 error.""" + + # create the room + user = self.register_user("foo", "pass") + user_tok = self.login("foo", "pass") + room_id = self.helper.create_room_as(user, tok=user_tok) + self.helper.leave(room_id, user, tok=user_tok) + + # delete the rooms and get joined roomed membership + url = f"/_matrix/client/r0/rooms/{room_id}/joined_members" + channel = self.make_request("GET", url.encode("ascii"), access_token=user_tok) + self.assertEqual(HTTPStatus.FORBIDDEN, channel.code, msg=channel.json_body) + self.assertEqual(Codes.FORBIDDEN, channel.json_body["errcode"]) + class JoinAliasRoomTestCase(unittest.HomeserverTestCase): From 92d21faf12c982a8d27ad465eb94f2fed0e8b32f Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 3 Aug 2022 10:57:38 -0500 Subject: [PATCH 09/10] Instrument `/messages` for understandable traces in Jaeger (#13368) In Jaeger: - Before: huge list of uncategorized database calls - After: nice and collapsible into units of work --- changelog.d/13368.misc | 1 + synapse/api/auth.py | 8 +++++++- synapse/federation/federation_client.py | 2 ++ synapse/handlers/federation.py | 2 ++ synapse/handlers/federation_event.py | 5 +++++ synapse/handlers/pagination.py | 2 ++ synapse/handlers/relations.py | 2 ++ synapse/storage/controllers/state.py | 5 +++++ synapse/storage/databases/main/stream.py | 2 ++ synapse/streams/events.py | 2 ++ synapse/visibility.py | 2 ++ 11 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 changelog.d/13368.misc diff --git a/changelog.d/13368.misc b/changelog.d/13368.misc new file mode 100644 index 0000000000..4b433a5107 --- /dev/null +++ b/changelog.d/13368.misc @@ -0,0 +1 @@ +Instrument `/messages` for understandable traces in Jaeger. diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 82e6475ef5..523bad0c55 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -31,7 +31,12 @@ from synapse.api.errors import ( from synapse.appservice import ApplicationService from synapse.http import get_request_user_agent from synapse.http.site import SynapseRequest -from synapse.logging.opentracing import active_span, force_tracing, start_active_span +from synapse.logging.opentracing import ( + active_span, + force_tracing, + start_active_span, + trace, +) from synapse.storage.databases.main.registration import TokenLookupResult from synapse.types import Requester, UserID, create_requester @@ -567,6 +572,7 @@ class Auth: return query_params[0].decode("ascii") + @trace async def check_user_in_room_or_world_readable( self, room_id: str, user_id: str, allow_departed_users: bool = False ) -> Tuple[str, Optional[str]]: diff --git a/synapse/federation/federation_client.py b/synapse/federation/federation_client.py index 6a8d76529b..54ffbd8170 100644 --- a/synapse/federation/federation_client.py +++ b/synapse/federation/federation_client.py @@ -61,6 +61,7 @@ from synapse.federation.federation_base import ( ) from synapse.federation.transport.client import SendJoinResponse from synapse.http.types import QueryParams +from synapse.logging.opentracing import trace from synapse.types import JsonDict, UserID, get_domain_from_id from synapse.util.async_helpers import concurrently_execute from synapse.util.caches.expiringcache import ExpiringCache @@ -233,6 +234,7 @@ class FederationClient(FederationBase): destination, content, timeout ) + @trace async def backfill( self, dest: str, room_id: str, limit: int, extremities: Collection[str] ) -> Optional[List[EventBase]]: diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 57ad6e5dce..30f1585a85 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -59,6 +59,7 @@ from synapse.events.validator import EventValidator from synapse.federation.federation_client import InvalidResponseError from synapse.http.servlet import assert_params_in_dict from synapse.logging.context import nested_logging_context +from synapse.logging.opentracing import trace from synapse.metrics.background_process_metrics import run_as_background_process from synapse.module_api import NOT_SPAM from synapse.replication.http.federation import ( @@ -180,6 +181,7 @@ class FederationHandler: "resume_sync_partial_state_room", self._resume_sync_partial_state_room ) + @trace async def maybe_backfill( self, room_id: str, current_depth: int, limit: int ) -> bool: diff --git a/synapse/handlers/federation_event.py b/synapse/handlers/federation_event.py index 91d1439191..8968b705d4 100644 --- a/synapse/handlers/federation_event.py +++ b/synapse/handlers/federation_event.py @@ -59,6 +59,7 @@ from synapse.events import EventBase from synapse.events.snapshot import EventContext from synapse.federation.federation_client import InvalidResponseError from synapse.logging.context import nested_logging_context +from synapse.logging.opentracing import trace from synapse.metrics.background_process_metrics import run_as_background_process from synapse.replication.http.devices import ReplicationUserDevicesResyncRestServlet from synapse.replication.http.federation import ( @@ -566,6 +567,7 @@ class FederationEventHandler: event.event_id ) + @trace async def backfill( self, dest: str, room_id: str, limit: int, extremities: Collection[str] ) -> None: @@ -610,6 +612,7 @@ class FederationEventHandler: backfilled=True, ) + @trace async def _get_missing_events_for_pdu( self, origin: str, pdu: EventBase, prevs: Set[str], min_depth: int ) -> None: @@ -710,6 +713,7 @@ class FederationEventHandler: logger.info("Got %d prev_events", len(missing_events)) await self._process_pulled_events(origin, missing_events, backfilled=False) + @trace async def _process_pulled_events( self, origin: str, events: Iterable[EventBase], backfilled: bool ) -> None: @@ -748,6 +752,7 @@ class FederationEventHandler: with nested_logging_context(ev.event_id): await self._process_pulled_event(origin, ev, backfilled=backfilled) + @trace async def _process_pulled_event( self, origin: str, event: EventBase, backfilled: bool ) -> None: diff --git a/synapse/handlers/pagination.py b/synapse/handlers/pagination.py index 6262a35822..e1e34e3b16 100644 --- a/synapse/handlers/pagination.py +++ b/synapse/handlers/pagination.py @@ -24,6 +24,7 @@ from synapse.api.errors import SynapseError from synapse.api.filtering import Filter from synapse.events.utils import SerializeEventConfig from synapse.handlers.room import ShutdownRoomResponse +from synapse.logging.opentracing import trace from synapse.metrics.background_process_metrics import run_as_background_process from synapse.storage.state import StateFilter from synapse.streams.config import PaginationConfig @@ -416,6 +417,7 @@ class PaginationHandler: await self._storage_controllers.purge_events.purge_room(room_id) + @trace async def get_messages( self, requester: Requester, diff --git a/synapse/handlers/relations.py b/synapse/handlers/relations.py index 8f797e3ae9..72d25df8c8 100644 --- a/synapse/handlers/relations.py +++ b/synapse/handlers/relations.py @@ -19,6 +19,7 @@ import attr from synapse.api.constants import RelationTypes from synapse.api.errors import SynapseError from synapse.events import EventBase, relation_from_event +from synapse.logging.opentracing import trace from synapse.storage.databases.main.relations import _RelatedEvent from synapse.types import JsonDict, Requester, StreamToken, UserID from synapse.visibility import filter_events_for_client @@ -361,6 +362,7 @@ class RelationsHandler: return results + @trace async def get_bundled_aggregations( self, events: Iterable[EventBase], user_id: str ) -> Dict[str, BundledAggregations]: diff --git a/synapse/storage/controllers/state.py b/synapse/storage/controllers/state.py index 1e35046e07..0d480f1014 100644 --- a/synapse/storage/controllers/state.py +++ b/synapse/storage/controllers/state.py @@ -29,6 +29,7 @@ from typing import ( from synapse.api.constants import EventTypes from synapse.events import EventBase +from synapse.logging.opentracing import trace from synapse.storage.state import StateFilter from synapse.storage.util.partial_state_events_tracker import ( PartialCurrentStateTracker, @@ -179,6 +180,7 @@ class StateStorageController: return self.stores.state._get_state_groups_from_groups(groups, state_filter) + @trace async def get_state_for_events( self, event_ids: Collection[str], state_filter: Optional[StateFilter] = None ) -> Dict[str, StateMap[EventBase]]: @@ -225,6 +227,7 @@ class StateStorageController: return {event: event_to_state[event] for event in event_ids} + @trace async def get_state_ids_for_events( self, event_ids: Collection[str], @@ -287,6 +290,7 @@ class StateStorageController: ) return state_map[event_id] + @trace async def get_state_ids_for_event( self, event_id: str, state_filter: Optional[StateFilter] = None ) -> StateMap[str]: @@ -327,6 +331,7 @@ class StateStorageController: groups, state_filter or StateFilter.all() ) + @trace async def get_state_group_for_events( self, event_ids: Collection[str], diff --git a/synapse/storage/databases/main/stream.py b/synapse/storage/databases/main/stream.py index 2590b52f73..a347430aa7 100644 --- a/synapse/storage/databases/main/stream.py +++ b/synapse/storage/databases/main/stream.py @@ -58,6 +58,7 @@ from twisted.internet import defer from synapse.api.filtering import Filter from synapse.events import EventBase from synapse.logging.context import make_deferred_yieldable, run_in_background +from synapse.logging.opentracing import trace from synapse.storage._base import SQLBaseStore from synapse.storage.database import ( DatabasePool, @@ -1346,6 +1347,7 @@ class StreamWorkerStore(EventsWorkerStore, SQLBaseStore): return rows, next_token + @trace async def paginate_room_events( self, room_id: str, diff --git a/synapse/streams/events.py b/synapse/streams/events.py index 54e0b1a23b..bcd840bd88 100644 --- a/synapse/streams/events.py +++ b/synapse/streams/events.py @@ -21,6 +21,7 @@ from synapse.handlers.presence import PresenceEventSource from synapse.handlers.receipts import ReceiptEventSource from synapse.handlers.room import RoomEventSource from synapse.handlers.typing import TypingNotificationEventSource +from synapse.logging.opentracing import trace from synapse.streams import EventSource from synapse.types import StreamToken @@ -69,6 +70,7 @@ class EventSources: ) return token + @trace async def get_current_token_for_pagination(self, room_id: str) -> StreamToken: """Get the current token for a given room to be used to paginate events. diff --git a/synapse/visibility.py b/synapse/visibility.py index 9abbaa5a64..d947edde66 100644 --- a/synapse/visibility.py +++ b/synapse/visibility.py @@ -23,6 +23,7 @@ from synapse.api.constants import EventTypes, HistoryVisibility, Membership from synapse.events import EventBase from synapse.events.snapshot import EventContext from synapse.events.utils import prune_event +from synapse.logging.opentracing import trace from synapse.storage.controllers import StorageControllers from synapse.storage.databases.main import DataStore from synapse.storage.state import StateFilter @@ -51,6 +52,7 @@ MEMBERSHIP_PRIORITY = ( _HISTORY_VIS_KEY: Final[Tuple[str, str]] = (EventTypes.RoomHistoryVisibility, "") +@trace async def filter_events_for_client( storage: StorageControllers, user_id: str, From a648a06d52715d0d4ad1ec72d042df1b3fd1be71 Mon Sep 17 00:00:00 2001 From: Shay Date: Wed, 3 Aug 2022 10:19:34 -0700 Subject: [PATCH 10/10] Add some tracing spans to give insight into local joins (#13439) --- changelog.d/13439.misc | 1 + synapse/handlers/message.py | 15 +++++---- synapse/handlers/room_member.py | 57 +++++++++++++++++---------------- 3 files changed, 40 insertions(+), 33 deletions(-) create mode 100644 changelog.d/13439.misc diff --git a/changelog.d/13439.misc b/changelog.d/13439.misc new file mode 100644 index 0000000000..4aa73d7075 --- /dev/null +++ b/changelog.d/13439.misc @@ -0,0 +1 @@ +Add some tracing to give more insight into local room joins. diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index ee0773988e..6b03603598 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -52,6 +52,7 @@ from synapse.events.builder import EventBuilder from synapse.events.snapshot import EventContext from synapse.events.validator import EventValidator from synapse.handlers.directory import DirectoryHandler +from synapse.logging import opentracing from synapse.logging.context import make_deferred_yieldable, run_in_background from synapse.metrics.background_process_metrics import run_as_background_process from synapse.replication.http.send_event import ReplicationSendEventRestServlet @@ -1374,9 +1375,10 @@ class EventCreationHandler: # and `state_groups` because they have `prev_events` that aren't persisted yet # (historical messages persisted in reverse-chronological order). if not event.internal_metadata.is_historical(): - await self._bulk_push_rule_evaluator.action_for_event_by_user( - event, context - ) + with opentracing.start_active_span("calculate_push_actions"): + await self._bulk_push_rule_evaluator.action_for_event_by_user( + event, context + ) try: # If we're a worker we need to hit out to the master. @@ -1463,9 +1465,10 @@ class EventCreationHandler: state = await state_entry.get_state( self._storage_controllers.state, StateFilter.all() ) - joined_hosts = await self.store.get_joined_hosts( - event.room_id, state, state_entry - ) + with opentracing.start_active_span("get_joined_hosts"): + joined_hosts = await self.store.get_joined_hosts( + event.room_id, state, state_entry + ) # Note that the expiry times must be larger than the expiry time in # _external_cache_joined_hosts_updates. diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index 520c52e013..70dc69c809 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -32,6 +32,7 @@ from synapse.event_auth import get_named_level, get_power_level_event from synapse.events import EventBase from synapse.events.snapshot import EventContext from synapse.handlers.profile import MAX_AVATAR_URL_LEN, MAX_DISPLAYNAME_LEN +from synapse.logging import opentracing from synapse.module_api import NOT_SPAM from synapse.storage.state import StateFilter from synapse.types import ( @@ -428,14 +429,14 @@ class RoomMemberHandler(metaclass=abc.ABCMeta): await self._join_rate_per_room_limiter.ratelimit( requester, key=room_id, update=False ) - - result_event = await self.event_creation_handler.handle_new_client_event( - requester, - event, - context, - extra_users=[target], - ratelimit=ratelimit, - ) + with opentracing.start_active_span("handle_new_client_event"): + result_event = await self.event_creation_handler.handle_new_client_event( + requester, + event, + context, + extra_users=[target], + ratelimit=ratelimit, + ) if event.membership == Membership.LEAVE: if prev_member_event_id: @@ -564,25 +565,26 @@ class RoomMemberHandler(metaclass=abc.ABCMeta): # by application services), and then by room ID. async with self.member_as_limiter.queue(as_id): async with self.member_linearizer.queue(key): - result = await self.update_membership_locked( - requester, - target, - room_id, - action, - txn_id=txn_id, - remote_room_hosts=remote_room_hosts, - third_party_signed=third_party_signed, - ratelimit=ratelimit, - content=content, - new_room=new_room, - require_consent=require_consent, - outlier=outlier, - historical=historical, - allow_no_prev_events=allow_no_prev_events, - prev_event_ids=prev_event_ids, - state_event_ids=state_event_ids, - depth=depth, - ) + with opentracing.start_active_span("update_membership_locked"): + result = await self.update_membership_locked( + requester, + target, + room_id, + action, + txn_id=txn_id, + remote_room_hosts=remote_room_hosts, + third_party_signed=third_party_signed, + ratelimit=ratelimit, + content=content, + new_room=new_room, + require_consent=require_consent, + outlier=outlier, + historical=historical, + allow_no_prev_events=allow_no_prev_events, + prev_event_ids=prev_event_ids, + state_event_ids=state_event_ids, + depth=depth, + ) return result @@ -649,6 +651,7 @@ class RoomMemberHandler(metaclass=abc.ABCMeta): Returns: A tuple of the new event ID and stream ID. """ + content_specified = bool(content) if content is None: content = {}