Skip to content

ref(grouping): Change exception subcomponent order in grouping info #102281

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 29, 2025
Merged

ref(grouping): Change exception subcomponent order in grouping info #102281

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2e151a8
switch order of elements in grouping info
lobsterkatie Oct 28, 2025
c5dbfb2
fix tests
lobsterkatie Oct 28, 2025
959dffe
update snapshots
lobsterkatie Oct 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions src/sentry/grouping/component.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,22 @@ def __init__(
def key(self) -> str:
return _get_exception_component_key(self)

def as_dict(self) -> dict[str, Any]:
"""
Convert to a dictionary, first rearranging the values so they show up in the order we want
in grouping info.
"""
ordered_values: Any = []

for component_id in ["type", "value", "ns_error", "stacktrace"]:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I am concerned about the hardcoded list of values not being future-proofed... but recognize that this is a pretty small class definition and we'd probably notice. Still, I think my other suggestion will address this.

subcomponent = self.get_subcomponent(component_id)
if subcomponent:
ordered_values.append(subcomponent)

self.values = ordered_values
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You know more about grouping than I do — and I'll defer to your expertise — but I worry that we might be dropping some values here. BaseGroupingComponent seems to explicitly support having multiple subcomponents of the same ID (via iter_subcomponents) and this would only preserve the first one.

Instead could we just do:

self.values.sort(
  key=lambda subcomponent: {"type": 0, "value": 1, "ns_error": 2, "stacktrace": 3}.get(subcomponent, 999)
)

This would order everything in-place, preserve items with duplicate IDs, and not drop things with unexpected IDs.

Copy link
Member Author

@lobsterkatie lobsterkatie Oct 29, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In an abstract sense you're right, but it's not an issue here because exception components can only ever have one copy of any given subcomponent type. (This mirrors the fact that a given error only ever is of one type, only ever has a single error message, etc. Yes, there can be chained exceptions, but each exception in the chain still only has a single type/value/stacktrace/etc.)

This is also only a temporary work-around, to get us from the universe where we do values = [stacktrace_component, type_component, ns_error_component, value_component] when creating the exception component to the universe where we do values = [type_component, value_component, ns_error_component, stacktrace_component] without it being noticeable to the user. (The change here will be visible, of course, but this will get us to the final state visually even before we've finished transitioning under the hood.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coolio, I'll approve — there is a plan to change the original values order (and then we can remove this method)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, that's what these two parts in the description were trying to say:

the new grouping config, in which the order of the subcomponents in ExceptionGroupingComponent will be changed

and

Once we're fully switched over to the new config, we can delete this shim.


return super().as_dict()


class ChainedExceptionGroupingComponent(BaseGroupingComponent[ExceptionGroupingComponent]):
id: str = "chained_exception"
Expand Down
74 changes: 37 additions & 37 deletions ...ntry/grouping/snapshots/grouping_info/test_grouping_info/newstyle@2023_01_11/actix.pysnap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
---
created: '2025-10-06T21:57:32.649693+00:00'
created: '2025-10-27T23:46:00.557065+00:00'
creator: sentry
source: tests/sentry/grouping/test_grouping_info.py
---
Expand All @@ -17,6 +17,24 @@ source: tests/sentry/grouping/test_grouping_info.py
"id": "exception",
"name": "exception",
"values": [
{
"contributes": true,
"hint": null,
"id": "type",
"name": null,
"values": [
"actix_web::pipeline"
]
},
{
"contributes": false,
"hint": "ignored because stacktrace takes precedence",
"id": "value",
"name": null,
"values": [
"Error occurred during request handling, status: <int> Internal Server Error Something went really wrong here"
]
},
{
"contributes": true,
"hint": null,
Expand Down Expand Up @@ -2561,24 +2579,6 @@ source: tests/sentry/grouping/test_grouping_info.py
]
}
]
},
{
"contributes": true,
"hint": null,
"id": "type",
"name": null,
"values": [
"actix_web::pipeline"
]
},
{
"contributes": false,
"hint": "ignored because stacktrace takes precedence",
"id": "value",
"name": null,
"values": [
"Error occurred during request handling, status: <int> Internal Server Error Something went really wrong here"
]
}
]
}
Expand Down Expand Up @@ -2624,6 +2624,24 @@ source: tests/sentry/grouping/test_grouping_info.py
"id": "exception",
"name": "exception",
"values": [
{
"contributes": true,
"hint": null,
"id": "type",
"name": null,
"values": [
"actix_web::pipeline"
]
},
{
"contributes": false,
"hint": "ignored because stacktrace takes precedence",
"id": "value",
"name": null,
"values": [
"Error occurred during request handling, status: <int> Internal Server Error Something went really wrong here"
]
},
{
"contributes": true,
"hint": null,
Expand Down Expand Up @@ -5168,24 +5186,6 @@ source: tests/sentry/grouping/test_grouping_info.py
]
}
]
},
{
"contributes": true,
"hint": null,
"id": "type",
"name": null,
"values": [
"actix_web::pipeline"
]
},
{
"contributes": false,
"hint": "ignored because stacktrace takes precedence",
"id": "value",
"name": null,
"values": [
"Error occurred during request handling, status: <int> Internal Server Error Something went really wrong here"
]
}
]
}
Expand Down
74 changes: 37 additions & 37 deletions ...rouping/snapshots/grouping_info/test_grouping_info/newstyle@2023_01_11/android_anr.pysnap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
---
created: '2025-10-07T22:42:53.838129+00:00'
created: '2025-10-27T23:46:00.583927+00:00'
creator: sentry
source: tests/sentry/grouping/test_grouping_info.py
---
Expand All @@ -17,6 +17,24 @@ source: tests/sentry/grouping/test_grouping_info.py
"id": "exception",
"name": "exception",
"values": [
{
"contributes": true,
"hint": null,
"id": "type",
"name": null,
"values": [
"ApplicationNotResponding"
]
},
{
"contributes": true,
"hint": "stripped event-specific values",
"id": "value",
"name": null,
"values": [
"Application Not Responding for at least <int> ms."
]
},
{
"contributes": false,
"hint": "ignored because it contains no in-app frames",
Expand Down Expand Up @@ -3804,24 +3822,6 @@ source: tests/sentry/grouping/test_grouping_info.py
]
}
]
},
{
"contributes": true,
"hint": null,
"id": "type",
"name": null,
"values": [
"ApplicationNotResponding"
]
},
{
"contributes": true,
"hint": "stripped event-specific values",
"id": "value",
"name": null,
"values": [
"Application Not Responding for at least <int> ms."
]
}
]
},
Expand Down Expand Up @@ -3953,6 +3953,24 @@ source: tests/sentry/grouping/test_grouping_info.py
"id": "exception",
"name": "exception",
"values": [
{
"contributes": true,
"hint": null,
"id": "type",
"name": null,
"values": [
"ApplicationNotResponding"
]
},
{
"contributes": false,
"hint": "ignored because stacktrace takes precedence",
"id": "value",
"name": null,
"values": [
"Application Not Responding for at least <int> ms."
]
},
{
"contributes": true,
"hint": null,
Expand Down Expand Up @@ -7740,24 +7758,6 @@ source: tests/sentry/grouping/test_grouping_info.py
]
}
]
},
{
"contributes": true,
"hint": null,
"id": "type",
"name": null,
"values": [
"ApplicationNotResponding"
]
},
{
"contributes": false,
"hint": "ignored because stacktrace takes precedence",
"id": "value",
"name": null,
"values": [
"Application Not Responding for at least <int> ms."
]
}
]
},
Expand Down
74 changes: 37 additions & 37 deletions ...grouping/snapshots/grouping_info/test_grouping_info/newstyle@2023_01_11/aspnetcore.pysnap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
---
created: '2025-10-06T21:57:32.694439+00:00'
created: '2025-10-27T23:46:00.603773+00:00'
creator: sentry
source: tests/sentry/grouping/test_grouping_info.py
---
Expand All @@ -17,6 +17,24 @@ source: tests/sentry/grouping/test_grouping_info.py
"id": "exception",
"name": "exception",
"values": [
{
"contributes": true,
"hint": null,
"id": "type",
"name": null,
"values": [
"System.Exception"
]
},
{
"contributes": false,
"hint": "ignored because stacktrace takes precedence",
"id": "value",
"name": null,
"values": [
"sync exception"
]
},
{
"contributes": true,
"hint": null,
Expand Down Expand Up @@ -1049,24 +1067,6 @@ source: tests/sentry/grouping/test_grouping_info.py
]
}
]
},
{
"contributes": true,
"hint": null,
"id": "type",
"name": null,
"values": [
"System.Exception"
]
},
{
"contributes": false,
"hint": "ignored because stacktrace takes precedence",
"id": "value",
"name": null,
"values": [
"sync exception"
]
}
]
}
Expand Down Expand Up @@ -1157,6 +1157,24 @@ source: tests/sentry/grouping/test_grouping_info.py
"id": "exception",
"name": "exception",
"values": [
{
"contributes": true,
"hint": null,
"id": "type",
"name": null,
"values": [
"System.Exception"
]
},
{
"contributes": false,
"hint": "ignored because stacktrace takes precedence",
"id": "value",
"name": null,
"values": [
"sync exception"
]
},
{
"contributes": true,
"hint": null,
Expand Down Expand Up @@ -2189,24 +2207,6 @@ source: tests/sentry/grouping/test_grouping_info.py
]
}
]
},
{
"contributes": true,
"hint": null,
"id": "type",
"name": null,
"values": [
"System.Exception"
]
},
{
"contributes": false,
"hint": "ignored because stacktrace takes precedence",
"id": "value",
"name": null,
"values": [
"sync exception"
]
}
]
}
Expand Down
Loading
Loading