From da3946bef7921e5a6df0d55fc2f556b641884ada Mon Sep 17 00:00:00 2001 From: Chris Saunders Date: Fri, 4 Apr 2014 16:04:22 +0000 Subject: [PATCH] Refactor fingerprinting into a Fingerprint module --- README.md | 4 ++-- app/models/error_report.rb | 2 +- app/models/fingerprint.rb | 41 ----------------------------------------- app/models/fingerprint/md5.rb | 22 ++++++++++++++++++++++ app/models/fingerprint/sha1.rb | 43 +++++++++++++++++++++++++++++++++++++++++++ app/models/fingerprints/legacy_fingerprint.rb | 20 -------------------- spec/models/error_report_spec.rb | 2 +- spec/models/fingerprint/md5_spec.rb | 43 +++++++++++++++++++++++++++++++++++++++++++ spec/models/fingerprint/sha1_spec.rb | 87 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ spec/models/fingerprint_spec.rb | 87 --------------------------------------------------------------------------------------- spec/models/fingerprints/legacy_fingerprint_spec.rb | 43 ------------------------------------------- 11 files changed, 199 insertions(+), 195 deletions(-) delete mode 100644 app/models/fingerprint.rb create mode 100644 app/models/fingerprint/md5.rb create mode 100644 app/models/fingerprint/sha1.rb delete mode 100644 app/models/fingerprints/legacy_fingerprint.rb create mode 100644 spec/models/fingerprint/md5_spec.rb create mode 100644 spec/models/fingerprint/sha1_spec.rb delete mode 100644 spec/models/fingerprint_spec.rb delete mode 100644 spec/models/fingerprints/legacy_fingerprint_spec.rb diff --git a/README.md b/README.md index a94cb02..9457372 100644 --- a/README.md +++ b/README.md @@ -399,11 +399,11 @@ Using custom fingerprinting methods ----------------------------------- Errbit allows you to use your own Fingerprinting Strategy. -If you are upgrading from a very old version of errbit, you can use the `LegacyFingerprint` for compatibility. The fingerprint strategy can be changed by adding an initializer to errbit: +If you are upgrading from a very old version of errbit, you can use the `Fingerprint::MD5` for compatibility. The fingerprint strategy can be changed by adding an initializer to errbit: ```ruby # config/fingerprint.rb -ErrorReport.fingerprint_strategy = LegacyFingerprint +ErrorReport.fingerprint_strategy = Fingerprint::MD5 ``` The easiest way to add custom fingerprint methods is to simply subclass `Fingerprint` diff --git a/app/models/error_report.rb b/app/models/error_report.rb index 1b79ba7..e3c4868 100644 --- a/app/models/error_report.rb +++ b/app/models/error_report.rb @@ -19,7 +19,7 @@ class ErrorReport :notifier, :user_attributes, :framework, :notice cattr_accessor :fingerprint_strategy do - Fingerprint + Fingerprint::Sha1 end def initialize(xml_or_attributes) diff --git a/app/models/fingerprint.rb b/app/models/fingerprint.rb deleted file mode 100644 index 66001c5..0000000 --- a/app/models/fingerprint.rb +++ /dev/null @@ -1,41 +0,0 @@ -require 'digest/sha1' - -class Fingerprint - - 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 diff --git a/app/models/fingerprint/md5.rb b/app/models/fingerprint/md5.rb new file mode 100644 index 0000000..6bd76dd --- /dev/null +++ b/app/models/fingerprint/md5.rb @@ -0,0 +1,22 @@ +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 new file mode 100644 index 0000000..78961fd --- /dev/null +++ b/app/models/fingerprint/sha1.rb @@ -0,0 +1,43 @@ +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/fingerprints/legacy_fingerprint.rb b/app/models/fingerprints/legacy_fingerprint.rb deleted file mode 100644 index fe4dfa0..0000000 --- a/app/models/fingerprints/legacy_fingerprint.rb +++ /dev/null @@ -1,20 +0,0 @@ -require 'digest/md5' - -class LegacyFingerprint < Fingerprint - 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 diff --git a/spec/models/error_report_spec.rb b/spec/models/error_report_spec.rb index cec55b2..f8237bd 100644 --- a/spec/models/error_report_spec.rb +++ b/spec/models/error_report_spec.rb @@ -49,7 +49,7 @@ describe ErrorReport do describe "#fingerprint_strategy" do after(:all) { - error_report.fingerprint_strategy = Fingerprint + error_report.fingerprint_strategy = Fingerprint::Sha1 } it "should be possible to change how fingerprints are generated" do diff --git a/spec/models/fingerprint/md5_spec.rb b/spec/models/fingerprint/md5_spec.rb new file mode 100644 index 0000000..63599be --- /dev/null +++ b/spec/models/fingerprint/md5_spec.rb @@ -0,0 +1,43 @@ +require 'spec_helper' + +describe Fingerprint::MD5 do + context 'being created' do + let(:backtrace) do + Backtrace.create(:raw => [ + { + "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 + backtrace + backtrace.lines.last.method = '_run__FRAGMENT__process_action__FRAGMENT__callbacks' + backtrace.save + backtrace + 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 + backtrace + backtrace.lines.last.method = '_run__998857585768765__process_action__1231231312321313__callbacks' + backtrace.save + backtrace + 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 new file mode 100644 index 0000000..be834a9 --- /dev/null +++ b/spec/models/fingerprint/sha1_spec.rb @@ -0,0 +1,87 @@ +require 'spec_helper' + +describe Fingerprint::Sha1 do + + context '#generate' do + let(:backtrace) { + Backtrace.create(:raw => [ + {"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) { + backtrace + backtrace.lines.last.number = 401 + backtrace.send(:generate_fingerprint) + backtrace.save + backtrace + } + 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 + should eq "NoMethodError: undefined method `foo' for #" + end + end + + context "multiple object strings in message" do + let(:message) { "# #" } + + it 'removes memory addresses globally' do + should eq "# #" + end + end + + end + +end + diff --git a/spec/models/fingerprint_spec.rb b/spec/models/fingerprint_spec.rb deleted file mode 100644 index 0e9613c..0000000 --- a/spec/models/fingerprint_spec.rb +++ /dev/null @@ -1,87 +0,0 @@ -require 'spec_helper' - -describe Fingerprint do - - context '#generate' do - let(:backtrace) { - Backtrace.create(:raw => [ - {"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.generate(notice1, "api key")).to eq Fingerprint.generate(notice2, "api key") - end - end - - context "with different backtrace with only last line change" do - let(:backtrace_2) { - backtrace - backtrace.lines.last.number = 401 - backtrace.send(:generate_fingerprint) - backtrace.save - backtrace - } - it 'should not same fingerprint' do - expect( - Fingerprint.generate(notice1, "api key") - ).not_to eql Fingerprint.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.generate(notice1, 'api key')).to eq Fingerprint.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.generate(notice1, 'api key')).to_not eq Fingerprint.generate(notice2, 'api key') - end - end - end - - describe '#unified_message' do - subject{ Fingerprint.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 - should eq "NoMethodError: undefined method `foo' for #" - end - end - - context "multiple object strings in message" do - let(:message) { "# #" } - - it 'removes memory addresses globally' do - should eq "# #" - end - end - - end - -end - diff --git a/spec/models/fingerprints/legacy_fingerprint_spec.rb b/spec/models/fingerprints/legacy_fingerprint_spec.rb deleted file mode 100644 index f1f019c..0000000 --- a/spec/models/fingerprints/legacy_fingerprint_spec.rb +++ /dev/null @@ -1,43 +0,0 @@ -require 'spec_helper' - -describe LegacyFingerprint do - context 'being created' do - let(:backtrace) do - Backtrace.create(:raw => [ - { - "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 - backtrace - backtrace.lines.last.method = '_run__FRAGMENT__process_action__FRAGMENT__callbacks' - backtrace.save - backtrace - end - - it "normalizes the fingerprint of generated methods" do - expect(LegacyFingerprint.generate(notice1, "api key")).to eql LegacyFingerprint.generate(notice2, "api key") - end - end - - context "with same backtrace where FRAGMENT has not been extracted" do - let(:backtrace_2) do - backtrace - backtrace.lines.last.method = '_run__998857585768765__process_action__1231231312321313__callbacks' - backtrace.save - backtrace - end - - it "normalizes the fingerprint of generated methods" do - expect(LegacyFingerprint.generate(notice1, "api key")).to eql LegacyFingerprint.generate(notice2, "api key") - end - end - end -end -- libgit2 0.21.2