Skip to content

Log delivered_at and delivery_latency for immediate email deliveries#2128

Merged
theseanything merged 2 commits into
mainfrom
theseanything/log-immediate-delivery-timing
May 29, 2026
Merged

Log delivered_at and delivery_latency for immediate email deliveries#2128
theseanything merged 2 commits into
mainfrom
theseanything/log-immediate-delivery-timing

Conversation

@theseanything
Copy link
Copy Markdown
Contributor

Slow immediate email deliveries are hard to investigate when latency only lives in CloudWatch. This adds delivered_at and delivery_latency to job logs so we can spot patterns in log queries—e.g. whether delays cluster on a particular form—without cross-referencing metrics for every case.

Batch deliveries are unchanged. delivery_latency is milliseconds from submission creation to SES delivery confirmation.

stephencdaly
stephencdaly previously approved these changes May 28, 2026
Copy link
Copy Markdown
Contributor

@stephencdaly stephencdaly left a comment

Choose a reason for hiding this comment

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

This seems fine. I was wondering whether it makes sense to pass delivery_duration_ms in a hash as the second argument to EventLogger.log_form_event, as we only need to log this one place.

The intention of CurrentJobLoggingAttributes is more for setting attributes that will be set on most/many log lines. I don't think it's a big deal either way though.

@theseanything theseanything force-pushed the theseanything/log-immediate-delivery-timing branch 2 times, most recently from aad9d0e to 8ac0ad0 Compare May 28, 2026 15:47
@theseanything theseanything force-pushed the theseanything/log-immediate-delivery-timing branch from 8ac0ad0 to 7f3f7e2 Compare May 28, 2026 15:48
@theseanything
Copy link
Copy Markdown
Contributor Author

This seems fine. I was wondering whether it makes sense to pass delivery_duration_ms in a hash as the second argument to EventLogger.log_form_event, as we only need to log this one place.

The intention of CurrentJobLoggingAttributes is more for setting attributes that will be set on most/many log lines. I don't think it's a big deal either way though.

Ahhhh yeah that makes more sense - just updated it do that instead.

@github-actions
Copy link
Copy Markdown
Contributor

🎉 A review copy of this PR has been deployed! You can reach it at: https://pr-2128.submit.review.forms.service.gov.uk/

It may take 5 minutes or so for the application to be fully deployed and working. If it still isn't ready
after 5 minutes, there may be something wrong with the ECS task. You will need to go to the integration AWS account
to debug, or otherwise ask an infrastructure person.

For the sign in details and more information, see the review apps wiki page.

@theseanything theseanything added this pull request to the merge queue May 29, 2026
Merged via the queue into main with commit 6e11938 May 29, 2026
3 checks passed
@theseanything theseanything deleted the theseanything/log-immediate-delivery-timing branch May 29, 2026 07:18
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