Skip to content

Commit 27429d5

Browse files
authored
ref(grouping): Make GroupingComponent generic and add subclasses (#80722)
This adds a full taxonomy of `BaseGroupingCompontent` subclasses, partially just as a form of documentation (we create this deeply nested `variants` structure when grouping, and it's helpful to understand what kind of data is in each part), and partially so that it's easier to write helpers dealing with different kinds of components and not have to constantly be checking for the presence of this or that attribute. Key changes: - Make `BaseGroupingComponent` generic, so that subclasses can specify the types allowed in `values`. - Allow `id` to be a class attribute (no longer require it to be passed to `__init__`), so that it can be specified in each subclass. - Add overloads of `get_single_grouping_component`. - Use a `TypeVar` to make `ReturnedVariants` covaraint. This is a bit of a hack, but fixing it will require a larger change than felt reasonable here. (Basically, we'll have to make everywhere it's used treat it as read-only.) I left a TODO.
1 parent 855e9ab commit 27429d5

File tree

8 files changed

+299
-129
lines changed

8 files changed

+299
-129
lines changed

src/sentry/grouping/component.py

Lines changed: 149 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
from __future__ import annotations
22

33
from collections.abc import Generator, Iterator, Sequence
4-
from typing import Any
4+
from typing import Any, Self
55

66
from sentry.grouping.utils import hash_from_values
77

@@ -23,32 +23,32 @@
2323
}
2424

2525

26-
def _calculate_contributes(values: Sequence[str | int | BaseGroupingComponent]) -> bool:
26+
def _calculate_contributes[ValuesType](values: Sequence[ValuesType]) -> bool:
2727
for value in values or ():
2828
if not isinstance(value, BaseGroupingComponent) or value.contributes:
2929
return True
3030
return False
3131

3232

33-
class BaseGroupingComponent:
33+
class BaseGroupingComponent[ValuesType: str | int | BaseGroupingComponent[Any]]:
3434
"""A grouping component is a recursive structure that is flattened
3535
into components to make a hash for grouping purposes.
3636
"""
3737

3838
id: str = "default"
3939
hint: str | None = None
4040
contributes: bool = False
41-
values: Sequence[str | int | BaseGroupingComponent]
41+
values: Sequence[ValuesType]
4242

4343
def __init__(
4444
self,
45-
id: str,
45+
id: str | None = None,
4646
hint: str | None = None,
4747
contributes: bool | None = None,
48-
values: Sequence[str | int | BaseGroupingComponent] | None = None,
48+
values: Sequence[ValuesType] | None = None,
4949
variant_provider: bool = False,
5050
):
51-
self.id = id
51+
self.id = id or self.id
5252
self.variant_provider = variant_provider
5353

5454
self.update(
@@ -73,7 +73,7 @@ def description(self) -> str:
7373
paths = []
7474

7575
def _walk_components(
76-
component: BaseGroupingComponent, current_path: list[str | None]
76+
component: BaseGroupingComponent[Any], current_path: list[str | None]
7777
) -> None:
7878
# Keep track of the names of the nodes from the root of the component tree to here
7979
current_path.append(component.name)
@@ -101,13 +101,13 @@ def _walk_components(
101101

102102
def get_subcomponent(
103103
self, id: str, only_contributing: bool = False
104-
) -> str | int | BaseGroupingComponent | None:
104+
) -> str | int | BaseGroupingComponent[Any] | None:
105105
"""Looks up a subcomponent by the id and returns the first or `None`."""
106106
return next(self.iter_subcomponents(id=id, only_contributing=only_contributing), None)
107107

108108
def iter_subcomponents(
109109
self, id: str, recursive: bool = False, only_contributing: bool = False
110-
) -> Iterator[str | int | BaseGroupingComponent | None]:
110+
) -> Iterator[str | int | BaseGroupingComponent[Any] | None]:
111111
"""Finds all subcomponents matching an id, optionally recursively."""
112112
for value in self.values:
113113
if isinstance(value, BaseGroupingComponent):
@@ -125,7 +125,7 @@ def update(
125125
self,
126126
hint: str | None = None,
127127
contributes: bool | None = None,
128-
values: Sequence[str | int | BaseGroupingComponent] | None = None,
128+
values: Sequence[ValuesType] | None = None,
129129
) -> None:
130130
"""Updates an already existing component with new values."""
131131
if hint is not None:
@@ -137,14 +137,14 @@ def update(
137137
if contributes is not None:
138138
self.contributes = contributes
139139

140-
def shallow_copy(self) -> BaseGroupingComponent:
140+
def shallow_copy(self) -> Self:
141141
"""Creates a shallow copy."""
142142
rv = object.__new__(self.__class__)
143143
rv.__dict__.update(self.__dict__)
144144
rv.values = list(self.values)
145145
return rv
146146

147-
def iter_values(self) -> Generator[str | int | BaseGroupingComponent]:
147+
def iter_values(self) -> Generator[str | int | BaseGroupingComponent[Any]]:
148148
"""Recursively walks the component and flattens it into a list of
149149
values.
150150
"""
@@ -183,3 +183,139 @@ def as_dict(self) -> dict[str, Any]:
183183

184184
def __repr__(self) -> str:
185185
return f"{self.__class__.__name__}({self.id!r}, hint={self.hint!r}, contributes={self.contributes!r}, values={self.values!r})"
186+
187+
188+
# NOTE: In all of the classes below, the type(s) passed to `BaseGroupingComponent` represent
189+
# the type(s) which can appear in the `values` attribute
190+
191+
192+
# Error-related inner components
193+
194+
195+
class ContextLineGroupingComponent(BaseGroupingComponent[str]):
196+
id: str = "context-line"
197+
198+
199+
class ErrorTypeGroupingComponent(BaseGroupingComponent[str]):
200+
id: str = "type"
201+
202+
203+
class ErrorValueGroupingComponent(BaseGroupingComponent[str]):
204+
id: str = "value"
205+
206+
207+
class FilenameGroupingComponent(BaseGroupingComponent[str]):
208+
id: str = "filename"
209+
210+
211+
class FunctionGroupingComponent(BaseGroupingComponent[str]):
212+
id: str = "function"
213+
214+
215+
class LineNumberGroupingComponent(BaseGroupingComponent[str]):
216+
id: str = "lineno"
217+
218+
219+
class ModuleGroupingComponent(BaseGroupingComponent[str]):
220+
id: str = "module"
221+
222+
223+
class NSErrorGroupingComponent(BaseGroupingComponent[str | int]):
224+
id: str = "ns-error"
225+
226+
227+
class SymbolGroupingComponent(BaseGroupingComponent[str]):
228+
id: str = "symbol"
229+
230+
231+
class FrameGroupingComponent(
232+
BaseGroupingComponent[
233+
ContextLineGroupingComponent
234+
| FilenameGroupingComponent
235+
| FunctionGroupingComponent
236+
| LineNumberGroupingComponent # only in legacy config
237+
| ModuleGroupingComponent
238+
| SymbolGroupingComponent # only in legacy config
239+
]
240+
):
241+
id: str = "frame"
242+
243+
244+
# Security-related inner components
245+
246+
247+
class HostnameGroupingComponent(BaseGroupingComponent[str]):
248+
id: str = "hostname"
249+
250+
251+
class SaltGroupingComponent(BaseGroupingComponent[str]):
252+
id: str = "salt"
253+
hint: str = "a static salt"
254+
255+
256+
class ViolationGroupingComponent(BaseGroupingComponent[str]):
257+
id: str = "violation"
258+
259+
260+
class URIGroupingComponent(BaseGroupingComponent[str]):
261+
id: str = "uri"
262+
263+
264+
# Top-level components
265+
266+
267+
class MessageGroupingComponent(BaseGroupingComponent[str]):
268+
id: str = "message"
269+
270+
271+
class StacktraceGroupingComponent(BaseGroupingComponent[FrameGroupingComponent]):
272+
id: str = "stacktrace"
273+
274+
275+
class ExceptionGroupingComponent(
276+
BaseGroupingComponent[
277+
ErrorTypeGroupingComponent
278+
| ErrorValueGroupingComponent
279+
| NSErrorGroupingComponent
280+
| StacktraceGroupingComponent
281+
]
282+
):
283+
id: str = "exception"
284+
285+
286+
class ChainedExceptionGroupingComponent(BaseGroupingComponent[ExceptionGroupingComponent]):
287+
id: str = "chained-exception"
288+
289+
290+
class ThreadsGroupingComponent(BaseGroupingComponent[StacktraceGroupingComponent]):
291+
id: str = "threads"
292+
293+
294+
class CSPGroupingComponent(
295+
BaseGroupingComponent[SaltGroupingComponent | ViolationGroupingComponent | URIGroupingComponent]
296+
):
297+
id: str = "csp"
298+
299+
300+
class ExpectCTGroupingComponent(
301+
BaseGroupingComponent[HostnameGroupingComponent | SaltGroupingComponent]
302+
):
303+
id: str = "expect-ct"
304+
305+
306+
class ExpectStapleGroupingComponent(
307+
BaseGroupingComponent[HostnameGroupingComponent | SaltGroupingComponent]
308+
):
309+
id: str = "expect-staple"
310+
311+
312+
class HPKPGroupingComponent(
313+
BaseGroupingComponent[HostnameGroupingComponent | SaltGroupingComponent]
314+
):
315+
id: str = "hpkp"
316+
317+
318+
class TemplateGroupingComponent(
319+
BaseGroupingComponent[ContextLineGroupingComponent | FilenameGroupingComponent]
320+
):
321+
id: str = "template"

src/sentry/grouping/enhancer/__init__.py

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
from sentry_ophio.enhancers import Enhancements as RustEnhancements
1616

1717
from sentry import projectoptions
18-
from sentry.grouping.component import BaseGroupingComponent
18+
from sentry.grouping.component import FrameGroupingComponent, StacktraceGroupingComponent
1919
from sentry.stacktraces.functions import set_in_app
2020
from sentry.utils.safe import get_path, set_path
2121

@@ -164,11 +164,11 @@ def apply_modifications_to_frame(
164164

165165
def assemble_stacktrace_component(
166166
self,
167-
components: list[BaseGroupingComponent],
167+
components: list[FrameGroupingComponent],
168168
frames: list[dict[str, Any]],
169169
platform: str | None,
170170
exception_data: dict[str, Any] | None = None,
171-
) -> tuple[BaseGroupingComponent, bool]:
171+
) -> tuple[StacktraceGroupingComponent, bool]:
172172
"""
173173
This assembles a `stacktrace` grouping component out of the given
174174
`frame` components and source frames.
@@ -186,8 +186,7 @@ def assemble_stacktrace_component(
186186
for py_component, rust_component in zip(components, rust_components):
187187
py_component.update(contributes=rust_component.contributes, hint=rust_component.hint)
188188

189-
component = BaseGroupingComponent(
190-
id="stacktrace",
189+
component = StacktraceGroupingComponent(
191190
values=components,
192191
hint=rust_results.hint,
193192
contributes=rust_results.contributes,

src/sentry/grouping/strategies/base.py

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,19 @@
11
import inspect
22
from collections.abc import Callable, Iterator, Sequence
3-
from typing import Any, Generic, Protocol, TypeVar
3+
from typing import Any, Generic, Protocol, TypeVar, overload
44

55
from sentry import projectoptions
66
from sentry.eventstore.models import Event
7-
from sentry.grouping.component import BaseGroupingComponent
7+
from sentry.grouping.component import (
8+
BaseGroupingComponent,
9+
ExceptionGroupingComponent,
10+
FrameGroupingComponent,
11+
StacktraceGroupingComponent,
12+
)
813
from sentry.grouping.enhancer import Enhancements
914
from sentry.interfaces.base import Interface
15+
from sentry.interfaces.exception import SingleException
16+
from sentry.interfaces.stacktrace import Frame, Stacktrace
1017

1118
STRATEGIES: dict[str, "Strategy[Any]"] = {}
1219

@@ -24,7 +31,10 @@
2431
DEFAULT_GROUPING_ENHANCEMENTS_BASE = "common:2019-03-23"
2532
DEFAULT_GROUPING_FINGERPRINTING_BASES: list[str] = []
2633

27-
ReturnedVariants = dict[str, BaseGroupingComponent]
34+
# TODO: Hack to make `ReturnedVariants` (no pun intended) covariant. At some point we should
35+
# probably turn `ReturnedVariants` into a Mapping (immutable), since in practice it's read-only.
36+
GroupingComponent = TypeVar("GroupingComponent", bound=BaseGroupingComponent[Any])
37+
ReturnedVariants = dict[str, GroupingComponent]
2838
ConcreteInterface = TypeVar("ConcreteInterface", bound=Interface, contravariant=True)
2939

3040

@@ -115,6 +125,21 @@ def get_grouping_component(
115125
"""
116126
return self._get_strategy_dict(interface, event=event, **kwargs)
117127

128+
@overload
129+
def get_single_grouping_component(
130+
self, interface: Frame, *, event: Event, **kwargs: Any
131+
) -> FrameGroupingComponent: ...
132+
133+
@overload
134+
def get_single_grouping_component(
135+
self, interface: SingleException, *, event: Event, **kwargs: Any
136+
) -> ExceptionGroupingComponent: ...
137+
138+
@overload
139+
def get_single_grouping_component(
140+
self, interface: Stacktrace, *, event: Event, **kwargs: Any
141+
) -> StacktraceGroupingComponent: ...
142+
118143
def get_single_grouping_component(
119144
self, interface: Interface, *, event: Event, **kwargs: Any
120145
) -> BaseGroupingComponent:

0 commit comments

Comments
 (0)