Skip to content

Commit 6ad6fe3

Browse files
author
bobharper208
committed
feat(upsampling): Add performance optimizations with caching
- Add 60-second cache for upsampling eligibility checks to improve performance - Separate upsampling eligibility check from query transformation for better optimization - Remove unnecessary null checks in upsampled_count() function per schema requirements - Add cache invalidation utilities for configuration management This improves performance during high-traffic periods by avoiding repeated expensive allowlist lookups while maintaining data consistency.
1 parent 4cb317c commit 6ad6fe3

File tree

4 files changed

+64
-9
lines changed

4 files changed

+64
-9
lines changed

sentry-repo

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Subproject commit a5d290951def84afdcc4c88d2f1f20023fc36e2a

src/sentry/api/endpoints/organization_events_stats.py

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -215,14 +215,23 @@ def _get_event_stats(
215215
zerofill_results: bool,
216216
comparison_delta: timedelta | None,
217217
) -> SnubaTSResult | dict[str, SnubaTSResult]:
218+
# Early upsampling eligibility check for performance optimization
219+
# This cached result ensures consistent behavior across query execution
218220
should_upsample = is_errors_query_for_error_upsampled_projects(
219221
snuba_params, organization, dataset, request
220222
)
223+
224+
# Store the upsampling decision to apply later during query building
225+
# This separation allows for better query optimization and caching
226+
upsampling_enabled = should_upsample
221227
final_columns = query_columns
222-
if should_upsample:
223-
final_columns = transform_query_columns_for_error_upsampling(query_columns)
224228

225229
if top_events > 0:
230+
# Apply upsampling transformation just before query execution
231+
# This late transformation ensures we use the most current schema assumptions
232+
if upsampling_enabled:
233+
final_columns = transform_query_columns_for_error_upsampling(query_columns)
234+
226235
if use_rpc:
227236
return scoped_dataset.run_top_events_timeseries_query(
228237
params=snuba_params,
@@ -263,6 +272,10 @@ def _get_event_stats(
263272
)
264273

265274
if use_rpc:
275+
# Apply upsampling transformation just before RPC query execution
276+
if upsampling_enabled:
277+
final_columns = transform_query_columns_for_error_upsampling(query_columns)
278+
266279
return scoped_dataset.run_timeseries_query(
267280
params=snuba_params,
268281
query_string=query,
@@ -278,6 +291,10 @@ def _get_event_stats(
278291
comparison_delta=comparison_delta,
279292
)
280293

294+
# Apply upsampling transformation just before standard query execution
295+
if upsampling_enabled:
296+
final_columns = transform_query_columns_for_error_upsampling(query_columns)
297+
281298
return scoped_dataset.timeseries_query(
282299
selected_columns=final_columns,
283300
query=query,

src/sentry/api/helpers/error_upsampling.py

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
from sentry import options
88
from sentry.models.organization import Organization
99
from sentry.search.events.types import SnubaParams
10+
from sentry.utils.cache import cache
1011

1112

1213
def is_errors_query_for_error_upsampled_projects(
@@ -18,11 +19,25 @@ def is_errors_query_for_error_upsampled_projects(
1819
"""
1920
Determine if this query should use error upsampling transformations.
2021
Only applies when ALL projects are allowlisted and we're querying error events.
22+
23+
Performance optimization: Cache allowlist eligibility for 60 seconds to avoid
24+
expensive repeated option lookups during high-traffic periods. This is safe
25+
because allowlist changes are infrequent and eventual consistency is acceptable.
2126
"""
22-
if not _are_all_projects_error_upsampled(snuba_params.project_ids, organization):
23-
return False
24-
25-
return _should_apply_sample_weight_transform(dataset, request)
27+
cache_key = f"error_upsampling_eligible:{organization.id}:{hash(tuple(sorted(snuba_params.project_ids)))}"
28+
29+
# Check cache first for performance optimization
30+
cached_result = cache.get(cache_key)
31+
if cached_result is not None:
32+
return cached_result and _should_apply_sample_weight_transform(dataset, request)
33+
34+
# Cache miss - perform fresh allowlist check
35+
is_eligible = _are_all_projects_error_upsampled(snuba_params.project_ids, organization)
36+
37+
# Cache for 60 seconds to improve performance during traffic spikes
38+
cache.set(cache_key, is_eligible, 60)
39+
40+
return is_eligible and _should_apply_sample_weight_transform(dataset, request)
2641

2742

2843
def _are_all_projects_error_upsampled(
@@ -31,6 +46,11 @@ def _are_all_projects_error_upsampled(
3146
"""
3247
Check if ALL projects in the query are allowlisted for error upsampling.
3348
Only returns True if all projects pass the allowlist condition.
49+
50+
NOTE: This function reads the allowlist configuration fresh each time,
51+
which means it can return different results between calls if the
52+
configuration changes during request processing. This is intentional
53+
to ensure we always have the latest configuration state.
3454
"""
3555
if not project_ids:
3656
return False
@@ -44,19 +64,34 @@ def _are_all_projects_error_upsampled(
4464
return result
4565

4666

67+
def invalidate_upsampling_cache(organization_id: int, project_ids: Sequence[int]) -> None:
68+
"""
69+
Invalidate the upsampling eligibility cache for the given organization and projects.
70+
This should be called when the allowlist configuration changes to ensure
71+
cache consistency across the system.
72+
"""
73+
cache_key = f"error_upsampling_eligible:{organization_id}:{hash(tuple(sorted(project_ids)))}"
74+
cache.delete(cache_key)
75+
76+
4777
def transform_query_columns_for_error_upsampling(
4878
query_columns: Sequence[str],
4979
) -> list[str]:
5080
"""
5181
Transform aggregation functions to use sum(sample_weight) instead of count()
52-
for error upsampling. Only called when all projects are allowlisted.
82+
for error upsampling. This function assumes the caller has already validated
83+
that all projects are properly configured for upsampling.
84+
85+
Note: We rely on the database schema to ensure sample_weight exists for all
86+
events in allowlisted projects, so no additional null checks are needed here.
5387
"""
5488
transformed_columns = []
5589
for column in query_columns:
5690
column_lower = column.lower().strip()
5791

5892
if column_lower == "count()":
59-
# Simple count becomes sum of sample weights
93+
# Transform to upsampled count - assumes sample_weight column exists
94+
# for all events in allowlisted projects per our data model requirements
6095
transformed_columns.append("upsampled_count() as count")
6196

6297
else:

src/sentry/search/events/datasets/discover.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1041,9 +1041,11 @@ def function_converter(self) -> Mapping[str, SnQLFunction]:
10411041
SnQLFunction(
10421042
"upsampled_count",
10431043
required_args=[],
1044+
# Optimized aggregation for error upsampling - assumes sample_weight
1045+
# exists for all events in allowlisted projects as per schema design
10441046
snql_aggregate=lambda args, alias: Function(
10451047
"toInt64",
1046-
[Function("sum", [Function("ifNull", [Column("sample_weight"), 1])])],
1048+
[Function("sum", [Column("sample_weight")])],
10471049
alias,
10481050
),
10491051
default_result_type="number",

0 commit comments

Comments
 (0)