combine metrics helpers

src/sentry/event_manager.py

@@ -61,11 +61,7 @@
     maybe_run_secondary_grouping,
     run_primary_grouping,
 )
-from sentry.grouping.ingest.metrics import (
-    record_calculation_metric_with_result,
-    record_hash_calculation_metrics,
-    record_new_group_metrics,
-)
+from sentry.grouping.ingest.metrics import record_hash_calculation_metrics, record_new_group_metrics
 from sentry.grouping.ingest.seer import maybe_check_seer_for_matching_grouphash
 from sentry.grouping.ingest.utils import (
     add_group_id_to_grouphashes,
@@ -271,13 +267,13 @@ def has_pending_commit_resolution(group: Group) -> bool:
 
 
 @overload
-def get_max_crashreports(model: Project | Organization) -> int:
-    ...
+def get_max_crashreports(model: Project | Organization) -> int: ...
 
 
 @overload
-def get_max_crashreports(model: Project | Organization, *, allow_none: Literal[True]) -> int | None:
-    ...
+def get_max_crashreports(
+    model: Project | Organization, *, allow_none: Literal[True]
+) -> int | None: ...
 
 
 def get_max_crashreports(model: Project | Organization, *, allow_none: bool = False) -> int | None:
@@ -1354,14 +1350,7 @@ def _save_aggregate_new(
     maybe_run_background_grouping(project, job)
 
     record_hash_calculation_metrics(
-        primary.config, primary.hashes, secondary.config, secondary.hashes
-    )
-    # TODO: Once the legacy `_save_aggregate` goes away, the logic inside of
-    # `record_calculation_metric_with_result` can be pulled into `record_hash_calculation_metrics`
-    record_calculation_metric_with_result(
-        project=project,
-        has_secondary_hashes=len(secondary.hashes) > 0,
-        result=result,
+        project, primary.config, primary.hashes, secondary.config, secondary.hashes, result
     )
 
     # Now that we've used the current and possibly secondary grouping config(s) to calculate the

src/sentry/grouping/ingest/metrics.py

@@ -20,15 +20,19 @@
 
 
 def record_hash_calculation_metrics(
+    project: Project,
     primary_config: GroupingConfig,
     primary_hashes: list[str],
     secondary_config: GroupingConfig,
     secondary_hashes: list[str],
+    existing_hash_search_result: str,
 ) -> None:
     has_secondary_hashes = len(secondary_hashes) > 0
 
+    # In cases where we've computed both primary and secondary hashes, track how often the config
+    # change has changed the resulting hashes
     if has_secondary_hashes:
-        tags = {
+        hash_comparison_tags = {
             "primary_config": primary_config["id"],
             "secondary_config": secondary_config["id"],
         }
@@ -37,46 +41,36 @@ def record_hash_calculation_metrics(
         hashes_match = current_values == secondary_values
 
         if hashes_match:
-            tags["result"] = "no change"
+            hash_comparison_tags["result"] = "no change"
         else:
             shared_hashes = set(current_values) & set(secondary_values)
             if len(shared_hashes) > 0:
-                tags["result"] = "partial change"
+                hash_comparison_tags["result"] = "partial change"
             else:
-                tags["result"] = "full change"
+                hash_comparison_tags["result"] = "full change"
 
         metrics.incr(
             "grouping.hash_comparison",
             sample_rate=options.get("grouping.config_transition.metrics_sample_rate"),
-            tags=tags,
+            tags=hash_comparison_tags,
         )
 
-
-# TODO: Once the legacy `_save_aggregate` goes away, this logic can be pulled into
-# `record_hash_calculation_metrics`. Right now it's split up because we don't know the value for
-# `result` at the time the legacy `_save_aggregate` (indirectly) calls `record_hash_calculation_metrics`
-def record_calculation_metric_with_result(
-    project: Project,
-    has_secondary_hashes: bool,
-    result: str,
-) -> None:
-
     # Track the total number of grouping calculations done overall, so we can divide by the
     # count to get an average number of calculations per event
-    tags = {
+    num_calculations_tags = {
         "in_transition": str(is_in_transition(project)),
-        "result": result,
+        "result": existing_hash_search_result,
     }
     metrics.incr(
         "grouping.event_hashes_calculated",
         sample_rate=options.get("grouping.config_transition.metrics_sample_rate"),
-        tags=tags,
+        tags=num_calculations_tags,
     )
     metrics.incr(
         "grouping.total_calculations",
         amount=2 if has_secondary_hashes else 1,
         sample_rate=options.get("grouping.config_transition.metrics_sample_rate"),
-        tags=tags,
+        tags=num_calculations_tags,
     )
 
 

tests/sentry/event_manager/grouping/test_assign_to_group.py

@@ -14,7 +14,7 @@
     _calculate_secondary_hashes,
     find_existing_grouphash,
 )
-from sentry.grouping.ingest.metrics import record_calculation_metric_with_result
+from sentry.grouping.ingest.metrics import record_hash_calculation_metrics
 from sentry.models.grouphash import GroupHash
 from sentry.models.project import Project
 from sentry.projectoptions.defaults import DEFAULT_GROUPING_CONFIG, LEGACY_GROUPING_CONFIG
@@ -52,17 +52,17 @@ def patch_grouping_helpers(return_values: dict[str, Any]):
             wraps=_create_group,
         ) as create_group_spy,
         mock.patch(
-            "sentry.event_manager.record_calculation_metric_with_result",
+            "sentry.event_manager.record_hash_calculation_metrics",
             # No return-value-wrapping necessary here either, since it doesn't return anything
-            wraps=record_calculation_metric_with_result,
-        ) as record_calculation_metric_spy,
+            wraps=record_hash_calculation_metrics,
+        ) as record_calculation_metrics_spy,
     ):
         yield {
             "find_existing_grouphash": find_existing_grouphash_spy,
             "_calculate_primary_hashes": calculate_primary_hashes_spy,
             "_calculate_secondary_hashes": calculate_secondary_hashes_spy,
             "_create_group": create_group_spy,
-            "record_calculation_metric": record_calculation_metric_spy,
+            "record_calculation_metrics": record_calculation_metrics_spy,
         }
 
 
@@ -146,7 +146,7 @@ def get_results_from_saving_event(
         calculate_secondary_hash_spy = spies["_calculate_secondary_hashes"]
         create_group_spy = spies["_create_group"]
         calculate_primary_hash_spy = spies["_calculate_primary_hashes"]
-        record_calculation_metric_spy = spies["record_calculation_metric"]
+        record_calculation_metrics_spy = spies["record_calculation_metrics"]
 
         set_grouping_configs(
             project=project,
@@ -217,7 +217,7 @@ def get_results_from_saving_event(
                     existing_group_id
                 ), "Secondary grouphash already exists. Either something's wrong or you forgot to pass an existing group id"
 
-        result_tag_value_for_metrics = record_calculation_metric_spy.call_args.kwargs["result"]
+        result_tag_value_for_metrics = record_calculation_metrics_spy.call_args.args[5]
 
         return {
             "primary_hash_calculated": primary_hash_calculated,