feat: subscribe to process.action_mailer notifications

[mrsimo]

I mentioned this last month.

I believe the process.action_mailer event is also very interesting since it will contain the template rendering. Here's an example of how it looks:

A couple things I'd like mention with regards to the implementation:

• I added two new options to set the disallowed keys and payload transform. I didn't know what to call them, the original settings had fairly generic names. I imagine it was assumed we'd only subscribe to one event, or that they could be reused for all of them. I don't feel like the payload_transform should be reused, since even the default is very specifically about renaming the other event's payload to follow the semantic conventions.
• Since they had different settings I split the Railtie code into two explicit calls to subscribe instead of iterating over an array of events.

I tried this branch with my own app, and I did find an initial issue with the Railtie code I'd written. I'm a bit nervous that the code from the Railtie isn't tested.

Also there's no test that actually sends an email with ActionMailer. I understand we're relying on ActiveSupport::Notifications, but I feel like it would be beneficial even if just to know if/when future Rails versions change the contents of the payload so we can decide if we want to handle that to maintain future compatibility. Would we be open to that?

Thank you!

[kaylareopelle]

I think this is heading in the right direction! Thank you for your work so far.

I have some documentation suggestions in the comments.

As for the testing concern you raised in the PR description, I think it would be a great idea to find a way to simulate the process of sending mail within the test suite. Especially since you ran into failures using the tool that weren't revealed by running the tests.

If you're looking for an example, I work for New Relic and our Ruby agent tests get a little closer to the mail sending process: https://github.com/newrelic/newrelic-ruby-agent/blob/dev/test/new_relic/agent/instrumentation/rails/action_mailer_subscriber.rb

[kaylareopelle]

Could you add a section describing the attributes captured by the process.action_mailer notification by default?
https://edgeguides.rubyonrails.org/active_support_instrumentation.html#process-action-mailer

[mrsimo]

Pushed some changes:

• The documentation changes requested, always appreciate if someone can double check them, english is not my first language. But thanks a lot for the suggestions 🧡, I love some good docs on the gems we use!
• An extra test that does a basic email delivery. Happy to iterate on it, pull it out to a different PR or whatever you prefer.
• I've been trying to figure out a way to test the railtie code, best I could come up with is pulling some bits out to class methods.

Thanks a lot for the review!!

[mrsimo]

I took the liberty of adding this, when adding the new tests that do ActionMailer deliveries the mail gems generates lots of warnings. I saw other instrumentation libraries already had it.

[mrsimo]

I moved these bits into class methods that the railtie callback calls. This way I can call them from the tests to subscribe (and unsubscribe) with different configurations.

[kaylareopelle]

I think this looks great! Thank you, @mrsimo!! 🚀

[mrsimo]

@kaylareopelle thank you so much. Just so I'm not missing anything, is there anything left to do here? Wait patiently?

[kaylareopelle]

@mrsimo, nothing else for you to do! I left the PR open to see if anyone else wanted look at it. If there aren't any new reviews by the end of the day tomorrow, I'll merge it in!