From b80774efb2ea5ae780ab751aad5de52ac7a2b0de Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 17 Feb 2026 13:15:57 -0600 Subject: [PATCH] Better instrument `JoinRoomAliasServlet` with tracing (#19461) So we can better see why it decides to do a local vs remote join. Spawning from [investigating a join issue on `matrix.org`](https://matrix.to/#/!SGNQGPGUwtcPBUotTL:matrix.org/$Odvd47QtkRscxilzkhcFOsDZWNvJUSEhSrD8GpukKWo?via=jki.re&via=element.io&via=matrix.org). --- changelog.d/19461.misc | 1 + synapse/handlers/room_member.py | 24 +++++++++++++++++++++--- synapse/handlers/room_member_worker.py | 5 +++++ 3 files changed, 27 insertions(+), 3 deletions(-) create mode 100644 changelog.d/19461.misc diff --git a/changelog.d/19461.misc b/changelog.d/19461.misc new file mode 100644 index 0000000000..9c444ffe3c --- /dev/null +++ b/changelog.d/19461.misc @@ -0,0 +1 @@ +Better instrument `JoinRoomAliasServlet` with tracing. diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index a8935fded6..0c6be72716 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -49,6 +49,7 @@ from synapse.handlers.profile import MAX_AVATAR_URL_LEN, MAX_DISPLAYNAME_LEN from synapse.handlers.state_deltas import MatchChange, StateDeltasHandler from synapse.handlers.worker_lock import NEW_EVENT_DURING_PURGE_LOCK_NAME from synapse.logging import opentracing +from synapse.logging.opentracing import SynapseTags, set_tag, tag_args, trace from synapse.metrics import SERVER_NAME_LABEL, event_processing_positions from synapse.replication.http.push import ReplicationCopyPusherRestServlet from synapse.storage.databases.main.state_deltas import StateDelta @@ -390,6 +391,7 @@ class RoomMemberHandler(metaclass=abc.ABCMeta): if requester is not None: await self._invites_per_issuer_limiter.ratelimit(requester) + @trace async def _local_membership_update( self, *, @@ -1221,6 +1223,8 @@ class RoomMemberHandler(metaclass=abc.ABCMeta): if result is None or result == (None, None): raise AuthError(403, f"User {user_id} has no membership in room {room_id}") + @trace + @tag_args async def _should_perform_remote_join( self, user_id: str, @@ -1275,6 +1279,12 @@ class RoomMemberHandler(metaclass=abc.ABCMeta): prev_member_event = await self.store.get_event(prev_member_event_id) previous_membership = prev_member_event.membership + # Interesting because it's used in the logic below to make decisions + set_tag( + SynapseTags.RESULT_PREFIX + "previous_membership", + str(previous_membership), + ) + # If we are not fully joined yet, and the target is not already in the room, # let's do a remote join so another server with the full state can validate # that the user has not been banned for example. @@ -1315,15 +1325,19 @@ class RoomMemberHandler(metaclass=abc.ABCMeta): state_key: event_map[event_id] for state_key, event_id in state_before_join.items() } - allowed_servers = get_servers_from_users( + servers_that_can_issue_invite = get_servers_from_users( get_users_which_can_issue_invite(current_state) ) + set_tag( + SynapseTags.RESULT_PREFIX + "servers_that_can_issue_invite", + str(servers_that_can_issue_invite), + ) # If the local server is not one of allowed servers, then a remote # join must be done. Return the list of prospective servers based on # which can issue invites. - if self.hs.hostname not in allowed_servers: - return True, list(allowed_servers) + if self.hs.hostname not in servers_that_can_issue_invite: + return True, list(servers_that_can_issue_invite) # Ensure the member should be allowed access via membership in a room. await self.event_auth_handler.check_restricted_join_rules( @@ -1897,6 +1911,7 @@ class RoomMemberMasterHandler(RoomMemberHandler): return complexity["v1"] > max_complexity + @trace async def _remote_join( self, requester: Requester, @@ -1975,6 +1990,7 @@ class RoomMemberMasterHandler(RoomMemberHandler): return event_id, stream_id + @trace async def remote_reject_invite( self, invite_event_id: str, @@ -2012,6 +2028,7 @@ class RoomMemberMasterHandler(RoomMemberHandler): invite_event, txn_id, requester, content ) + @trace async def remote_rescind_knock( self, knock_event_id: str, @@ -2124,6 +2141,7 @@ class RoomMemberMasterHandler(RoomMemberHandler): return result_event.event_id, result_event.internal_metadata.stream_ordering + @trace async def remote_knock( self, requester: Requester, diff --git a/synapse/handlers/room_member_worker.py b/synapse/handlers/room_member_worker.py index b56519ab0a..da63525598 100644 --- a/synapse/handlers/room_member_worker.py +++ b/synapse/handlers/room_member_worker.py @@ -23,6 +23,7 @@ import logging from typing import TYPE_CHECKING from synapse.handlers.room_member import NoKnownServersError, RoomMemberHandler +from synapse.logging.opentracing import trace from synapse.replication.http.membership import ( ReplicationRemoteJoinRestServlet as ReplRemoteJoin, ReplicationRemoteKnockRestServlet as ReplRemoteKnock, @@ -48,6 +49,7 @@ class RoomMemberWorkerHandler(RoomMemberHandler): self._remote_rescind_client = ReplRescindKnock.make_client(hs) self._notify_change_client = ReplJoinedLeft.make_client(hs) + @trace async def _remote_join( self, requester: Requester, @@ -70,6 +72,7 @@ class RoomMemberWorkerHandler(RoomMemberHandler): return ret["event_id"], ret["stream_id"] + @trace async def remote_reject_invite( self, invite_event_id: str, @@ -90,6 +93,7 @@ class RoomMemberWorkerHandler(RoomMemberHandler): ) return ret["event_id"], ret["stream_id"] + @trace async def remote_rescind_knock( self, knock_event_id: str, @@ -118,6 +122,7 @@ class RoomMemberWorkerHandler(RoomMemberHandler): ) return ret["event_id"], ret["stream_id"] + @trace async def remote_knock( self, requester: Requester,