add action mailer instrumentation #1280
Conversation
| end | ||
|
|
||
| def span_type | ||
| # ActionMailer creates emails like a controller |
There was a problem hiding this comment.
Could you clarify this? Are these top-level spans then? Do they happen on the same execution context as, say, a Rack request?
There was a problem hiding this comment.
i believe they are not top level spans , but i am going to try to stress test this in a sample app, will report back when i have more. tl;dr i believe i've botched the span_type here and need to update, but want to confirm that and just generally get a bit more familiar with what these actions represent within the mailer lifecycle
There was a problem hiding this comment.
So, to update: So this integration is similar to active record or action view in that there's a few different spans which could all be "top level" in the sense that they're the 1st span for the service, but generally speaking they'll rarely be the root span of the trace, usualy they're a child of action_pack controller span or a job consumer (delayed job, sidekiq, etc) span.
Here's a few examples:
- async job queue (deliver_later)
- syncronous, no job queue (deliver_now)
I've tried to update the span_types to reflect existing standards that are closer to their more appropriate usage, marking them as either a template (which is what .process spans do, they render the mailer templates) or worker (which is what .deliver spans do, they send emails).
Anyway lmk what you think
There was a problem hiding this comment.
Makes sense. I like the traces you showed above, they all seem reasonable to me 👍.
|
|
||
| RSpec.shared_context 'ActionMailer helpers' do | ||
| before(:each) do | ||
| if ActionMailer::Base.respond_to?(:delivery_method) |
There was a problem hiding this comment.
What is this condition for? Different versions of Rails?
Unless it's a trivial explanation, it would be good to have it as a comment, so we can know the rationale in the future while browsing this code.
delner
added
feature
|
Are the information about the email being sent (from, to, subject, payload size) not available in the ActiveSupport notification? |
|
The actual network operations also seem to be missing, but I think that the current instrumentation level provides enough value as it. |
Codecov Report
@@ Coverage Diff @@
## master #1280 +/- ##
==========================================
- Coverage 98.31% 98.30% -0.01%
==========================================
Files 913 924 +11
Lines 44247 44542 +295
==========================================
+ Hits 43501 43789 +288
- Misses 746 753 +7
Continue to review full report at Codecov.
|
|
@marcotc lmk what you think, sorry didnt realize this was still open |
| @@ -32,6 +32,19 @@ def process(span, event, _id, payload) | |||
| span.span_type = span_type | |||
| span.set_tag(Ext::TAG_MAILER, payload[:mailer]) | |||
| span.set_tag(Ext::TAG_MSG_ID, payload[:message_id]) | |||
|
|
|||
| # Since email date can contain PII we disable by default | |||
| # Some of these fields can be either strings or arrays, so we try to normalize | |||
|
I figure you folks have this covered but just in case not, this isn't yet in the changelog for 0.53 |
|
Thanks @hale for the note! We usually filter down all items in the milestone when writing the release notes, and in this case we filtered a bit too much 😅 . I'll open a PR to add this item to the 0.53.0 changelog, as I agree with you that this definitely deserves being there. Thanks @ericmustin for the great work ;) |
This was added in #1280 and it's definitely relevant enough to include in the changelog :)
This was added in #1280 and it's definitely relevant enough to include in the changelog :)
This PR provides instrumentation support for Rails 5/6 ActionMailer.
This PR addresses #250 , instruments the
process.action_maileranddeliver.action_mailerActiveSupport::Notification Events for the Rails ActionMailer Module.There's perhaps some additional metadata that could be collected as span tags but a lot of it (
from,tosubjectbodyetc etc) seems like it would contain PII, so i've limited the tags to what's useful for measuring execution times and errors between specific mailers and actions.I will try to update this PR shortly with an example trace for a small sample application