Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configurable Throttle Limits for each Notification Type #2653

Closed
Lakshmi2107 opened this issue Oct 10, 2022 · 2 comments · Fixed by #2654
Closed

Configurable Throttle Limits for each Notification Type #2653

Lakshmi2107 opened this issue Oct 10, 2022 · 2 comments · Fixed by #2654
Labels
enhancement New feature or request

Comments

@Lakshmi2107
Copy link
Contributor

Currently, we have rate limiting configuration set for each notification type per contact method. This is directly set in the code and not dynamic. If there's a need to change in the rate limits it needs redeployment of code every time.

Solution:
We can make the rate limits configurable in UI under admin panel. We have config limits set for unacknowledged alerts, contact methods per user etc..Similarly we can have it for throttling which can be modified by the admin according to their needs. We can have entries added in config_limits table with the maximum limit as value.

Example Screenshot:
Screenshot 2022-10-10 at 9 50 01 PM

@mastercactapus
Copy link
Member

mastercactapus commented Oct 17, 2022

For some context, there are three main concerns throttling rules are intended to solve:

  • Noise to the on-call user during various scenarios (probably the most relevant to this issue as it's subjective)
  • known API limits (e.g., Slack, Twilio set this, hard-coded values are based on public documentation for this req.)
  • Hidden rate limits imposed by cell carriers (these are set based on SMS provider recommendations)

The last two are important to prevent API lockout or to have all of your messages/calls dropped as spam (i.e., if you send too many too fast, your number gets blocked from the cell network entirely, at least for a time).


We discussed this a lot when we first put throttling in. The underlying provider/notification system will change to be more dynamic with the plugin work for #2639, so the underlying code/approach will likely be rewritten entirely.

I agree the admin should get the final say, but could you explain more about the specific use case this is solving for?
What problems exist with the current rules that should be looked at?

This ^ would clarify a lot and help design the new approach.

@Lakshmi2107
Copy link
Contributor Author

@mastercactapus Sorry for the late reply. We might need this feature during peak time when the same person might receive multiple alerts within a short interval. If we can make it admin configurable, we will be able to configure it based on our needs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants