diff --git a/src/sentry/event_manager.py b/src/sentry/event_manager.py index 36d98982e7e0f0..9977b0b28e4d78 100644 --- a/src/sentry/event_manager.py +++ b/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 diff --git a/src/sentry/grouping/ingest/metrics.py b/src/sentry/grouping/ingest/metrics.py index 8469048fa7dc80..4f76adf9340fab 100644 --- a/src/sentry/grouping/ingest/metrics.py +++ b/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, ) diff --git a/tests/sentry/event_manager/grouping/test_assign_to_group.py b/tests/sentry/event_manager/grouping/test_assign_to_group.py index e958658b374ddc..81e07ecb5684e7 100644 --- a/tests/sentry/event_manager/grouping/test_assign_to_group.py +++ b/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,