diff options
| -rw-r--r-- | changelog.d/12513.feature | 1 | ||||
| -rw-r--r-- | synapse/events/spamcheck.py | 81 | ||||
| -rw-r--r-- | synapse/server.py | 2 | ||||
| -rw-r--r-- | tests/rest/client/test_rooms.py | 6 |
4 files changed, 64 insertions, 26 deletions
diff --git a/changelog.d/12513.feature b/changelog.d/12513.feature new file mode 100644 index 0000000000..01bf1d9d2c --- /dev/null +++ b/changelog.d/12513.feature @@ -0,0 +1 @@ +Measure the time taken in spam-checking callbacks and expose those measurements as metrics. diff --git a/synapse/events/spamcheck.py b/synapse/events/spamcheck.py index 3b6795d40f..f30207376a 100644 --- a/synapse/events/spamcheck.py +++ b/synapse/events/spamcheck.py @@ -32,6 +32,7 @@ from synapse.rest.media.v1.media_storage import ReadableFileWrapper from synapse.spam_checker_api import RegistrationBehaviour from synapse.types import RoomAlias, UserProfile from synapse.util.async_helpers import delay_cancellation, maybe_awaitable +from synapse.util.metrics import Measure if TYPE_CHECKING: import synapse.events @@ -162,7 +163,10 @@ def load_legacy_spam_checkers(hs: "synapse.server.HomeServer") -> None: class SpamChecker: - def __init__(self) -> None: + def __init__(self, hs: "synapse.server.HomeServer") -> None: + self.hs = hs + self.clock = hs.get_clock() + self._check_event_for_spam_callbacks: List[CHECK_EVENT_FOR_SPAM_CALLBACK] = [] self._user_may_join_room_callbacks: List[USER_MAY_JOIN_ROOM_CALLBACK] = [] self._user_may_invite_callbacks: List[USER_MAY_INVITE_CALLBACK] = [] @@ -255,7 +259,10 @@ class SpamChecker: will be used as the error message returned to the user. """ for callback in self._check_event_for_spam_callbacks: - res: Union[bool, str] = await delay_cancellation(callback(event)) + with Measure( + self.clock, "{}.{}".format(callback.__module__, callback.__qualname__) + ): + res: Union[bool, str] = await delay_cancellation(callback(event)) if res: return res @@ -276,9 +283,12 @@ class SpamChecker: Whether the user may join the room """ for callback in self._user_may_join_room_callbacks: - may_join_room = await delay_cancellation( - callback(user_id, room_id, is_invited) - ) + with Measure( + self.clock, "{}.{}".format(callback.__module__, callback.__qualname__) + ): + may_join_room = await delay_cancellation( + callback(user_id, room_id, is_invited) + ) if may_join_room is False: return False @@ -300,9 +310,12 @@ class SpamChecker: True if the user may send an invite, otherwise False """ for callback in self._user_may_invite_callbacks: - may_invite = await delay_cancellation( - callback(inviter_userid, invitee_userid, room_id) - ) + with Measure( + self.clock, "{}.{}".format(callback.__module__, callback.__qualname__) + ): + may_invite = await delay_cancellation( + callback(inviter_userid, invitee_userid, room_id) + ) if may_invite is False: return False @@ -328,9 +341,12 @@ class SpamChecker: True if the user may send the invite, otherwise False """ for callback in self._user_may_send_3pid_invite_callbacks: - may_send_3pid_invite = await delay_cancellation( - callback(inviter_userid, medium, address, room_id) - ) + with Measure( + self.clock, "{}.{}".format(callback.__module__, callback.__qualname__) + ): + may_send_3pid_invite = await delay_cancellation( + callback(inviter_userid, medium, address, room_id) + ) if may_send_3pid_invite is False: return False @@ -348,7 +364,10 @@ class SpamChecker: True if the user may create a room, otherwise False """ for callback in self._user_may_create_room_callbacks: - may_create_room = await delay_cancellation(callback(userid)) + with Measure( + self.clock, "{}.{}".format(callback.__module__, callback.__qualname__) + ): + may_create_room = await delay_cancellation(callback(userid)) if may_create_room is False: return False @@ -369,9 +388,12 @@ class SpamChecker: True if the user may create a room alias, otherwise False """ for callback in self._user_may_create_room_alias_callbacks: - may_create_room_alias = await delay_cancellation( - callback(userid, room_alias) - ) + with Measure( + self.clock, "{}.{}".format(callback.__module__, callback.__qualname__) + ): + may_create_room_alias = await delay_cancellation( + callback(userid, room_alias) + ) if may_create_room_alias is False: return False @@ -390,7 +412,10 @@ class SpamChecker: True if the user may publish the room, otherwise False """ for callback in self._user_may_publish_room_callbacks: - may_publish_room = await delay_cancellation(callback(userid, room_id)) + with Measure( + self.clock, "{}.{}".format(callback.__module__, callback.__qualname__) + ): + may_publish_room = await delay_cancellation(callback(userid, room_id)) if may_publish_room is False: return False @@ -412,9 +437,13 @@ class SpamChecker: True if the user is spammy. """ for callback in self._check_username_for_spam_callbacks: - # Make a copy of the user profile object to ensure the spam checker cannot - # modify it. - if await delay_cancellation(callback(user_profile.copy())): + with Measure( + self.clock, "{}.{}".format(callback.__module__, callback.__qualname__) + ): + # Make a copy of the user profile object to ensure the spam checker cannot + # modify it. + res = await delay_cancellation(callback(user_profile.copy())) + if res: return True return False @@ -442,9 +471,12 @@ class SpamChecker: """ for callback in self._check_registration_for_spam_callbacks: - behaviour = await delay_cancellation( - callback(email_threepid, username, request_info, auth_provider_id) - ) + with Measure( + self.clock, "{}.{}".format(callback.__module__, callback.__qualname__) + ): + behaviour = await delay_cancellation( + callback(email_threepid, username, request_info, auth_provider_id) + ) assert isinstance(behaviour, RegistrationBehaviour) if behaviour != RegistrationBehaviour.ALLOW: return behaviour @@ -486,7 +518,10 @@ class SpamChecker: """ for callback in self._check_media_file_for_spam_callbacks: - spam = await delay_cancellation(callback(file_wrapper, file_info)) + with Measure( + self.clock, "{}.{}".format(callback.__module__, callback.__qualname__) + ): + spam = await delay_cancellation(callback(file_wrapper, file_info)) if spam: return True diff --git a/synapse/server.py b/synapse/server.py index 7daa7b9334..ee60cce8eb 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -681,7 +681,7 @@ class HomeServer(metaclass=abc.ABCMeta): @cache_in_self def get_spam_checker(self) -> SpamChecker: - return SpamChecker() + return SpamChecker(self) @cache_in_self def get_third_party_event_rules(self) -> ThirdPartyEventRules: diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index ad416e2fd8..d0197aca94 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -925,7 +925,7 @@ class RoomJoinTestCase(RoomBase): ) -> bool: return return_value - callback_mock = Mock(side_effect=user_may_join_room) + callback_mock = Mock(side_effect=user_may_join_room, spec=lambda *x: None) self.hs.get_spam_checker()._user_may_join_room_callbacks.append(callback_mock) # Join a first room, without being invited to it. @@ -2856,7 +2856,9 @@ class ThreepidInviteTestCase(unittest.HomeserverTestCase): # Add a mock to the spamchecker callbacks for user_may_send_3pid_invite. Make it # allow everything for now. - mock = Mock(return_value=make_awaitable(True)) + # `spec` argument is needed for this function mock to have `__qualname__`, which + # is needed for `Measure` metrics buried in SpamChecker. + mock = Mock(return_value=make_awaitable(True), spec=lambda *x: None) self.hs.get_spam_checker()._user_may_send_3pid_invite_callbacks.append(mock) # Send a 3PID invite into the room and check that it succeeded. |