Skip to content

Commit 6327d5a

Browse files
authored
feat(error-upsampling): Apply upsampling when ANY project is allowlisted (#95824)
Changes the error upsampling logic from requiring ALL projects in a query to be allowlisted to requiring only ANY project to be allowlisted. The upsampling returns the same data for non upsampled projects so there is no need to be strict and require all projects.
1 parent 0bf4051 commit 6327d5a

File tree

7 files changed

+27
-28
lines changed

7 files changed

+27
-28
lines changed

src/sentry/api/bases/organization_events.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
from sentry.api.base import CURSOR_LINK_HEADER
1919
from sentry.api.bases import NoProjects
2020
from sentry.api.bases.organization import FilterParamsDateNotNull, OrganizationEndpoint
21-
from sentry.api.helpers.error_upsampling import are_all_projects_error_upsampled
21+
from sentry.api.helpers.error_upsampling import are_any_projects_error_upsampled
2222
from sentry.api.helpers.mobile import get_readable_device_name
2323
from sentry.api.helpers.teams import get_teams
2424
from sentry.api.serializers.snuba import SnubaTSResultSerializer
@@ -431,7 +431,7 @@ def handle_error_upsampling(self, project_ids: Sequence[int], results: dict[str,
431431
and update the meta fields to reflect the new field names. This works around a limitation in
432432
how aliases are handled in the SnQL parser.
433433
"""
434-
if are_all_projects_error_upsampled(project_ids):
434+
if are_any_projects_error_upsampled(project_ids):
435435
data = results.get("data", [])
436436
fields_meta = results.get("meta", {}).get("fields", {})
437437

src/sentry/api/helpers/error_upsampling.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,18 +19,18 @@ def is_errors_query_for_error_upsampled_projects(
1919
) -> bool:
2020
"""
2121
Determine if this query should use error upsampling transformations.
22-
Only applies when ALL projects are allowlisted and we're querying error events.
22+
Only applies when ANY projects are allowlisted and we're querying error events.
2323
"""
24-
if not are_all_projects_error_upsampled(snuba_params.project_ids):
24+
if not are_any_projects_error_upsampled(snuba_params.project_ids):
2525
return False
2626

2727
return _should_apply_sample_weight_transform(dataset, request)
2828

2929

30-
def are_all_projects_error_upsampled(project_ids: Sequence[int]) -> bool:
30+
def are_any_projects_error_upsampled(project_ids: Sequence[int]) -> bool:
3131
"""
32-
Check if ALL projects in the query are allowlisted for error upsampling.
33-
Only returns True if all projects pass the allowlist condition.
32+
Check if ANY projects in the query are allowlisted for error upsampling.
33+
Only returns True if any project pass the allowlist condition.
3434
"""
3535
if not project_ids:
3636
return False
@@ -39,8 +39,8 @@ def are_all_projects_error_upsampled(project_ids: Sequence[int]) -> bool:
3939
if not allowlist:
4040
return False
4141

42-
# All projects must be in the allowlist
43-
result = all(project_id in allowlist for project_id in project_ids)
42+
# Any project must be in the allowlist
43+
result = any(project_id in allowlist for project_id in project_ids)
4444
return result
4545

4646

src/sentry/api/serializers/models/group.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
from django.db.models import Min, prefetch_related_objects
1515

1616
from sentry import features, tagstore
17-
from sentry.api.helpers.error_upsampling import are_all_projects_error_upsampled
17+
from sentry.api.helpers.error_upsampling import are_any_projects_error_upsampled
1818
from sentry.api.serializers import Serializer, register, serialize
1919
from sentry.api.serializers.models.actor import ActorSerializer
2020
from sentry.api.serializers.models.plugin import is_plugin_deprecated
@@ -1074,8 +1074,8 @@ def _execute_error_seen_stats_query(
10741074
["max", "timestamp", "last_seen"],
10751075
["uniq", "tags[sentry:user]", "count"],
10761076
]
1077-
# Check if all projects are allowlisted for error upsampling
1078-
is_upsampled = are_all_projects_error_upsampled(project_ids)
1077+
# Check if any projects are allowlisted for error upsampling
1078+
is_upsampled = are_any_projects_error_upsampled(project_ids)
10791079
if is_upsampled:
10801080
aggregations[0] = ["upsampled_count", "", "times_seen"]
10811081

src/sentry/api/serializers/models/group_stream.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
from sentry import features, release_health, tsdb
1313
from sentry.api.helpers.error_upsampling import (
1414
UPSAMPLED_ERROR_AGGREGATION,
15-
are_all_projects_error_upsampled,
15+
are_any_projects_error_upsampled,
1616
)
1717
from sentry.api.serializers import serialize
1818
from sentry.api.serializers.models.group import (
@@ -395,8 +395,7 @@ def get_attrs(
395395
if self.stats_period and not self._collapse("stats"):
396396
aggregation_override = None
397397
if self.project_ids:
398-
is_upsampled = are_all_projects_error_upsampled(self.project_ids)
399-
if is_upsampled:
398+
if are_any_projects_error_upsampled(self.project_ids):
400399
aggregation_override = UPSAMPLED_ERROR_AGGREGATION
401400

402401
partial_get_stats = functools.partial(

tests/sentry/api/helpers/test_error_upsampling.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
from sentry.api.helpers.error_upsampling import (
88
_is_error_focused_query,
99
_should_apply_sample_weight_transform,
10-
are_all_projects_error_upsampled,
10+
are_any_projects_error_upsampled,
1111
transform_query_columns_for_error_upsampling,
1212
)
1313
from sentry.models.organization import Organization
@@ -35,21 +35,21 @@ def setUp(self) -> None:
3535
self.request.GET = QueryDict("")
3636

3737
@patch("sentry.api.helpers.error_upsampling.options")
38-
def test_are_all_projects_error_upsampled(self, mock_options: Mock) -> None:
38+
def test_are_any_projects_error_upsampled(self, mock_options: Mock) -> None:
3939
# Test when all projects are allowlisted
4040
mock_options.get.return_value = self.project_ids
41-
assert are_all_projects_error_upsampled(self.project_ids) is True
41+
assert are_any_projects_error_upsampled(self.project_ids) is True
4242

43-
# Test when some projects are not allowlisted
43+
# Test when some projects are allowlisted
4444
mock_options.get.return_value = self.project_ids[:-1]
45-
assert are_all_projects_error_upsampled(self.project_ids) is False
45+
assert are_any_projects_error_upsampled(self.project_ids) is True
4646

4747
# Test when no projects are allowlisted
4848
mock_options.get.return_value = []
49-
assert are_all_projects_error_upsampled(self.project_ids) is False
49+
assert are_any_projects_error_upsampled(self.project_ids) is False
5050

5151
# Test when no project IDs provided
52-
assert are_all_projects_error_upsampled([]) is False
52+
assert are_any_projects_error_upsampled([]) is False
5353

5454
def test_transform_query_columns_for_error_upsampling(self) -> None:
5555
# Test count() transformation

tests/snuba/api/endpoints/test_organization_events.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6776,7 +6776,7 @@ def test_error_upsampling_with_no_allowlist(self):
67766776
assert response.data["data"][0]["count()"] == 1
67776777

67786778
def test_error_upsampling_with_partial_allowlist(self):
6779-
"""Test that count() is not upsampled when only some projects are allowlisted."""
6779+
"""Test that count() is upsampled when any project in the query is allowlisted."""
67806780
# Create a second project
67816781
project2 = self.create_project(organization=self.organization)
67826782

@@ -6819,8 +6819,8 @@ def test_error_upsampling_with_partial_allowlist(self):
68196819
features = {"organizations:discover-basic": True, "organizations:global-views": True}
68206820
response = self.do_request(query, features=features)
68216821
assert response.status_code == 200, response.content
6822-
# Expect no upsampling since not all projects are allowlisted
6823-
assert response.data["data"][0]["count()"] == 2
6822+
# Expect upsampling since any project is allowlisted (both events upsampled: 10 + 10 = 20)
6823+
assert response.data["data"][0]["count()"] == 20
68246824

68256825
def test_is_status(self):
68266826
self.store_event(

tests/snuba/api/endpoints/test_organization_events_stats.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3578,9 +3578,9 @@ def test_error_upsampling_with_partial_allowlist(self, mock_options):
35783578
assert response.status_code == 200, response.content
35793579
data = response.data["data"]
35803580
assert len(data) == 2 # Two time buckets
3581-
# Should use regular count() since not all projects are allowlisted
3582-
assert data[0][1][0]["count"] == 1
3583-
assert data[1][1][0]["count"] == 1
3581+
# Should use upsampled count() since any project is allowlisted
3582+
assert data[0][1][0]["count"] == 10
3583+
assert data[1][1][0]["count"] == 10
35843584

35853585
@mock.patch("sentry.api.helpers.error_upsampling.options")
35863586
def test_error_upsampling_with_transaction_events(self, mock_options):

0 commit comments

Comments
 (0)