Browse Source

fix(stats-detectors): Use correct fingerprint when redirecting escala… (#61758)

…tions

Functions and endpoints are fingerprinted differently, so make sure
functions are fingerprinted consistently.
Tony Xiao 1 year ago
parent
commit
85a0888d4a

+ 12 - 11
src/sentry/tasks/statistical_detectors.py

@@ -962,6 +962,15 @@ def limit_regressions_by_project(
             yield regression
 
 
+def generate_fingerprint(regression_type: RegressionType, regression: BreakpointData) -> str:
+    if regression_type == RegressionType.ENDPOINT:
+        return fingerprint_regression(regression["transaction"])
+    elif regression_type == RegressionType.FUNCTION:
+        return f"{int(regression['transaction']):x}"
+    else:
+        raise ValueError(f"Unsupported RegressionType: {regression_type}")
+
+
 def redirect_escalations(
     regressions: Generator[BreakpointData, None, None],
     regression_type: RegressionType,
@@ -975,7 +984,7 @@ def redirect_escalations(
                 [
                     (
                         int(regression["project"]),
-                        fingerprint_regression(regression["transaction"]),
+                        generate_fingerprint(regression_type, regression),
                     )
                     for regression in regression_chunk
                 ],
@@ -984,7 +993,7 @@ def redirect_escalations(
 
         for regression in regression_chunk:
             project_id = int(regression["project"])
-            fingerprint = fingerprint_regression(regression["transaction"])
+            fingerprint = generate_fingerprint(regression_type, regression)
             group = existing_regression_groups.get((project_id, fingerprint))
 
             if group is None:
@@ -1005,14 +1014,6 @@ def save_versioned_regressions(
     regression_type: RegressionType,
     batch_size=100,
 ) -> Generator[BreakpointData, None, None]:
-    def generate_fingerprint(regression) -> str:
-        if regression_type == RegressionType.ENDPOINT:
-            return fingerprint_regression(regression["transaction"])
-        elif regression_type == RegressionType.FUNCTION:
-            return f"{int(regression['transaction']):x}"
-        else:
-            raise ValueError(f"Unsupported RegressionType: {regression_type}")
-
     for regression_chunk in chunked(versioned_regressions, batch_size):
         RegressionGroup.objects.bulk_create(
             [
@@ -1024,7 +1025,7 @@ def save_versioned_regressions(
                     version=version,
                     active=True,
                     project_id=int(regression["project"]),
-                    fingerprint=generate_fingerprint(regression),
+                    fingerprint=generate_fingerprint(regression_type, regression),
                     baseline=regression["aggregate_range_1"],
                     regressed=regression["aggregate_range_2"],
                 )

+ 29 - 14
tests/sentry/tasks/test_statistical_detectors.py

@@ -24,6 +24,7 @@ from sentry.tasks.statistical_detectors import (
     detect_transaction_change_points,
     detect_transaction_trends,
     emit_function_regression_issue,
+    generate_fingerprint,
     limit_regressions_by_project,
     query_functions,
     query_transactions,
@@ -511,36 +512,36 @@ def test_limit_regressions_by_project(ratelimit, timestamp, expected_idx):
         pytest.param([], [1], id="no existing"),
         pytest.param(
             [
-                (1, False, "transaction_1"),
-                (2, False, "transaction_1"),
+                (1, False, "1"),
+                (2, False, "1"),
             ],
             [3],
             id="existing inactive",
         ),
         pytest.param(
             [
-                (1, False, "transaction_1"),
-                (2, True, "transaction_1"),
+                (1, False, "1"),
+                (2, True, "1"),
             ],
             [None],
             id="existing active",
         ),
         pytest.param(
             [
-                (1, False, "transaction_1"),
-                (2, False, "transaction_1"),
-                (1, False, "transaction_2"),
-                (2, False, "transaction_2"),
-                (3, False, "transaction_2"),
-                (4, True, "transaction_2"),
+                (1, False, "1"),
+                (2, False, "1"),
+                (1, False, "2"),
+                (2, False, "2"),
+                (3, False, "2"),
+                (4, True, "2"),
             ],
             [3, None],
             id="mixed active and inactive",
         ),
         pytest.param(
             [
-                (1, True, "transaction_1"),
-                (2, False, "transaction_1"),
+                (1, True, "1"),
+                (2, False, "1"),
             ],
             [3],
             id="use latest version",
@@ -563,7 +564,21 @@ def test_redirect_escalations(
                 version=version,
                 active=active,
                 project_id=project.id,
-                fingerprint=fingerprint_regression(transaction),
+                fingerprint=generate_fingerprint(
+                    regression_type,
+                    {
+                        "absolute_percentage_change": 5.0,
+                        "aggregate_range_1": 100000000.0,
+                        "aggregate_range_2": 500000000.0,
+                        "breakpoint": 1687323600,
+                        "project": str(project.id),
+                        "transaction": transaction,
+                        "trend_difference": 400000000.0,
+                        "trend_percentage": 5.0,
+                        "unweighted_p_value": 0.0,
+                        "unweighted_t_value": -float("inf"),
+                    },
+                ),
                 baseline=100000000.0,
                 regressed=500000000.0,
             )
@@ -577,7 +592,7 @@ def test_redirect_escalations(
             "aggregate_range_2": 500000000.0,
             "breakpoint": 1687323600,
             "project": str(project.id),
-            "transaction": "transaction_1",
+            "transaction": "1",
             "trend_difference": 400000000.0,
             "trend_percentage": 5.0,
             "unweighted_p_value": 0.0,