Download as
Browse Code »

Commit 9c40727dc527c43d2f2c3dedbcf94b9dec2aaa4c

Authored by Stephen Crosby
1 parent 24db1d16
Exists in master and in 1 other branch production

configurable notice fingerprinting

Showing 17 changed files with 264 additions and 221 deletions   Show diff stats
README.md
  Diff comments   View file @9c40727
... ... @@ -230,21 +230,6 @@ Airbrake.setProject("ERRBIT API KEY", "ERRBIT API KEY");
230 230 Airbrake.setHost("http://errbit.yourdomain.com");
231 231 ```
232 232  
233   -Using custom fingerprinting methods
234   ------------------------------------
235   -
236   -Errbit collates errors into groups using a fingerprinting strategy. If you find
237   -your errors are not getting grouped the way you would expect, you may need to
238   -implement your own strategy. A fingerprinting strategy is just a class that
239   -implements a ::generate class method. See the classes in
240   -`app/models/fingerprint/` if you need some inspiration. You can install it with
241   -an initializer like:
242   -
243   -```ruby
244   -# config/initializers/fingerprint.rb
245   -ErrorReport.fingerprint_strategy = MyStrategy
246   -```
247   -
248 233 Plugins and Integrations
249 234 ------------------------
250 235 You can extend Errbit by adding Ruby gems and plugins which are typically gems.
... ...
app/controllers/site_config_controller.rb 0 → 100644
  Diff comments   View file @9c40727
... ... @@ -0,0 +1,27 @@
  1 +class SiteConfigController < ApplicationController
  2 + before_action :require_admin!
  3 +
  4 + def index
  5 + @config = SiteConfig.document
  6 + end
  7 +
  8 + def update
  9 + SiteConfig.document.update_attributes(
  10 + notice_fingerprinter: filtered_update_params)
  11 + flash[:success] = 'Updated site config'
  12 + redirect_to action: :index
  13 + end
  14 +
  15 + private def filtered_update_params
  16 + params
  17 + .require(:site_config)
  18 + .require(:notice_fingerprinter_attributes)
  19 + .permit(
  20 + :error_class,
  21 + :message,
  22 + :backtrace_lines,
  23 + :component,
  24 + :action,
  25 + :environment_name)
  26 + end
  27 +end
... ...
app/models/app.rb
  Diff comments   View file @9c40727
... ... @@ -27,6 +27,7 @@ class App
27 27 embeds_many :deploys
28 28 embeds_one :issue_tracker, :class_name => 'IssueTracker'
29 29 embeds_one :notification_service
  30 + embeds_one :notice_fingerprinter, autobuild: true
30 31  
31 32 has_many :problems, :inverse_of => :app, :dependent => :destroy
32 33  
... ... @@ -38,6 +39,7 @@ class App
38 39 validates_uniqueness_of :name, :allow_blank => true
39 40 validates_uniqueness_of :api_key, :allow_blank => true
40 41 validates_associated :watchers
  42 + validates_associated :notice_fingerprinter
41 43 validate :check_issue_tracker
42 44  
43 45 accepts_nested_attributes_for :watchers, :allow_destroy => true,
... ... @@ -46,6 +48,7 @@ class App
46 48 :reject_if => proc { |attrs| !ErrbitPlugin::Registry.issue_trackers.keys.map(&:to_s).include?(attrs[:type_tracker].to_s) }
47 49 accepts_nested_attributes_for :notification_service, :allow_destroy => true,
48 50 :reject_if => proc { |attrs| !NotificationService.subclasses.map(&:to_s).include?(attrs[:type].to_s) }
  51 + accepts_nested_attributes_for :notice_fingerprinter
49 52  
50 53 scope :watched_by, ->(user) do
51 54 where watchers: { "$elemMatch" => { "user_id" => user.id } }
... ...
app/models/error_report.rb
  Diff comments   View file @9c40727
... ... @@ -26,10 +26,6 @@ class ErrorReport
26 26 attr_reader :server_environment
27 27 attr_reader :user_attributes
28 28  
29   - cattr_accessor :fingerprint_strategy do
30   - Fingerprint::Sha1
31   - end
32   -
33 29 def initialize(xml_or_attributes)
34 30 @attributes = xml_or_attributes
35 31 @attributes = Hoptoad.parse_xml!(@attributes) if @attributes.is_a? String
... ... @@ -130,9 +126,7 @@ class ErrorReport
130 126 Gem::Version.new(app_version) >= Gem::Version.new(current_version)
131 127 end
132 128  
133   - private
134   -
135 129 def fingerprint
136   - @fingerprint ||= fingerprint_strategy.generate(notice, api_key)
  130 + app.notice_fingerprinter.generate(api_key, notice, backtrace)
137 131 end
138 132 end
... ...
app/models/fingerprint/md5.rb
View file @24db1d1
... ... @@ -1,22 +0,0 @@
1   -require 'digest/md5'
2   -
3   -module Fingerprint
4   - class MD5 < Sha1
5   - def to_s
6   - Digest::MD5.hexdigest(fingerprint_source)
7   - end
8   -
9   - def fingerprint_source
10   - location['method'] &&= sanitized_method_signature
11   - end
12   -
13   - private
14   - def sanitized_method_signature
15   - location['method'].gsub(/[0-9]+|FRAGMENT/, '#').gsub(/_+#/, '_#')
16   - end
17   -
18   - def location
19   - notice.backtrace.lines.first
20   - end
21   - end
22   -end
app/models/fingerprint/sha1.rb
View file @24db1d1
... ... @@ -1,43 +0,0 @@
1   -require 'digest/sha1'
2   -
3   -module Fingerprint
4   - class Sha1
5   -
6   - attr_reader :notice, :api_key
7   -
8   - def self.generate(notice, api_key)
9   - self.new(notice, api_key).to_s
10   - end
11   -
12   - def initialize(notice, api_key)
13   - @notice = notice
14   - @api_key = api_key
15   - end
16   -
17   - def to_s
18   - Digest::SHA1.hexdigest(fingerprint_source.to_s)
19   - end
20   -
21   - def fingerprint_source
22   - {
23   - :file_or_message => file_or_message,
24   - :error_class => notice.error_class,
25   - :component => notice.component || 'unknown',
26   - :action => notice.action,
27   - :environment => notice.environment_name || 'development',
28   - :api_key => api_key
29   - }
30   - end
31   -
32   - def file_or_message
33   - @file_or_message ||= unified_message + notice.backtrace.fingerprint
34   - end
35   -
36   - # filter memory addresses out of object strings
37   - # example: "#<Object:0x007fa2b33d9458>" becomes "#<Object>"
38   - def unified_message
39   - notice.message.gsub(/(#<.+?):[0-9a-f]x[0-9a-f]+(>)/, '\1\2')
40   - end
41   -
42   - end
43   -end
app/models/notice_fingerprinter.rb 0 → 100644
  Diff comments   View file @9c40727
... ... @@ -0,0 +1,32 @@
  1 +class NoticeFingerprinter
  2 + include Mongoid::Document
  3 + include Mongoid::Timestamps
  4 +
  5 + field :error_class, default: true, type: Boolean
  6 + field :message, default: true, type: Boolean
  7 + field :backtrace_lines, default: -1, type: Integer
  8 + field :component, default: true, type: Boolean
  9 + field :action, default: true, type: Boolean
  10 + field :environment_name, default: true, type: Boolean
  11 + field :source, type: String
  12 +
  13 + embedded_in :app
  14 + embedded_in :site_config
  15 +
  16 + def generate(api_key, notice, backtrace)
  17 + material = []
  18 + material << notice.error_class if error_class
  19 + material << notice.message if message
  20 + material << notice.component if component
  21 + material << notice.action if action
  22 + material << notice.environment_name if environment_name
  23 +
  24 + if backtrace_lines < 0
  25 + material << backtrace.lines
  26 + else
  27 + material << backtrace.lines.slice(0, backtrace_lines)
  28 + end
  29 +
  30 + Digest::MD5.hexdigest(material.reduce('') { |c, m| c << m.to_s; c })
  31 + end
  32 +end
... ...
app/models/site_config.rb 0 → 100644
  Diff comments   View file @9c40727
... ... @@ -0,0 +1,36 @@
  1 +class SiteConfig
  2 + CONFIG_SOURCE_SITE = 'site'.freeze
  3 + CONFIG_SOURCE_APP = 'app'.freeze
  4 +
  5 + include Mongoid::Document
  6 + include Mongoid::Timestamps
  7 +
  8 + before_save :denormalize
  9 +
  10 + embeds_one :notice_fingerprinter, autobuild: true
  11 + validates_associated :notice_fingerprinter
  12 + accepts_nested_attributes_for :notice_fingerprinter
  13 +
  14 + # Get the one and only SiteConfig document
  15 + def self.document
  16 + first || create
  17 + end
  18 +
  19 + # Denormalize SiteConfig onto individual apps so that this record doesn't
  20 + # need to be accessed when inserting new error notices
  21 + def denormalize
  22 + notice_fingerprinter_attributes = notice_fingerprinter.attributes.tap do |attrs|
  23 + attrs.delete('_id')
  24 + attrs[:source] = :site
  25 + end
  26 +
  27 + App.each do |app|
  28 + f = app.notice_fingerprinter
  29 +
  30 + if !f || f.source == CONFIG_SOURCE_SITE
  31 + app.update_attributes(
  32 + notice_fingerprinter: notice_fingerprinter_attributes)
  33 + end
  34 + end
  35 + end
  36 +end
... ...
app/views/shared/_navigation.html.haml
  Diff comments   View file @9c40727
... ... @@ -13,4 +13,8 @@
13 13 = link_to users_path do
14 14 %i.fa.fa-users
15 15 = t('.users')
  16 + %li{:class => active_if_here(:site_config)}
  17 + = link_to site_config_index_path do
  18 + %i.fa.fa-wrench
  19 + = t('.config')
16 20 %div.clear
... ...
app/views/site_config/index.html.haml 0 → 100644
  Diff comments   View file @9c40727
... ... @@ -0,0 +1,39 @@
  1 +- content_for :title, 'Config'
  2 +
  3 +- content_for :action_bar, link_to('cancel', users_path, :class => 'button')
  4 +
  5 += form_for @config, url: site_config_index_path, method: 'put' do |f|
  6 + = errors_for @config
  7 +
  8 + %fieldset
  9 + %legend Notice Fingerprinter
  10 + %p
  11 + The notice fingerprinter governs how error notifications are grouped.
  12 + Each item counts toward an error's uniqueness if enabled.
  13 +
  14 + = f.fields_for :notice_fingerprinter do |g|
  15 + .checkbox
  16 + = g.check_box :error_class
  17 + = g.label :error_class, 'Error class'
  18 +
  19 + .checkbox
  20 + = g.check_box :message
  21 + = g.label :message, 'Error message'
  22 +
  23 + %div
  24 + = g.label :backtrace_lines, 'Number of backtrace lines (-1 for unlimited)'
  25 + = g.text_field :backtrace_lines
  26 +
  27 + .checkbox
  28 + = g.check_box :component
  29 + = g.label :component, 'Component (or controller)'
  30 +
  31 + .checkbox
  32 + = g.check_box :action
  33 + = g.label :action, 'Action'
  34 +
  35 + .checkbox
  36 + = g.check_box :environment_name
  37 + = g.label :environment_name, 'Environment name'
  38 +
  39 + %div.buttons= f.submit 'Update Config'
... ...
config/routes.rb
  Diff comments   View file @9c40727
... ... @@ -14,7 +14,14 @@ Rails.application.routes.draw do
14 14 delete :unlink_github
15 15 end
16 16 end
17   - resources :problems, :only => [:index] do
  17 +
  18 + resources :site_config, :only => [:index] do
  19 + collection do
  20 + put :update
  21 + end
  22 + end
  23 +
  24 + resources :problems, :only => [:index] do
18 25 collection do
19 26 post :destroy_several
20 27 post :resolve_several
... ...
spec/controllers/site_config_controller_spec.rb 0 → 100644
  Diff comments   View file @9c40727
... ... @@ -0,0 +1,48 @@
  1 +describe SiteConfigController, type: 'controller' do
  2 + it_requires_admin_privileges for: {
  3 + index: :get,
  4 + update: :put
  5 + }
  6 +
  7 + let(:admin) { Fabricate(:admin) }
  8 +
  9 + before { sign_in admin }
  10 +
  11 + describe '#index' do
  12 + it 'has an index action' do
  13 + get :index
  14 + end
  15 + end
  16 +
  17 + describe '#update' do
  18 + it 'updates' do
  19 + put :update, site_config: {
  20 + notice_fingerprinter_attributes: {
  21 + backtrace_lines: 3,
  22 + environment_name: false
  23 + }
  24 + }
  25 +
  26 + fingerprinter = SiteConfig.document.notice_fingerprinter
  27 +
  28 + expect(fingerprinter.environment_name).to be false
  29 + expect(fingerprinter.backtrace_lines).to be 3
  30 + end
  31 +
  32 + it 'redirects to the index' do
  33 + put :update, site_config: {
  34 + notice_fingerprinter_attributes: { error_class: true }
  35 + }
  36 +
  37 + expect(response).to redirect_to(site_config_index_path)
  38 + end
  39 +
  40 + it 'flashes a confirmation' do
  41 + put :update, site_config: {
  42 + notice_fingerprinter_attributes: { error_class: true }
  43 + }
  44 +
  45 + expect(request.flash[:success]).to eq 'Updated site config'
  46 + end
  47 + end
  48 +end
... ...
spec/fabricators/err_fabricator.rb
  Diff comments   View file @9c40727
... ... @@ -6,7 +6,8 @@ end
6 6 Fabricator :notice do
7 7 app
8 8 err
9   - message 'FooError: Too Much Bar'
  9 + error_class 'FooError'
  10 + message 'Too Much Bar'
10 11 backtrace
11 12 server_environment { {'environment-name' => 'production'} }
12 13 request {{ 'component' => 'foo', 'action' => 'bar' }}
... ...
spec/models/error_report_spec.rb
  Diff comments   View file @9c40727
... ... @@ -42,20 +42,6 @@ describe ErrorReport do
42 42 end
43 43 end
44 44  
45   - describe "#fingerprint_strategy" do
46   - it "should be possible to change how fingerprints are generated" do
47   - def error_report.fingerprint_strategy
48   - Class.new do
49   - def self.generate(*args)
50   - 'fingerprintzzz'
51   - end
52   - end
53   - end
54   -
55   - expect(error_report.error.fingerprint).to eq('fingerprintzzz')
56   - end
57   - end
58   -
59 45 describe "#generate_notice!" do
60 46 it "save a notice" do
61 47 expect {
... ...
spec/models/fingerprint/md5_spec.rb
View file @24db1d1
... ... @@ -1,39 +0,0 @@
1   -describe Fingerprint::MD5, type: 'model' do
2   - context 'being created' do
3   - let(:backtrace) do
4   - Backtrace.find_or_create([
5   - {
6   - "number"=>"17",
7   - "file"=>"[GEM_ROOT]/gems/activesupport/lib/active_support/callbacks.rb",
8   - "method"=>"_run__2497084960985961383__process_action__2062871603614456254__callbacks"
9   - }
10   - ])
11   - end
12   - let(:notice1) { Fabricate.build(:notice, :backtrace => backtrace) }
13   - let(:notice2) { Fabricate.build(:notice, :backtrace => backtrace_2) }
14   -
15   - context "with same backtrace" do
16   - let(:backtrace_2) do
17   - new_lines = backtrace.lines.dup
18   - new_lines.last[:method] = '_run__FRAGMENT__process_action__FRAGMENT__callbacks'
19   - Backtrace.find_or_create(new_lines)
20   - end
21   -
22   - it "normalizes the fingerprint of generated methods" do
23   - expect(Fingerprint::MD5.generate(notice1, "api key")).to eql Fingerprint::MD5.generate(notice2, "api key")
24   - end
25   - end
26   -
27   - context "with same backtrace where FRAGMENT has not been extracted" do
28   - let(:backtrace_2) do
29   - new_lines = backtrace.lines.dup
30   - new_lines.last[:method] = '_run__998857585768765__process_action__1231231312321313__callbacks'
31   - Backtrace.find_or_create(new_lines)
32   - end
33   -
34   - it "normalizes the fingerprint of generated methods" do
35   - expect(Fingerprint::MD5.generate(notice1, "api key")).to eql Fingerprint::MD5.generate(notice2, "api key")
36   - end
37   - end
38   - end
39   -end
spec/models/fingerprint/sha1_spec.rb
View file @24db1d1
... ... @@ -1,79 +0,0 @@
1   -describe Fingerprint::Sha1, type: 'model' do
2   - context '#generate' do
3   - let(:backtrace) {
4   - Backtrace.find_or_create([
5   - {"number"=>"425", "file"=>"[GEM_ROOT]/gems/activesupport-3.0.0.rc/lib/active_support/callbacks.rb", "method"=>"_run__2115867319__process_action__262109504__callbacks"},
6   - {"number"=>"404", "file"=>"[GEM_ROOT]/gems/activesupport-3.0.0.rc/lib/active_support/callbacks.rb", "method"=>"send"},
7   - {"number"=>"404", "file"=>"[GEM_ROOT]/gems/activesupport-3.0.0.rc/lib/active_support/callbacks.rb", "method"=>"_run_process_action_callbacks"}
8   - ])
9   - }
10   - let(:notice1) { Fabricate.build(:notice, :backtrace => backtrace) }
11   - let(:notice2) { Fabricate.build(:notice, :backtrace => backtrace_2) }
12   -
13   - context "with same backtrace" do
14   - let(:backtrace_2) { backtrace }
15   - it 'should create the same fingerprint for two notices' do
16   - expect(Fingerprint::Sha1.generate(notice1, "api key")).to eq Fingerprint::Sha1.generate(notice2, "api key")
17   - end
18   - end
19   -
20   - context "with different backtrace with only last line change" do
21   - let(:backtrace_2) {
22   - new_lines = backtrace.lines.dup
23   - new_lines.last[:number] = 401
24   - Backtrace.find_or_create backtrace.lines
25   - }
26   - it 'should not same fingerprint' do
27   - expect(
28   - Fingerprint::Sha1.generate(notice1, "api key")
29   - ).not_to eql Fingerprint::Sha1.generate(notice2, "api key")
30   - end
31   - end
32   -
33   - context 'with messages differing in object string memory addresses' do
34   - let(:backtrace_2) { backtrace }
35   -
36   - before do
37   - notice1.message = "NoMethodError: undefined method `foo' for #<ActiveSupport::HashWithIndifferentAccess:0x007f6bfe3287e8>"
38   - notice2.message = "NoMethodError: undefined method `foo' for #<ActiveSupport::HashWithIndifferentAccess:0x007f6bfd9f5338>"
39   - end
40   -
41   - its 'fingerprints should be equal' do
42   - expect(Fingerprint::Sha1.generate(notice1, 'api key')).to eq Fingerprint::Sha1.generate(notice2, 'api key')
43   - end
44   - end
45   -
46   - context 'with different messages at same stacktrace' do
47   - let(:backtrace_2) { backtrace }
48   -
49   - before do
50   - notice1.message = "NoMethodError: undefined method `bar' for #<ActiveSupport::HashWithIndifferentAccess:0x007f6bfe3287e8>"
51   - notice2.message = "NoMethodError: undefined method `bar' for nil:NilClass"
52   - end
53   -
54   - its 'fingerprints should not be equal' do
55   - expect(Fingerprint::Sha1.generate(notice1, 'api key')).to_not eq Fingerprint::Sha1.generate(notice2, 'api key')
56   - end
57   - end
58   - end
59   -
60   - describe '#unified_message' do
61   - subject{ Fingerprint::Sha1.new(double('notice', message: message), 'api key').unified_message }
62   -
63   - context "full error message" do
64   - let(:message) { "NoMethodError: undefined method `foo' for #<ActiveSupport::HashWithIndifferentAccess:0x007f6bfe3287e8>" }
65   -
66   - it 'removes memory address from object strings' do
67   - is_expected.to eq "NoMethodError: undefined method `foo' for #<ActiveSupport::HashWithIndifferentAccess>"
68   - end
69   - end
70   -
71   - context "multiple object strings in message" do
72   - let(:message) { "#<ActiveSupport::HashWithIndifferentAccess:0x007f6bfe3287e8> #<Object:0x007fa2b33d9458>" }
73   -
74   - it 'removes memory addresses globally' do
75   - is_expected.to eq "#<ActiveSupport::HashWithIndifferentAccess> #<Object>"
76   - end
77   - end
78   - end
79   -end
spec/models/notice_fingerprinter_spec.rb 0 → 100644
  Diff comments   View file @9c40727
... ... @@ -0,0 +1,64 @@
  1 +describe NoticeFingerprinter, type: 'model' do
  2 + let(:fingerprinter) { described_class.new }
  3 + let(:notice) { Fabricate(:notice) }
  4 + let(:backtrace) { Fabricate(:backtrace) }
  5 +
  6 + context '#generate' do
  7 + it 'generates the same fingerprint for the same notice' do
  8 + f1 = fingerprinter.generate('123', notice, backtrace)
  9 + f2 = fingerprinter.generate('123', notice, backtrace)
  10 + expect(f1).to eq(f2)
  11 + end
  12 +
  13 + %w(error_class message component action environment_name).each do |i|
  14 + it "affects the fingerprint when #{i} is false" do
  15 + f1 = fingerprinter.generate('123', notice, backtrace)
  16 + f2 = fingerprinter.generate('123', notice, backtrace)
  17 +
  18 + fingerprinter.send((i << '=').to_sym, false)
  19 + f3 = fingerprinter.generate('123', notice, backtrace)
  20 +
  21 + expect(f1).to eq(f2)
  22 + expect(f1).to_not eq(f3)
  23 + end
  24 + end
  25 +
  26 + it 'affects the fingerprint with different backtrace_lines config' do
  27 + f1 = fingerprinter.generate('123', notice, backtrace)
  28 + f2 = fingerprinter.generate('123', notice, backtrace)
  29 +
  30 + fingerprinter.backtrace_lines = 2
  31 + f3 = fingerprinter.generate('123', notice, backtrace)
  32 +
  33 + expect(f1).to eq(f2)
  34 + expect(f1).to_not eq(f3)
  35 + end
  36 +
  37 + context 'two backtraces have the same first two lines' do
  38 + let(:backtrace1) { Fabricate(:backtrace) }
  39 + let(:backtrace2) { Fabricate(:backtrace) }
  40 +
  41 + before do
  42 + backtrace1.lines[0] = backtrace2.lines[0]
  43 + backtrace1.lines[1] = backtrace2.lines[1]
  44 + backtrace1.lines[2] = { number: 1, file: 'a', method: :b }
  45 + end
  46 +
  47 + it 'has the same fingerprint when only considering two lines' do
  48 + fingerprinter.backtrace_lines = 2
  49 + f1 = fingerprinter.generate('123', notice, backtrace1)
  50 + f2 = fingerprinter.generate('123', notice, backtrace2)
  51 +
  52 + expect(f1).to eq(f2)
  53 + end
  54 +
  55 + it 'has a different fingerprint when considering three lines' do
  56 + fingerprinter.backtrace_lines = 3
  57 + f1 = fingerprinter.generate('123', notice, backtrace1)
  58 + f2 = fingerprinter.generate('123', notice, backtrace2)
  59 +
  60 + expect(f1).to_not eq(f2)
  61 + end
  62 + end
  63 + end
  64 +end
... ...