diff --git a/README.md b/README.md index ceb3b03..f908972 100644 --- a/README.md +++ b/README.md @@ -230,21 +230,6 @@ Airbrake.setProject("ERRBIT API KEY", "ERRBIT API KEY"); Airbrake.setHost("http://errbit.yourdomain.com"); ``` -Using custom fingerprinting methods ------------------------------------ - -Errbit collates errors into groups using a fingerprinting strategy. If you find -your errors are not getting grouped the way you would expect, you may need to -implement your own strategy. A fingerprinting strategy is just a class that -implements a ::generate class method. See the classes in -`app/models/fingerprint/` if you need some inspiration. You can install it with -an initializer like: - -```ruby -# config/initializers/fingerprint.rb -ErrorReport.fingerprint_strategy = MyStrategy -``` - Plugins and Integrations ------------------------ You can extend Errbit by adding Ruby gems and plugins which are typically gems. diff --git a/app/controllers/site_config_controller.rb b/app/controllers/site_config_controller.rb new file mode 100644 index 0000000..7d767ed --- /dev/null +++ b/app/controllers/site_config_controller.rb @@ -0,0 +1,27 @@ +class SiteConfigController < ApplicationController + before_action :require_admin! + + def index + @config = SiteConfig.document + end + + def update + SiteConfig.document.update_attributes( + notice_fingerprinter: filtered_update_params) + flash[:success] = 'Updated site config' + redirect_to action: :index + end + + private def filtered_update_params + params + .require(:site_config) + .require(:notice_fingerprinter_attributes) + .permit( + :error_class, + :message, + :backtrace_lines, + :component, + :action, + :environment_name) + end +end diff --git a/app/models/app.rb b/app/models/app.rb index c581ba1..dbc1463 100644 --- a/app/models/app.rb +++ b/app/models/app.rb @@ -27,6 +27,7 @@ class App embeds_many :deploys embeds_one :issue_tracker, :class_name => 'IssueTracker' embeds_one :notification_service + embeds_one :notice_fingerprinter, autobuild: true has_many :problems, :inverse_of => :app, :dependent => :destroy @@ -38,6 +39,7 @@ class App validates_uniqueness_of :name, :allow_blank => true validates_uniqueness_of :api_key, :allow_blank => true validates_associated :watchers + validates_associated :notice_fingerprinter validate :check_issue_tracker accepts_nested_attributes_for :watchers, :allow_destroy => true, @@ -46,6 +48,7 @@ class App :reject_if => proc { |attrs| !ErrbitPlugin::Registry.issue_trackers.keys.map(&:to_s).include?(attrs[:type_tracker].to_s) } accepts_nested_attributes_for :notification_service, :allow_destroy => true, :reject_if => proc { |attrs| !NotificationService.subclasses.map(&:to_s).include?(attrs[:type].to_s) } + accepts_nested_attributes_for :notice_fingerprinter scope :watched_by, ->(user) do where watchers: { "$elemMatch" => { "user_id" => user.id } } diff --git a/app/models/error_report.rb b/app/models/error_report.rb index 09d79ff..d2b4020 100644 --- a/app/models/error_report.rb +++ b/app/models/error_report.rb @@ -26,10 +26,6 @@ class ErrorReport attr_reader :server_environment attr_reader :user_attributes - cattr_accessor :fingerprint_strategy do - Fingerprint::Sha1 - end - def initialize(xml_or_attributes) @attributes = xml_or_attributes @attributes = Hoptoad.parse_xml!(@attributes) if @attributes.is_a? String @@ -130,9 +126,7 @@ class ErrorReport Gem::Version.new(app_version) >= Gem::Version.new(current_version) end - private - def fingerprint - @fingerprint ||= fingerprint_strategy.generate(notice, api_key) + app.notice_fingerprinter.generate(api_key, notice, backtrace) end end diff --git a/app/models/fingerprint/md5.rb b/app/models/fingerprint/md5.rb deleted file mode 100644 index 6bd76dd..0000000 --- a/app/models/fingerprint/md5.rb +++ /dev/null @@ -1,22 +0,0 @@ -require 'digest/md5' - -module Fingerprint - class MD5 < Sha1 - def to_s - Digest::MD5.hexdigest(fingerprint_source) - end - - def fingerprint_source - location['method'] &&= sanitized_method_signature - end - - private - def sanitized_method_signature - location['method'].gsub(/[0-9]+|FRAGMENT/, '#').gsub(/_+#/, '_#') - end - - def location - notice.backtrace.lines.first - end - end -end diff --git a/app/models/fingerprint/sha1.rb b/app/models/fingerprint/sha1.rb deleted file mode 100644 index 78961fd..0000000 --- a/app/models/fingerprint/sha1.rb +++ /dev/null @@ -1,43 +0,0 @@ -require 'digest/sha1' - -module Fingerprint - class Sha1 - - attr_reader :notice, :api_key - - def self.generate(notice, api_key) - self.new(notice, api_key).to_s - end - - def initialize(notice, api_key) - @notice = notice - @api_key = api_key - end - - def to_s - Digest::SHA1.hexdigest(fingerprint_source.to_s) - end - - def fingerprint_source - { - :file_or_message => file_or_message, - :error_class => notice.error_class, - :component => notice.component || 'unknown', - :action => notice.action, - :environment => notice.environment_name || 'development', - :api_key => api_key - } - end - - def file_or_message - @file_or_message ||= unified_message + notice.backtrace.fingerprint - end - - # filter memory addresses out of object strings - # example: "#" becomes "#" - def unified_message - notice.message.gsub(/(#<.+?):[0-9a-f]x[0-9a-f]+(>)/, '\1\2') - end - - end -end diff --git a/app/models/notice_fingerprinter.rb b/app/models/notice_fingerprinter.rb new file mode 100644 index 0000000..0ce1cf8 --- /dev/null +++ b/app/models/notice_fingerprinter.rb @@ -0,0 +1,32 @@ +class NoticeFingerprinter + include Mongoid::Document + include Mongoid::Timestamps + + field :error_class, default: true, type: Boolean + field :message, default: true, type: Boolean + field :backtrace_lines, default: -1, type: Integer + field :component, default: true, type: Boolean + field :action, default: true, type: Boolean + field :environment_name, default: true, type: Boolean + field :source, type: String + + embedded_in :app + embedded_in :site_config + + def generate(api_key, notice, backtrace) + material = [] + material << notice.error_class if error_class + material << notice.message if message + material << notice.component if component + material << notice.action if action + material << notice.environment_name if environment_name + + if backtrace_lines < 0 + material << backtrace.lines + else + material << backtrace.lines.slice(0, backtrace_lines) + end + + Digest::MD5.hexdigest(material.reduce('') { |c, m| c << m.to_s; c }) + end +end diff --git a/app/models/site_config.rb b/app/models/site_config.rb new file mode 100644 index 0000000..434a93e --- /dev/null +++ b/app/models/site_config.rb @@ -0,0 +1,36 @@ +class SiteConfig + CONFIG_SOURCE_SITE = 'site'.freeze + CONFIG_SOURCE_APP = 'app'.freeze + + include Mongoid::Document + include Mongoid::Timestamps + + before_save :denormalize + + embeds_one :notice_fingerprinter, autobuild: true + validates_associated :notice_fingerprinter + accepts_nested_attributes_for :notice_fingerprinter + + # Get the one and only SiteConfig document + def self.document + first || create + end + + # Denormalize SiteConfig onto individual apps so that this record doesn't + # need to be accessed when inserting new error notices + def denormalize + notice_fingerprinter_attributes = notice_fingerprinter.attributes.tap do |attrs| + attrs.delete('_id') + attrs[:source] = :site + end + + App.each do |app| + f = app.notice_fingerprinter + + if !f || f.source == CONFIG_SOURCE_SITE + app.update_attributes( + notice_fingerprinter: notice_fingerprinter_attributes) + end + end + end +end diff --git a/app/views/shared/_navigation.html.haml b/app/views/shared/_navigation.html.haml index df9b4bf..d901b8b 100644 --- a/app/views/shared/_navigation.html.haml +++ b/app/views/shared/_navigation.html.haml @@ -13,4 +13,8 @@ = link_to users_path do %i.fa.fa-users = t('.users') + %li{:class => active_if_here(:site_config)} + = link_to site_config_index_path do + %i.fa.fa-wrench + = t('.config') %div.clear diff --git a/app/views/site_config/index.html.haml b/app/views/site_config/index.html.haml new file mode 100644 index 0000000..9358f5a --- /dev/null +++ b/app/views/site_config/index.html.haml @@ -0,0 +1,39 @@ +- content_for :title, 'Config' + +- content_for :action_bar, link_to('cancel', users_path, :class => 'button') + += form_for @config, url: site_config_index_path, method: 'put' do |f| + = errors_for @config + + %fieldset + %legend Notice Fingerprinter + %p + The notice fingerprinter governs how error notifications are grouped. + Each item counts toward an error's uniqueness if enabled. + + = f.fields_for :notice_fingerprinter do |g| + .checkbox + = g.check_box :error_class + = g.label :error_class, 'Error class' + + .checkbox + = g.check_box :message + = g.label :message, 'Error message' + + %div + = g.label :backtrace_lines, 'Number of backtrace lines (-1 for unlimited)' + = g.text_field :backtrace_lines + + .checkbox + = g.check_box :component + = g.label :component, 'Component (or controller)' + + .checkbox + = g.check_box :action + = g.label :action, 'Action' + + .checkbox + = g.check_box :environment_name + = g.label :environment_name, 'Environment name' + + %div.buttons= f.submit 'Update Config' diff --git a/config/routes.rb b/config/routes.rb index 6a1455a..df63f5b 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -14,7 +14,14 @@ Rails.application.routes.draw do delete :unlink_github end end - resources :problems, :only => [:index] do + + resources :site_config, :only => [:index] do + collection do + put :update + end + end + + resources :problems, :only => [:index] do collection do post :destroy_several post :resolve_several diff --git a/spec/controllers/site_config_controller_spec.rb b/spec/controllers/site_config_controller_spec.rb new file mode 100644 index 0000000..9195b8e --- /dev/null +++ b/spec/controllers/site_config_controller_spec.rb @@ -0,0 +1,48 @@ +describe SiteConfigController, type: 'controller' do + it_requires_admin_privileges for: { + index: :get, + update: :put + } + + let(:admin) { Fabricate(:admin) } + + before { sign_in admin } + + describe '#index' do + it 'has an index action' do + get :index + end + end + + describe '#update' do + it 'updates' do + put :update, site_config: { + notice_fingerprinter_attributes: { + backtrace_lines: 3, + environment_name: false + } + } + + fingerprinter = SiteConfig.document.notice_fingerprinter + + expect(fingerprinter.environment_name).to be false + expect(fingerprinter.backtrace_lines).to be 3 + end + + it 'redirects to the index' do + put :update, site_config: { + notice_fingerprinter_attributes: { error_class: true } + } + + expect(response).to redirect_to(site_config_index_path) + end + + it 'flashes a confirmation' do + put :update, site_config: { + notice_fingerprinter_attributes: { error_class: true } + } + + expect(request.flash[:success]).to eq 'Updated site config' + end + end +end diff --git a/spec/fabricators/err_fabricator.rb b/spec/fabricators/err_fabricator.rb index 23d01f6..3a3dcb2 100644 --- a/spec/fabricators/err_fabricator.rb +++ b/spec/fabricators/err_fabricator.rb @@ -6,7 +6,8 @@ end Fabricator :notice do app err - message 'FooError: Too Much Bar' + error_class 'FooError' + message 'Too Much Bar' backtrace server_environment { {'environment-name' => 'production'} } request {{ 'component' => 'foo', 'action' => 'bar' }} diff --git a/spec/models/error_report_spec.rb b/spec/models/error_report_spec.rb index bffe33b..049bd9f 100644 --- a/spec/models/error_report_spec.rb +++ b/spec/models/error_report_spec.rb @@ -42,20 +42,6 @@ describe ErrorReport do end end - describe "#fingerprint_strategy" do - it "should be possible to change how fingerprints are generated" do - def error_report.fingerprint_strategy - Class.new do - def self.generate(*args) - 'fingerprintzzz' - end - end - end - - expect(error_report.error.fingerprint).to eq('fingerprintzzz') - end - end - describe "#generate_notice!" do it "save a notice" do expect { diff --git a/spec/models/fingerprint/md5_spec.rb b/spec/models/fingerprint/md5_spec.rb deleted file mode 100644 index 283e56c..0000000 --- a/spec/models/fingerprint/md5_spec.rb +++ /dev/null @@ -1,39 +0,0 @@ -describe Fingerprint::MD5, type: 'model' do - context 'being created' do - let(:backtrace) do - Backtrace.find_or_create([ - { - "number"=>"17", - "file"=>"[GEM_ROOT]/gems/activesupport/lib/active_support/callbacks.rb", - "method"=>"_run__2497084960985961383__process_action__2062871603614456254__callbacks" - } - ]) - end - let(:notice1) { Fabricate.build(:notice, :backtrace => backtrace) } - let(:notice2) { Fabricate.build(:notice, :backtrace => backtrace_2) } - - context "with same backtrace" do - let(:backtrace_2) do - new_lines = backtrace.lines.dup - new_lines.last[:method] = '_run__FRAGMENT__process_action__FRAGMENT__callbacks' - Backtrace.find_or_create(new_lines) - end - - it "normalizes the fingerprint of generated methods" do - expect(Fingerprint::MD5.generate(notice1, "api key")).to eql Fingerprint::MD5.generate(notice2, "api key") - end - end - - context "with same backtrace where FRAGMENT has not been extracted" do - let(:backtrace_2) do - new_lines = backtrace.lines.dup - new_lines.last[:method] = '_run__998857585768765__process_action__1231231312321313__callbacks' - Backtrace.find_or_create(new_lines) - end - - it "normalizes the fingerprint of generated methods" do - expect(Fingerprint::MD5.generate(notice1, "api key")).to eql Fingerprint::MD5.generate(notice2, "api key") - end - end - end -end diff --git a/spec/models/fingerprint/sha1_spec.rb b/spec/models/fingerprint/sha1_spec.rb deleted file mode 100644 index d83c264..0000000 --- a/spec/models/fingerprint/sha1_spec.rb +++ /dev/null @@ -1,79 +0,0 @@ -describe Fingerprint::Sha1, type: 'model' do - context '#generate' do - let(:backtrace) { - Backtrace.find_or_create([ - {"number"=>"425", "file"=>"[GEM_ROOT]/gems/activesupport-3.0.0.rc/lib/active_support/callbacks.rb", "method"=>"_run__2115867319__process_action__262109504__callbacks"}, - {"number"=>"404", "file"=>"[GEM_ROOT]/gems/activesupport-3.0.0.rc/lib/active_support/callbacks.rb", "method"=>"send"}, - {"number"=>"404", "file"=>"[GEM_ROOT]/gems/activesupport-3.0.0.rc/lib/active_support/callbacks.rb", "method"=>"_run_process_action_callbacks"} - ]) - } - let(:notice1) { Fabricate.build(:notice, :backtrace => backtrace) } - let(:notice2) { Fabricate.build(:notice, :backtrace => backtrace_2) } - - context "with same backtrace" do - let(:backtrace_2) { backtrace } - it 'should create the same fingerprint for two notices' do - expect(Fingerprint::Sha1.generate(notice1, "api key")).to eq Fingerprint::Sha1.generate(notice2, "api key") - end - end - - context "with different backtrace with only last line change" do - let(:backtrace_2) { - new_lines = backtrace.lines.dup - new_lines.last[:number] = 401 - Backtrace.find_or_create backtrace.lines - } - it 'should not same fingerprint' do - expect( - Fingerprint::Sha1.generate(notice1, "api key") - ).not_to eql Fingerprint::Sha1.generate(notice2, "api key") - end - end - - context 'with messages differing in object string memory addresses' do - let(:backtrace_2) { backtrace } - - before do - notice1.message = "NoMethodError: undefined method `foo' for #" - notice2.message = "NoMethodError: undefined method `foo' for #" - end - - its 'fingerprints should be equal' do - expect(Fingerprint::Sha1.generate(notice1, 'api key')).to eq Fingerprint::Sha1.generate(notice2, 'api key') - end - end - - context 'with different messages at same stacktrace' do - let(:backtrace_2) { backtrace } - - before do - notice1.message = "NoMethodError: undefined method `bar' for #" - notice2.message = "NoMethodError: undefined method `bar' for nil:NilClass" - end - - its 'fingerprints should not be equal' do - expect(Fingerprint::Sha1.generate(notice1, 'api key')).to_not eq Fingerprint::Sha1.generate(notice2, 'api key') - end - end - end - - describe '#unified_message' do - subject{ Fingerprint::Sha1.new(double('notice', message: message), 'api key').unified_message } - - context "full error message" do - let(:message) { "NoMethodError: undefined method `foo' for #" } - - it 'removes memory address from object strings' do - is_expected.to eq "NoMethodError: undefined method `foo' for #" - end - end - - context "multiple object strings in message" do - let(:message) { "# #" } - - it 'removes memory addresses globally' do - is_expected.to eq "# #" - end - end - end -end diff --git a/spec/models/notice_fingerprinter_spec.rb b/spec/models/notice_fingerprinter_spec.rb new file mode 100644 index 0000000..850c8a6 --- /dev/null +++ b/spec/models/notice_fingerprinter_spec.rb @@ -0,0 +1,64 @@ +describe NoticeFingerprinter, type: 'model' do + let(:fingerprinter) { described_class.new } + let(:notice) { Fabricate(:notice) } + let(:backtrace) { Fabricate(:backtrace) } + + context '#generate' do + it 'generates the same fingerprint for the same notice' do + f1 = fingerprinter.generate('123', notice, backtrace) + f2 = fingerprinter.generate('123', notice, backtrace) + expect(f1).to eq(f2) + end + + %w(error_class message component action environment_name).each do |i| + it "affects the fingerprint when #{i} is false" do + f1 = fingerprinter.generate('123', notice, backtrace) + f2 = fingerprinter.generate('123', notice, backtrace) + + fingerprinter.send((i << '=').to_sym, false) + f3 = fingerprinter.generate('123', notice, backtrace) + + expect(f1).to eq(f2) + expect(f1).to_not eq(f3) + end + end + + it 'affects the fingerprint with different backtrace_lines config' do + f1 = fingerprinter.generate('123', notice, backtrace) + f2 = fingerprinter.generate('123', notice, backtrace) + + fingerprinter.backtrace_lines = 2 + f3 = fingerprinter.generate('123', notice, backtrace) + + expect(f1).to eq(f2) + expect(f1).to_not eq(f3) + end + + context 'two backtraces have the same first two lines' do + let(:backtrace1) { Fabricate(:backtrace) } + let(:backtrace2) { Fabricate(:backtrace) } + + before do + backtrace1.lines[0] = backtrace2.lines[0] + backtrace1.lines[1] = backtrace2.lines[1] + backtrace1.lines[2] = { number: 1, file: 'a', method: :b } + end + + it 'has the same fingerprint when only considering two lines' do + fingerprinter.backtrace_lines = 2 + f1 = fingerprinter.generate('123', notice, backtrace1) + f2 = fingerprinter.generate('123', notice, backtrace2) + + expect(f1).to eq(f2) + end + + it 'has a different fingerprint when considering three lines' do + fingerprinter.backtrace_lines = 3 + f1 = fingerprinter.generate('123', notice, backtrace1) + f2 = fingerprinter.generate('123', notice, backtrace2) + + expect(f1).to_not eq(f2) + end + end + end +end -- libgit2 0.21.2