Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Option to suppress resource exceeded alerting #6173

Merged
merged 19 commits into from
Oct 24, 2019
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/6173.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add config option to suppress client side resource limit alerting.
8 changes: 7 additions & 1 deletion docs/sample_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,6 @@ listeners:
#
#hs_disabled: false
#hs_disabled_message: 'Human readable reason for why the HS is blocked'
#hs_disabled_limit_type: 'error code(str), to help clients decode reason'

# Monthly Active User Blocking
#
Expand All @@ -261,9 +260,16 @@ listeners:
# sign up in a short space of time never to return after their initial
# session.
#
# 'mau_limit_alerting' is a means of limiting client side alerting
# should the mau limit be reached. This is useful for small instances
# where the admin has 5 mau seats (say) for 5 specific people and no
# interest increasing the mau limit further. Defaults to True, which
# means that alerting is enabled
#
#limit_usage_by_mau: false
#max_mau_value: 50
#mau_trial_days: 2
#mau_limit_alerting: false

# If enabled, the metrics for the number of monthly active users will
# be populated, however no one will be limited. If limit_usage_by_mau
Expand Down
12 changes: 9 additions & 3 deletions synapse/api/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,13 @@
import synapse.logging.opentracing as opentracing
import synapse.types
from synapse import event_auth
from synapse.api.constants import EventTypes, JoinRules, Membership, UserTypes
from synapse.api.constants import (
EventTypes,
JoinRules,
LimitBlockingTypes,
Membership,
UserTypes,
)
from synapse.api.errors import (
AuthError,
Codes,
Expand Down Expand Up @@ -726,7 +732,7 @@ def check_auth_blocking(self, user_id=None, threepid=None, user_type=None):
self.hs.config.hs_disabled_message,
errcode=Codes.RESOURCE_LIMIT_EXCEEDED,
admin_contact=self.hs.config.admin_contact,
limit_type=self.hs.config.hs_disabled_limit_type,
limit_type=LimitBlockingTypes.HS_DISABLED,
)
if self.hs.config.limit_usage_by_mau is True:
assert not (user_id and threepid)
Expand Down Expand Up @@ -759,5 +765,5 @@ def check_auth_blocking(self, user_id=None, threepid=None, user_type=None):
"Monthly Active User Limit Exceeded",
admin_contact=self.hs.config.admin_contact,
errcode=Codes.RESOURCE_LIMIT_EXCEEDED,
limit_type="monthly_active_user",
limit_type=LimitBlockingTypes.MONTHLY_ACTIVE_USER,
)
7 changes: 7 additions & 0 deletions synapse/api/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,3 +131,10 @@ class RelationTypes(object):
ANNOTATION = "m.annotation"
REPLACE = "m.replace"
REFERENCE = "m.reference"


class LimitBlockingTypes(object):
"""Reasons that a server may be blocked"""

MONTHLY_ACTIVE_USER = "monthly_active_user"
HS_DISABLED = "hs_disabled"
10 changes: 8 additions & 2 deletions synapse/config/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ def read_config(self, config, **kwargs):
)

self.mau_trial_days = config.get("mau_trial_days", 0)
self.mau_limit_alerting = config.get("mau_limit_alerting", True)

# How long to keep redacted events in the database in unredacted form
# before redacting them.
Expand All @@ -192,7 +193,6 @@ def read_config(self, config, **kwargs):
# Options to disable HS
self.hs_disabled = config.get("hs_disabled", False)
self.hs_disabled_message = config.get("hs_disabled_message", "")
self.hs_disabled_limit_type = config.get("hs_disabled_limit_type", "")

# Admin uri to direct users at should their instance become blocked
# due to resource constraints
Expand Down Expand Up @@ -675,7 +675,6 @@ def generate_config_section(
#
#hs_disabled: false
#hs_disabled_message: 'Human readable reason for why the HS is blocked'
#hs_disabled_limit_type: 'error code(str), to help clients decode reason'

# Monthly Active User Blocking
#
Expand All @@ -695,9 +694,16 @@ def generate_config_section(
# sign up in a short space of time never to return after their initial
# session.
#
# 'mau_limit_alerting' is a means of limiting client side alerting
# should the mau limit be reached. This is useful for small instances
# where the admin has 5 mau seats (say) for 5 specific people and no
# interest increasing the mau limit further. Defaults to True, which
# means that alerting is enabled
#
#limit_usage_by_mau: false
#max_mau_value: 50
#mau_trial_days: 2
#mau_limit_alerting: false

# If enabled, the metrics for the number of monthly active users will
# be populated, however no one will be limited. If limit_usage_by_mau
Expand Down
110 changes: 72 additions & 38 deletions synapse/server_notices/resource_limits_server_notices.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

from synapse.api.constants import (
EventTypes,
LimitBlockingTypes,
ServerNoticeLimitReached,
ServerNoticeMsgType,
)
Expand Down Expand Up @@ -70,7 +71,7 @@ def maybe_send_server_notice_to_user(self, user_id):
return

if not self._server_notices_manager.is_enabled():
# Don't try and send server notices unles they've been enabled
# Don't try and send server notices unless they've been enabled
return

timestamp = yield self._store.user_last_seen_monthly_active(user_id)
Expand All @@ -79,59 +80,92 @@ def maybe_send_server_notice_to_user(self, user_id):
# In practice, not sure we can ever get here
return

# Determine current state of room

room_id = yield self._server_notices_manager.get_notice_room_for_user(user_id)

if not room_id:
logger.warn("Failed to get server notices room")
return

yield self._check_and_set_tags(user_id, room_id)

# Determine current state of room
currently_blocked, ref_events = yield self._is_room_currently_blocked(room_id)

limit_msg = None
limit_type = None
try:
# Normally should always pass in user_id if you have it, but in
# this case are checking what would happen to other users if they
# were to arrive.
try:
yield self._auth.check_auth_blocking()
is_auth_blocking = False
except ResourceLimitError as e:
is_auth_blocking = True
event_content = e.msg
event_limit_type = e.limit_type

if currently_blocked and not is_auth_blocking:
# Room is notifying of a block, when it ought not to be.
# Remove block notification
content = {"pinned": ref_events}
yield self._server_notices_manager.send_notice(
user_id, content, EventTypes.Pinned, ""
)
# Normally should always pass in user_id to check_auth_blocking
# if you have it, but in this case are checking what would happen
# to other users if they were to arrive.
yield self._auth.check_auth_blocking()
except ResourceLimitError as e:
limit_msg = e.msg
limit_type = e.limit_type

elif not currently_blocked and is_auth_blocking:
try:
if (
limit_type == LimitBlockingTypes.MONTHLY_ACTIVE_USER
and not self._config.mau_limit_alerting
):
# We have hit the MAU limit, but MAU alerting is disabled:
# reset room if necessary and return
if currently_blocked:
self._remove_limit_block_notification(user_id, ref_events)
return

if currently_blocked and not limit_type:
# Room is notifying of a block, when it ought not to be.
yield self._remove_limit_block_notification(user_id, ref_events)
elif not currently_blocked and limit_type:
# Room is not notifying of a block, when it ought to be.
# Add block notification
content = {
"body": event_content,
"msgtype": ServerNoticeMsgType,
"server_notice_type": ServerNoticeLimitReached,
"admin_contact": self._config.admin_contact,
"limit_type": event_limit_type,
}
event = yield self._server_notices_manager.send_notice(
user_id, content, EventTypes.Message
yield self._apply_limit_block_notification(
user_id, limit_msg, limit_type
)

content = {"pinned": [event.event_id]}
yield self._server_notices_manager.send_notice(
user_id, content, EventTypes.Pinned, ""
)

except SynapseError as e:
logger.error("Error sending resource limits server notice: %s", e)

@defer.inlineCallbacks
def _remove_limit_block_notification(self, user_id, ref_events):
"""Utility method to remove limit block notifications from the server
notices room.

Args:
user_id (str): user to notify
ref_events (list[str]): The event_ids of pinned events that are unrelated to
limit blocking and need to be preserved.
"""
content = {"pinned": ref_events}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems to be an existing problem (so not necessarily for fixing as part of this PR), but why do we preserve the other events in this case and not in the _apply_limit_block_notification case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From memory the idea was to shut everything down and so all tags should be discarded, but in retrospect this seems unhelpful.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unfixed, will address in a separate PR.

yield self._server_notices_manager.send_notice(
user_id, content, EventTypes.Pinned, ""
)

@defer.inlineCallbacks
def _apply_limit_block_notification(self, user_id, event_body, event_limit_type):
"""Utility method to apply limit block notifications in the server
notices room.

Args:
user_id (str): user to notify
event_body(str): The human readable text that describes the block.
event_limit_type(str): Specifies the type of block e.g. monthly active user
limit has been exceeded.
"""
content = {
"body": event_body,
"msgtype": ServerNoticeMsgType,
"server_notice_type": ServerNoticeLimitReached,
"admin_contact": self._config.admin_contact,
"limit_type": event_limit_type,
}
event = yield self._server_notices_manager.send_notice(
user_id, content, EventTypes.Message
)

content = {"pinned": [event.event_id]}
yield self._server_notices_manager.send_notice(
user_id, content, EventTypes.Pinned, ""
)

@defer.inlineCallbacks
def _check_and_set_tags(self, user_id, room_id):
"""
Expand Down
57 changes: 56 additions & 1 deletion tests/server_notices/test_resource_limits_server_notices.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

from twisted.internet import defer

from synapse.api.constants import EventTypes, ServerNoticeMsgType
from synapse.api.constants import EventTypes, LimitBlockingTypes, ServerNoticeMsgType
from synapse.api.errors import ResourceLimitError
from synapse.server_notices.resource_limits_server_notices import (
ResourceLimitsServerNotices,
Expand Down Expand Up @@ -158,6 +158,61 @@ def test_maybe_send_server_notice_to_user_not_in_mau_cohort(self):

self._send_notice.assert_not_called()

def test_maybe_send_server_notice_when_alerting_suppressed_room_unblocked(self):
"""
Test that when server is over MAU limit and alerting is suppressed, then
an alert message is not sent into the room
"""
self.hs.config.mau_limit_alerting = False
self._rlsn._auth.check_auth_blocking = Mock(
side_effect=ResourceLimitError(
403, "foo", limit_type=LimitBlockingTypes.MONTHLY_ACTIVE_USER
)
)
self.get_success(self._rlsn.maybe_send_server_notice_to_user(self.user_id))

self.assertTrue(self._send_notice.call_count == 0)

def test_check_hs_disabled_unaffected_by_mau_alert_suppression(self):
"""
Test that when a server is disabled, that MAU limit alerting is ignored.
"""
self.hs.config.mau_limit_alerting = False
self._rlsn._auth.check_auth_blocking = Mock(
side_effect=ResourceLimitError(
403, "foo", limit_type=LimitBlockingTypes.HS_DISABLED
)
)
self.get_success(self._rlsn.maybe_send_server_notice_to_user(self.user_id))

# Would be better to check contents, but 2 calls == set blocking event
self.assertEqual(self._send_notice.call_count, 2)

def test_maybe_send_server_notice_when_alerting_suppressed_room_blocked(self):
"""
When the room is already in a blocked state, test that when alerting
is suppressed that the room is returned to an unblocked state.
"""
self.hs.config.mau_limit_alerting = False
self._rlsn._auth.check_auth_blocking = Mock(
side_effect=ResourceLimitError(
403, "foo", limit_type=LimitBlockingTypes.MONTHLY_ACTIVE_USER
)
)
self._rlsn._server_notices_manager.__is_room_currently_blocked = Mock(
return_value=defer.succeed((True, []))
)

mock_event = Mock(
type=EventTypes.Message, content={"msgtype": ServerNoticeMsgType}
)
self._rlsn._store.get_events = Mock(
return_value=defer.succeed({"123": mock_event})
)
self.get_success(self._rlsn.maybe_send_server_notice_to_user(self.user_id))

self._send_notice.assert_called_once()


class TestResourceLimitsServerNoticesWithRealRooms(unittest.HomeserverTestCase):
def prepare(self, reactor, clock, hs):
Expand Down
1 change: 0 additions & 1 deletion tests/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,6 @@ def default_config(name, parse=False):
"limit_usage_by_mau": False,
"hs_disabled": False,
"hs_disabled_message": "",
"hs_disabled_limit_type": "",
"max_mau_value": 50,
"mau_trial_days": 0,
"mau_stats_only": False,
Expand Down