Skip to content

Process SES Notifications into S3 logs #67312

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

Open
wants to merge 1 commit into
base: staging
Choose a base branch
from

Conversation

cat5inthecradle
Copy link
Contributor

@cat5inthecradle cat5inthecradle commented Jul 23, 2025

image

This change captures our SES logging pipeline in Infra-as-code. Today, this is manually configured and the SNS notifications are delivered to an email inbox. I'm changing this to save to S3, in an Athena-ready format.

This creates new environment-specific SNS topics (but only in prod) and I will manually edit SES to use these new topics once live. The old topics, named simply Deliveries, Bounces, and Complaints will be deleted. It should be straightforward to adapt this for further environment separation in the future.

I'm interested in feedback on the lambda implementation in Cloudformation. Attn: @sureshc and @anthony-jackson-code.

I'm also curious about the log bucket choice. Attn: @unlox775-code-dot-org

Links

Testing story

Deployment strategy

Follow-up work

It would be useful to analyze "Bounces", because it could imply that the associated Code.org website user is invalid, or at the very least blocked from ever resetting their password or verifying their email. This is why I went to the trouble of adding a more robust node lambda implementation (even though I'm only deploying index.js) - we can add special handling for "Bounce" or other notifications. For example: notify Dashboard that the given user is bouncing, so they can be flagged or dealt with appropriately.

Privacy

Security

Caching

PR Checklist:

  • Tests provide adequate coverage
  • Privacy and Security impacts have been assessed
  • Code is well-commented
  • New features are translatable or updates will not break translations
  • Relevant documentation has been added or updated
  • User impact is well-understood and desirable
  • Pull Request is labeled appropriately
  • Follow-up work items (including potential tech debt) are tracked and linked

@cat5inthecradle cat5inthecradle requested a review from a team as a code owner July 23, 2025 21:57
Comment on lines +58 to +60
LogsBucketName:
Type: String
Default: cdo-logs
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Capturing this here to make future environment segregation easier.

@sureshc
Copy link
Contributor

sureshc commented Jul 23, 2025

If our primary using of Simple Email Service (SES) is to send transactional emails from our learning platform (Dashboard), could we write create a table / ActiveRecord model / controller to store these directly in the database? Might help with reconciling problems (workshop registration email / survey not received, password reset email not received, etc.) with the Dashboard user who was impacted.

@cat5inthecradle
Copy link
Contributor Author

If our primary using of Simple Email Service (SES) is to send transactional emails from our learning platform (Dashboard), could we write create a table / ActiveRecord model / controller to store these directly in the database? Might help with reconciling problems (workshop registration email / survey not received, password reset email not received, etc.) with the Dashboard user who was impacted.

These notifications are from SES based on what happens with the messages after Dashboard has fired off the request. Are you suggesting having the lambda in this PR send a PUT to dashboard, which will then save the notifications in the database?

I think we should store these notification logs in S3 for simplicity. There's security overhead and performance costs associated with making a /store-ses-notifications route in Dashboard. However, I do think it makes sense to add a /report-email-bounce endpoint that does what you're talking about but is focused on that specific business logic.

@cat5inthecradle
Copy link
Contributor Author

I think we should move forward with this PR as-is. The urgent short term goal is to eliminate the email logs Google user, which this accomplishes. Handling these notifications has never been a priority, but this change makes it easier to do so in the future by putting the notifications in an easier-to-query location and creates the lambda which we could add additional logic to in the future.

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

Successfully merging this pull request may close these issues.

2 participants