Commit 7aa332b42461c0c3e15ff36b3363eb4fb06e50d4
1 parent
dffb8d28
Exists in
master
and in
1 other branch
cache notices statistics in problem
Showing
9 changed files
with
175 additions
and
46 deletions
Show diff stats
app/helpers/application_helper.rb
| 1 | 1 | module ApplicationHelper |
| 2 | 2 | def message_graph(problem) |
| 3 | - create_percentage_table_for(problem) {|notice| notice.message} | |
| 3 | + create_percentage_table_for(problem.messages) | |
| 4 | 4 | end |
| 5 | 5 | |
| 6 | 6 | def generate_problem_ical(notices) |
| ... | ... | @@ -35,35 +35,19 @@ module ApplicationHelper |
| 35 | 35 | end |
| 36 | 36 | |
| 37 | 37 | def user_agent_graph(problem) |
| 38 | - create_percentage_table_for(problem) {|notice| pretty_user_agent(notice.user_agent)} | |
| 39 | - end | |
| 40 | - | |
| 41 | - def pretty_user_agent(user_agent) | |
| 42 | - (user_agent.nil? || user_agent.none?) ? "N/A" : "#{user_agent.browser} #{user_agent.version}" | |
| 38 | + create_percentage_table_for(problem.user_agents) | |
| 43 | 39 | end |
| 44 | 40 | |
| 45 | 41 | def tenant_graph(problem) |
| 46 | - create_percentage_table_for(problem) {|notice| get_host(notice.request['url'])} | |
| 47 | - end | |
| 48 | - | |
| 49 | - def get_host(url) | |
| 50 | - begin | |
| 51 | - uri = url && URI.parse(url) | |
| 52 | - uri.blank? ? "N/A" : uri.host | |
| 53 | - rescue URI::InvalidURIError | |
| 54 | - "N/A" | |
| 55 | - end | |
| 42 | + create_percentage_table_for(problem.hosts) | |
| 56 | 43 | end |
| 57 | 44 | |
| 58 | - | |
| 59 | - def create_percentage_table_for(problem, &block) | |
| 60 | - tallies = tally(problem.notices, &block) | |
| 61 | - create_percentage_table_from_tallies(tallies, :total => problem.notices.count) | |
| 45 | + def create_percentage_table_for(collection) | |
| 46 | + create_percentage_table_from_tallies(tally(collection)) | |
| 62 | 47 | end |
| 63 | 48 | |
| 64 | - def tally(collection, &block) | |
| 65 | - collection.inject({}) do |tallies, item| | |
| 66 | - value = yield item | |
| 49 | + def tally(collection) | |
| 50 | + collection.inject({}) do |tallies, value| | |
| 67 | 51 | tallies[value] = (tallies[value] || 0) + 1 |
| 68 | 52 | tallies |
| 69 | 53 | end | ... | ... |
app/models/notice.rb
| ... | ... | @@ -19,7 +19,7 @@ class Notice |
| 19 | 19 | after_create :increase_counter_cache, :cache_attributes_on_problem, :unresolve_problem |
| 20 | 20 | after_create :deliver_notification, :if => :should_notify? |
| 21 | 21 | before_save :sanitize |
| 22 | - before_destroy :decrease_counter_cache | |
| 22 | + before_destroy :decrease_counter_cache, :remove_cached_attributes_from_problem | |
| 23 | 23 | |
| 24 | 24 | validates_presence_of :backtrace, :server_environment, :notifier |
| 25 | 25 | |
| ... | ... | @@ -33,6 +33,10 @@ class Notice |
| 33 | 33 | agent_string.blank? ? nil : UserAgent.parse(agent_string) |
| 34 | 34 | end |
| 35 | 35 | |
| 36 | + def user_agent_string | |
| 37 | + (user_agent.nil? || user_agent.none?) ? "N/A" : "#{user_agent.browser} #{user_agent.version}" | |
| 38 | + end | |
| 39 | + | |
| 36 | 40 | def environment_name |
| 37 | 41 | server_environment['server-environment'] || server_environment['environment-name'] |
| 38 | 42 | end |
| ... | ... | @@ -59,6 +63,17 @@ class Notice |
| 59 | 63 | read_attribute(:request) || {} |
| 60 | 64 | end |
| 61 | 65 | |
| 66 | + def url | |
| 67 | + request['url'] | |
| 68 | + end | |
| 69 | + | |
| 70 | + def host | |
| 71 | + uri = url && URI.parse(url) | |
| 72 | + uri.blank? ? "N/A" : uri.host | |
| 73 | + rescue URI::InvalidURIError | |
| 74 | + "N/A" | |
| 75 | + end | |
| 76 | + | |
| 62 | 77 | def env_vars |
| 63 | 78 | request['cgi-data'] || {} |
| 64 | 79 | end |
| ... | ... | @@ -94,11 +109,14 @@ class Notice |
| 94 | 109 | problem.inc(:notices_count, -1) if err |
| 95 | 110 | end |
| 96 | 111 | |
| 112 | + def remove_cached_attributes_from_problem | |
| 113 | + problem.remove_cached_notice_attribures(self) | |
| 114 | + end | |
| 115 | + | |
| 97 | 116 | def unresolve_problem |
| 98 | 117 | problem.update_attribute(:resolved, false) if problem.resolved? |
| 99 | 118 | end |
| 100 | 119 | |
| 101 | - | |
| 102 | 120 | def cache_attributes_on_problem |
| 103 | 121 | problem.cache_notice_attributes(self) |
| 104 | 122 | end | ... | ... |
app/models/problem.rb
| ... | ... | @@ -18,6 +18,9 @@ class Problem |
| 18 | 18 | field :environment |
| 19 | 19 | field :klass |
| 20 | 20 | field :where |
| 21 | + field :user_agents, :type => Array, :default => [] | |
| 22 | + field :messages, :type => Array, :default => [] | |
| 23 | + field :hosts, :type => Array, :default => [] | |
| 21 | 24 | |
| 22 | 25 | index :app_id |
| 23 | 26 | index :app_name |
| ... | ... | @@ -123,8 +126,20 @@ class Problem |
| 123 | 126 | :message => notice.message, |
| 124 | 127 | :environment => notice.environment_name, |
| 125 | 128 | :klass => notice.klass, |
| 126 | - :where => notice.where) if notice | |
| 129 | + :where => notice.where, | |
| 130 | + :messages => messages.push(notice.message), | |
| 131 | + :hosts => hosts.push(notice.host), | |
| 132 | + :user_agents => user_agents.push(notice.user_agent_string) | |
| 133 | + ) if notice | |
| 127 | 134 | update_attributes!(attrs) |
| 128 | 135 | end |
| 136 | + | |
| 137 | + def remove_cached_notice_attribures(notice) | |
| 138 | + messages.delete_at(messages.index(notice.message)) | |
| 139 | + hosts.delete_at(hosts.index(notice.host)) | |
| 140 | + user_agents.delete_at(user_agents.index(notice.user_agent_string)) | |
| 141 | + save! | |
| 142 | + end | |
| 143 | + | |
| 129 | 144 | end |
| 130 | 145 | ... | ... |
app/views/errs/show.html.haml
app/views/notices/_summary.html.haml
| ... | ... | @@ -2,7 +2,7 @@ |
| 2 | 2 | %table.summary |
| 3 | 3 | %tr |
| 4 | 4 | %th Message |
| 5 | - %td.main.nowrap= message_graph(notice.problem) | |
| 5 | + %td.main.nowrap= message_graph(problem) | |
| 6 | 6 | - if notice.request['url'].present? |
| 7 | 7 | %tr |
| 8 | 8 | %th URL |
| ... | ... | @@ -15,13 +15,13 @@ |
| 15 | 15 | %td= notice.created_at.to_s(:micro) |
| 16 | 16 | %tr |
| 17 | 17 | %th Similar |
| 18 | - %td= notice.problem.notices.count - 1 | |
| 18 | + %td= problem.notices_count - 1 | |
| 19 | 19 | %tr |
| 20 | 20 | %th Browser |
| 21 | - %td= user_agent_graph(notice.problem) | |
| 21 | + %td= user_agent_graph(problem) | |
| 22 | 22 | %tr |
| 23 | 23 | %th Tenant |
| 24 | - %td= tenant_graph(notice.problem) | |
| 24 | + %td= tenant_graph(problem) | |
| 25 | 25 | %tr |
| 26 | 26 | %th App Server |
| 27 | 27 | %td= notice.server_environment && notice.server_environment["hostname"] | ... | ... |
| ... | ... | @@ -0,0 +1,18 @@ |
| 1 | +class CacheProblemStatistics < Mongoid::Migration | |
| 2 | + def self.up | |
| 3 | + Problem.all.each do |problem| | |
| 4 | + problem.notices.each do |notice| | |
| 5 | + problem.messages << notice.message | |
| 6 | + problem.hosts << notice.host | |
| 7 | + problem.user_agents << notice.user_agent_string | |
| 8 | + end | |
| 9 | + problem.save! | |
| 10 | + end | |
| 11 | + end | |
| 12 | + | |
| 13 | + def self.down | |
| 14 | + Problem.all.each do |problem| | |
| 15 | + problem.update_attributes(:messages => [], :hosts => [], :user_agents => []) | |
| 16 | + end | |
| 17 | + end | |
| 18 | +end | |
| 0 | 19 | \ No newline at end of file | ... | ... |
spec/helpers/application_helper_spec.rb
| ... | ... | @@ -10,18 +10,3 @@ require 'spec_helper' |
| 10 | 10 | # end |
| 11 | 11 | # end |
| 12 | 12 | # end |
| 13 | -describe ApplicationHelper do | |
| 14 | - describe "get_host" do | |
| 15 | - it "returns host if url is valid" do | |
| 16 | - helper.get_host("http://example.com/resource/12").should == 'example.com' | |
| 17 | - end | |
| 18 | - | |
| 19 | - it "returns 'N/A' when url is not valid" do | |
| 20 | - helper.get_host("some string").should == 'N/A' | |
| 21 | - end | |
| 22 | - | |
| 23 | - it "returns 'N/A' when url is empty" do | |
| 24 | - helper.get_host({}).should == 'N/A' | |
| 25 | - end | |
| 26 | - end | |
| 27 | -end | ... | ... |
spec/models/notice_spec.rb
| ... | ... | @@ -82,6 +82,36 @@ describe Notice do |
| 82 | 82 | end |
| 83 | 83 | end |
| 84 | 84 | |
| 85 | + describe "user agent string" do | |
| 86 | + it "should be parsed and human-readable" do | |
| 87 | + notice = Factory.build(:notice, :request => {'cgi-data' => {'HTTP_USER_AGENT' => 'Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_6_7; en-US) AppleWebKit/534.16 (KHTML, like Gecko) Chrome/10.0.648.204 Safari/534.16'}}) | |
| 88 | + notice.user_agent_string.should == 'Chrome 10.0.648.204' | |
| 89 | + end | |
| 90 | + | |
| 91 | + it "should be nil if HTTP_USER_AGENT is blank" do | |
| 92 | + notice = Factory.build(:notice) | |
| 93 | + notice.user_agent_string.should == "N/A" | |
| 94 | + end | |
| 95 | + end | |
| 96 | + | |
| 97 | + describe "host" do | |
| 98 | + it "returns host if url is valid" do | |
| 99 | + notice = Factory.build(:notice, :request => {'url' => "http://example.com/resource/12"}) | |
| 100 | + notice.host.should == 'example.com' | |
| 101 | + end | |
| 102 | + | |
| 103 | + it "returns 'N/A' when url is not valid" do | |
| 104 | + notice = Factory.build(:notice, :request => {'url' => "some string"}) | |
| 105 | + notice.host.should == 'N/A' | |
| 106 | + end | |
| 107 | + | |
| 108 | + it "returns 'N/A' when url is empty" do | |
| 109 | + notice = Factory.build(:notice, :request => {}) | |
| 110 | + notice.host.should == 'N/A' | |
| 111 | + end | |
| 112 | + | |
| 113 | + end | |
| 114 | + | |
| 85 | 115 | |
| 86 | 116 | describe "email notifications (configured individually for each app)" do |
| 87 | 117 | custom_thresholds = [2, 4, 8, 16, 32, 64] | ... | ... |
spec/models/problem_spec.rb
| ... | ... | @@ -197,5 +197,84 @@ describe Problem do |
| 197 | 197 | }.should change(problem, :last_deploy_at).from(@last_deploy).to(next_deploy) |
| 198 | 198 | end |
| 199 | 199 | end |
| 200 | + | |
| 201 | + context "notice messages cache" do | |
| 202 | + before do | |
| 203 | + @app = Factory(:app) | |
| 204 | + @problem = Factory(:problem, :app => @app) | |
| 205 | + @err = Factory(:err, :problem => @problem) | |
| 206 | + end | |
| 207 | + | |
| 208 | + it "#messages returns [] by default" do | |
| 209 | + @problem.messages.should == [] | |
| 210 | + end | |
| 211 | + | |
| 212 | + it "adding a notice adds a string to #messages" do | |
| 213 | + lambda { | |
| 214 | + Factory(:notice, :err => @err, :message => 'ERR 1') | |
| 215 | + }.should change(@problem, :messages).from([]).to(['ERR 1']) | |
| 216 | + end | |
| 217 | + | |
| 218 | + it "removing a notice removes string from #messages" do | |
| 219 | + notice1 = Factory(:notice, :err => @err, :message => 'ERR 1') | |
| 220 | + lambda { | |
| 221 | + @err.notices.first.destroy | |
| 222 | + @problem.reload | |
| 223 | + }.should change(@problem, :messages).from(['ERR 1']).to([]) | |
| 224 | + end | |
| 225 | + end | |
| 226 | + | |
| 227 | + context "notice hosts cache" do | |
| 228 | + before do | |
| 229 | + @app = Factory(:app) | |
| 230 | + @problem = Factory(:problem, :app => @app) | |
| 231 | + @err = Factory(:err, :problem => @problem) | |
| 232 | + end | |
| 233 | + | |
| 234 | + it "#hosts returns [] by default" do | |
| 235 | + @problem.hosts.should == [] | |
| 236 | + end | |
| 237 | + | |
| 238 | + it "adding a notice adds a string to #hosts" do | |
| 239 | + lambda { | |
| 240 | + Factory(:notice, :err => @err, :request => {'url' => "http://example.com/resource/12"}) | |
| 241 | + }.should change(@problem, :hosts).from([]).to(['example.com']) | |
| 242 | + end | |
| 243 | + | |
| 244 | + it "removing a notice removes string from #hosts" do | |
| 245 | + notice1 = Factory(:notice, :err => @err, :request => {'url' => "http://example.com/resource/12"}) | |
| 246 | + lambda { | |
| 247 | + @err.notices.first.destroy | |
| 248 | + @problem.reload | |
| 249 | + }.should change(@problem, :hosts).from(['example.com']).to([]) | |
| 250 | + end | |
| 251 | + end | |
| 252 | + | |
| 253 | + context "notice user_agents cache" do | |
| 254 | + before do | |
| 255 | + @app = Factory(:app) | |
| 256 | + @problem = Factory(:problem, :app => @app) | |
| 257 | + @err = Factory(:err, :problem => @problem) | |
| 258 | + end | |
| 259 | + | |
| 260 | + it "#user_agents returns [] by default" do | |
| 261 | + @problem.user_agents.should == [] | |
| 262 | + end | |
| 263 | + | |
| 264 | + it "adding a notice adds a string to #user_agents" do | |
| 265 | + lambda { | |
| 266 | + Factory(:notice, :err => @err, :request => {'cgi-data' => {'HTTP_USER_AGENT' => 'Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_6_7; en-US) AppleWebKit/534.16 (KHTML, like Gecko) Chrome/10.0.648.204 Safari/534.16'}}) | |
| 267 | + }.should change(@problem, :user_agents).from([]).to(['Chrome 10.0.648.204']) | |
| 268 | + end | |
| 269 | + | |
| 270 | + it "removing a notice removes string from #user_agents" do | |
| 271 | + notice1 = Factory(:notice, :err => @err, :request => {'cgi-data' => {'HTTP_USER_AGENT' => 'Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_6_7; en-US) AppleWebKit/534.16 (KHTML, like Gecko) Chrome/10.0.648.204 Safari/534.16'}}) | |
| 272 | + lambda { | |
| 273 | + @err.notices.first.destroy | |
| 274 | + @problem.reload | |
| 275 | + }.should change(@problem, :user_agents).from(['Chrome 10.0.648.204']).to([]) | |
| 276 | + end | |
| 277 | + end | |
| 278 | + | |
| 200 | 279 | end |
| 201 | 280 | ... | ... |