Improve visibility/tracing of email delivery

[sergei-maertens]

Related to #3326

Currently emails are delivered asynchronously - this applies to all transactional emails (registration backend, confirmation email...) using django-yubin. This causes the logs for a submission to report a success status because the email was queued successfully.

However, the actual sending/delivery can fail for a multitude of reasons (not limited to just attachment size) and we cannot correlate that back to the submission now.

Ideally, we can attach metadata to the email to be able to correlate it to the record/instance that caused it to be sent so that we can update the relevant logs/audit trails.

E.g. using custom headers for the e-mail:

• X-OF-Content-Type: submissions.submission
• X-OF-Content-UUID: c2e1c102-34c5-4f67-b207-02484454b92b
• X-OF-event: registration-email

We can then periodically (using Celery?) process the mail queue and create TimelineLog records for the statuses/events like succesful delivery or failed delivery. This then also allows us to create digest e-mails to send to the people that have to deal with these issues.

Technical specification

• For outgoing submission registration e-mails, add custom headers:
  • X-OF-Content-Type: submission.submission
  • X-OF-Content-UUID: <uuid of the submission>
  • X-OF-Event: registration
• Ensure that a timeline log (audit) event is created whenever an e-mail sending attempt is performed. Ensure that the success status is logged of this. The log entry must be related to the submission (look this up from the e-mail headers). Some pointers:
  • https://github.com/APSL/django-yubin/blob/master/django_yubin/models.py#L136
  • https://github.com/APSL/django-yubin/blob/master/django_yubin/models.py#L99
  • https://github.com/APSL/django-yubin/blob/master/django_yubin/tasks.py
  • possibly we need to hook into this model's post_save to record every (re)-send attempt
• Set up a daily digest e-mail
  • Ensure you can configure (a list of?) e-mail addresses that should receive the daily digest e-mail in the global configuration
  • Set up a Celery task to collect (relevant) timeline log records that require attention (in this case: failed registration e-mail deliveries)
    • Each record should clearly state what is wrong (e.g. "Email could not be sent") and to which submission it applies (use the public reference so people can search in the admin)
    • Entries should be de-duplicated - django-yubin will probably automatically try re-sending so you will likely have multiple identical errors about mail delivery not succeeding
    • The e-mail should be sent once every day to all configured recipients, configure this using celery beat. Doing this at 11pm seems appropriate, so that it's in people's TODO list at the start of the next work day. Only consider log entries from that day (e.g. do not include errors from yesterday/the day before yesterday)
    • Do not send any mail if there's nothing to report.

Please break up the tasks into separate, small, easy to review PRs!

[joeribekker]

Not sure if the ordering of events allows it but isn't it easier to create a Message instance and link this message to the submission and THEN schedule it?

[sergei-maertens]

Not sure if the ordering of events allows it but isn't it easier to create a Message instance and link this message to the submission and THEN schedule it?

This feels like duplicating django-yubin, they already have a Message model instance in the DB that stores all the information of the email (which is then used in the queue to actually deliver the emails).

This approach would also have to rely on generic foreign keys which bring their own set of problems and don't have any integrity guarantees. Using the email headers is imo much more end-to-end reliable since you can correlate what is being sent and logged on our end with the actual emails being delivered (or not) to mailboxes. It's a similar approach to (distributed) tracing like APM/Jaeger for microservices.

[joeribekker]

I'm leaving the technical details to the team, but using the header sounded less reliable (lees: houtje touwtje) than FKs.

[sergei-maertens]

I'm leaving the technical details to the team, but using the header sounded less reliable (lees: houtje touwtje) than FKs.

👍 I understand that, I'm afraid it's the only option though. Since e-mail is actually sent in application code through django.core.mail, we don't get any reference to the resulting email message at the location where we do the e-mail sending.