Skip to content

Commit 794a19d

Browse files
committed
Instrumentation for Incoming Mail Processing
Fixes CNVS-20436 This does two things: 1) Reports to StatsD each time a message is processed 2) Every 5 minutes, reports the unprocessed size of each incoming mailbox Test Plan: Regression test incoming mail processing. Will want to check the basic cases of incoming messages still being processed properly. Smoke test the new Instrumentation delayed job. 1) Setup incoming mail processing by adding a config/incoming_mail.yml with the right content - Wheeler can tell you what the file should contain. (not shown because passwords) 2) Start canvas jobs `bundle exec script/delayed_job run` 3) Watch /error_reports for any new error reports. You should see none. Change-Id: Ia737906cedb67e183fec0f41571e002883c058c8 Reviewed-on: https://gerrit.instructure.com/57892 Reviewed-by: Joel Hough <joel@instructure.com> Tested-by: Jenkins QA-Review: Steven Shepherd <sshepherd@instructure.com> Product-Review: Matthew Wheeler <mwheeler@instructure.com>
1 parent 6f1dac6 commit 794a19d

12 files changed

Lines changed: 149 additions & 2 deletions

File tree

config/initializers/periodic_jobs.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,10 @@ def with_each_shard_by_database(klass, method, *args)
8080
end
8181
end
8282

83+
Delayed::Periodic.cron 'IncomingMailProcessor::Instrumentation#process', '*/5 * * * *' do
84+
IncomingMailProcessor::Instrumentation.process
85+
end
86+
8387
Delayed::Periodic.cron 'ErrorReport.destroy_error_reports', '2-59/5 * * * *' do
8488
cutoff = Setting.get('error_reports_retain_for', 3.months.to_s).to_i
8589
if cutoff > 0

gems/incoming_mail_processor/incoming_mail_processor.gemspec

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ Gem::Specification.new do |spec|
1919
spec.add_dependency "mail", "2.5.4"
2020
spec.add_dependency "html_text_helper"
2121
spec.add_dependency "utf8_cleaner"
22+
spec.add_dependency "canvas_statsd"
2223

2324
spec.add_development_dependency "bundler", "~> 1.5"
2425
spec.add_development_dependency "rspec", "2.99.0"

gems/incoming_mail_processor/lib/incoming_mail_processor.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
require "html_text_helper"
22
require "mail"
33
require "utf8_cleaner"
4+
require "canvas_statsd"
45

56
module IncomingMailProcessor
67
require "incoming_mail_processor/pop3_mailbox"
@@ -11,4 +12,5 @@ module IncomingMailProcessor
1112
require "incoming_mail_processor/incoming_message_processor"
1213
require "incoming_mail_processor/mailbox_account"
1314
require "incoming_mail_processor/settings"
15+
require "incoming_mail_processor/instrumentation"
1416
end

gems/incoming_mail_processor/lib/incoming_mail_processor/directory_mailbox.rb

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,11 @@ def move_message(filename, target_folder)
6565
move_file(folder, filename, target_folder)
6666
end
6767

68+
def unprocessed_message_count
69+
# not implemented, and used only for performance monitoring.
70+
nil
71+
end
72+
6873
private
6974
def folder_exists?(folder, subfolder = nil)
7075
to_check = subfolder ? File.join(folder, subfolder) : folder

gems/incoming_mail_processor/lib/incoming_mail_processor/imap_mailbox.rb

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,13 @@ def move_message(message_id, target_folder)
7474
delete_message(message_id)
7575
end
7676

77+
def unprocessed_message_count
78+
connect
79+
@imap.select("INBOX")
80+
length = @imap.search(["X-GM-RAW", "label:unread"]).length
81+
disconnect
82+
length
83+
end
7784
end
7885

7986
end

gems/incoming_mail_processor/lib/incoming_mail_processor/incoming_message_processor.rb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,9 @@ def process_single(incoming_message, tag, mailbox_account = IncomingMailProcesso
8787

8888
body, html_body = extract_body(incoming_message)
8989

90-
@message_handler.handle(mailbox_account.address, body, html_body, incoming_message, tag)
90+
handle = @message_handler.handle(mailbox_account.address, body, html_body, incoming_message, tag)
91+
CanvasStatsd::Statsd.increment("incoming_mail_processor.incoming_message_processed")
92+
handle
9193
end
9294

9395
private
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
#
2+
# Copyright (C) 2011-2015 Instructure, Inc.
3+
#
4+
# This file is part of Canvas.
5+
#
6+
# Canvas is free software: you can redistribute it and/or modify it under
7+
# the terms of the GNU Affero General Public License as published by the Free
8+
# Software Foundation, version 3 of the License.
9+
#
10+
# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
11+
# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
12+
# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
13+
# details.
14+
#
15+
# You should have received a copy of the GNU Affero General Public License along
16+
# with this program. If not, see <http://www.gnu.org/licenses/>.
17+
#
18+
19+
module IncomingMailProcessor
20+
class Instrumentation
21+
def self.process
22+
unreads = []
23+
24+
mailbox_accounts.each do |a|
25+
unreads << IncomingMailProcessor::IncomingMessageProcessor.create_mailbox(a).unprocessed_message_count
26+
end
27+
28+
report_unreads(unreads)
29+
end
30+
31+
def self.mailbox_accounts
32+
IncomingMailProcessor::IncomingMessageProcessor.mailbox_accounts
33+
end
34+
private_class_method :mailbox_accounts
35+
36+
def self.report_unreads(unreads)
37+
result = Hash[mailbox_accounts.map{|a| a.config[:username]}.zip(unreads)]
38+
result.delete_if { |_k, v| v.nil? }
39+
result.each_pair do |username, count|
40+
name = "incoming_mail_processor." + CanvasStatsd::Statsd.escape(username)
41+
CanvasStatsd::Statsd.gauge name, count
42+
end
43+
end
44+
private_class_method :report_unreads
45+
end
46+
end

gems/incoming_mail_processor/lib/incoming_mail_processor/pop3_mailbox.rb

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,10 @@ def move_message(pop_message, target_folder)
6666
# pop can't do this -- just delete the message
6767
delete_message(pop_message)
6868
end
69-
end
7069

70+
def unprocessed_message_count
71+
# not implemented, and used only for performance monitoring.
72+
nil
73+
end
74+
end
7175
end

gems/incoming_mail_processor/spec/incoming_mail_processor/directory_mailbox_spec.rb

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,12 @@ def default_config
8989
end
9090
end
9191

92+
describe '#unprocessed_message_count' do
93+
it "should return nil" do
94+
@mailbox.unprocessed_message_count.should be_nil
95+
end
96+
end
97+
9298
context "with simple foo file" do
9399

94100
before do

gems/incoming_mail_processor/spec/incoming_mail_processor/imap_mailbox_spec.rb

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,18 @@ class <<@imap_mock
8787
end
8888
end
8989

90+
describe '#unprocessed_message_count' do
91+
it 'returns zero if there are no messages' do
92+
@imap_mock.expects(:search).with(["X-GM-RAW", "label:unread"]).once.returns([])
93+
@mailbox.unprocessed_message_count.should eql 0
94+
end
95+
96+
it 'returns the number of messages if there are any' do
97+
@imap_mock.expects(:search).with(["X-GM-RAW", "label:unread"]).once.returns([1,2,3,58,42])
98+
@mailbox.unprocessed_message_count.should eql 5
99+
end
100+
end
101+
90102
context "connected" do
91103
before do
92104
@mailbox.connect

0 commit comments

Comments
 (0)