Download as
Browse Code »

Commit f646cf500aef1d37aeb28a4a11f5bf2a4930837c

Authored by Stephen Crosby
1 parent 816110d7
Exists in master and in 1 other branch production

reduce number of queries on insert 

These changes reduce the typical number of queries needed to insert an
error from nine to five.
Showing 5 changed files with 81 additions and 71 deletions   Show diff stats
app/models/app.rb
  Diff comments   View file @f646cf5
... ... @@ -57,8 +57,10 @@ class App
57 57 def find_or_create_err!(attrs)
58 58 Err.where(
59 59 :fingerprint => attrs[:fingerprint]
60   - ).first ||
61   - problems.create!(attrs.slice(:error_class, :environment)).errs.create!(attrs.slice(:fingerprint, :problem_id))
  60 + ).first || (
  61 + problem = problems.create!(attrs.slice(:error_class, :environment))
  62 + problem.errs.create!(attrs.slice(:fingerprint, :problem_id))
  63 + )
62 64 end
63 65  
64 66 # Mongoid Bug: find(id) on association proxies returns an Enumerator
... ...
app/models/backtrace.rb
  Diff comments   View file @f646cf5
... ... @@ -14,12 +14,13 @@ class Backtrace
14 14  
15 15 delegate :app, :to => :notice
16 16  
17   - def self.find_or_create(attributes = {})
18   - new(attributes).similar.find_and_modify({ '$setOnInsert' => attributes })
19   - end
  17 + def self.find_or_create(lines)
  18 + fingerprint = generate_fingerprint(lines)
20 19  
21   - def similar
22   - Backtrace.where(:fingerprint => fingerprint)
  20 + where(fingerprint: generate_fingerprint(lines)).
  21 + find_one_and_update(
  22 + { '$setOnInsert' => { fingerprint: fingerprint, lines: lines } },
  23 + { return_document: :after, upsert: true })
23 24 end
24 25  
25 26 def raw=(raw)
... ... @@ -30,6 +31,6 @@ class Backtrace
30 31  
31 32 private
32 33 def generate_fingerprint
33   - self.fingerprint = Digest::SHA1.hexdigest(lines.map(&:to_s).join)
  34 + self.fingerprint = self.class.generate_fingerprint(lines)
34 35 end
35 36 end
... ...
app/models/error_report.rb
  Diff comments   View file @f646cf5
... ... @@ -40,24 +40,88 @@ class ErrorReport
40 40 end
41 41  
42 42 def backtrace
43   - @normalized_backtrace ||= Backtrace.find_or_create(raw: @backtrace)
  43 + @normalized_backtrace ||= Backtrace.find_or_create(@backtrace)
44 44 end
45 45  
46 46 def generate_notice!
47 47 return unless valid?
48 48 return @notice if @notice
  49 +
  50 + make_notice
  51 + error.notices << @notice
  52 + cache_attributes_on_problem
  53 + email_notification
  54 + services_notification
  55 + @notice
  56 + end
  57 +
  58 + def make_notice
49 59 @notice = Notice.new(
50 60 message: message,
51 61 error_class: error_class,
52   - backtrace_id: backtrace.id,
  62 + backtrace: backtrace,
53 63 request: request,
54 64 server_environment: server_environment,
55 65 notifier: notifier,
56 66 user_attributes: user_attributes,
57 67 framework: framework
58 68 )
59   - error.notices << @notice
60   - @notice
  69 + end
  70 +
  71 + # Update problem cache with information about this notice
  72 + def cache_attributes_on_problem
  73 + # increment notice count
  74 + message_digest = Digest::MD5.hexdigest(@notice.message)
  75 + host_digest = Digest::MD5.hexdigest(@notice.host)
  76 + user_agent_digest = Digest::MD5.hexdigest(@notice.user_agent_string)
  77 +
  78 + @problem = Problem.where("_id" => @error.problem_id).find_one_and_update(
  79 + '$set' => {
  80 + 'app_name' => app.name,
  81 + 'environment' => @notice.environment_name,
  82 + 'error_class' => @notice.error_class,
  83 + 'last_notice_at' => @notice.created_at,
  84 + 'message' => @notice.message,
  85 + 'resolved' => false,
  86 + 'resolved_at' => nil,
  87 + 'where' => @notice.where,
  88 + "messages.#{message_digest}.value" => @notice.message,
  89 + "hosts.#{host_digest}.value" => @notice.host,
  90 + "user_agents.#{user_agent_digest}.value" => @notice.user_agent_string,
  91 + },
  92 + '$inc' => {
  93 + 'notices_count' => 1,
  94 + "messages.#{message_digest}.count" => 1,
  95 + "hosts.#{host_digest}.count" => 1,
  96 + "user_agents.#{user_agent_digest}.count" => 1,
  97 + }
  98 + )
  99 + end
  100 +
  101 + def similar_count
  102 + @similar_count ||= @problem.notices_count
  103 + end
  104 +
  105 + # Send email notification if needed
  106 + def email_notification
  107 + return false unless app.emailable?
  108 + return false unless app.email_at_notices.include?(similar_count)
  109 + Mailer.err_notification(@notice).deliver
  110 + rescue => e
  111 + HoptoadNotifier.notify(e)
  112 + end
  113 +
  114 + def should_notify?
  115 + app.notification_service.notify_at_notices.include?(0) ||
  116 + app.notification_service.notify_at_notices.include?(similar_count)
  117 + end
  118 +
  119 + # Launch all notification define on the app associate to this notice
  120 + def services_notification
  121 + return true unless app.notification_service_configured? and should_notify?
  122 + app.notification_service.create_notification(problem)
  123 + rescue => e
  124 + HoptoadNotifier.notify(e)
61 125 end
62 126  
63 127 ##
... ...
app/models/notice.rb
  Diff comments   View file @f646cf5
... ... @@ -20,13 +20,10 @@ class Notice
20 20 index(:created_at => 1)
21 21 index(:err_id => 1, :created_at => 1, :_id => 1)
22 22  
23   - after_create :cache_attributes_on_problem, :unresolve_problem
24   - after_create :email_notification
25   - after_create :services_notification
26 23 before_save :sanitize
27 24 before_destroy :decrease_counter_cache, :remove_cached_attributes_from_problem
28 25  
29   - validates_presence_of :backtrace, :server_environment, :notifier
  26 + validates_presence_of :backtrace_id, :server_environment, :notifier
30 27  
31 28 scope :ordered, ->{ order_by(:created_at.asc) }
32 29 scope :reverse_ordered, ->{ order_by(:created_at.desc) }
... ... @@ -110,22 +107,6 @@ class Notice
110 107 backtrace_lines.in_app
111 108 end
112 109  
113   - def similar_count
114   - problem.notices_count
115   - end
116   -
117   - def emailable?
118   - app.email_at_notices.include?(similar_count)
119   - end
120   -
121   - def should_email?
122   - app.emailable? && emailable?
123   - end
124   -
125   - def should_notify?
126   - app.notification_service.notify_at_notices.include?(0) || app.notification_service.notify_at_notices.include?(similar_count)
127   - end
128   -
129 110 ##
130 111 # TODO: Move on decorator maybe
131 112 #
... ... @@ -151,21 +132,12 @@ class Notice
151 132 problem.remove_cached_notice_attributes(self) if err
152 133 end
153 134  
154   - def unresolve_problem
155   - problem.update_attributes!(:resolved => false, :resolved_at => nil, :notices_count => 1) if problem.resolved?
156   - end
157   -
158   - def cache_attributes_on_problem
159   - ProblemUpdaterCache.new(problem, self).update
160   - end
161   -
162 135 def sanitize
163 136 [:server_environment, :request, :notifier].each do |h|
164 137 send("#{h}=",sanitize_hash(send(h)))
165 138 end
166 139 end
167 140  
168   -
169 141 def sanitize_hash(h)
170 142 h.recurse do |h|
171 143 h.inject({}) do |h,(k,v)|
... ... @@ -178,25 +150,4 @@ class Notice
178 150 end
179 151 end
180 152 end
181   -
182   - private
183   -
184   - ##
185   - # Send email notification if needed
186   - def email_notification
187   - return true unless should_email?
188   - Mailer.err_notification(self).deliver
189   - rescue => e
190   - HoptoadNotifier.notify(e)
191   - end
192   -
193   - ##
194   - # Launch all notification define on the app associate to this notice
195   - def services_notification
196   - return true unless app.notification_service_configured? and should_notify?
197   - app.notification_service.create_notification(problem)
198   - rescue => e
199   - HoptoadNotifier.notify(e)
200   - end
201   -
202 153 end
... ...
app/models/problem.rb
  Diff comments   View file @f646cf5
... ... @@ -132,15 +132,7 @@ class Problem
132 132 def cache_app_attributes
133 133 if app
134 134 self.app_name = app.name
135   - self.last_deploy_at = if (last_deploy = app.deploys.where(:environment => self.environment).last)
136   - last_deploy.created_at.utc
137   - end
138   - collection.
139   - find('_id' => self.id).
140   - update_one({'$set' => {
141   - 'app_name' => self.app_name,
142   - 'last_deploy_at' => self.last_deploy_at.try(:utc)
143   - }})
  135 + self.last_deploy_at = app.last_deploy_at
144 136 end
145 137 end
146 138  
... ...