Move creation of event outside of mailer

Metric events are created for service desk related outgoing emails,
but to assure that these events are not generated multiple times
(when delivery is retried), these events are now created before
Notify is called.

app/mailers/emails/service_desk.rb

@@ -20,9 +20,7 @@ module Emails
       options = service_desk_options(email_sender, 'thank_you', @issue.external_author)
                   .merge(subject: "Re: #{subject_base}")
 
-      mail_new_thread(@issue, options).tap do
-        Gitlab::Metrics::BackgroundTransaction.current&.add_event(:service_desk_thank_you_email)
-      end
+      mail_new_thread(@issue, options)
     end
 
     def service_desk_new_note_email(issue_id, note_id, recipient)
@@ -33,9 +31,7 @@ module Emails
       options = service_desk_options(email_sender, 'new_note', recipient)
                   .merge(subject: subject_base)
 
-      mail_answer_thread(@issue, options).tap do
-        Gitlab::Metrics::BackgroundTransaction.current&.add_event(:service_desk_new_note_email)
-      end
+      mail_answer_thread(@issue, options)
     end
 
     private

app/services/notification_service.rb

@@ -396,6 +396,7 @@ class NotificationService
 
     recipients.each do |recipient|
       mailer.service_desk_new_note_email(issue.id, note.id, recipient).deliver_later
+      Gitlab::Metrics::BackgroundTransaction.current&.add_event(:service_desk_new_note_email)
     end
   end
 

lib/gitlab/email/handler/service_desk_handler.rb

@@ -95,6 +95,7 @@ module Gitlab
 
         def send_thank_you_email
           Notify.service_desk_thank_you_email(@issue.id).deliver_later
+          Gitlab::Metrics::BackgroundTransaction.current&.add_event(:service_desk_thank_you_email)
         end
 
         def message_including_template

spec/lib/gitlab/email/handler/service_desk_handler_spec.rb

@@ -50,6 +50,15 @@ RSpec.describe Gitlab::Email::Handler::ServiceDeskHandler do
       it 'sends thank you email' do
         expect { receiver.execute }.to have_enqueued_job.on_queue('mailers')
       end
+
+      it 'adds metric events for incoming and reply emails' do
+        metric_transaction = double('Gitlab::Metrics::WebTransaction', increment: true, observe: true)
+        allow(::Gitlab::Metrics::BackgroundTransaction).to receive(:current).and_return(metric_transaction)
+        expect(metric_transaction).to receive(:add_event).with(:receive_email_service_desk, anything)
+        expect(metric_transaction).to receive(:add_event).with(:service_desk_thank_you_email)
+
+        receiver.execute
+      end
     end
 
     context 'when everything is fine' do

spec/mailers/emails/service_desk_spec.rb

@@ -115,16 +115,6 @@ RSpec.describe Emails::ServiceDesk do
     end
   end
 
-  shared_examples 'notification with metric event' do |event_type|
-    it 'adds metric event' do
-      metric_transaction = double('Gitlab::Metrics::WebTransaction', increment: true, observe: true)
-      allow(::Gitlab::Metrics::BackgroundTransaction).to receive(:current).and_return(metric_transaction)
-      expect(metric_transaction).to receive(:add_event).with(event_type)
-
-      subject.content_type
-    end
-  end
-
   describe '.service_desk_thank_you_email' do
     let_it_be(:reply_in_subject) { true }
     let_it_be(:default_text) do
@@ -134,7 +124,6 @@ RSpec.describe Emails::ServiceDesk do
     subject { ServiceEmailClass.service_desk_thank_you_email(issue.id) }
 
     it_behaves_like 'read template from repository', 'thank_you'
-    it_behaves_like 'notification with metric event', :service_desk_thank_you_email
 
     context 'handling template markdown' do
       context 'with a simple text' do
@@ -175,7 +164,6 @@ RSpec.describe Emails::ServiceDesk do
     subject { ServiceEmailClass.service_desk_new_note_email(issue.id, note.id, email) }
 
     it_behaves_like 'read template from repository', 'new_note'
-    it_behaves_like 'notification with metric event', :service_desk_new_note_email
 
     context 'handling template markdown' do
       context 'with a simple text' do

spec/services/notification_service_spec.rb

@@ -376,41 +376,31 @@ RSpec.describe NotificationService, :mailer do
 
         let(:subject) { NotificationService.new }
         let(:mailer) { double(deliver_later: true) }
+        let(:issue) { create(:issue, author: User.support_bot) }
+        let(:project) { issue.project }
+        let(:note) { create(:note, noteable: issue, project: project) }
 
-        def should_email!
-          expect(Notify).to receive(:service_desk_new_note_email)
-            .with(issue.id, note.id, issue.external_author)
-        end
+        shared_examples 'notification with exact metric events' do |number_of_events|
+          it 'adds metric event' do
+            metric_transaction = double('Gitlab::Metrics::WebTransaction', increment: true, observe: true)
+            allow(::Gitlab::Metrics::BackgroundTransaction).to receive(:current).and_return(metric_transaction)
+            expect(metric_transaction).to receive(:add_event).with(:service_desk_new_note_email).exactly(number_of_events).times
 
-        def should_not_email!
-          expect(Notify).not_to receive(:service_desk_new_note_email)
+            subject.new_note(note)
+          end
         end
 
-        def execute!
-          subject.new_note(note)
-        end
+        shared_examples 'no participants are notified' do
+          it 'does not send the email' do
+            expect(Notify).not_to receive(:service_desk_new_note_email)
 
-        def self.it_should_email!
-          it 'sends the email' do
-            should_email!
-            execute!
+            subject.new_note(note)
           end
-        end
 
-        def self.it_should_not_email!
-          it 'doesn\'t send the email' do
-            should_not_email!
-            execute!
-          end
+          it_behaves_like 'notification with exact metric events', 0
         end
 
-        let(:issue) { create(:issue, author: User.support_bot) }
-        let(:project) { issue.project }
-        let(:note) { create(:note, noteable: issue, project: project) }
-
-        context 'do not exist' do
-          it_should_not_email!
-        end
+        it_behaves_like 'no participants are notified'
 
         context 'do exist and note not confidential' do
           let!(:issue_email_participant) { issue.issue_email_participants.create!(email: 'service.desk@example.com') }
@@ -420,7 +410,14 @@ RSpec.describe NotificationService, :mailer do
             project.update!(service_desk_enabled: true)
           end
 
-          it_should_email!
+          it 'sends the email' do
+            expect(Notify).to receive(:service_desk_new_note_email)
+              .with(issue.id, note.id, issue.external_author)
+
+            subject.new_note(note)
+          end
+
+          it_behaves_like 'notification with exact metric events', 1
         end
 
         context 'do exist and note is confidential' do
@@ -432,7 +429,7 @@ RSpec.describe NotificationService, :mailer do
             project.update!(service_desk_enabled: true)
           end
 
-          it_should_not_email!
+          it_behaves_like 'no participants are notified'
         end
       end