From 9d128b15b46933860c9b3d8dd4063e01ae7fd040 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Thu, 7 Aug 2025 08:42:24 -0700 Subject: [PATCH 1/7] pull variant name into separate variable ^ Conflicts: ^ src/sentry/grouping/strategies/message.py --- src/sentry/grouping/strategies/base.py | 4 +++- src/sentry/grouping/strategies/message.py | 6 ++++-- src/sentry/grouping/strategies/newstyle.py | 3 ++- src/sentry/grouping/strategies/security.py | 16 ++++++++++++---- src/sentry/grouping/strategies/template.py | 6 ++++-- 5 files changed, 25 insertions(+), 10 deletions(-) diff --git a/src/sentry/grouping/strategies/base.py b/src/sentry/grouping/strategies/base.py index 30c66a8fdb5cb7..75acb44320aa49 100644 --- a/src/sentry/grouping/strategies/base.py +++ b/src/sentry/grouping/strategies/base.py @@ -182,12 +182,14 @@ 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"] + 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 diff --git a/src/sentry/grouping/strategies/message.py b/src/sentry/grouping/strategies/message.py index 4d24fdc4f95af9..f4c7bc72c479f5 100644 --- a/src/sentry/grouping/strategies/message.py +++ b/src/sentry/grouping/strategies/message.py @@ -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"] + # 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 ""], ) } diff --git a/src/sentry/grouping/strategies/newstyle.py b/src/sentry/grouping/strategies/newstyle.py index d448f76583a0c2..867a6aa2fb6849 100644 --- a/src/sentry/grouping/strategies/newstyle.py +++ b/src/sentry/grouping/strategies/newstyle.py @@ -300,6 +300,7 @@ def frame( ) -> ReturnedVariants: frame = interface platform = frame.platform or event.platform + variant_name = context["variant_name"] # Safari throws [native code] frames in for calls like ``forEach`` # whereas Chrome ignores these. Let's remove it from the hashing algo @@ -371,7 +372,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( diff --git a/src/sentry/grouping/strategies/security.py b/src/sentry/grouping/strategies/security.py index 93991dce5bc04f..053af719da39ab 100644 --- a/src/sentry/grouping/strategies/security.py +++ b/src/sentry/grouping/strategies/security.py @@ -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]), @@ -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]), @@ -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]), @@ -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() @@ -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, diff --git a/src/sentry/grouping/strategies/template.py b/src/sentry/grouping/strategies/template.py index 034c7a6401b6ea..d2dd2ff9736918 100644 --- a/src/sentry/grouping/strategies/template.py +++ b/src/sentry/grouping/strategies/template.py @@ -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]) @@ -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], ) } From 20cca95b8c32ace203fcdfc1e889d1b2f7cc89fe Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Thu, 7 Aug 2025 08:42:27 -0700 Subject: [PATCH 2/7] use `.get` for getting variant name --- src/sentry/grouping/strategies/base.py | 2 +- src/sentry/grouping/strategies/newstyle.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sentry/grouping/strategies/base.py b/src/sentry/grouping/strategies/base.py index 75acb44320aa49..058e7740c83969 100644 --- a/src/sentry/grouping/strategies/base.py +++ b/src/sentry/grouping/strategies/base.py @@ -486,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 diff --git a/src/sentry/grouping/strategies/newstyle.py b/src/sentry/grouping/strategies/newstyle.py index 867a6aa2fb6849..e86dd1fdbb3d4a 100644 --- a/src/sentry/grouping/strategies/newstyle.py +++ b/src/sentry/grouping/strategies/newstyle.py @@ -410,7 +410,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, From beb7610e60d510e250ccd6be64a906898d9c8259 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Thu, 7 Aug 2025 08:42:29 -0700 Subject: [PATCH 3/7] assert on variant name --- src/sentry/grouping/strategies/base.py | 1 + src/sentry/grouping/strategies/newstyle.py | 2 ++ 2 files changed, 3 insertions(+) diff --git a/src/sentry/grouping/strategies/base.py b/src/sentry/grouping/strategies/base.py index 058e7740c83969..c168be4abce0a2 100644 --- a/src/sentry/grouping/strategies/base.py +++ b/src/sentry/grouping/strategies/base.py @@ -183,6 +183,7 @@ def get_single_grouping_component( 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 diff --git a/src/sentry/grouping/strategies/newstyle.py b/src/sentry/grouping/strategies/newstyle.py index e86dd1fdbb3d4a..8fd16761c33d93 100644 --- a/src/sentry/grouping/strategies/newstyle.py +++ b/src/sentry/grouping/strategies/newstyle.py @@ -301,6 +301,7 @@ def frame( 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 @@ -426,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 From 5995af5874d2ef1143f9550751ddd16cf768e1bb Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Thu, 7 Aug 2025 08:42:31 -0700 Subject: [PATCH 4/7] remove `variant_name` from initial context --- src/sentry/grouping/strategies/configurations.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/sentry/grouping/strategies/configurations.py b/src/sentry/grouping/strategies/configurations.py index e430e5dd050afc..3e461c596dd2da 100644 --- a/src/sentry/grouping/strategies/configurations.py +++ b/src/sentry/grouping/strategies/configurations.py @@ -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. - "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 From e69742a7bd9f878c5ac0b69e20f4945231c0a90b Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Thu, 7 Aug 2025 08:42:33 -0700 Subject: [PATCH 5/7] stop setting default `None` for variant name in context --- src/sentry/grouping/strategies/base.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/sentry/grouping/strategies/base.py b/src/sentry/grouping/strategies/base.py index c168be4abce0a2..ec4fefd742b9d5 100644 --- a/src/sentry/grouping/strategies/base.py +++ b/src/sentry/grouping/strategies/base.py @@ -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 From e56f7bb5035ee2ded72cc7a497468c2fe45a5861 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Thu, 7 Aug 2025 08:42:35 -0700 Subject: [PATCH 6/7] use `.get` when getting exception data from context --- src/sentry/grouping/strategies/newstyle.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry/grouping/strategies/newstyle.py b/src/sentry/grouping/strategies/newstyle.py index 8fd16761c33d93..84b7e16169ab05 100644 --- a/src/sentry/grouping/strategies/newstyle.py +++ b/src/sentry/grouping/strategies/newstyle.py @@ -470,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 From 51c9950223af2177b3423a9d157141848638af51 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Thu, 7 Aug 2025 08:42:36 -0700 Subject: [PATCH 7/7] stop setting default `None` for exception data in context --- src/sentry/grouping/strategies/configurations.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/sentry/grouping/strategies/configurations.py b/src/sentry/grouping/strategies/configurations.py index 3e461c596dd2da..7546bd8da61877 100644 --- a/src/sentry/grouping/strategies/configurations.py +++ b/src/sentry/grouping/strategies/configurations.py @@ -37,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, }, )