cabot icon indicating copy to clipboard operation
cabot copied to clipboard

Question about the cabot-alert-email plugin

Open jchoksi-whitbread opened this issue 7 years ago • 5 comments

We configured Cabot to use AWS's SES service for sending e-mail.

AWS's SES service has a hard limit of a maximum of 50 recipients for every message you send.

We recently added a service to Cabot which had quite a few Cabot users setup to be alerted. Say around 30 people.

The single HTTP check in this service, was configured to have an importance of CRITICAL.

When this service triggered an alert, an e-mail failed to be sent out.

We saw a SMTPDataError: (554, 'Transaction failed: Recipient count exceeds 50.') error in Cabot's worker logs.

We couldn't see how the e-mail was being sent out to >=50 recipients until we came across the following code in: https://github.com/arachnys/cabot-alert-email/blob/master/cabot_alert_email/models.py#L39

emails += [u.email for u in users if u.email]

That code seems to be duplicating the list of email addresses already gathered in line 29.

Could you please confirm whether line 39 is doing something in error?

Edit: FYI. we worked around the issue by setting the HTTP check's importance to be ERROR.

jchoksi-whitbread avatar Oct 17 '17 20:10 jchoksi-whitbread

Looks like a simple error. I have no idea what that line is meant to be doing! @frankh ?

dbuxton avatar Oct 18 '17 00:10 dbuxton

This looks like it was introduced in https://github.com/cabotapp/cabot-alert-email/commit/2d499fd22381aa72bb1ec669b4bcb9f973091bf4 − it does make more sense to be adding the duty officers.

P.S. I’ll be deleting the arachys/cabot-alert-email fork as the cabotapp one should be the source of truth, and it’s confusing

JeanFred avatar Oct 18 '17 07:10 JeanFred

I submitted https://github.com/cabotapp/cabot-alert-email/pull/5 which should fix it

JeanFred avatar Oct 18 '17 08:10 JeanFred

Additionally, the plugin should be deduplicating redundant emails by using a set − this would have mitigated the issue @jchoksi-whitbread encountered. Will also push a fix.

JeanFred avatar Oct 18 '17 08:10 JeanFred

@jchoksi-whitbread are you still experiencing this issue? i am also considering aws and would like to understand if this can be closed.

whysthatso avatar Nov 24 '18 15:11 whysthatso