Commit 8c7bcf33a58a37050b4f6a864bdbc944da11f8e1
Exists in
master
and in
1 other branch
Merge pull request #682 from csaunders/properly_namespace_legacy_fingerprint
Refactor fingerprinting into a Fingerprint module
Showing
11 changed files
with
199 additions
and
195 deletions
Show diff stats
README.md
@@ -399,11 +399,11 @@ Using custom fingerprinting methods | @@ -399,11 +399,11 @@ Using custom fingerprinting methods | ||
399 | ----------------------------------- | 399 | ----------------------------------- |
400 | 400 | ||
401 | Errbit allows you to use your own Fingerprinting Strategy. | 401 | Errbit allows you to use your own Fingerprinting Strategy. |
402 | -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: | 402 | +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: |
403 | 403 | ||
404 | ```ruby | 404 | ```ruby |
405 | # config/fingerprint.rb | 405 | # config/fingerprint.rb |
406 | -ErrorReport.fingerprint_strategy = LegacyFingerprint | 406 | +ErrorReport.fingerprint_strategy = Fingerprint::MD5 |
407 | ``` | 407 | ``` |
408 | 408 | ||
409 | The easiest way to add custom fingerprint methods is to simply subclass `Fingerprint` | 409 | The easiest way to add custom fingerprint methods is to simply subclass `Fingerprint` |
app/models/error_report.rb
@@ -19,7 +19,7 @@ class ErrorReport | @@ -19,7 +19,7 @@ class ErrorReport | ||
19 | :notifier, :user_attributes, :framework, :notice | 19 | :notifier, :user_attributes, :framework, :notice |
20 | 20 | ||
21 | cattr_accessor :fingerprint_strategy do | 21 | cattr_accessor :fingerprint_strategy do |
22 | - Fingerprint | 22 | + Fingerprint::Sha1 |
23 | end | 23 | end |
24 | 24 | ||
25 | def initialize(xml_or_attributes) | 25 | def initialize(xml_or_attributes) |
app/models/fingerprint.rb
@@ -1,41 +0,0 @@ | @@ -1,41 +0,0 @@ | ||
1 | -require 'digest/sha1' | ||
2 | - | ||
3 | -class Fingerprint | ||
4 | - | ||
5 | - attr_reader :notice, :api_key | ||
6 | - | ||
7 | - def self.generate(notice, api_key) | ||
8 | - self.new(notice, api_key).to_s | ||
9 | - end | ||
10 | - | ||
11 | - def initialize(notice, api_key) | ||
12 | - @notice = notice | ||
13 | - @api_key = api_key | ||
14 | - end | ||
15 | - | ||
16 | - def to_s | ||
17 | - Digest::SHA1.hexdigest(fingerprint_source.to_s) | ||
18 | - end | ||
19 | - | ||
20 | - def fingerprint_source | ||
21 | - { | ||
22 | - :file_or_message => file_or_message, | ||
23 | - :error_class => notice.error_class, | ||
24 | - :component => notice.component || 'unknown', | ||
25 | - :action => notice.action, | ||
26 | - :environment => notice.environment_name || 'development', | ||
27 | - :api_key => api_key | ||
28 | - } | ||
29 | - end | ||
30 | - | ||
31 | - def file_or_message | ||
32 | - @file_or_message ||= unified_message + notice.backtrace.fingerprint | ||
33 | - end | ||
34 | - | ||
35 | - # filter memory addresses out of object strings | ||
36 | - # example: "#<Object:0x007fa2b33d9458>" becomes "#<Object>" | ||
37 | - def unified_message | ||
38 | - notice.message.gsub(/(#<.+?):[0-9a-f]x[0-9a-f]+(>)/, '\1\2') | ||
39 | - end | ||
40 | - | ||
41 | -end |
@@ -0,0 +1,22 @@ | @@ -0,0 +1,22 @@ | ||
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 |
@@ -0,0 +1,43 @@ | @@ -0,0 +1,43 @@ | ||
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/fingerprints/legacy_fingerprint.rb
@@ -1,20 +0,0 @@ | @@ -1,20 +0,0 @@ | ||
1 | -require 'digest/md5' | ||
2 | - | ||
3 | -class LegacyFingerprint < Fingerprint | ||
4 | - def to_s | ||
5 | - Digest::MD5.hexdigest(fingerprint_source) | ||
6 | - end | ||
7 | - | ||
8 | - def fingerprint_source | ||
9 | - location['method'] &&= sanitized_method_signature | ||
10 | - end | ||
11 | - | ||
12 | - private | ||
13 | - def sanitized_method_signature | ||
14 | - location['method'].gsub(/[0-9]+|FRAGMENT/, '#').gsub(/_+#/, '_#') | ||
15 | - end | ||
16 | - | ||
17 | - def location | ||
18 | - notice.backtrace.lines.first | ||
19 | - end | ||
20 | -end |
spec/models/error_report_spec.rb
@@ -49,7 +49,7 @@ describe ErrorReport do | @@ -49,7 +49,7 @@ describe ErrorReport do | ||
49 | 49 | ||
50 | describe "#fingerprint_strategy" do | 50 | describe "#fingerprint_strategy" do |
51 | after(:all) { | 51 | after(:all) { |
52 | - error_report.fingerprint_strategy = Fingerprint | 52 | + error_report.fingerprint_strategy = Fingerprint::Sha1 |
53 | } | 53 | } |
54 | 54 | ||
55 | it "should be possible to change how fingerprints are generated" do | 55 | it "should be possible to change how fingerprints are generated" do |
@@ -0,0 +1,43 @@ | @@ -0,0 +1,43 @@ | ||
1 | +require 'spec_helper' | ||
2 | + | ||
3 | +describe Fingerprint::MD5 do | ||
4 | + context 'being created' do | ||
5 | + let(:backtrace) do | ||
6 | + Backtrace.create(:raw => [ | ||
7 | + { | ||
8 | + "number"=>"17", | ||
9 | + "file"=>"[GEM_ROOT]/gems/activesupport/lib/active_support/callbacks.rb", | ||
10 | + "method"=>"_run__2497084960985961383__process_action__2062871603614456254__callbacks" | ||
11 | + } | ||
12 | + ]) | ||
13 | + end | ||
14 | + let(:notice1) { Fabricate.build(:notice, :backtrace => backtrace) } | ||
15 | + let(:notice2) { Fabricate.build(:notice, :backtrace => backtrace_2) } | ||
16 | + | ||
17 | + context "with same backtrace" do | ||
18 | + let(:backtrace_2) do | ||
19 | + backtrace | ||
20 | + backtrace.lines.last.method = '_run__FRAGMENT__process_action__FRAGMENT__callbacks' | ||
21 | + backtrace.save | ||
22 | + backtrace | ||
23 | + end | ||
24 | + | ||
25 | + it "normalizes the fingerprint of generated methods" do | ||
26 | + expect(Fingerprint::MD5.generate(notice1, "api key")).to eql Fingerprint::MD5.generate(notice2, "api key") | ||
27 | + end | ||
28 | + end | ||
29 | + | ||
30 | + context "with same backtrace where FRAGMENT has not been extracted" do | ||
31 | + let(:backtrace_2) do | ||
32 | + backtrace | ||
33 | + backtrace.lines.last.method = '_run__998857585768765__process_action__1231231312321313__callbacks' | ||
34 | + backtrace.save | ||
35 | + backtrace | ||
36 | + end | ||
37 | + | ||
38 | + it "normalizes the fingerprint of generated methods" do | ||
39 | + expect(Fingerprint::MD5.generate(notice1, "api key")).to eql Fingerprint::MD5.generate(notice2, "api key") | ||
40 | + end | ||
41 | + end | ||
42 | + end | ||
43 | +end |
@@ -0,0 +1,87 @@ | @@ -0,0 +1,87 @@ | ||
1 | +require 'spec_helper' | ||
2 | + | ||
3 | +describe Fingerprint::Sha1 do | ||
4 | + | ||
5 | + context '#generate' do | ||
6 | + let(:backtrace) { | ||
7 | + Backtrace.create(:raw => [ | ||
8 | + {"number"=>"425", "file"=>"[GEM_ROOT]/gems/activesupport-3.0.0.rc/lib/active_support/callbacks.rb", "method"=>"_run__2115867319__process_action__262109504__callbacks"}, | ||
9 | + {"number"=>"404", "file"=>"[GEM_ROOT]/gems/activesupport-3.0.0.rc/lib/active_support/callbacks.rb", "method"=>"send"}, | ||
10 | + {"number"=>"404", "file"=>"[GEM_ROOT]/gems/activesupport-3.0.0.rc/lib/active_support/callbacks.rb", "method"=>"_run_process_action_callbacks"} | ||
11 | + ]) | ||
12 | + } | ||
13 | + let(:notice1) { Fabricate.build(:notice, :backtrace => backtrace) } | ||
14 | + let(:notice2) { Fabricate.build(:notice, :backtrace => backtrace_2) } | ||
15 | + | ||
16 | + context "with same backtrace" do | ||
17 | + let(:backtrace_2) { backtrace } | ||
18 | + it 'should create the same fingerprint for two notices' do | ||
19 | + expect(Fingerprint::Sha1.generate(notice1, "api key")).to eq Fingerprint::Sha1.generate(notice2, "api key") | ||
20 | + end | ||
21 | + end | ||
22 | + | ||
23 | + context "with different backtrace with only last line change" do | ||
24 | + let(:backtrace_2) { | ||
25 | + backtrace | ||
26 | + backtrace.lines.last.number = 401 | ||
27 | + backtrace.send(:generate_fingerprint) | ||
28 | + backtrace.save | ||
29 | + backtrace | ||
30 | + } | ||
31 | + it 'should not same fingerprint' do | ||
32 | + expect( | ||
33 | + Fingerprint::Sha1.generate(notice1, "api key") | ||
34 | + ).not_to eql Fingerprint::Sha1.generate(notice2, "api key") | ||
35 | + end | ||
36 | + end | ||
37 | + | ||
38 | + context 'with messages differing in object string memory addresses' do | ||
39 | + let(:backtrace_2) { backtrace } | ||
40 | + | ||
41 | + before do | ||
42 | + notice1.message = "NoMethodError: undefined method `foo' for #<ActiveSupport::HashWithIndifferentAccess:0x007f6bfe3287e8>" | ||
43 | + notice2.message = "NoMethodError: undefined method `foo' for #<ActiveSupport::HashWithIndifferentAccess:0x007f6bfd9f5338>" | ||
44 | + end | ||
45 | + | ||
46 | + its 'fingerprints should be equal' do | ||
47 | + expect(Fingerprint::Sha1.generate(notice1, 'api key')).to eq Fingerprint::Sha1.generate(notice2, 'api key') | ||
48 | + end | ||
49 | + end | ||
50 | + | ||
51 | + context 'with different messages at same stacktrace' do | ||
52 | + let(:backtrace_2) { backtrace } | ||
53 | + | ||
54 | + before do | ||
55 | + notice1.message = "NoMethodError: undefined method `bar' for #<ActiveSupport::HashWithIndifferentAccess:0x007f6bfe3287e8>" | ||
56 | + notice2.message = "NoMethodError: undefined method `bar' for nil:NilClass" | ||
57 | + end | ||
58 | + | ||
59 | + its 'fingerprints should not be equal' do | ||
60 | + expect(Fingerprint::Sha1.generate(notice1, 'api key')).to_not eq Fingerprint::Sha1.generate(notice2, 'api key') | ||
61 | + end | ||
62 | + end | ||
63 | + end | ||
64 | + | ||
65 | + describe '#unified_message' do | ||
66 | + subject{ Fingerprint::Sha1.new(double('notice', message: message), 'api key').unified_message } | ||
67 | + | ||
68 | + context "full error message" do | ||
69 | + let(:message) { "NoMethodError: undefined method `foo' for #<ActiveSupport::HashWithIndifferentAccess:0x007f6bfe3287e8>" } | ||
70 | + | ||
71 | + it 'removes memory address from object strings' do | ||
72 | + should eq "NoMethodError: undefined method `foo' for #<ActiveSupport::HashWithIndifferentAccess>" | ||
73 | + end | ||
74 | + end | ||
75 | + | ||
76 | + context "multiple object strings in message" do | ||
77 | + let(:message) { "#<ActiveSupport::HashWithIndifferentAccess:0x007f6bfe3287e8> #<Object:0x007fa2b33d9458>" } | ||
78 | + | ||
79 | + it 'removes memory addresses globally' do | ||
80 | + should eq "#<ActiveSupport::HashWithIndifferentAccess> #<Object>" | ||
81 | + end | ||
82 | + end | ||
83 | + | ||
84 | + end | ||
85 | + | ||
86 | +end | ||
87 | + |
spec/models/fingerprint_spec.rb
@@ -1,87 +0,0 @@ | @@ -1,87 +0,0 @@ | ||
1 | -require 'spec_helper' | ||
2 | - | ||
3 | -describe Fingerprint do | ||
4 | - | ||
5 | - context '#generate' do | ||
6 | - let(:backtrace) { | ||
7 | - Backtrace.create(:raw => [ | ||
8 | - {"number"=>"425", "file"=>"[GEM_ROOT]/gems/activesupport-3.0.0.rc/lib/active_support/callbacks.rb", "method"=>"_run__2115867319__process_action__262109504__callbacks"}, | ||
9 | - {"number"=>"404", "file"=>"[GEM_ROOT]/gems/activesupport-3.0.0.rc/lib/active_support/callbacks.rb", "method"=>"send"}, | ||
10 | - {"number"=>"404", "file"=>"[GEM_ROOT]/gems/activesupport-3.0.0.rc/lib/active_support/callbacks.rb", "method"=>"_run_process_action_callbacks"} | ||
11 | - ]) | ||
12 | - } | ||
13 | - let(:notice1) { Fabricate.build(:notice, :backtrace => backtrace) } | ||
14 | - let(:notice2) { Fabricate.build(:notice, :backtrace => backtrace_2) } | ||
15 | - | ||
16 | - context "with same backtrace" do | ||
17 | - let(:backtrace_2) { backtrace } | ||
18 | - it 'should create the same fingerprint for two notices' do | ||
19 | - expect(Fingerprint.generate(notice1, "api key")).to eq Fingerprint.generate(notice2, "api key") | ||
20 | - end | ||
21 | - end | ||
22 | - | ||
23 | - context "with different backtrace with only last line change" do | ||
24 | - let(:backtrace_2) { | ||
25 | - backtrace | ||
26 | - backtrace.lines.last.number = 401 | ||
27 | - backtrace.send(:generate_fingerprint) | ||
28 | - backtrace.save | ||
29 | - backtrace | ||
30 | - } | ||
31 | - it 'should not same fingerprint' do | ||
32 | - expect( | ||
33 | - Fingerprint.generate(notice1, "api key") | ||
34 | - ).not_to eql Fingerprint.generate(notice2, "api key") | ||
35 | - end | ||
36 | - end | ||
37 | - | ||
38 | - context 'with messages differing in object string memory addresses' do | ||
39 | - let(:backtrace_2) { backtrace } | ||
40 | - | ||
41 | - before do | ||
42 | - notice1.message = "NoMethodError: undefined method `foo' for #<ActiveSupport::HashWithIndifferentAccess:0x007f6bfe3287e8>" | ||
43 | - notice2.message = "NoMethodError: undefined method `foo' for #<ActiveSupport::HashWithIndifferentAccess:0x007f6bfd9f5338>" | ||
44 | - end | ||
45 | - | ||
46 | - its 'fingerprints should be equal' do | ||
47 | - expect(Fingerprint.generate(notice1, 'api key')).to eq Fingerprint.generate(notice2, 'api key') | ||
48 | - end | ||
49 | - end | ||
50 | - | ||
51 | - context 'with different messages at same stacktrace' do | ||
52 | - let(:backtrace_2) { backtrace } | ||
53 | - | ||
54 | - before do | ||
55 | - notice1.message = "NoMethodError: undefined method `bar' for #<ActiveSupport::HashWithIndifferentAccess:0x007f6bfe3287e8>" | ||
56 | - notice2.message = "NoMethodError: undefined method `bar' for nil:NilClass" | ||
57 | - end | ||
58 | - | ||
59 | - its 'fingerprints should not be equal' do | ||
60 | - expect(Fingerprint.generate(notice1, 'api key')).to_not eq Fingerprint.generate(notice2, 'api key') | ||
61 | - end | ||
62 | - end | ||
63 | - end | ||
64 | - | ||
65 | - describe '#unified_message' do | ||
66 | - subject{ Fingerprint.new(double('notice', message: message), 'api key').unified_message } | ||
67 | - | ||
68 | - context "full error message" do | ||
69 | - let(:message) { "NoMethodError: undefined method `foo' for #<ActiveSupport::HashWithIndifferentAccess:0x007f6bfe3287e8>" } | ||
70 | - | ||
71 | - it 'removes memory address from object strings' do | ||
72 | - should eq "NoMethodError: undefined method `foo' for #<ActiveSupport::HashWithIndifferentAccess>" | ||
73 | - end | ||
74 | - end | ||
75 | - | ||
76 | - context "multiple object strings in message" do | ||
77 | - let(:message) { "#<ActiveSupport::HashWithIndifferentAccess:0x007f6bfe3287e8> #<Object:0x007fa2b33d9458>" } | ||
78 | - | ||
79 | - it 'removes memory addresses globally' do | ||
80 | - should eq "#<ActiveSupport::HashWithIndifferentAccess> #<Object>" | ||
81 | - end | ||
82 | - end | ||
83 | - | ||
84 | - end | ||
85 | - | ||
86 | -end | ||
87 | - |
spec/models/fingerprints/legacy_fingerprint_spec.rb
@@ -1,43 +0,0 @@ | @@ -1,43 +0,0 @@ | ||
1 | -require 'spec_helper' | ||
2 | - | ||
3 | -describe LegacyFingerprint do | ||
4 | - context 'being created' do | ||
5 | - let(:backtrace) do | ||
6 | - Backtrace.create(:raw => [ | ||
7 | - { | ||
8 | - "number"=>"17", | ||
9 | - "file"=>"[GEM_ROOT]/gems/activesupport/lib/active_support/callbacks.rb", | ||
10 | - "method"=>"_run__2497084960985961383__process_action__2062871603614456254__callbacks" | ||
11 | - } | ||
12 | - ]) | ||
13 | - end | ||
14 | - let(:notice1) { Fabricate.build(:notice, :backtrace => backtrace) } | ||
15 | - let(:notice2) { Fabricate.build(:notice, :backtrace => backtrace_2) } | ||
16 | - | ||
17 | - context "with same backtrace" do | ||
18 | - let(:backtrace_2) do | ||
19 | - backtrace | ||
20 | - backtrace.lines.last.method = '_run__FRAGMENT__process_action__FRAGMENT__callbacks' | ||
21 | - backtrace.save | ||
22 | - backtrace | ||
23 | - end | ||
24 | - | ||
25 | - it "normalizes the fingerprint of generated methods" do | ||
26 | - expect(LegacyFingerprint.generate(notice1, "api key")).to eql LegacyFingerprint.generate(notice2, "api key") | ||
27 | - end | ||
28 | - end | ||
29 | - | ||
30 | - context "with same backtrace where FRAGMENT has not been extracted" do | ||
31 | - let(:backtrace_2) do | ||
32 | - backtrace | ||
33 | - backtrace.lines.last.method = '_run__998857585768765__process_action__1231231312321313__callbacks' | ||
34 | - backtrace.save | ||
35 | - backtrace | ||
36 | - end | ||
37 | - | ||
38 | - it "normalizes the fingerprint of generated methods" do | ||
39 | - expect(LegacyFingerprint.generate(notice1, "api key")).to eql LegacyFingerprint.generate(notice2, "api key") | ||
40 | - end | ||
41 | - end | ||
42 | - end | ||
43 | -end |