feat(perf-detector-threshold-configuration) Added lowered defaults and new threshold for N+1 API Calls (#53401)

For project: [Detector Threshold
Configurations](https://www.notion.so/sentry/Detector-Threshold-Configuration-f8cb07e7cceb42388cedb09ea05fc116)
Lowered defaults values and effects:
[link](https://www.notion.so/sentry/Detector-Dry-Run-Results-40dc7a3e4d8b4b9ea8da90608fe54747)

- We are GA'ing the project with lowered default values for thresholds. 
- Added lowered defaults for Large http payload, Uncompressed Assets and
Render Blocking assets.
- Removed Dry run code, initially added to test the effects of lowering
defaults, on detection.
- Updated N+1 API Call detector to add new total duration threshold with
a default of 300ms.
- Updated N+1 API Call detector tests to ensure new total duration
threshold is respected.
- Removed Extended N+1 API Calls detector and tests.

---------

Co-authored-by: Abdullah Khan <abdullahkhan@PG9Y57YDXQ.local>

src/sentry/api/endpoints/project_performance_issue_settings.py

@@ -57,6 +57,7 @@ class ConfigurableThresholds(Enum):
     CONSECUTIVE_DB_QUERIES_MIN_TIME_SAVED = "consecutive_db_min_time_saved_threshold"
     RENDER_BLOCKING_ASSET_FCP_RATIO = "render_blocking_fcp_ratio"
     SLOW_DB_QUERY_DURATION = "slow_db_query_duration_threshold"
+    N_PLUS_API_CALLS_DURATION = "n_plus_one_api_calls_total_duration_threshold"
 
 
 internal_only_project_settings_to_group_map: Dict[str, Type[GroupType]] = {
@@ -82,6 +83,7 @@ configurable_thresholds_to_internal_settings_map: Dict[str, str] = {
     ConfigurableThresholds.CONSECUTIVE_DB_QUERIES_MIN_TIME_SAVED.value: InternalProjectOptions.CONSECUTIVE_DB_QUERIES.value,
     ConfigurableThresholds.RENDER_BLOCKING_ASSET_FCP_RATIO.value: InternalProjectOptions.RENDER_BLOCKING_ASSET.value,
     ConfigurableThresholds.SLOW_DB_QUERY_DURATION.value: InternalProjectOptions.SLOW_DB_QUERY.value,
+    ConfigurableThresholds.N_PLUS_API_CALLS_DURATION.value: InternalProjectOptions.N_PLUS_ONE_API_CALLS.value,
 }
 
 
@@ -120,6 +122,9 @@ class ProjectPerformanceIssueSettingsSerializer(serializers.Serializer):
     consecutive_db_min_time_saved_threshold = serializers.IntegerField(
         required=False, min_value=50, max_value=5000  # ms
     )
+    n_plus_one_api_calls_total_duration_threshold = serializers.IntegerField(
+        required=False, min_value=100, max_value=TEN_SECONDS  # ms
+    )
     uncompressed_assets_detection_enabled = serializers.BooleanField(required=False)
     consecutive_http_spans_detection_enabled = serializers.BooleanField(required=False)
     large_http_payload_detection_enabled = serializers.BooleanField(required=False)

src/sentry/options/defaults.py

@@ -1252,12 +1252,12 @@ register(
 )
 register(
     "performance.issues.render_blocking_assets.size_threshold",
-    default=1000000,
+    default=500000,
     flags=FLAG_AUTOMATOR_MODIFIABLE,
 )
 register(
     "performance.issues.consecutive_http.max_duration_between_spans",
-    default=1000,
+    default=500,
     flags=FLAG_AUTOMATOR_MODIFIABLE,
 )
 register(
@@ -1272,7 +1272,7 @@ register(
 )
 register(
     "performance.issues.large_http_payload.size_threshold",
-    default=1000000,
+    default=300000,
     flags=FLAG_AUTOMATOR_MODIFIABLE,
 )  # 1MB
 register(
@@ -1292,7 +1292,7 @@ register(
 )  # 512 kilo bytes
 register(
     "performance.issues.uncompressed_asset.duration_threshold",
-    default=500,
+    default=300,
     flags=FLAG_AUTOMATOR_MODIFIABLE,
 )  # ms
 register(
@@ -1305,6 +1305,11 @@ register(
     default=500,
     flags=FLAG_AUTOMATOR_MODIFIABLE,
 )  # ms
+register(
+    "performance.issues.n_plus_one_api_calls.total_duration",
+    default=300,
+    flags=FLAG_AUTOMATOR_MODIFIABLE,
+)  # ms
 
 # Dynamic Sampling system-wide options
 # Size of the sliding window used for dynamic sampling. It is defaulted to 24 hours.

src/sentry/utils/performance_issues/base.py

@@ -35,7 +35,6 @@ class DetectorType(Enum):
     N_PLUS_ONE_DB_QUERIES = "n_plus_one_db"
     N_PLUS_ONE_DB_QUERIES_EXTENDED = "n_plus_one_db_ext"
     N_PLUS_ONE_API_CALLS = "n_plus_one_api_calls"
-    N_PLUS_ONE_API_CALLS_EXTENDED = "n_plus_one_api_calls_ext"
     CONSECUTIVE_DB_OP = "consecutive_db"
     CONSECUTIVE_HTTP_OP = "consecutive_http"
     CONSECUTIVE_HTTP_OP_EXTENDED = "consecutive_http_ext"
@@ -53,7 +52,6 @@ DETECTOR_TYPE_TO_GROUP_TYPE = {
     DetectorType.N_PLUS_ONE_DB_QUERIES: PerformanceNPlusOneGroupType,
     DetectorType.N_PLUS_ONE_DB_QUERIES_EXTENDED: PerformanceNPlusOneGroupType,
     DetectorType.N_PLUS_ONE_API_CALLS: PerformanceNPlusOneAPICallsGroupType,
-    DetectorType.N_PLUS_ONE_API_CALLS_EXTENDED: PerformanceNPlusOneAPICallsGroupType,
     DetectorType.CONSECUTIVE_DB_OP: PerformanceConsecutiveDBQueriesGroupType,
     DetectorType.FILE_IO_MAIN_THREAD: PerformanceFileIOMainThreadGroupType,
     DetectorType.M_N_PLUS_ONE_DB: PerformanceMNPlusOneDBQueriesGroupType,

src/sentry/utils/performance_issues/detectors/n_plus_one_api_calls_detector.py

@@ -13,6 +13,7 @@ from sentry import features
 from sentry.issues.grouptype import PerformanceNPlusOneAPICallsGroupType
 from sentry.issues.issue_occurrence import IssueEvidence
 from sentry.models import Organization, Project
+from sentry.utils.performance_issues.detectors.utils import get_total_span_duration
 
 from ..base import (
     DETECTOR_TYPE_TO_GROUP_TYPE,
@@ -20,7 +21,6 @@ from ..base import (
     PerformanceDetector,
     fingerprint_http_spans,
     get_notification_attachment_body,
-    get_span_duration,
     get_span_evidence_value,
     get_url_from_span,
     parameterize_url,
@@ -61,12 +61,6 @@ class NPlusOneAPICallsDetector(PerformanceDetector):
         if op not in self.settings.get("allowed_span_ops", []):
             return
 
-        duration_threshold = timedelta(milliseconds=self.settings.get("duration_threshold"))
-        span_duration = get_span_duration(span)
-
-        if span_duration < duration_threshold:
-            return
-
         self.span_hashes[span["span_id"]] = get_span_hash(span)
 
         previous_span = self.spans[-1] if len(self.spans) > 0 else None
@@ -163,6 +157,10 @@ class NPlusOneAPICallsDetector(PerformanceDetector):
         if len(self.spans) < self.settings["count"]:
             return
 
+        total_duration = get_total_span_duration(self.spans)
+        if total_duration < self.settings["total_duration"]:
+            return
+
         last_span = self.spans[-1]
 
         fingerprint = self._fingerprint()
@@ -262,106 +260,6 @@ class NPlusOneAPICallsDetector(PerformanceDetector):
         )
 
 
-class NPlusOneAPICallsDetectorExtended(NPlusOneAPICallsDetector):
-    """
-    Detector goals:
-    - Extend N+1 API Calls detector to replace individual span duration threshold with total duration.
-    """
-
-    type = DetectorType.N_PLUS_ONE_API_CALLS_EXTENDED
-    settings_key = DetectorType.N_PLUS_ONE_API_CALLS_EXTENDED
-
-    def _maybe_store_problem(self):
-        if len(self.spans) < 1:
-            return
-
-        if len(self.spans) < self.settings["count"]:
-            return
-
-        total_duration = self._sum_span_duration(self.spans)
-        if total_duration < self.settings["total_duration"]:
-            return
-
-        last_span = self.spans[-1]
-
-        fingerprint = self._fingerprint()
-        if not fingerprint:
-            return
-
-        offender_span_ids = [span["span_id"] for span in self.spans]
-
-        self.stored_problems[fingerprint] = PerformanceProblem(
-            fingerprint=fingerprint,
-            op=last_span["op"],
-            desc=os.path.commonprefix([span.get("description", "") or "" for span in self.spans]),
-            type=DETECTOR_TYPE_TO_GROUP_TYPE[self.settings_key],
-            cause_span_ids=[],
-            parent_span_ids=[last_span.get("parent_span_id", None)],
-            offender_span_ids=offender_span_ids,
-            evidence_data={
-                "op": last_span["op"],
-                "cause_span_ids": [],
-                "parent_span_ids": [last_span.get("parent_span_id", None)],
-                "offender_span_ids": offender_span_ids,
-                "transaction_name": self._event.get("transaction", ""),
-                "num_repeating_spans": str(len(offender_span_ids)) if offender_span_ids else "",
-                "repeating_spans": self._get_path_prefix(self.spans[0]),
-                "repeating_spans_compact": get_span_evidence_value(self.spans[0], include_op=False),
-                "parameters": self._get_parameters(),
-            },
-            evidence_display=[
-                IssueEvidence(
-                    name="Offending Spans",
-                    value=get_notification_attachment_body(
-                        last_span["op"],
-                        os.path.commonprefix(
-                            [span.get("description", "") or "" for span in self.spans]
-                        ),
-                    ),
-                    # Has to be marked important to be displayed in the notifications
-                    important=True,
-                )
-            ],
-        )
-
-    def visit_span(self, span: Span) -> None:
-        if not NPlusOneAPICallsDetectorExtended.is_span_eligible(span):
-            return
-
-        op = span.get("op", None)
-        if op not in self.settings.get("allowed_span_ops", []):
-            return
-
-        self.span_hashes[span["span_id"]] = get_span_hash(span)
-
-        previous_span = self.spans[-1] if len(self.spans) > 0 else None
-
-        if previous_span is None:
-            self.spans.append(span)
-        elif self._spans_are_concurrent(previous_span, span) and self._spans_are_similar(
-            previous_span, span
-        ):
-            self.spans.append(span)
-        else:
-            self._maybe_store_problem()
-            self.spans = [span]
-
-    def _sum_span_duration(self, spans: list[Span]) -> float:
-        "Given a list of spans, find the sum of the span durations in milliseconds"
-        sum = 0.0
-        for span in spans:
-            sum += get_span_duration(span).total_seconds() * 1000
-        return sum
-
-    def is_creation_allowed_for_organization(self, organization: Organization) -> bool:
-        # Only collecting metrics.
-        return False
-
-    def is_creation_allowed_for_project(self, project: Project) -> bool:
-        # Only collecting metrics.
-        return False
-
-
 HTTP_METHODS = {
     "GET",
     "HEAD",

src/sentry/utils/performance_issues/performance_detection.py

@@ -17,9 +17,6 @@ from sentry.utils.event_frames import get_sdk_name
 from sentry.utils.performance_issues.detectors.consecutive_http_detector import (
     ConsecutiveHTTPSpanDetectorExtended,
 )
-from sentry.utils.performance_issues.detectors.n_plus_one_api_calls_detector import (
-    NPlusOneAPICallsDetectorExtended,
-)
 from sentry.utils.safe import get_path
 
 from .base import DetectorType, PerformanceDetector
@@ -183,6 +180,9 @@ def get_merged_settings(project_id: Optional[int] = None) -> Dict[str | Any, Any
         "http_request_delay_threshold": options.get(
             "performance.issues.http_overhead.http_request_delay_threshold"
         ),
+        "n_plus_one_api_calls_total_duration_threshold": options.get(
+            "performance.issues.n_plus_one_api_calls.total_duration"
+        ),
     }
 
     default_project_settings = (
@@ -265,18 +265,12 @@ def get_detection_settings(project_id: Optional[int] = None) -> Dict[DetectorTyp
             }
         ],
         DetectorType.N_PLUS_ONE_API_CALLS: {
-            "duration_threshold": 50,  # ms
+            "total_duration": settings["n_plus_one_api_calls_total_duration_threshold"],  # ms
             "concurrency_threshold": 5,  # ms
             "count": 10,
             "allowed_span_ops": ["http.client"],
             "detection_enabled": settings["n_plus_one_api_calls_detection_enabled"],
         },
-        DetectorType.N_PLUS_ONE_API_CALLS_EXTENDED: {
-            "total_duration": 300,  # ms
-            "concurrency_threshold": 5,  # ms
-            "count": 10,
-            "allowed_span_ops": ["http.client"],
-        },
         DetectorType.M_N_PLUS_ONE_DB: {
             "total_duration_threshold": 100.0,  # ms
             "minimum_occurrences_of_pattern": 3,
@@ -316,43 +310,6 @@ def get_detection_settings(project_id: Optional[int] = None) -> Dict[DetectorTyp
     }
 
 
-# Settings used to test out the effect of lowering default thresholds, on metrics.
-def get_dry_run_detection_settings(project_id: Optional[int] = None) -> Dict[DetectorType, Any]:
-    settings = get_merged_settings(project_id)
-
-    return {
-        DetectorType.N_PLUS_ONE_DB_QUERIES: {
-            "count": settings["n_plus_one_db_count"],
-            "duration_threshold": 50,  # ms
-            "detection_enabled": settings["n_plus_one_db_queries_detection_enabled"],
-        },
-        DetectorType.SLOW_DB_QUERY: [
-            {
-                "duration_threshold": 500,  # ms
-                "allowed_span_ops": ["db"],
-                "detection_enabled": settings["slow_db_queries_detection_enabled"],
-            },
-        ],
-        DetectorType.UNCOMPRESSED_ASSETS: {
-            "size_threshold_bytes": settings["uncompressed_asset_size_threshold"],
-            "duration_threshold": 300,  # ms
-            "allowed_span_ops": ["resource.css", "resource.script"],
-            "detection_enabled": settings["uncompressed_assets_detection_enabled"],
-        },
-        DetectorType.LARGE_HTTP_PAYLOAD: {
-            "payload_size_threshold": 300000,  # in bytes
-            "detection_enabled": settings["large_http_payload_detection_enabled"],
-        },
-        DetectorType.RENDER_BLOCKING_ASSET_SPAN: {
-            "fcp_minimum_threshold": settings["render_blocking_fcp_min"],  # ms
-            "fcp_maximum_threshold": settings["render_blocking_fcp_max"],  # ms
-            "fcp_ratio_threshold": settings["render_blocking_fcp_ratio"],  # in the range [0, 1]
-            "minimum_size_bytes": 500000,  # in bytes
-            "detection_enabled": settings["large_render_blocking_asset_detection_enabled"],
-        },
-    }
-
-
 def _detect_performance_problems(
     data: dict[str, Any], sdk_span: Any, project: Project
 ) -> List[PerformanceProblem]:
@@ -371,7 +328,6 @@ def _detect_performance_problems(
         NPlusOneDBSpanDetectorExtended(detection_settings, data),
         FileIOMainThreadDetector(detection_settings, data),
         NPlusOneAPICallsDetector(detection_settings, data),
-        NPlusOneAPICallsDetectorExtended(detection_settings, data),
         MNPlusOneDBSpanDetector(detection_settings, data),
         UncompressedAssetSpanDetector(detection_settings, data),
         LargeHTTPPayloadDetector(detection_settings, data),
@@ -384,23 +340,6 @@ def _detect_performance_problems(
     # Metrics reporting only for detection, not created issues.
     report_metrics_for_detectors(data, event_id, detectors, sdk_span, project.organization)
 
-    # TODO Abdullah Khan: Remove code after dry run evaluating changes in detection ---------------------
-    detection_dry_run_settings = get_dry_run_detection_settings(project_id)
-    dry_run_detectors: List[PerformanceDetector] = [
-        NPlusOneDBSpanDetector(detection_dry_run_settings, data),
-        UncompressedAssetSpanDetector(detection_dry_run_settings, data),
-        LargeHTTPPayloadDetector(detection_dry_run_settings, data),
-        SlowDBQueryDetector(detection_dry_run_settings, data),
-        RenderBlockingAssetSpanDetector(detection_dry_run_settings, data),
-    ]
-
-    for dry_run_detector in dry_run_detectors:
-        run_detector_on_data(dry_run_detector, data)
-
-    report_metrics_for_detectors(
-        data, event_id, dry_run_detectors, sdk_span, project.organization, True
-    )
-
     organization = cast(Organization, project.organization)
     if project is None or organization is None:
         return []
@@ -455,7 +394,6 @@ def report_metrics_for_detectors(
     detectors: Sequence[PerformanceDetector],
     sdk_span: Any,
     organization: Organization,
-    is_dry_run: bool = False,
 ):
     all_detected_problems = [i for d in detectors for i in d.stored_problems]
     has_detected_problems = bool(all_detected_problems)
@@ -504,7 +442,6 @@ def report_metrics_for_detectors(
     detected_tags = {
         "sdk_name": sdk_name,
         "is_early_adopter": organization.flags.early_adopter.is_set,
-        "is_dry_run": is_dry_run,
     }
 
     event_integrations = event.get("sdk", {}).get("integrations", []) or []

tests/sentry/api/endpoints/test_project_performance_issue_settings.py

@@ -91,6 +91,7 @@ class ProjectPerformanceIssueSettingsTest(APITestCase):
                 "performance.issues.uncompressed_asset.duration_threshold": 300,
                 "performance.issues.uncompressed_asset.size_threshold": 200000,
                 "performance.issues.consecutive_db.min_time_saved_threshold": 300,
+                "performance.issues.n_plus_one_api_calls.total_duration": 300,
             }
         ):
             with self.feature(PERFORMANCE_ISSUE_FEATURES):
@@ -108,6 +109,7 @@ class ProjectPerformanceIssueSettingsTest(APITestCase):
             assert response.data["uncompressed_asset_duration_threshold"] == 300
             assert response.data["uncompressed_asset_size_threshold"] == 200000
             assert response.data["consecutive_db_min_time_saved_threshold"] == 300
+            assert response.data["n_plus_one_api_calls_total_duration_threshold"] == 300
 
             get_value.return_value = {
                 "n_plus_one_db_duration_threshold": 10000,
@@ -119,6 +121,7 @@ class ProjectPerformanceIssueSettingsTest(APITestCase):
                 "db_on_main_thread_duration_threshold": 50,
                 "file_io_on_main_thread_duration_threshold": 33,
                 "consecutive_db_min_time_saved_threshold": 5000,
+                "n_plus_one_api_calls_total_duration_threshold": 500,
             }
 
             with self.feature(PERFORMANCE_ISSUE_FEATURES):
@@ -136,6 +139,7 @@ class ProjectPerformanceIssueSettingsTest(APITestCase):
             assert response.data["db_on_main_thread_duration_threshold"] == 50
             assert response.data["file_io_on_main_thread_duration_threshold"] == 33
             assert response.data["consecutive_db_min_time_saved_threshold"] == 5000
+            assert response.data["n_plus_one_api_calls_total_duration_threshold"] == 500
 
     def test_get_returns_error_without_feature_enabled(self):
         with self.feature({}):

tests/sentry/utils/performance_issues/test_n_plus_one_api_calls_detector.py

@@ -6,7 +6,6 @@ from uuid import uuid4
 import pytest
 
 from sentry.issues.grouptype import PerformanceNPlusOneAPICallsGroupType
-from sentry.models.options.project_option import ProjectOption
 from sentry.testutils import TestCase
 from sentry.testutils.performance_issues.event_generators import (
     create_event,
@@ -39,9 +38,7 @@ class NPlusOneAPICallsDetectorTest(TestCase):
         return list(detector.stored_problems.values())
 
     def create_event(self, description_maker: Callable[[int], str]) -> dict[str, Any]:
-        duration_threshold = (
-            self._settings[DetectorType.N_PLUS_ONE_API_CALLS]["duration_threshold"] + 1
-        )
+        total_duration = self._settings[DetectorType.N_PLUS_ONE_API_CALLS]["total_duration"] + 1
         count = self._settings[DetectorType.N_PLUS_ONE_API_CALLS]["count"] + 1
         hash = uuid4().hex[:16]
 
@@ -49,7 +46,7 @@ class NPlusOneAPICallsDetectorTest(TestCase):
             [
                 create_span(
                     "http.client",
-                    duration_threshold,
+                    total_duration / count,
                     description_maker(i),
                     hash=hash,
                 )
@@ -57,6 +54,21 @@ class NPlusOneAPICallsDetectorTest(TestCase):
             ]
         )
 
+    def create_eligible_spans(self, duration: float, count: int) -> list:
+        spans = []
+
+        for i in range(count):
+            spans.append(
+                create_span(
+                    "http.client",
+                    duration,
+                    f"GET /api/0/organizations/books?book_id={i}",
+                    f"hash{i}",
+                )
+            )
+
+        return spans
+
     def test_detects_problems_with_many_concurrent_calls_to_same_url(self):
         event = get_event("n-plus-one-api-calls/n-plus-one-api-calls-in-issue-stream")
 
@@ -133,45 +145,57 @@ class NPlusOneAPICallsDetectorTest(TestCase):
         ]
         assert problems[0].title == "N+1 API Call"
 
-    def test_does_not_detect_problem_with_unparameterized_urls(self):
-        event = get_event("n-plus-one-api-calls/n-plus-one-api-calls-in-weather-app")
-        assert self.find_problems(event) == []
+    def test_does_not_detect_problems_with_low_total_duration_of_spans(self):
+        event = get_event("n-plus-one-api-calls/n-plus-one-api-calls-in-issue-stream")
+        event["spans"] = self.create_eligible_spans(
+            100, 10
+        )  # total duration is 1s, greater than default
 
-    def test_does_not_detect_problem_with_concurrent_calls_to_different_urls(self):
-        event = get_event("n-plus-one-api-calls/not-n-plus-one-api-calls")
-        assert self.find_problems(event) == []
+        problems = self.find_problems(event)
+        assert len(problems) == 1
+
+        event["spans"] = self.create_eligible_spans(
+            10, 5
+        )  # total duration is 50ms, lower than default
 
-    def test_respects_feature_flag(self):
-        project = self.create_project()
+        problems = self.find_problems(event)
+        assert problems == []
+
+    def test_detects_problems_with_low_span_duration_high_total_duration(self):
         event = get_event("n-plus-one-api-calls/n-plus-one-api-calls-in-issue-stream")
+        event["spans"] = self.create_eligible_spans(100, 10)  # total duration is 1s
 
-        detector = NPlusOneAPICallsDetector(self._settings, event)
+        problems = self.find_problems(event)
+        assert len(problems) == 1
 
-        assert not detector.is_creation_allowed_for_organization(project.organization)
+        event["spans"] = self.create_eligible_spans(10, 50)  # total duration is 500ms
 
-        with self.feature({"organizations:performance-n-plus-one-api-calls-detector": True}):
-            assert detector.is_creation_allowed_for_organization(project.organization)
+        problems = self.find_problems(event)
+        assert len(problems) == 1
 
-    def test_respects_project_option(self):
-        project = self.create_project()
+    def test_does_not_detect_problems_with_low_span_count(self):
         event = get_event("n-plus-one-api-calls/n-plus-one-api-calls-in-issue-stream")
-        event["project_id"] = project.id
-
-        settings = get_detection_settings(project.id)
-        detector = NPlusOneAPICallsDetector(settings, event)
+        event["spans"] = self.create_eligible_spans(
+            1000, self._settings[DetectorType.N_PLUS_ONE_API_CALLS]["count"]
+        )
 
-        assert detector.is_creation_allowed_for_project(project)
+        problems = self.find_problems(event)
+        assert len(problems) == 1
 
-        ProjectOption.objects.set_value(
-            project=project,
-            key="sentry:performance_issue_settings",
-            value={"n_plus_one_api_calls_detection_enabled": False},
+        event["spans"] = self.create_eligible_spans(
+            1000, self._settings[DetectorType.N_PLUS_ONE_API_CALLS]["count"] - 1
         )
 
-        settings = get_detection_settings(project.id)
-        detector = NPlusOneAPICallsDetector(settings, event)
+        problems = self.find_problems(event)
+        assert problems == []
+
+    def test_does_not_detect_problem_with_unparameterized_urls(self):
+        event = get_event("n-plus-one-api-calls/n-plus-one-api-calls-in-weather-app")
+        assert self.find_problems(event) == []
 
-        assert not detector.is_creation_allowed_for_project(project)
+    def test_does_not_detect_problem_with_concurrent_calls_to_different_urls(self):
+        event = get_event("n-plus-one-api-calls/not-n-plus-one-api-calls")
+        assert self.find_problems(event) == []
 
     def test_fingerprints_events(self):
         event = self.create_event(lambda i: "GET /clients/11/info")

tests/sentry/utils/performance_issues/test_n_plus_one_api_calls_detector_extended.py

@@ -1,468 +0,0 @@
-from __future__ import annotations
-
-from typing import Any, Callable
-from uuid import uuid4
-
-import pytest
-
-from sentry.issues.grouptype import PerformanceNPlusOneAPICallsGroupType
-from sentry.testutils import TestCase
-from sentry.testutils.performance_issues.event_generators import (
-    create_event,
-    create_span,
-    get_event,
-)
-from sentry.testutils.silo import region_silo_test
-from sentry.utils.performance_issues.base import DetectorType, parameterize_url
-from sentry.utils.performance_issues.detectors.n_plus_one_api_calls_detector import (
-    NPlusOneAPICallsDetectorExtended,
-    without_query_params,
-)
-from sentry.utils.performance_issues.performance_detection import (
-    get_detection_settings,
-    run_detector_on_data,
-)
-from sentry.utils.performance_issues.performance_problem import PerformanceProblem
-
-
-@region_silo_test
-@pytest.mark.django_db
-class NPlusOneAPICallsDetectorExtendedTest(TestCase):
-    def setUp(self):
-        super().setUp()
-        self._settings = get_detection_settings()
-
-    def find_problems(self, event: dict[str, Any]) -> list[PerformanceProblem]:
-        detector = NPlusOneAPICallsDetectorExtended(self._settings, event)
-        run_detector_on_data(detector, event)
-        return list(detector.stored_problems.values())
-
-    def create_event(self, description_maker: Callable[[int], str]) -> dict[str, Any]:
-        duration_threshold = (
-            self._settings[DetectorType.N_PLUS_ONE_API_CALLS]["duration_threshold"] + 1
-        )
-        count = self._settings[DetectorType.N_PLUS_ONE_API_CALLS]["count"] + 1
-        hash = uuid4().hex[:16]
-
-        return create_event(
-            [
-                create_span(
-                    "http.client",
-                    duration_threshold,
-                    description_maker(i),
-                    hash=hash,
-                )
-                for i in range(count)
-            ]
-        )
-
-    def create_eligible_spans(self, duration: float, count: int) -> list:
-        spans = []
-
-        for i in range(count):
-            spans.append(
-                create_span(
-                    "http.client",
-                    duration,
-                    f"GET /api/0/organizations/books?book_id={i}",
-                    f"hash{i}",
-                )
-            )
-
-        return spans
-
-    def test_detects_problems_with_many_concurrent_calls_to_same_url(self):
-        event = get_event("n-plus-one-api-calls/n-plus-one-api-calls-in-issue-stream")
-
-        problems = self.find_problems(event)
-        assert self.find_problems(event) == [
-            PerformanceProblem(
-                fingerprint="1-1010-d750ce46bb1b13dd5780aac48098d5e20eea682c",
-                op="http.client",
-                type=PerformanceNPlusOneAPICallsGroupType,
-                desc="GET /api/0/organizations/sentry/events/?field=replayId&field=count%28%29&per_page=50&query=issue.id%3A",
-                parent_span_ids=["a0c39078d1570b00"],
-                cause_span_ids=[],
-                offender_span_ids=[
-                    "ba198ace55bdb20f",
-                    "8a20c71faa0fb6a7",
-                    "9269c825d935b33a",
-                    "9ea82f759505e0f3",
-                    "8c55019639e94ab3",
-                    "9b86746e9cc7fbf0",
-                    "806aa31fe1874495",
-                    "bf409b62d9c30197",
-                    "896ac7d28addb37f",
-                    "9c859aeaf6bfaea9",
-                    "950d8f569bbe3d9e",
-                    "b19a2811b457e87a",
-                    "b566d4ce5b46d4f0",
-                    "b33e9da4441a4800",
-                    "8b68818410aa45d8",
-                    "8ac4e73b53fc2077",
-                    "9fe4a1aff019e39e",
-                    "b29cd0c0cd85ae85",
-                    "b3ff0062caa3ea51",
-                    "a3fde2e38a66cc2c",
-                    "b78802cd80762f57",
-                    "9e2ea4d33b1c1bc6",
-                    "bb827dc7a11085f4",
-                    "a34089b08b6d0646",
-                    "950801c0d7576650",
-                ],
-                evidence_data={
-                    "op": "http.client",
-                    "parent_span_ids": ["a0c39078d1570b00"],
-                    "cause_span_ids": [],
-                    "offender_span_ids": [
-                        "ba198ace55bdb20f",
-                        "8a20c71faa0fb6a7",
-                        "9269c825d935b33a",
-                        "9ea82f759505e0f3",
-                        "8c55019639e94ab3",
-                        "9b86746e9cc7fbf0",
-                        "806aa31fe1874495",
-                        "bf409b62d9c30197",
-                        "896ac7d28addb37f",
-                        "9c859aeaf6bfaea9",
-                        "950d8f569bbe3d9e",
-                        "b19a2811b457e87a",
-                        "b566d4ce5b46d4f0",
-                        "b33e9da4441a4800",
-                        "8b68818410aa45d8",
-                        "8ac4e73b53fc2077",
-                        "9fe4a1aff019e39e",
-                        "b29cd0c0cd85ae85",
-                        "b3ff0062caa3ea51",
-                        "a3fde2e38a66cc2c",
-                        "b78802cd80762f57",
-                        "9e2ea4d33b1c1bc6",
-                        "bb827dc7a11085f4",
-                        "a34089b08b6d0646",
-                        "950801c0d7576650",
-                    ],
-                },
-                evidence_display=[],
-            )
-        ]
-        assert problems[0].title == "N+1 API Call"
-
-    def test_does_not_detect_problems_with_low_total_duration_of_spans(self):
-        event = get_event("n-plus-one-api-calls/n-plus-one-api-calls-in-issue-stream")
-        event["spans"] = self.create_eligible_spans(
-            100, 10
-        )  # total duration is 1s, greater than default
-
-        problems = self.find_problems(event)
-        assert len(problems) == 1
-
-        event["spans"] = self.create_eligible_spans(
-            10, 5
-        )  # total duration is 50ms, lower than default
-
-        problems = self.find_problems(event)
-        assert problems == []
-
-    def test_detects_problems_with_low_span_duration_high_total_duration(self):
-        event = get_event("n-plus-one-api-calls/n-plus-one-api-calls-in-issue-stream")
-        event["spans"] = self.create_eligible_spans(100, 10)  # total duration is 1s
-
-        problems = self.find_problems(event)
-        assert len(problems) == 1
-
-        event["spans"] = self.create_eligible_spans(10, 50)  # total duration is 500ms
-
-        problems = self.find_problems(event)
-        assert len(problems) == 1
-
-    def test_does_not_detect_problems_with_low_span_count(self):
-        event = get_event("n-plus-one-api-calls/n-plus-one-api-calls-in-issue-stream")
-        event["spans"] = self.create_eligible_spans(
-            1000, self._settings[DetectorType.N_PLUS_ONE_API_CALLS_EXTENDED]["count"]
-        )
-
-        problems = self.find_problems(event)
-        assert len(problems) == 1
-
-        event["spans"] = self.create_eligible_spans(
-            1000, self._settings[DetectorType.N_PLUS_ONE_API_CALLS_EXTENDED]["count"] - 1
-        )
-
-        problems = self.find_problems(event)
-        assert problems == []
-
-    def test_does_not_detect_problem_with_unparameterized_urls(self):
-        event = get_event("n-plus-one-api-calls/n-plus-one-api-calls-in-weather-app")
-        assert self.find_problems(event) == []
-
-    def test_does_not_detect_problem_with_concurrent_calls_to_different_urls(self):
-        event = get_event("n-plus-one-api-calls/not-n-plus-one-api-calls")
-        assert self.find_problems(event) == []
-
-    def test_fingerprints_events(self):
-        event = self.create_event(lambda i: "GET /clients/11/info")
-        [problem] = self.find_problems(event)
-
-        assert problem.fingerprint == "1-1010-e9daac10ea509a0bf84a8b8da45d36394868ad67"
-
-    def test_fingerprints_identical_relative_urls_together(self):
-        event1 = self.create_event(lambda i: "GET /clients/11/info")
-        [problem1] = self.find_problems(event1)
-
-        event2 = self.create_event(lambda i: "GET /clients/11/info")
-        [problem2] = self.find_problems(event2)
-
-        assert problem1.fingerprint == problem2.fingerprint
-
-    def test_fingerprints_same_relative_urls_together(self):
-        event1 = self.create_event(lambda i: f"GET /clients/42/info?id={i}")
-        [problem1] = self.find_problems(event1)
-
-        event2 = self.create_event(lambda i: f"GET /clients/42/info?id={i*2}")
-        [problem2] = self.find_problems(event2)
-
-        assert problem1.fingerprint == problem2.fingerprint
-
-    def test_fingerprints_same_parameterized_integer_relative_urls_together(self):
-        event1 = self.create_event(lambda i: f"GET /clients/17/info?id={i}")
-        [problem1] = self.find_problems(event1)
-
-        event2 = self.create_event(lambda i: f"GET /clients/16/info?id={i*2}")
-        [problem2] = self.find_problems(event2)
-
-        assert problem1.fingerprint == problem2.fingerprint
-
-    def test_fingerprints_different_relative_url_separately(self):
-        event1 = self.create_event(lambda i: f"GET /clients/11/info?id={i}")
-        [problem1] = self.find_problems(event1)
-
-        event2 = self.create_event(lambda i: f"GET /projects/11/details?pid={i}")
-        [problem2] = self.find_problems(event2)
-
-        assert problem1.fingerprint != problem2.fingerprint
-
-    def test_ignores_hostname_for_fingerprinting(self):
-        event1 = self.create_event(lambda i: f"GET http://service.io/clients/42/info?id={i}")
-        [problem1] = self.find_problems(event1)
-
-        event2 = self.create_event(lambda i: f"GET /clients/42/info?id={i}")
-        [problem2] = self.find_problems(event2)
-
-        assert problem1.fingerprint == problem2.fingerprint
-
-
-@pytest.mark.parametrize(
-    "url,parameterized_url",
-    [
-        (
-            "",
-            "",
-        ),
-        (
-            "http://service.io",
-            "http://service.io",
-        ),
-        (
-            "https://www.service.io/resources/11",
-            "https://www.service.io/resources/*",
-        ),
-        (
-            "https://www.service.io/resources/11/details",
-            "https://www.service.io/resources/*/details",
-        ),
-        (
-            "https://www.service.io/resources/11/details?id=1&sort=down",
-            "https://www.service.io/resources/*/details?id=*&sort=*",
-        ),
-        (
-            "https://www.service.io/resources/11/details?sort=down&id=1",
-            "https://www.service.io/resources/*/details?id=*&sort=*",
-        ),
-        (
-            "https://service.io/clients/somecord/details?id=17",
-            "https://service.io/clients/somecord/details?id=*",
-        ),
-        (
-            "/clients/11/project/1343",
-            "/clients/*/project/*",
-        ),
-        (
-            "/clients/11/project/1343-turtles",
-            "/clients/*/project/*",
-        ),
-        (
-            "/clients/11/project/1343turtles",
-            "/clients/*/project/1343turtles",
-        ),
-        (
-            "/clients/563712f9722fb0996ac8f3905b40786f/project/1343",  # md5
-            "/clients/*/project/*",
-        ),
-        (
-            "/clients/563712f9722fb0996z/project/",  # md5-like
-            "/clients/563712f9722fb0996z/project/",
-        ),
-        (
-            "/clients/403926033d001b5279df37cbbe5287b7c7c267fa/project/1343",  # sha1
-            "/clients/*/project/*",
-        ),
-        (
-            "/clients/8ff81d74-606d-4c75-ac5e-cee65cbbc866/project/1343",  # uuid
-            "/clients/*/project/*",
-        ),
-        (
-            "/clients/hello-123s/project/1343",  # uuid-like
-            "/clients/hello-123s/project/*",
-        ),
-        (
-            "/item/5c9b9b609c172be2a013f534/details",  # short hash
-            "/item/*/details",
-        ),
-        (
-            "/item/be9a25322d/details",  # shorter short hash
-            "/item/*/details",
-        ),
-        (
-            "/item/defaced12/details",  # false short hash
-            "/item/defaced12/details",
-        ),
-        (
-            "/item/defaced12-abba/details",  # false short hash 2
-            "/item/defaced12-abba/details",
-        ),
-    ],
-)
-def test_parameterizes_url(url, parameterized_url):
-    r = parameterize_url(url)
-    assert r == parameterized_url
-
-
-@pytest.mark.parametrize(
-    "span",
-    [
-        {
-            "span_id": "a",
-            "op": "http.client",
-            "hash": "b",
-            "description": "GET http://service.io/resource",
-        },
-        {
-            "span_id": "a",
-            "op": "http.client",
-            "description": "GET http://service.io/resource",
-            "hash": "a",
-            "data": {
-                "url": "/resource",
-            },
-        },
-        {
-            "span_id": "a",
-            "op": "http.client",
-            "description": "GET http://service.io/resource",
-            "hash": "a",
-            "data": {
-                "url": {
-                    "pathname": "/resource",
-                }
-            },
-        },
-        {
-            "span_id": "a",
-            "op": "http.client",
-            "description": "GET http://service.io/resource.json?param=something",
-            "hash": "a",
-        },
-    ],
-)
-def test_allows_eligible_spans(span):
-    assert NPlusOneAPICallsDetectorExtended.is_span_eligible(span)
-
-
-@pytest.mark.parametrize(
-    "span",
-    [
-        {"span_id": "a", "op": None},
-        {"op": "http.client"},
-        {
-            "span_id": "a",
-            "op": "http.client",
-            "hash": "a",
-            "description": "POST http://service.io/resource",
-        },
-        {
-            "span_id": "a",
-            "op": "http.client",
-            "description": "GET http://service.io/resource.js",
-            "hash": "a",
-        },
-        {
-            "span_id": "a",
-            "op": "http.client",
-            "description": "GET /resource.js",
-            "hash": "a",
-            "data": {"url": "/resource.js"},
-        },
-        {
-            "span_id": "a",
-            "op": "http.client",
-            "description": "GET http://service.io/resource?graphql=somequery",
-            "hash": "a",
-        },
-        {
-            "span_id": "a",
-            "op": "http.client",
-            "description": "GET http://service.io/resource",  # New JS SDK removes query string from description
-            "hash": "a",
-            "data": {
-                "http.query": "graphql=somequery",
-                "url": "http://service.io/resource",
-            },
-        },
-        {
-            "span_id": "a",
-            "op": "http.client",
-            "hash": "b",
-            "description": "GET /_next/data/LjdprRSkUtLP0bMUoWLur/items.json?collection=hello",
-        },
-        {
-            "span_id": "a",
-            "op": "http.client",
-            "hash": "b",
-            "description": "GET /__nextjs_original-stack-frame?isServerSide=false&file=webpack-internal%3A%2F%2F%2F.%2Fnode_modules%2Freact-dom%2Fcjs%2Freact-dom.development.js&methodName=Object.invokeGuardedCallbackDev&arguments=&lineNumber=73&column=3`",
-        },
-    ],
-)
-def test_rejects_ineligible_spans(span):
-    assert not NPlusOneAPICallsDetectorExtended.is_span_eligible(span)
-
-
-@pytest.mark.parametrize(
-    "url,url_without_query",
-    [
-        ("", ""),
-        ("http://service.io", "http://service.io"),
-        ("http://service.io/resource", "http://service.io/resource"),
-        ("/resource?id=1", "/resource"),
-        ("/resource?id=1&sort=down", "/resource"),
-    ],
-)
-def test_removes_query_params(url, url_without_query):
-    assert without_query_params(url) == url_without_query
-
-
-@pytest.mark.parametrize(
-    "event",
-    [get_event("n-plus-one-api-calls/not-n-plus-one-api-calls")],
-)
-def test_allows_eligible_events(event):
-    assert NPlusOneAPICallsDetectorExtended.is_event_eligible(event)
-
-
-@pytest.mark.parametrize(
-    "event",
-    [
-        {"contexts": {"trace": {"op": "task"}}},
-    ],
-)
-def test_rejects_ineligible_events(event):
-    assert not NPlusOneAPICallsDetectorExtended.is_event_eligible(event)

tests/sentry/utils/performance_issues/test_performance_detection.py

@@ -191,9 +191,11 @@ class PerformanceDetectionTest(TestCase):
         assert (
             default_settings[DetectorType.UNCOMPRESSED_ASSETS]["size_threshold_bytes"] == 500 * 1024
         )
-        assert default_settings[DetectorType.UNCOMPRESSED_ASSETS]["duration_threshold"] == 500
+        assert default_settings[DetectorType.UNCOMPRESSED_ASSETS]["duration_threshold"] == 300
         assert default_settings[DetectorType.CONSECUTIVE_DB_OP]["min_time_saved"] == 100
         assert default_settings[DetectorType.SLOW_DB_QUERY][0]["duration_threshold"] == 1000
+        assert default_settings[DetectorType.N_PLUS_ONE_API_CALLS]["total_duration"] == 300
+        assert default_settings[DetectorType.LARGE_HTTP_PAYLOAD]["payload_size_threshold"] == 300000
 
         self.project_option_mock.return_value = {
             "n_plus_one_db_duration_threshold": 100000,
@@ -205,6 +207,7 @@ class PerformanceDetectionTest(TestCase):
             "db_on_main_thread_duration_threshold": 50,
             "file_io_on_main_thread_duration_threshold": 33,
             "consecutive_db_min_time_saved_threshold": 500,
+            "n_plus_one_api_calls_total_duration_threshold": 500,
         }
 
         configured_settings = get_detection_settings(self.project)
@@ -212,6 +215,7 @@ class PerformanceDetectionTest(TestCase):
         assert (
             configured_settings[DetectorType.N_PLUS_ONE_DB_QUERIES]["duration_threshold"] == 100000
         )
+        assert configured_settings[DetectorType.N_PLUS_ONE_API_CALLS]["total_duration"] == 500
         assert configured_settings[DetectorType.SLOW_DB_QUERY][0]["duration_threshold"] == 5000
         assert (
             configured_settings[DetectorType.RENDER_BLOCKING_ASSET_SPAN]["fcp_ratio_threshold"]
@@ -413,7 +417,7 @@ class PerformanceDetectionTest(TestCase):
 
         perf_problems = _detect_performance_problems(n_plus_one_event, sdk_span_mock, self.project)
 
-        assert sdk_span_mock.containing_transaction.set_tag.call_count == 12
+        assert sdk_span_mock.containing_transaction.set_tag.call_count == 7
         sdk_span_mock.containing_transaction.set_tag.assert_has_calls(
             [
                 call(
@@ -472,7 +476,7 @@ class PerformanceDetectionTest(TestCase):
             for call in incr_mock.mock_calls
             if call.args[0] == "performance.performance_issue.detected"
         ]
-        assert len(detection_calls) == 2
+        assert len(detection_calls) == 1
         tags = detection_calls[0].kwargs["tags"]
 
         assert tags["uncompressed_assets"]

tests/sentry/utils/performance_issues/test_render_blocking_asset_detector.py

@@ -143,7 +143,7 @@ class RenderBlockingAssetDetectorTest(TestCase):
         event = _valid_render_blocking_asset_event("https://example.com/a.js")
         for span in event["spans"]:
             if span["op"] == "resource.script":
-                span["data"]["http.response_content_length"] = 900000
+                span["data"]["http.response_content_length"] = 400000
         assert self.find_problems(event) == []
 
     def test_does_not_detect_if_missing_size(self):