feat: add instrumentation support for action mailer

[xuan-cao-swi]

Description

Adding support for instrumenting action mailer using active support notification. This instrumentation is modified based on action_view.

Related: #293

Test

Sample span_data

#<struct OpenTelemetry::SDK::Trace::SpanData
 name="action_mailer deliver",
 kind=:internal,
 status=#<OpenTelemetry::Trace::Status:0x0000ffff8eb50908 @code=1, @description="">,
 parent_span_id="\x00\x00\x00\x00\x00\x00\x00\x00",
 total_recorded_attributes=7,
 total_recorded_events=0,
 total_recorded_links=0,
 start_timestamp=1713299681034888674,
 end_timestamp=1713299681044902841,
 attributes=
  {"email.message_id"=>"[email protected]",
   "email.subject"=>"Welcome to OpenTelemetry!",
   "email.x_mailer"=>"TestMailer",
   "email.to.address"=>["[email protected]"],
   "email.from.address"=>["[email protected]"],
   "email.cc.address"=>["[email protected]"],
   "email.bcc.address"=>["[email protected]"]},
 links=nil,
 events=nil,
 resource=
  #<OpenTelemetry::SDK::Resources::Resource:0x0000ffff85aec2e0
   @attributes=
    {"service.name"=>"unknown_service",
     "process.pid"=>363,
     "process.command"=>"trace_request_demonstration.ru",
     "process.runtime.name"=>"ruby",
     "process.runtime.version"=>"3.2.2",
     "process.runtime.description"=>"ruby 3.2.2 (2023-03-30 revision e51014f9c0) [aarch64-linux-musl]",
     "telemetry.sdk.name"=>"opentelemetry",
     "telemetry.sdk.language"=>"ruby",
     "telemetry.sdk.version"=>"1.4.1"}>,
 instrumentation_scope=#<struct OpenTelemetry::SDK::InstrumentationScope name="OpenTelemetry::Instrumentation::ActionMailer", version="0.1.0">,
 span_id="}\x1C\x91C\e\x88\x00v",
 trace_id="\xBF\xC0\xAD%\xBA\xF8\xAD\x974\xD0\x1A \xE2=F\xDB",
 trace_flags=#<OpenTelemetry::Trace::TraceFlags:0x0000ffff883d0030 @flags=1>,
 tracestate=#<OpenTelemetry::Trace::Tracestate:0x0000ffff883f3ee0 @hash={}>>

[github-actions]

👋 This pull request has been marked as stale because it has been open with no activity. You can: comment on the issue or remove the stale label to hold stale off for a while, add the keep label to hold stale off permanently, or do nothing. If you do nothing this pull request will be closed eventually by the stale bot

[arielvalentin]

I'll take a closer look later this week. Apologies for waiting so long to review.

[arielvalentin]

This is a micro optimization but checking if the values are nil before adding them to the hash is generally faster than using compact!

#693 (comment)

[kaylareopelle]

Thank you for writing this, Xuan!

[kaylareopelle]

Is the intent of this code block to show you can get the ActionMailer instrumentation from the solo opentelemetry-instrumentation-action_mailer gem or the opentelemetry-instrumentation-rails gem?

If so, I think the opentelemetry-instrumentation-rails.gemspec should be updated to add opentelemetry-instrumentation-action_mailer as a dependency.

It might also be helpful to add a comment within the code block that describes the two options more clearly. It could read something like:

# Install just the ActionMailer instrumentation
gem install opentelemetry-instrumentation-action_mailer
# Install the ActionMailer instrumentation along with the rest of the Rails-related instrumentation
gem install opentelemetry-instrumentation-rails

With how it's currently written, I think I have to install both gems to get ActionMailer instrumentation.

[xuan-cao-swi]

It might also be helpful to add a comment within the code block that describes the two options more clearly. It could read something like:

Updated

If so, I think the opentelemetry-instrumentation-rails.gemspec should be updated to add opentelemetry-instrumentation-action_mailer as a dependency.

I think it's better to have action_mailer gem released on rubygems then include rails (also for instrumentation-all)

[kaylareopelle]

Suggested change

	ActionMailer instrumentation doesn't expose email address by default, but if email address is needed, simply use `:email_address` option:
	ActionMailer instrumentation doesn't expose email addresses by default, but if email addresses are needed, simply use `:email_address` option:

[kaylareopelle]

Suggested change

	This instrumentation now relies entirely on `ActiveSupport::Notifications` and registers a custom Subscriber that listens to relevant events to report as spans.
	This instrumentation relies entirely on `ActiveSupport::Notifications` and registers a custom Subscriber that listens to relevant events to report as spans.

[kaylareopelle]

Is this saying the following attributes are on the action_mailer deliver spans?

[kaylareopelle]

I wonder if we should incorporate "notifications payload" somewhere into this sentence to make it clear that's where the attributes are coming from.

Maybe:

Suggested change

	Attributes that are specific to this instrumentation are recorded under `action_mailer deliver`:
	The following attributes from the notification payload for the `deliver.action_mailer` event are attached to `action_mailer deliver` spans:

[kaylareopelle]

Approved as-is, but I have a few documentation-related suggestions you're welcome to apply or ignore.

Thanks, Xuan!

[kaylareopelle]

One other thing (not sure I said this already or just thought it) -- could you either add this gem to the rails instrumentation gemspec as part of this PR or open an issue about this?

Currently the README is inaccurate. The ActionMailer instrumentation isn't a dependency of the Rails project, so it won't be installed when opentelemetry-instrumentation-rails is installed.

We've already done a lot in this PR, so I don't mind adding this on as a follow-up, but didn't want to assume your preference.

[xuan-cao-swi]

could you either add this gem to the rails instrumentation gemspec as part of this PR or open an issue about this?

Yes, I created an issue that is tagged on this PR

[mrsimo]

Not sure whether to open a new issue, but wanted to try here first.

We had added instrumentation for ActionMailer ourselves using OpenTelemetry::Instrumentation::ActiveSupport.subscribe, but had added process.action_mailer as well. The span itself doesn't have interesting attributes but it helps wrap everything necessary to render the email in a common span. We find this very useful. For example:

Helps see how long it takes to render the email vs actually deliver it:

Would you be open to adding this as well?

[arielvalentin]

We generally add functionality based on our user need and in many cases we tend to add features that we (maintainers) regularly use in production.

We are happy to review any contributions you'd like to submit to the community and look forward to your help.

[mrsimo]

Wonderful! I'll look into it!