ref(grouping): Use better descriptions in Grouping Info section #101042
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
the
Scope: Backend
shayna-ch
reviewed
Oct 6, 2025
src/sentry/grouping/grouping_info.py
Outdated
| "exception_type": "error type", | ||
| "message": "message", | ||
| "ns_error": "NSError", | ||
| "chained_exception_stacktrace": "chained exception stacktraces", |
There was a problem hiding this comment.
what is the reason for using exception in the description instead of error? It seems like the rest use 'error', like for chained_exception_messages the description is "chained error messages". Is it because its a stacktrace?
There was a problem hiding this comment.
I went back and forth on that one. "Error message" sounded more idiomatic to me (and more universal) than "exception message," which is why I went with that. For stacktraces, they can either be top-level, in a thread in event.threads, or in an exception in event.exception. Because it felt like ______ stacktrace was describing the location of the stacktrace in the first two cases, it made more sense to me to leave it as exception stacktrace rather than error stacktrace. That said, I get that's a little bit of a weird contrast. If I standardize, I'd probably go with exception message over error stacktrace.
WDYT?
There was a problem hiding this comment.
That explanation makes sense! To me standardizing still feels a little bit more understandable, at first glance the descriptions had me wondering if there was a difference between a chained error and a chained exception. But I'm also going back and forth so if you decide to keep it how it is I think that will be fine.
There was a problem hiding this comment.
Another option is something like this:
Error Message
Error Type
Stacktrace (from `exception`, in-app frames)
Stacktrace (from `exception`, all frames)
Stacktrace (from `threads`, in-app frames)
Stacktrace (from `threads`, all frames)
Stacktrace (event-level, in-app frames)
Stacktrace (event-level, all frames)
In which case maybe we could play around with the styling - make the parentheses part grey like the hint (or not), and move the hint to the next line or something. I originally had the in-app frames/all frames part in parens, but then it looked weird with the hint in parens next to it, so I switched to the m-dash.
There was a problem hiding this comment.
UPDATE: After some IRL conversation and playing around with possibilities, we decided to go with just switching error type and error message to exception type and exception message and otherwise going with what we have.
lobsterkatie
force-pushed
the
kmclb-use-better-descriptions-in-grouping-info-section
branch
from
de384c0 to
6637a09
Compare
shayna-ch
approved these changes
Oct 7, 2025
❌ 79 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
lobsterkatie
force-pushed
the
kmclb-use-better-descriptions-in-grouping-info-section
branch
from
6637a09 to
fb39bfa
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In the grouping info section of the issue details page, there are a number of problems with the way we describe grouping methods. In some cases, the description is incomplete (just
appor justsystem, rather thanapp _____orsystem _____). In some cases, it's not very descriptive (modified _____to indicate a hybrid fingerprint, when we could be a lot more specific about what "modified" means). In some cases, it's unclear (exceptionandexception stacktraceare different, but it's easy to get them confused). And in some cases there's no method listed at all, and it just shows up asdefault.The root of all of these problems is that the
descriptionproperty only looks at contributing components, and only "major" components at that. It's also directly reflective of the particular implementation of our grouping system, which works great for computation purposes, but less well for human understanding purposes.To fix these issues, this PR instead bases the description on a variant's key, which produces a different value for each possible grouping method, and which takes into account not only which components end up contributing to the final hash, but which components would contribute if each variant were looked at individually. It also provides a human-readable translation for each key rather than just concatenating component names together.
Because grouphash metadata uses the
descriptionproperty in its computations, its implementation has been left alone for now. Instead, the new description is looked up only when creating the JSON for the grouping info section.Examples of before and after heading values (not an exhaustive list):