Skip to content

Commit 92c3834

Browse files
Abdkhan14Abdullah Khan
authored and
chloeho7
committed
feat(perf-detector-threshold-configuration) Added new thresholds and … (#53460)
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 and adding new thresholds. - Changed default threshold for Consecutive HTTP to 500ms to match prod sentry_option value: [query](https://redash.getsentry.net/queries/4546/source) - Added lowered defaults for N+1 DB duration (iterating down to 50ms), Slow DB Queries duration(iterating down to 500ms), Consecutive http span duration (iterating down to 500ms) - Updated Consecutive HTTP Detector to add new total min time saved threshold with a default of 2000s. - Updated Consecutive HTTP Detector tests to ensure new total duration threshold is respected. - Removed Extended Consecutive HTTP Detector and tests. --------- Co-authored-by: Abdullah Khan <[email protected]>
1 parent 00e98d8 commit 92c3834

10 files changed

+219
-435
lines changed

Diff for: ‎src/sentry/api/endpoints/project_performance_issue_settings.py

+5
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ class ConfigurableThresholds(Enum):
5858
RENDER_BLOCKING_ASSET_FCP_RATIO = "render_blocking_fcp_ratio"
5959
SLOW_DB_QUERY_DURATION = "slow_db_query_duration_threshold"
6060
N_PLUS_API_CALLS_DURATION = "n_plus_one_api_calls_total_duration_threshold"
61+
CONSECUTIVE_HTTP_SPANS_MIN_TIME_SAVED = "consecutive_http_spans_min_time_saved_threshold"
6162

6263

6364
internal_only_project_settings_to_group_map: Dict[str, Type[GroupType]] = {
@@ -84,6 +85,7 @@ class ConfigurableThresholds(Enum):
8485
ConfigurableThresholds.RENDER_BLOCKING_ASSET_FCP_RATIO.value: InternalProjectOptions.RENDER_BLOCKING_ASSET.value,
8586
ConfigurableThresholds.SLOW_DB_QUERY_DURATION.value: InternalProjectOptions.SLOW_DB_QUERY.value,
8687
ConfigurableThresholds.N_PLUS_API_CALLS_DURATION.value: InternalProjectOptions.N_PLUS_ONE_API_CALLS.value,
88+
ConfigurableThresholds.CONSECUTIVE_HTTP_SPANS_MIN_TIME_SAVED.value: InternalProjectOptions.CONSECUTIVE_HTTP_SPANS.value,
8789
}
8890

8991

@@ -125,6 +127,9 @@ class ProjectPerformanceIssueSettingsSerializer(serializers.Serializer):
125127
n_plus_one_api_calls_total_duration_threshold = serializers.IntegerField(
126128
required=False, min_value=100, max_value=TEN_SECONDS # ms
127129
)
130+
consecutive_http_spans_min_time_saved_threshold = serializers.IntegerField(
131+
required=False, min_value=1000, max_value=TEN_SECONDS # ms
132+
)
128133
uncompressed_assets_detection_enabled = serializers.BooleanField(required=False)
129134
consecutive_http_spans_detection_enabled = serializers.BooleanField(required=False)
130135
large_http_payload_detection_enabled = serializers.BooleanField(required=False)

Diff for: ‎src/sentry/options/defaults.py

+9-4
Original file line numberDiff line numberDiff line change
@@ -1229,12 +1229,12 @@
12291229
)
12301230
register(
12311231
"performance.issues.n_plus_one_db.duration_threshold",
1232-
default=100.0,
1232+
default=90.0,
12331233
flags=FLAG_AUTOMATOR_MODIFIABLE,
12341234
)
12351235
register(
12361236
"performance.issues.slow_db_query.duration_threshold",
1237-
default=1000.0, # ms
1237+
default=900.0, # ms
12381238
flags=FLAG_AUTOMATOR_MODIFIABLE,
12391239
)
12401240
register(
@@ -1259,7 +1259,7 @@
12591259
)
12601260
register(
12611261
"performance.issues.consecutive_http.max_duration_between_spans",
1262-
default=500,
1262+
default=500, # ms
12631263
flags=FLAG_AUTOMATOR_MODIFIABLE,
12641264
)
12651265
register(
@@ -1269,7 +1269,12 @@
12691269
)
12701270
register(
12711271
"performance.issues.consecutive_http.span_duration_threshold",
1272-
default=1000,
1272+
default=900, # ms
1273+
flags=FLAG_AUTOMATOR_MODIFIABLE,
1274+
)
1275+
register(
1276+
"performance.issues.consecutive_http.min_time_saved_threshold",
1277+
default=2000, # ms
12731278
flags=FLAG_AUTOMATOR_MODIFIABLE,
12741279
)
12751280
register(

Diff for: ‎src/sentry/utils/performance_issues/base.py

-2
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ class DetectorType(Enum):
3737
N_PLUS_ONE_API_CALLS = "n_plus_one_api_calls"
3838
CONSECUTIVE_DB_OP = "consecutive_db"
3939
CONSECUTIVE_HTTP_OP = "consecutive_http"
40-
CONSECUTIVE_HTTP_OP_EXTENDED = "consecutive_http_ext"
4140
LARGE_HTTP_PAYLOAD = "large_http_payload"
4241
FILE_IO_MAIN_THREAD = "file_io_main_thread"
4342
M_N_PLUS_ONE_DB = "m_n_plus_one_db"
@@ -57,7 +56,6 @@ class DetectorType(Enum):
5756
DetectorType.M_N_PLUS_ONE_DB: PerformanceMNPlusOneDBQueriesGroupType,
5857
DetectorType.UNCOMPRESSED_ASSETS: PerformanceUncompressedAssetsGroupType,
5958
DetectorType.CONSECUTIVE_HTTP_OP: PerformanceConsecutiveHTTPQueriesGroupType,
60-
DetectorType.CONSECUTIVE_HTTP_OP_EXTENDED: PerformanceConsecutiveHTTPQueriesGroupType,
6159
DetectorType.DB_MAIN_THREAD: PerformanceDBMainThreadGroupType,
6260
DetectorType.LARGE_HTTP_PAYLOAD: PerformanceLargeHTTPPayloadGroupType,
6361
DetectorType.HTTP_OVERHEAD: PerformanceHTTPOverheadGroupType,

Diff for: ‎src/sentry/utils/performance_issues/detectors/consecutive_http_detector.py

+17-76
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,10 @@ def visit_span(self, span: Span) -> None:
5050
if not span_id or not self._is_eligible_http_span(span):
5151
return
5252

53+
span_duration = get_span_duration(span).total_seconds() * 1000
54+
if span_duration < self.settings.get("span_duration_threshold"):
55+
return
56+
5357
if self._overlaps_last_span(span):
5458
self._validate_and_store_performance_problem()
5559
self._reset_variables()
@@ -63,11 +67,12 @@ def _validate_and_store_performance_problem(self):
6367
exceeds_count_threshold = len(self.consecutive_http_spans) >= self.settings.get(
6468
"consecutive_count_threshold"
6569
)
66-
exceeds_span_duration_threshold = all(
67-
get_span_duration(span).total_seconds() * 1000
68-
> self.settings.get("span_duration_threshold")
69-
for span in self.consecutive_http_spans
70-
)
70+
71+
exceeds_min_time_saved_duration = False
72+
if self.consecutive_http_spans:
73+
exceeds_min_time_saved_duration = self._calculate_time_saved() >= self.settings.get(
74+
"min_time_saved"
75+
)
7176

7277
subceeds_duration_between_spans_threshold = all(
7378
get_duration_between_spans(
@@ -79,11 +84,17 @@ def _validate_and_store_performance_problem(self):
7984

8085
if (
8186
exceeds_count_threshold
82-
and exceeds_span_duration_threshold
8387
and subceeds_duration_between_spans_threshold
88+
and exceeds_min_time_saved_duration
8489
):
8590
self._store_performance_problem()
8691

92+
def _calculate_time_saved(self) -> float:
93+
total_time = get_total_span_duration(self.consecutive_http_spans)
94+
max_span_duration = get_max_span_duration(self.consecutive_http_spans)
95+
96+
return total_time - max_span_duration
97+
8798
def _store_performance_problem(self) -> None:
8899
fingerprint = self._fingerprint()
89100
offender_span_ids = [span.get("span_id", None) for span in self.consecutive_http_spans]
@@ -166,73 +177,3 @@ def is_creation_allowed_for_organization(self, organization: Organization) -> bo
166177

167178
def is_creation_allowed_for_project(self, project: Project) -> bool:
168179
return self.settings["detection_enabled"]
169-
170-
171-
class ConsecutiveHTTPSpanDetectorExtended(ConsecutiveHTTPSpanDetector):
172-
"""
173-
Detector goals:
174-
- Extend Consecutive HTTP Span Detector to mimic detection using thresholds from
175-
- Consecutive DB Queries Detector.
176-
"""
177-
178-
type = DetectorType.CONSECUTIVE_HTTP_OP_EXTENDED
179-
settings_key = DetectorType.CONSECUTIVE_HTTP_OP_EXTENDED
180-
181-
def visit_span(self, span: Span) -> None:
182-
if is_event_from_browser_javascript_sdk(self.event()):
183-
return
184-
185-
span_id = span.get("span_id", None)
186-
187-
if not span_id or not self._is_eligible_http_span(span):
188-
return
189-
190-
span_duration = get_span_duration(span).total_seconds() * 1000
191-
if span_duration < self.settings.get("span_duration_threshold"):
192-
return
193-
194-
if self._overlaps_last_span(span):
195-
self._validate_and_store_performance_problem()
196-
self._reset_variables()
197-
198-
self._add_problem_span(span)
199-
200-
def _validate_and_store_performance_problem(self):
201-
exceeds_count_threshold = len(self.consecutive_http_spans) >= self.settings.get(
202-
"consecutive_count_threshold"
203-
)
204-
205-
exceeds_min_time_saved_duration = False
206-
if self.consecutive_http_spans:
207-
exceeds_min_time_saved_duration = self._calculate_time_saved() >= self.settings.get(
208-
"min_time_saved"
209-
)
210-
211-
subceeds_duration_between_spans_threshold = all(
212-
get_duration_between_spans(
213-
self.consecutive_http_spans[idx - 1], self.consecutive_http_spans[idx]
214-
)
215-
< self.settings.get("max_duration_between_spans")
216-
for idx in range(1, len(self.consecutive_http_spans))
217-
)
218-
219-
if (
220-
exceeds_count_threshold
221-
and subceeds_duration_between_spans_threshold
222-
and exceeds_min_time_saved_duration
223-
):
224-
self._store_performance_problem()
225-
226-
def _calculate_time_saved(self) -> float:
227-
total_time = get_total_span_duration(self.consecutive_http_spans)
228-
max_span_duration = get_max_span_duration(self.consecutive_http_spans)
229-
230-
return total_time - max_span_duration
231-
232-
def is_creation_allowed_for_organization(self, organization: Organization) -> bool:
233-
# Only collecting metrics.
234-
return False
235-
236-
def is_creation_allowed_for_project(self, project: Project) -> bool:
237-
# Only collecting metrics.
238-
return False

Diff for: ‎src/sentry/utils/performance_issues/performance_detection.py

+4-10
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,6 @@
1414
from sentry.utils import metrics
1515
from sentry.utils.event import is_event_from_browser_javascript_sdk
1616
from sentry.utils.event_frames import get_sdk_name
17-
from sentry.utils.performance_issues.detectors.consecutive_http_detector import (
18-
ConsecutiveHTTPSpanDetectorExtended,
19-
)
2017
from sentry.utils.safe import get_path
2118

2219
from .base import DetectorType, PerformanceDetector
@@ -159,6 +156,9 @@ def get_merged_settings(project_id: Optional[int] = None) -> Dict[str | Any, Any
159156
"consecutive_http_spans_span_duration_threshold": options.get(
160157
"performance.issues.consecutive_http.span_duration_threshold"
161158
),
159+
"consecutive_http_spans_min_time_saved_threshold": options.get(
160+
"performance.issues.consecutive_http.min_time_saved_threshold"
161+
),
162162
"large_http_payload_size_threshold": options.get(
163163
"performance.issues.large_http_payload.size_threshold"
164164
),
@@ -287,18 +287,13 @@ def get_detection_settings(project_id: Optional[int] = None) -> Dict[DetectorTyp
287287
"span_duration_threshold": settings[
288288
"consecutive_http_spans_span_duration_threshold"
289289
], # ms
290+
"min_time_saved": settings["consecutive_http_spans_min_time_saved_threshold"], # ms
290291
"consecutive_count_threshold": settings["consecutive_http_spans_count_threshold"],
291292
"max_duration_between_spans": settings[
292293
"consecutive_http_spans_max_duration_between_spans"
293294
], # ms
294295
"detection_enabled": settings["consecutive_http_spans_detection_enabled"],
295296
},
296-
DetectorType.CONSECUTIVE_HTTP_OP_EXTENDED: {
297-
"span_duration_threshold": 500, # ms
298-
"min_time_saved": 2000, # time saved by running all queries in parallel
299-
"consecutive_count_threshold": 3,
300-
"max_duration_between_spans": 1000, # ms
301-
},
302297
DetectorType.LARGE_HTTP_PAYLOAD: {
303298
"payload_size_threshold": settings["large_http_payload_size_threshold"],
304299
"detection_enabled": settings["large_http_payload_detection_enabled"],
@@ -320,7 +315,6 @@ def _detect_performance_problems(
320315
detectors: List[PerformanceDetector] = [
321316
ConsecutiveDBSpanDetector(detection_settings, data),
322317
ConsecutiveHTTPSpanDetector(detection_settings, data),
323-
ConsecutiveHTTPSpanDetectorExtended(detection_settings, data),
324318
DBMainThreadDetector(detection_settings, data),
325319
SlowDBQueryDetector(detection_settings, data),
326320
RenderBlockingAssetSpanDetector(detection_settings, data),

Diff for: ‎tests/sentry/api/endpoints/test_project_performance_issue_settings.py

+4
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ def test_get_project_options_overrides_threshold_defaults(self, get_value):
9292
"performance.issues.uncompressed_asset.size_threshold": 200000,
9393
"performance.issues.consecutive_db.min_time_saved_threshold": 300,
9494
"performance.issues.n_plus_one_api_calls.total_duration": 300,
95+
"performance.issues.consecutive_http.min_time_saved_threshold": 2000,
9596
}
9697
):
9798
with self.feature(PERFORMANCE_ISSUE_FEATURES):
@@ -110,6 +111,7 @@ def test_get_project_options_overrides_threshold_defaults(self, get_value):
110111
assert response.data["uncompressed_asset_size_threshold"] == 200000
111112
assert response.data["consecutive_db_min_time_saved_threshold"] == 300
112113
assert response.data["n_plus_one_api_calls_total_duration_threshold"] == 300
114+
assert response.data["consecutive_http_spans_min_time_saved_threshold"] == 2000
113115

114116
get_value.return_value = {
115117
"n_plus_one_db_duration_threshold": 10000,
@@ -122,6 +124,7 @@ def test_get_project_options_overrides_threshold_defaults(self, get_value):
122124
"file_io_on_main_thread_duration_threshold": 33,
123125
"consecutive_db_min_time_saved_threshold": 5000,
124126
"n_plus_one_api_calls_total_duration_threshold": 500,
127+
"consecutive_http_spans_min_time_saved_threshold": 1000,
125128
}
126129

127130
with self.feature(PERFORMANCE_ISSUE_FEATURES):
@@ -140,6 +143,7 @@ def test_get_project_options_overrides_threshold_defaults(self, get_value):
140143
assert response.data["file_io_on_main_thread_duration_threshold"] == 33
141144
assert response.data["consecutive_db_min_time_saved_threshold"] == 5000
142145
assert response.data["n_plus_one_api_calls_total_duration_threshold"] == 500
146+
assert response.data["consecutive_http_spans_min_time_saved_threshold"] == 1000
143147

144148
def test_get_returns_error_without_feature_enabled(self):
145149
with self.feature({}):

0 commit comments

Comments
 (0)