Browse Source

feat(perf): Improve N+1 API Calls detector (#37984)

* Remove `http.server` from detection

We're only interested in `http.client` calls for now. This is a pretty
specific application.

* Update N+1 Detector

- add a similarity condition: must have same hash
- reduce concurrency condition from 5 to 3
George Gritsouk 2 years ago
parent
commit
09f0c48e88

+ 15 - 8
src/sentry/utils/performance_issues/performance_detection.py

@@ -95,9 +95,9 @@ def get_default_detection_settings():
         },
         DetectorType.N_PLUS_ONE_SPANS: [
             {
-                "count": 5,
+                "count": 3,
                 "start_time_threshold": 5.0,  # ms
-                "allowed_span_ops": ["http.client", "http.server"],
+                "allowed_span_ops": ["http.client"],
             }
         ],
     }
@@ -509,6 +509,7 @@ class NPlusOneSpanDetector(PerformanceDetector):
     def init(self):
         self.spans_involved = {}
         self.most_recent_start_time = {}
+        self.most_recent_hash = {}
         self.stored_issues = {}
 
     def visit_span(self, span: Span):
@@ -525,25 +526,31 @@ class NPlusOneSpanDetector(PerformanceDetector):
         if not fingerprint:
             return
 
+        hash = span.get("hash", None)
+        if not hash:
+            return
+
         if fingerprint not in self.spans_involved:
             self.spans_involved[fingerprint] = []
             self.most_recent_start_time[fingerprint] = 0
+            self.most_recent_hash[fingerprint] = ""
 
         delta_to_previous_span_start_time = timedelta(
             seconds=(span["start_timestamp"] - self.most_recent_start_time[fingerprint])
         )
 
+        is_concurrent_with_previous_span = delta_to_previous_span_start_time < start_time_threshold
+        has_same_hash_as_previous_span = span["hash"] == self.most_recent_hash[fingerprint]
+
         self.most_recent_start_time[fingerprint] = span["start_timestamp"]
+        self.most_recent_hash[fingerprint] = hash
 
-        if delta_to_previous_span_start_time >= start_time_threshold:
-            # This span is subsequent to the most recent span
+        if is_concurrent_with_previous_span and has_same_hash_as_previous_span:
+            self.spans_involved[fingerprint].append(span)
+        else:
             self.spans_involved[fingerprint] = [span_id]
             return
 
-        else:
-            # This span is approximately concurrent with the most recent span
-            self.spans_involved[fingerprint].append(span)
-
         if not self.stored_issues.get(fingerprint, False):
             if len(self.spans_involved[fingerprint]) >= count:
                 self.stored_issues[fingerprint] = PerformanceSpanIssue(

+ 12 - 10
tests/sentry/utils/performance_issues/test_performance_detection.py

@@ -191,20 +191,22 @@ class PerformanceDetectionTest(unittest.TestCase):
 
     def test_calls_n_plus_one_spans_calls(self):
         # ├── GET list.json
-        # │   ├── GET 1.json
-        # │   ├──  GET 2.json
-        # │   ├──   GET 3.json
-        # │   ├──    GET 4.json
-        # │   └──      GET 5.json
+        # │   ├── GET /events.json?q=1
+        # │   ├──  GET /events.json?q=2
+        # │   ├──   GET /events.json?q=3
 
         n_plus_one_event = create_event(
             [
                 create_span("http.client", 250, "GET /list.json"),
-                modify_span_start(create_span("http.client", 180, "GET /1.json"), 101),
-                modify_span_start(create_span("http.client", 178, "GET /2.json"), 105),
-                modify_span_start(create_span("http.client", 163, "GET /3.json"), 109),
-                modify_span_start(create_span("http.client", 152, "GET /4.json"), 113),
-                modify_span_start(create_span("http.client", 191, "GET /5.json"), 116),
+                modify_span_start(
+                    create_span("http.client", 180, "GET /events.json?q=1", "c0c0c0c0"), 101
+                ),
+                modify_span_start(
+                    create_span("http.client", 178, "GET /events.json?q=2", "c0c0c0c0"), 105
+                ),
+                modify_span_start(
+                    create_span("http.client", 163, "GET /events.json?q=3", "c0c0c0c0"), 109
+                ),
             ]
         )