Skip to content

Conversation

@jvdp
Copy link
Contributor

@jvdp jvdp commented Dec 19, 2018

Since production applications typically run with log level info and email adresses should be considered as sensitive data (or personal data under GDPR) we want to prevent them from ending up in the logs. In development mode (with log level debug) they are still logged as part of the Mail::Message object.

While we could add another config option for this (#19293) I propose we fix the privacy issue by doing the right thing by default, and custom log subscribers could be written to restore the old functionality.

(As a bonus, the wording in the logs is now consistent with the event name ;))

Since production applications typically run with log level info and
email adresses should be considered as sensitive data we want to prevent 
them from ending up in the logs. In development mode (with log level
debug) they are still logged as part of the Mail::Message object.
@georgeclaghorn
Copy link
Contributor

👍 for not logging personal data when it’s not needed. Can we log the Message-ID instead, so that outbound emails in application logs can still be correlated with mail server logs?

@robzolkos
Copy link
Contributor

robzolkos commented Jan 14, 2019

I'm not fond of the Delivered mail text as thats not actually what happened. The app has sent the email but it isn't responsible for delivering it or knowledgable if it has been delivered.

Yes the mail is part of a delivery but to call it delivered is not accurate imo.

Everything else about this change however is spot on. Love the removal of private info from logs.

@jvdp
Copy link
Contributor Author

jvdp commented Feb 13, 2019

👍 for not logging personal data when it’s not needed. Can we log the Message-ID instead, so that outbound emails in application logs can still be correlated with mail server logs?

Good idea! Took me a while to get around to this though. I've also run into (and fixed) a bug where auto-generated message ids are not included in the payload. (It seems they aren't generated until you call Mail::Message#encoded.)

I'm not fond of the Delivered mail text as thats not actually what happened. The app has sent the email but it isn't responsible for delivering it or knowledgable if it has been delivered.

Yes the mail is part of a delivery but to call it delivered is not accurate imo.

Everything else about this change however is spot on. Love the removal of private info from logs.

I get where you're coming from but there's also another perspective: Rails has 'delivered' the mail to the subsystem responsible for the next step towards delivery. And it's now consistent with the event name.

@jvdp jvdp force-pushed the dont-log-emails-as-info branch from 2e00096 to 12c8b2a Compare February 13, 2019 14:04
@georgeclaghorn georgeclaghorn merged commit 2488901 into rails:master Feb 13, 2019
@jvdp jvdp deleted the dont-log-emails-as-info branch February 13, 2019 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants