Skip to content

Commit 9b7aa16

Browse files
lobsterkatieshashjar
authored andcommitted
ref(grouping): Small refactors to prep for new config (#102339)
This includes three small refactors to grouping code, pulled from upcoming PRs to keep them scoped to their actual purpose. - Simplify adding values to exception grouping components. - Pull useful values into variables in grouping config upgrade function. - In stacktrace strategy, use `has_url_origin` helper over `is_url` for consistency with the rest of the grouping code.
1 parent e52f1be commit 9b7aa16

File tree

2 files changed

+20
-32
lines changed

2 files changed

+20
-32
lines changed

src/sentry/grouping/ingest/config.py

Lines changed: 13 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -35,22 +35,22 @@ def update_or_set_grouping_config_if_needed(project: Project, source: str) -> st
3535
use by scripts.
3636
"""
3737
current_config = project.get_option("sentry:grouping_config")
38+
current_config_is_valid = current_config in GROUPING_CONFIG_CLASSES.keys()
39+
40+
# If the project's current config comes back as the default one, it might be because that's
41+
# actually what's set in the database for that project, or it might be relying on the default
42+
# value of that project option. In the latter case, we can use this upgrade check as a chance to
43+
# set it. (We want projects to have their own record of the config they're using, so that when
44+
# we introduce a new one, we know to transition them.)
45+
project_option_exists = ProjectOption.objects.filter(
46+
key="sentry:grouping_config", project_id=project.id
47+
).exists()
3848

3949
if current_config == BETA_GROUPING_CONFIG:
4050
return "skipped - beta config"
4151

42-
if current_config == DEFAULT_GROUPING_CONFIG:
43-
# If the project's current config comes back as the default one, it might be because that's
44-
# actually what's set in the database for that project, or it might be relying on the
45-
# default value of that project option. In the latter case, we can use this upgrade check as
46-
# a chance to set it. (We want projects to have their own record of the config they're
47-
# using, so that when we introduce a new one, we know to transition them.)
48-
project_option_exists = ProjectOption.objects.filter(
49-
key="sentry:grouping_config", project_id=project.id
50-
).exists()
51-
52-
if project_option_exists:
53-
return "skipped - up-to-date record exists"
52+
if current_config == DEFAULT_GROUPING_CONFIG and project_option_exists:
53+
return "skipped - up-to-date record exists"
5454

5555
# We want to try to write the audit log entry and project option change just once, so we use a
5656
# cache key to avoid raciness. It's not perfect, but it reduces the risk significantly.
@@ -69,10 +69,7 @@ def update_or_set_grouping_config_if_needed(project: Project, source: str) -> st
6969
changes: dict[str, str | int] = {"sentry:grouping_config": DEFAULT_GROUPING_CONFIG}
7070

7171
# If the current config is out of date but still valid, start a transition period
72-
if (
73-
current_config != DEFAULT_GROUPING_CONFIG
74-
and current_config in GROUPING_CONFIG_CLASSES.keys()
75-
):
72+
if current_config != DEFAULT_GROUPING_CONFIG and current_config_is_valid:
7673
# This is when we will stop calculating the old hash in cases where we don't find the
7774
# new hash (which we do in an effort to preserve group continuity).
7875
transition_expiry = (
@@ -86,10 +83,6 @@ def update_or_set_grouping_config_if_needed(project: Project, source: str) -> st
8683
}
8784
)
8885

89-
project_option_exists = ProjectOption.objects.filter(
90-
key="sentry:grouping_config", project_id=project.id
91-
).exists()
92-
9386
for key, value in changes.items():
9487
project.update_option(key, value)
9588

src/sentry/grouping/strategies/newstyle.py

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
ErrorTypeGroupingComponent,
1414
ErrorValueGroupingComponent,
1515
ExceptionGroupingComponent,
16+
ExceptionGroupingComponentChildren,
1617
FilenameGroupingComponent,
1718
FrameGroupingComponent,
1819
FunctionGroupingComponent,
@@ -509,7 +510,7 @@ def _single_stacktrace_variant(
509510
and frame_components[0].contributes
510511
and get_behavior_family_for_platform(frames[0].platform or event.platform) == "javascript"
511512
and not frames[0].function
512-
and frames[0].is_url()
513+
and has_url_origin(frames[0].abs_path, files_count_as_urls=True)
513514
):
514515
frame_components[0].update(
515516
contributes=False, hint="ignored single non-URL JavaScript frame"
@@ -586,16 +587,6 @@ def single_exception(
586587
exception_components_by_variant = {}
587588

588589
for variant_name, stacktrace_component in stacktrace_components_by_variant.items():
589-
values: list[
590-
ErrorTypeGroupingComponent
591-
| ErrorValueGroupingComponent
592-
| NSErrorGroupingComponent
593-
| StacktraceGroupingComponent
594-
] = [stacktrace_component, type_component]
595-
596-
if ns_error_component is not None:
597-
values.append(ns_error_component)
598-
599590
value_component = ErrorValueGroupingComponent()
600591

601592
raw = exception.value
@@ -621,7 +612,11 @@ def single_exception(
621612
hint="ignored because ns-error info takes precedence",
622613
)
623614

624-
values.append(value_component)
615+
values: list[ExceptionGroupingComponentChildren] = []
616+
if ns_error_component is not None:
617+
values = [stacktrace_component, type_component, ns_error_component, value_component]
618+
else:
619+
values = [stacktrace_component, type_component, value_component]
625620

626621
exception_components_by_variant[variant_name] = ExceptionGroupingComponent(
627622
values=values, frame_counts=stacktrace_component.frame_counts

0 commit comments

Comments
 (0)