Skip to content

Commit

Permalink
chore(grouping): Clean up obsolete code (#81929)
Browse files Browse the repository at this point in the history
This is a clean-up of the grouping code, removing a number of bits which have become obsolete:

- When the code for parameterizing messages [was abstracted into a `Parameterizer` class](#71078), the tests for message parameterization were simply copied over from `test_normalize_message.py` into `test_parameterization.py`. Under the hood, `normalize_message_for_grouping`, which the former tests, just calls `Parameterizer.parameterize_all`, which is exactly what is tested by the latter. Thus, the tests duplicate one another, and so this deletes the older `test_normalize_message.py`.

- We have two different constants for limiting the number of frames sent to Seer, but only one is used, so this deletes the other one (the one in `group_similar_issues_embeddings.py`).

- `BaseGroupingCompoenent` has a `variant_provider` attribute, whose purpose is not explained anywhere and the last use of which was [removed in April of 2021](#25690), so this removes it.

- Many of the tests for built-in fingerprints have docstrings which reference a feature flag which was [removed in February](#65137), so this updates those docstrings.

- `apply_server_fingerprinting` takes an `allow_custom_title` parameter, whose value was originally based on the `organizations:custom-event-title` feature flag. But that flag was [defaulted to `True` in early 2021](#23090) and [removed entirely in mid 2023](#48808), in favor of its value being hardcoded to `True` in [the one place `apply_server_fingerprinting` is called](https://github.com/getsentry/sentry/blob/474b5c5f3066ba2584d16ae19b5c9a14e7e10416/src/sentry/grouping/ingest/hashing.py#L69-L73). This therefore removes the paramter entirely.

- `Enhancements.assemble_stacktrace_component` currently returns a tuple, the second value of which (`rust_results.invert_stacktrace`) was part of hierarchical grouping and [stoped being used](#77235) when hierarchical grouping was removed in September. This therefore changes `assemble_stacktrace_component` to stop returning it and only return the stacktrace component.
  • Loading branch information
lobsterkatie authored Dec 11, 2024
1 parent ad13053 commit a4422b0
Show file tree
Hide file tree
Showing 9 changed files with 13 additions and 241 deletions.
6 changes: 2 additions & 4 deletions src/sentry/grouping/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -249,9 +249,7 @@ def get_fingerprinting_config_for_project(


def apply_server_fingerprinting(
event: MutableMapping[str, Any],
fingerprinting_config: FingerprintingRules,
allow_custom_title: bool = True,
event: MutableMapping[str, Any], fingerprinting_config: FingerprintingRules
) -> None:
fingerprint_info = {}

Expand All @@ -268,7 +266,7 @@ def apply_server_fingerprinting(

# A custom title attribute is stored in the event to override the
# default title.
if "title" in attributes and allow_custom_title:
if "title" in attributes:
event["title"] = expand_title_template(attributes["title"], event)
event["fingerprint"] = new_fingerprint

Expand Down
3 changes: 0 additions & 3 deletions src/sentry/grouping/component.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,7 @@ def __init__(
hint: str | None = None,
contributes: bool | None = None,
values: Sequence[ValuesType] | None = None,
variant_provider: bool = False,
):
self.variant_provider = variant_provider

self.update(
hint=hint,
contributes=contributes,
Expand Down
4 changes: 2 additions & 2 deletions src/sentry/grouping/enhancer/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ def assemble_stacktrace_component(
frames: list[dict[str, Any]],
platform: str | None,
exception_data: dict[str, Any] | None = None,
) -> tuple[StacktraceGroupingComponent, bool]:
) -> StacktraceGroupingComponent:
"""
This assembles a `stacktrace` grouping component out of the given
`frame` components and source frames.
Expand Down Expand Up @@ -205,7 +205,7 @@ def assemble_stacktrace_component(
frame_counts=frame_counts,
)

return stacktrace_component, rust_results.invert_stacktrace
return stacktrace_component

def as_dict(self, with_rules=False):
rv = {
Expand Down
4 changes: 1 addition & 3 deletions src/sentry/grouping/ingest/hashing.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,7 @@ def _calculate_event_grouping(
# look at `grouping_config` to pick the right parameters.
event.data["fingerprint"] = event.data.data.get("fingerprint") or ["{{ default }}"]
apply_server_fingerprinting(
event.data.data,
get_fingerprinting_config_for_project(project),
allow_custom_title=True,
event.data.data, get_fingerprinting_config_for_project(project)
)

with metrics.timer("event_manager.event.get_hashes", tags=metric_tags):
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/grouping/strategies/legacy.py
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,7 @@ def stacktrace_legacy(
frames_for_filtering.append(frame.get_raw_data())
prev_frame = frame

stacktrace_component, _ = context.config.enhancements.assemble_stacktrace_component(
stacktrace_component = context.config.enhancements.assemble_stacktrace_component(
frame_components, frames_for_filtering, event.platform
)
stacktrace_component.update(contributes=contributes, hint=hint)
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/grouping/strategies/newstyle.py
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,7 @@ def _single_stacktrace_variant(
contributes=False, hint="ignored single non-URL JavaScript frame"
)

stacktrace_component, _ = context.config.enhancements.assemble_stacktrace_component(
stacktrace_component = context.config.enhancements.assemble_stacktrace_component(
frame_components,
frames_for_filtering,
event.platform,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
from sentry.utils.safe import get_path

logger = logging.getLogger(__name__)
MAX_FRAME_COUNT = 50


class FormattedSimilarIssuesEmbeddingsData(TypedDict):
Expand Down
12 changes: 6 additions & 6 deletions tests/sentry/grouping/test_builtin_fingerprinting.py
Original file line number Diff line number Diff line change
Expand Up @@ -610,8 +610,8 @@ def test_built_in_chunkload_rules_wrong_sdk(self):

def test_built_in_hydration_rules_same_transactions(self):
"""
With the flag enabled, hydration errors with the same transaction should be grouped and
the built-in rules for hydration errors should be applied.
Hydration errors with the same transaction should be grouped and the built-in rules for
hydration errors should be applied.
"""

event_message1 = self.store_event(data=self.hydration_error_trace, project_id=self.project)
Expand Down Expand Up @@ -650,8 +650,8 @@ def test_built_in_hydration_rules_same_transactions(self):

def test_built_in_hydration_rules_different_transactions(self):
"""
With the flag enabled, hydration errors with different transactions should not be grouped and
the built-in rules for hydration errors should be applied.
Hydration errors with different transactions should not be grouped and the built-in rules
for hydration errors should be applied.
"""

event_transaction_slash = self.store_event(
Expand Down Expand Up @@ -698,8 +698,8 @@ def test_built_in_hydration_rules_different_transactions(self):

def test_built_in_hydration_rules_no_transactions(self):
"""
With the flag enabled, for hydration errors with no transactions
the built-in HydrationError rules should NOT be applied.
For hydration errors with no transactions the built-in HydrationError rules should NOT be
applied.
"""

data_transaction_no_tx = self.hydration_error_trace
Expand Down
220 changes: 0 additions & 220 deletions tests/sentry/grouping/test_normalize_message.py

This file was deleted.

0 comments on commit a4422b0

Please sign in to comment.