Skip to content

ref(grouping): Simplify handling of variant name and exception data #97458

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.

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 7 commits into from
Aug 8, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 5 additions & 3 deletions src/sentry/grouping/strategies/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,6 @@ def __init__(self, strategy_config: StrategyConfiguration, event: Event):
self.config = strategy_config
self.event = event
self._push_context_layer()
self["variant_name"] = None

def __setitem__(self, key: str, value: ContextValue) -> None:
# Add the key-value pair to the context layer at the top of the stack
Expand Down Expand Up @@ -182,12 +181,15 @@ def get_single_grouping_component(
Invoke the delegate grouping strategy corresponding to the given interface, returning the
grouping component for the variant set on the context.
"""
variant_name = self["variant_name"]
assert variant_name is not None

components_by_variant = self._get_grouping_components_for_interface(
interface, event=event, **kwargs
)

assert len(components_by_variant) == 1
return components_by_variant[self["variant_name"]]
return components_by_variant[variant_name]

def _get_grouping_components_for_interface(
self, interface: Interface, *, event: Event, **kwargs: Any
Expand Down Expand Up @@ -484,7 +486,7 @@ def call_with_variants(
**kwargs: Any,
) -> ReturnedVariants:
context = kwargs["context"]
incoming_variant_name = context["variant_name"]
incoming_variant_name = context.get("variant_name")

if incoming_variant_name is not None:
# For the case where the variant is already determined, we act as a delegate strategy. To
Expand Down
5 changes: 0 additions & 5 deletions src/sentry/grouping/strategies/configurations.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,6 @@
],
delegates=["frame:v1", "stacktrace:v1", "single-exception:v1"],
initial_context={
# This key in the context tells the system which variant should
# be produced. TODO: phase this out.
Copy link
Member

Choose a reason for hiding this comment

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

ah the joy of removing a 4 year old TODO in code :)

"variant_name": None,
# This is a flag that can be used by any delegate to respond to
# a detected recursion. This is currently used by the frame
# strategy to disable itself. Recursion is detected by the outer
Expand All @@ -40,8 +37,6 @@
"normalize_message": True,
# Platforms for which context line should be taken into account when grouping.
"contextline_platforms": ("javascript", "node", "python", "php", "ruby"),
# Stacktrace is produced in the context of this exception
"exception_data": None,
},
)

Expand Down
6 changes: 4 additions & 2 deletions src/sentry/grouping/strategies/message.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,15 +72,17 @@ def normalize_message_for_grouping(message: str, event: Event) -> str:
def message_v1(
interface: Message, event: Event, context: GroupingContext, **kwargs: Any
) -> ReturnedVariants:
variant_name = context["variant_name"]
Copy link
Member

Choose a reason for hiding this comment

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

I know the code previously didnt, but should we not asserting for existence here as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

The produces_variants decorator (indirectly) sets variant_name in the context, so we know it will be there.

(To be more precise, the decorator calls call_with_variants, and call_with_variants sets variant_name immediately before calling strategy_func, which in this case is the function message_v1 that you're looking at.)


# This is true for all but our test config
if context["normalize_message"]:
raw_message = interface.message or interface.formatted or ""
normalized = normalize_message_for_grouping(raw_message, event)
hint = "stripped event-specific values" if raw_message != normalized else None
return {context["variant_name"]: MessageGroupingComponent(values=[normalized], hint=hint)}
return {variant_name: MessageGroupingComponent(values=[normalized], hint=hint)}
else:
return {
context["variant_name"]: MessageGroupingComponent(
variant_name: MessageGroupingComponent(
values=[interface.message or interface.formatted or ""],
)
}
9 changes: 6 additions & 3 deletions src/sentry/grouping/strategies/newstyle.py
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,8 @@ def frame(
) -> ReturnedVariants:
frame = interface
platform = frame.platform or event.platform
variant_name = context["variant_name"]
assert variant_name is not None

# Safari throws [native code] frames in for calls like ``forEach``
# whereas Chrome ignores these. Let's remove it from the hashing algo
Expand Down Expand Up @@ -371,7 +373,7 @@ def frame(
if context["is_recursion"]:
frame_component.update(contributes=False, hint="ignored due to recursion")

return {context["variant_name"]: frame_component}
return {variant_name: frame_component}


def get_contextline_component(
Expand Down Expand Up @@ -409,7 +411,7 @@ def get_contextline_component(
def stacktrace(
interface: Stacktrace, event: Event, context: GroupingContext, **kwargs: Any
) -> ReturnedVariants:
assert context["variant_name"] is None
assert context.get("variant_name") is None

return call_with_variants(
_single_stacktrace_variant,
Expand All @@ -425,6 +427,7 @@ def _single_stacktrace_variant(
stacktrace: Stacktrace, event: Event, context: GroupingContext, kwargs: dict[str, Any]
) -> ReturnedVariants:
variant_name = context["variant_name"]
assert variant_name is not None

frames = stacktrace.frames

Expand Down Expand Up @@ -467,7 +470,7 @@ def _single_stacktrace_variant(
frame_components,
raw_frames,
event.platform,
exception_data=context["exception_data"],
exception_data=context.get("exception_data"),
)

# This context value is set by the grouping info endpoint, so that the frame order of the
Expand Down
16 changes: 12 additions & 4 deletions src/sentry/grouping/strategies/security.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,10 @@
def expect_ct_v1(
interface: ExpectCT, event: Event, context: GroupingContext, **kwargs: Any
) -> ReturnedVariants:
variant_name = context["variant_name"]

return {
context["variant_name"]: ExpectCTGroupingComponent(
variant_name: ExpectCTGroupingComponent(
values=[
SaltGroupingComponent(values=["expect-ct"]),
HostnameGroupingComponent(values=[interface.hostname]),
Expand All @@ -44,8 +46,10 @@ def expect_ct_v1(
def expect_staple_v1(
interface: ExpectStaple, event: Event, context: GroupingContext, **kwargs: Any
) -> ReturnedVariants:
variant_name = context["variant_name"]

return {
context["variant_name"]: ExpectStapleGroupingComponent(
variant_name: ExpectStapleGroupingComponent(
values=[
SaltGroupingComponent(values=["expect-staple"]),
HostnameGroupingComponent(values=[interface.hostname]),
Expand All @@ -59,8 +63,10 @@ def expect_staple_v1(
def hpkp_v1(
interface: Hpkp, event: Event, context: GroupingContext, **kwargs: Any
) -> ReturnedVariants:
variant_name = context["variant_name"]

return {
context["variant_name"]: HPKPGroupingComponent(
variant_name: HPKPGroupingComponent(
values=[
SaltGroupingComponent(values=["hpkp"]),
HostnameGroupingComponent(values=[interface.hostname]),
Expand All @@ -74,6 +80,8 @@ def hpkp_v1(
def csp_v1(
interface: Csp, event: Event, context: GroupingContext, **kwargs: Any
) -> ReturnedVariants:
variant_name = context["variant_name"]

violation_component = ViolationGroupingComponent()
uri_component = URIGroupingComponent()

Expand All @@ -89,7 +97,7 @@ def csp_v1(
uri_component.update(values=[interface.normalized_blocked_uri])

return {
context["variant_name"]: CSPGroupingComponent(
variant_name: CSPGroupingComponent(
values=[
SaltGroupingComponent(values=[interface.effective_directive]),
violation_component,
Expand Down
6 changes: 4 additions & 2 deletions src/sentry/grouping/strategies/template.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
def template_v1(
interface: Template, event: Event, context: GroupingContext, **kwargs: Any
) -> ReturnedVariants:
variant_name = context["variant_name"]

filename_component = FilenameGroupingComponent()
if interface.filename is not None:
filename_component.update(values=[interface.filename])
Expand All @@ -33,7 +35,7 @@ def template_v1(
context_line_component.update(values=[interface.context_line])

return {
context["variant_name"]: TemplateGroupingComponent(
values=[filename_component, context_line_component]
variant_name: TemplateGroupingComponent(
values=[filename_component, context_line_component],
)
}
Loading