feat(grouping): Add better support for native fingerprinting/grouping use-cases#102653
feat(grouping): Add better support for native fingerprinting/grouping use-cases#102653mujacica wants to merge 13 commits into
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #102653 +/- ##
=========================================
Coverage 80.41% 80.42%
=========================================
Files 9393 9394 +1
Lines 403390 403628 +238
Branches 25923 25923
=========================================
+ Hits 324400 324613 +213
- Misses 78550 78575 +25
Partials 440 440 |
7d884a1 to
e4cdb9c
Compare
lobsterkatie
left a comment
There was a problem hiding this comment.
This looks good!
My only question is if the tests would be better as snapshot tests, because then we'd be able to do versioned tests, the same way we do with enhancement rules. It would also allow you to get rid of some of the boilerplate in these tests. WDYT?
e4cdb9c to
fe55639
Compare
|
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you add the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
fe55639 to
91a01bb
Compare
There was a problem hiding this comment.
Bug: Thread matcher JSON round-trip fails on deserialization
The thread matchers cannot be properly deserialized from JSON after serialization. The _to_config_structure method serializes using the internal key (e.g., "thread_name"), but _from_config_structure passes this key directly to FingerprintMatcher.__init__, which expects the external key (e.g., "thread.name") to look up in MATCHERS. Since "thread_name" is not a key in MATCHERS (only a value), deserialization fails with InvalidFingerprintingConfig. This affects all new thread matchers (thread.id, thread.name, thread.state, thread.crashed, thread.current) and breaks any workflow that saves fingerprinting configs to JSON and loads them back, like to_json() followed by from_json().
src/sentry/grouping/fingerprinting/matchers.py#L140-L174
sentry/src/sentry/grouping/fingerprinting/matchers.py
Lines 140 to 174 in 91a01bb
|
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you add the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
7f87790 to
347c377
Compare
|
Hey @lobsterkatie - any more info/insights for this one - I would like to slowly start integrating this. You mentioned snapshot testing as a potential improvement - I tried it out but without a lot of success. Is that blocking for the merge? |
347c377 to
6c35595
Compare
This reverts commit 715ecf7.
6c35595 to
4606685
Compare
Added support for fingerprinting based on the thread info
thread.name:MainThread type:RuntimeError -> main-thread-errorthread.crashed:true -> crashed-threadAdded support sibling matching for fingerprinting
[ function:handler ] | function:process -> handler-processAdded support for thread info in client-side fingerprinting
Sentry.setFingerprint(["{{ thread.name }}", "{{ transaction }}"]);sentry_sdk.set_fingerprint(["{{ thread.name }}", "database-error"])