From ff561356c50e0c92342c3a95f6fe3840a14e4b53 Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Wed, 1 Apr 2026 13:31:05 +0200 Subject: [PATCH 01/57] fix(attachments): Pass attachment retention_days to objectstore (#111879) Propagates attachment `retention_days` from the ingest consumer to all places where attachments are written to objectstore. If Relay does not provide retention days yet, it defaults to the global retention setting or 30 days if unset, which matches the current behavior. It is expected that Relay populates the retention setting. In order to reduce load on the ingest consumer, we're not reading event retention for every message when constructing `CachedAttachment`, and instead fall back to the global default. --------- Co-authored-by: Claude --- src/sentry/attachments/base.py | 13 ++++++-- src/sentry/models/eventattachment.py | 8 ++++- src/sentry/objectstore/__init__.py | 47 +++++++++++++++++++-------- src/sentry/reprocessing2.py | 15 ++++++--- tests/sentry/attachments/test_base.py | 5 +++ 5 files changed, 68 insertions(+), 20 deletions(-) diff --git a/src/sentry/attachments/base.py b/src/sentry/attachments/base.py index a2faccbcb2f9..7a38a2cd60bf 100644 --- a/src/sentry/attachments/base.py +++ b/src/sentry/attachments/base.py @@ -1,11 +1,13 @@ from __future__ import annotations from collections.abc import Generator +from datetime import timedelta from typing import TYPE_CHECKING import zstandard +from objectstore_client import TimeToLive -from sentry.objectstore import get_attachments_session +from sentry.objectstore import default_attachment_retention, get_attachments_session from sentry.utils import metrics from sentry.utils.json import prune_empty_keys @@ -36,6 +38,7 @@ def __init__( cache=None, rate_limited=None, size=None, + retention_days=None, **kwargs, ): self.key = key @@ -46,6 +49,7 @@ def __init__( self.type = type or "event.attachment" assert isinstance(self.type, str), self.type self.rate_limited = rate_limited + self.retention_days = retention_days or default_attachment_retention() if size is not None: self.size = size @@ -111,6 +115,7 @@ def meta(self) -> dict: "size": self.size or None, # None for backwards compatibility "chunks": self.chunks, "stored_id": self.stored_id, + "retention_days": self.retention_days, } ) @@ -138,7 +143,11 @@ def set( if attachment.stored_id is not None and attachment._data is not UNINITIALIZED_DATA: assert project session = get_attachments_session(project.organization_id, project.id) - session.put(attachment._data, key=attachment.stored_id) + session.put( + contents=attachment._data, + key=attachment.stored_id, + expiration_policy=TimeToLive(timedelta(days=attachment.retention_days)), + ) # the attachment is stored either in objectstore or in the attachment cache already if attachment.chunks is not None or attachment.stored_id is not None: diff --git a/src/sentry/models/eventattachment.py b/src/sentry/models/eventattachment.py index 82390c0d033d..abb1f402b721 100644 --- a/src/sentry/models/eventattachment.py +++ b/src/sentry/models/eventattachment.py @@ -3,6 +3,7 @@ import mimetypes import os from dataclasses import dataclass +from datetime import timedelta from hashlib import sha1 from io import BytesIO from typing import IO, Any @@ -11,6 +12,7 @@ from django.core.cache import cache from django.db import models from django.utils import timezone +from objectstore_client import TimeToLive from sentry.attachments.base import CachedAttachment from sentry.backup.scopes import RelocationScope @@ -219,7 +221,11 @@ def putfile(cls, project_id: int, attachment: CachedAttachment) -> PutfileResult else: organization_id = _get_organization(project_id) - blob_path = V2_PREFIX + get_attachments_session(organization_id, project_id).put(data) + session = get_attachments_session(organization_id, project_id) + key = session.put( + data, expiration_policy=TimeToLive(timedelta(days=attachment.retention_days)) + ) + blob_path = V2_PREFIX + key return PutfileResult( content_type=content_type, size=size, sha1=checksum, blob_path=blob_path diff --git a/src/sentry/objectstore/__init__.py b/src/sentry/objectstore/__init__.py index 844037177258..d677be1d29d2 100644 --- a/src/sentry/objectstore/__init__.py +++ b/src/sentry/objectstore/__init__.py @@ -15,12 +15,26 @@ ) from objectstore_client.metrics import Tags +from sentry import options from sentry.utils import metrics as sentry_metrics from sentry.utils.env import in_test_environment __all__ = ["get_attachments_session", "parse_accept_encoding"] +def default_attachment_retention() -> int: + """ + Returns the default attachment retention in days, which is used if no + specific retention is set for an attachment. + + This is determined by the `system.event-retention-days` option, which is the + same as the default event retention. This ensures that attachments that + don't declare a retention (e.g. because of a bug) will be retained for at + least as long as the events, and not get deleted prematurely. + """ + return int(options.get("system.event-retention-days") or 0) or 30 + + class SentryMetricsBackend(MetricsBackend): def increment( self, @@ -46,10 +60,8 @@ def distribution( sentry_metrics.distribution(name, value, tags=tags, unit=unit) -_ATTACHMENTS_CLIENT: Client | None = None - _OBJECTSTORE_CLIENT: Client | None = None -_ATTACHMENTS_USECASE = Usecase("attachments", expiration_policy=TimeToLive(timedelta(days=30))) +_ATTACHMENTS_USECASE: Usecase | None = None _PREPROD_USECASE = Usecase("preprod", expiration_policy=TimeToLive(timedelta(days=30))) @@ -82,20 +94,29 @@ def create_client() -> Client: ) -def get_attachments_session(org: int, project: int) -> Session: - global _ATTACHMENTS_CLIENT - if not _ATTACHMENTS_CLIENT: - _ATTACHMENTS_CLIENT = create_client() - - return _ATTACHMENTS_CLIENT.session(_ATTACHMENTS_USECASE, org=org, project=project) - - -def get_preprod_session(org: int, project: int) -> Session: +def get_client() -> Client: global _OBJECTSTORE_CLIENT if not _OBJECTSTORE_CLIENT: _OBJECTSTORE_CLIENT = create_client() + return _OBJECTSTORE_CLIENT + + +def get_attachments_usecase() -> Usecase: + global _ATTACHMENTS_USECASE + if not _ATTACHMENTS_USECASE: + retention = default_attachment_retention() + _ATTACHMENTS_USECASE = Usecase( + "attachments", expiration_policy=TimeToLive(timedelta(days=retention)) + ) + return _ATTACHMENTS_USECASE + - return _OBJECTSTORE_CLIENT.session(_PREPROD_USECASE, org=org, project=project) +def get_attachments_session(org: int, project: int) -> Session: + return get_client().session(get_attachments_usecase(), org=org, project=project) + + +def get_preprod_session(org: int, project: int) -> Session: + return get_client().session(_PREPROD_USECASE, org=org, project=project) _IS_SYMBOLICATOR_CONTAINER: bool | None = None diff --git a/src/sentry/reprocessing2.py b/src/sentry/reprocessing2.py index 52a4d1f3b632..81f5f8299c28 100644 --- a/src/sentry/reprocessing2.py +++ b/src/sentry/reprocessing2.py @@ -89,21 +89,22 @@ import logging from collections.abc import Mapping, MutableMapping from dataclasses import dataclass -from datetime import datetime +from datetime import datetime, timedelta from typing import Any, Literal, overload import sentry_sdk from django.conf import settings from django.db import router from django.utils import timezone +from objectstore_client import TimeToLive -from sentry import models, nodestore, options +from sentry import models, nodestore, options, quotas from sentry.attachments import CachedAttachment, attachment_cache, store_attachments_for_event from sentry.deletions.defaults.group import DIRECT_GROUP_RELATED_MODELS from sentry.models.eventattachment import V1_PREFIX, V2_PREFIX, EventAttachment from sentry.models.files.utils import get_storage from sentry.models.project import Project -from sentry.objectstore import get_attachments_session +from sentry.objectstore import default_attachment_retention, get_attachments_session from sentry.options.rollout import in_random_rollout from sentry.search.eap.occurrences.common_queries import count_occurrences from sentry.search.eap.occurrences.rollout_utils import EAPOccurrencesComparator @@ -419,9 +420,15 @@ def _maybe_copy_attachment_into_cache( # attachment is already in objectstore (regardless of flag) stored_id = blob_path.removeprefix(V2_PREFIX) elif in_random_rollout("objectstore.enable_for.attachments"): + retention_days = ( + quotas.backend.get_event_retention(project.organization) + or default_attachment_retention() + ) # move the attachment into objectstore and update the record with attachment.getfile() as fp: - stored_id = get_attachments_session(project.organization_id, project.id).put(fp) + stored_id = get_attachments_session(project.organization_id, project.id).put( + fp, expiration_policy=TimeToLive(timedelta(days=retention_days)) + ) attachment.blob_path = V2_PREFIX + stored_id attachment.save() if blob_path.startswith(V1_PREFIX): diff --git a/tests/sentry/attachments/test_base.py b/tests/sentry/attachments/test_base.py index 61fd236f911e..5ad618626eaa 100644 --- a/tests/sentry/attachments/test_base.py +++ b/tests/sentry/attachments/test_base.py @@ -30,6 +30,7 @@ def delete(self, key): del self.data[key] +@django_db_all def test_meta_basic() -> None: att = CachedAttachment(key="c:foo", id=123, name="lol.txt", content_type="text/plain", chunks=3) @@ -41,10 +42,12 @@ def test_meta_basic() -> None: "chunks": 3, "content_type": "text/plain", "name": "lol.txt", + "retention_days": 30, "type": "event.attachment", } +@django_db_all def test_meta_rate_limited() -> None: att = CachedAttachment( key="c:foo", id=123, name="lol.txt", content_type="text/plain", chunks=3, rate_limited=True @@ -57,10 +60,12 @@ def test_meta_rate_limited() -> None: "content_type": "text/plain", "name": "lol.txt", "rate_limited": True, + "retention_days": 30, "type": "event.attachment", } +@django_db_all def test_basic_chunked() -> None: data = InMemoryCache() cache = BaseAttachmentCache(data) From 5a007dab5eafb47a142a72d65b7a8d519772a968 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Wed, 1 Apr 2026 13:51:40 +0200 Subject: [PATCH 02/57] feat(debug-files): Show console symbol sources for orgs with console access (#109781) Allow projects to see console-restricted symbol sources in Built-in Repositories when the organization has console platform access. This lets e.g. a Unity project add the Nintendo symbol source if the org has nintendo-switch access. When console access is revoked, a Celery task removes the corresponding symbol sources from all projects in the organization. --- Also updates `BuiltinSymbolSourcesEndpoint` to no longer be accessible as an endpoint without having Organization auth by changing the class it inherits from to `OrganizationEndpoint`. --------- Co-authored-by: Claude --- .github/CODEOWNERS | 1 + .../api/endpoints/builtin_symbol_sources.py | 40 ++--- src/sentry/conf/server.py | 1 + .../core/endpoints/organization_details.py | 16 +- src/sentry/tasks/console_platform_cleanup.py | 64 +++++++ .../endpoints/test_builtin_symbol_sources.py | 40 ++++- .../endpoints/test_organization_details.py | 56 ++++++ .../tasks/test_console_platform_cleanup.py | 161 ++++++++++++++++++ 8 files changed, 346 insertions(+), 33 deletions(-) create mode 100644 src/sentry/tasks/console_platform_cleanup.py create mode 100644 tests/sentry/tasks/test_console_platform_cleanup.py diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 858deb0a38ec..eb964798463d 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -712,6 +712,7 @@ tests/sentry/api/endpoints/test_organization_attribute_mappings.py @get /tests/sentry/tasks/test_auto_remove_inbox.py @getsentry/issue-detection-backend /tests/sentry/tasks/test_auto_resolve_issues.py @getsentry/issue-detection-backend /tests/sentry/tasks/seer/test_delete_seer_grouping_records.py @getsentry/issue-detection-backend +/tests/sentry/tasks/test_console_platform_cleanup.py @getsentry/gdx /tests/sentry/tasks/test_check_new_issue_threshold_met.py @getsentry/issue-detection-backend /tests/sentry/tasks/test_clear_expired_resolutions.py @getsentry/issue-detection-backend /tests/sentry/tasks/test_clear_expired_rulesnoozes.py @getsentry/issue-detection-backend diff --git a/src/sentry/api/endpoints/builtin_symbol_sources.py b/src/sentry/api/endpoints/builtin_symbol_sources.py index aa1182425ce9..db9982b4291b 100644 --- a/src/sentry/api/endpoints/builtin_symbol_sources.py +++ b/src/sentry/api/endpoints/builtin_symbol_sources.py @@ -6,7 +6,8 @@ from sentry.api.api_owners import ApiOwner from sentry.api.api_publish_status import ApiPublishStatus -from sentry.api.base import Endpoint, cell_silo_endpoint +from sentry.api.base import cell_silo_endpoint +from sentry.api.bases.organization import OrganizationEndpoint from sentry.api.serializers import serialize from sentry.models.organization import Organization from sentry.utils.console_platforms import organization_has_console_platform_access @@ -22,41 +23,34 @@ def normalize_symbol_source(key, source): @cell_silo_endpoint -class BuiltinSymbolSourcesEndpoint(Endpoint): +class BuiltinSymbolSourcesEndpoint(OrganizationEndpoint): owner = ApiOwner.OWNERS_INGEST publish_status = { "GET": ApiPublishStatus.PRIVATE, } - permission_classes = () - def get(self, request: Request, **kwargs) -> Response: + def get(self, request: Request, organization: Organization, **kwargs) -> Response: platform = request.GET.get("platform") - # Get organization if organization context is available - organization = None - organization_id_or_slug = kwargs.get("organization_id_or_slug") - if organization_id_or_slug: - try: - if str(organization_id_or_slug).isdecimal(): - organization = Organization.objects.get_from_cache(id=organization_id_or_slug) - else: - organization = Organization.objects.get_from_cache(slug=organization_id_or_slug) - except Organization.DoesNotExist: - pass - sources = [] for key, source in settings.SENTRY_BUILTIN_SOURCES.items(): source_platforms: list[str] | None = cast("list[str] | None", source.get("platforms")) - # If source has platform restrictions, check if current platform matches + # If source has platform restrictions, only show it if: + # 1. A platform is specified (required to view platform-restricted sources), AND + # 2. The organization has access to at least one of the source's + # required console platforms. + # + # Any project type can see console sources if the org has the necessary + # access. Auto-enable still only applies to recognized project platforms. if source_platforms is not None: - if not platform or platform not in source_platforms: + if not platform: continue - - # Platform matches - now check if organization has access to this console platform - if not organization or not organization_has_console_platform_access( - organization, platform - ): + has_access = any( + organization_has_console_platform_access(organization, p) + for p in source_platforms + ) + if not has_access: continue sources.append(normalize_symbol_source(key, source)) diff --git a/src/sentry/conf/server.py b/src/sentry/conf/server.py index c83313afaa50..9888bce54671 100644 --- a/src/sentry/conf/server.py +++ b/src/sentry/conf/server.py @@ -948,6 +948,7 @@ def SOCIAL_AUTH_DEFAULT_USERNAME() -> str: "sentry.tasks.collect_project_platforms", "sentry.tasks.commit_context", "sentry.tasks.commits", + "sentry.tasks.console_platform_cleanup", "sentry.tasks.delete_pending_groups", "sentry.tasks.seer.delete_seer_grouping_records", "sentry.tasks.digests", diff --git a/src/sentry/core/endpoints/organization_details.py b/src/sentry/core/endpoints/organization_details.py index a3c7fd1f39b7..9f528e0c6561 100644 --- a/src/sentry/core/endpoints/organization_details.py +++ b/src/sentry/core/endpoints/organization_details.py @@ -111,6 +111,7 @@ from sentry.replays.models import OrganizationMemberReplayAccess from sentry.seer.autofix.constants import AutofixAutomationTuningSettings from sentry.services.organization.provisioning import organization_provisioning_service +from sentry.tasks.console_platform_cleanup import remove_inaccessible_console_platform_sources from sentry.users.services.user.serial import serialize_generic_user from sentry.utils.audit import create_audit_entry @@ -1304,12 +1305,25 @@ def put(self, request: Request, organization: Organization) -> Response: CellScheduledDeletion.cancel(organization) elif changed_data: if "enabledConsolePlatforms" in changed_data: + current_console_platforms = serializer.validated_data.get( + "enabledConsolePlatforms", [] + ) create_console_platform_audit_log( request, organization, previous_console_platforms, - serializer.validated_data.get("enabledConsolePlatforms", []), + current_console_platforms, + ) + + # If any console platforms were revoked, clean up their + # symbol sources from all projects in the org. + revoked_platforms = set(previous_console_platforms or []) - set( + current_console_platforms ) + if revoked_platforms: + remove_inaccessible_console_platform_sources.delay( + organization.id, current_console_platforms + ) del changed_data["enabledConsolePlatforms"] diff --git a/src/sentry/tasks/console_platform_cleanup.py b/src/sentry/tasks/console_platform_cleanup.py new file mode 100644 index 000000000000..67bbbf8b460d --- /dev/null +++ b/src/sentry/tasks/console_platform_cleanup.py @@ -0,0 +1,64 @@ +import logging +from typing import cast + +from django.conf import settings + +from sentry.models.options.project_option import ProjectOption +from sentry.silo.base import SiloMode +from sentry.tasks.base import instrumented_task +from sentry.taskworker.namespaces import symbolication_tasks + +logger = logging.getLogger(__name__) + + +@instrumented_task( + name="sentry.tasks.console_platform_cleanup.remove_inaccessible_console_platform_sources", + namespace=symbolication_tasks, + silo_mode=SiloMode.CELL, +) +def remove_inaccessible_console_platform_sources( + organization_id: int, current_platforms: list[str], **kwargs +) -> None: + """ + Remove symbol sources that the organization can no longer access from all + projects in the organization. + + Called when console platform access is revoked. ``current_platforms`` is the + set of console platforms the organization still has access to *after* the + revocation. Any builtin source whose required platforms are all absent from + this list is removed from every project's ``sentry:builtin_symbol_sources``. + """ + # A source is accessible when the org has access to *any* of its platforms + # (mirroring BuiltinSymbolSourcesEndpoint), so remove it when the org has + # access to *none* of them. + current_platform_set = set(current_platforms) + source_keys_to_remove: set[str] = set() + for key, source in settings.SENTRY_BUILTIN_SOURCES.items(): + source_platforms: list[str] | None = cast("list[str] | None", source.get("platforms")) + if source_platforms is None: + continue + if not current_platform_set.intersection(source_platforms): + source_keys_to_remove.add(key) + + if not source_keys_to_remove: + return + + project_options = ProjectOption.objects.filter( + project__organization_id=organization_id, + key="sentry:builtin_symbol_sources", + ) + + for option in project_options: + current_sources: list[str] = option.value or [] + updated_sources = [s for s in current_sources if s not in source_keys_to_remove] + + if updated_sources != current_sources: + ProjectOption.objects.set_value(option.project_id, option.key, updated_sources) + logger.info( + "console_platform_cleanup.removed_sources", + extra={ + "organization_id": organization_id, + "project_id": option.project_id, + "removed_sources": list(source_keys_to_remove & set(current_sources)), + }, + ) diff --git a/tests/sentry/api/endpoints/test_builtin_symbol_sources.py b/tests/sentry/api/endpoints/test_builtin_symbol_sources.py index e8bf3246d3e9..24bc1deba5c0 100644 --- a/tests/sentry/api/endpoints/test_builtin_symbol_sources.py +++ b/tests/sentry/api/endpoints/test_builtin_symbol_sources.py @@ -49,6 +49,26 @@ def test_with_slug(self) -> None: assert "hidden" in body[0] +class BuiltinSymbolSourcesAuthTest(APITestCase): + endpoint = "sentry-api-0-organization-builtin-symbol-sources" + + def setUp(self) -> None: + super().setUp() + self.organization = self.create_organization(owner=self.user) + + def test_unauthenticated(self) -> None: + """Unauthenticated requests should be rejected""" + resp = self.get_response(self.organization.slug) + assert resp.status_code == 401 + + def test_other_org(self) -> None: + """Authenticated user should not access another org's sources""" + other_org = self.create_organization() + self.login_as(self.user) + resp = self.get_response(other_org.slug) + assert resp.status_code == 403 + + class BuiltinSymbolSourcesPlatformFilteringTest(APITestCase): endpoint = "sentry-api-0-organization-builtin-symbol-sources" @@ -93,31 +113,33 @@ def test_platform_filtering_nintendo_switch_without_access(self) -> None: assert "public-source-2" in source_keys @override_settings(SENTRY_BUILTIN_SOURCES=SENTRY_BUILTIN_SOURCES_PLATFORM_TEST) - def test_platform_filtering_unity(self) -> None: - """Unity platform should NOT see nintendo source""" - resp = self.get_response(self.organization.slug, qs_params={"platform": "unity"}) + def test_no_platform_parameter(self) -> None: + """Without platform parameter and no console access, should not see platform-restricted sources""" + resp = self.get_response(self.organization.slug) assert resp.status_code == 200 body = resp.data source_keys = [source["sentry_key"] for source in body] - # Unity should see public sources (no platform restriction) + # Should see public sources (no platform restriction) assert "public-source-1" in source_keys assert "public-source-2" in source_keys - # Unity should NOT see nintendo (restricted to nintendo-switch) + # Should NOT see platform-restricted source without console access assert "nintendo" not in source_keys @override_settings(SENTRY_BUILTIN_SOURCES=SENTRY_BUILTIN_SOURCES_PLATFORM_TEST) - def test_no_platform_parameter(self) -> None: - """Without platform parameter, should see public sources but not platform-restricted ones""" + def test_no_platform_with_console_access(self) -> None: + """Without platform parameter, should NOT see platform-restricted sources even with access""" + self.organization.update_option("sentry:enabled_console_platforms", ["nintendo-switch"]) + resp = self.get_response(self.organization.slug) assert resp.status_code == 200 body = resp.data source_keys = [source["sentry_key"] for source in body] - # Should see public sources (no platform restriction) + # Should see public sources assert "public-source-1" in source_keys assert "public-source-2" in source_keys - # Should NOT see platform-restricted source when no platform is provided + # Should NOT see nintendo without a gaming platform specified assert "nintendo" not in source_keys diff --git a/tests/sentry/core/endpoints/test_organization_details.py b/tests/sentry/core/endpoints/test_organization_details.py index f01d258575ad..e203d076de85 100644 --- a/tests/sentry/core/endpoints/test_organization_details.py +++ b/tests/sentry/core/endpoints/test_organization_details.py @@ -1442,6 +1442,62 @@ def test_enabled_and_disabled_console_platforms(self) -> None: == "Enabled platforms: PlayStation, Xbox; Disabled platforms: Nintendo Switch" ) + @patch( + "sentry.tasks.console_platform_cleanup.remove_inaccessible_console_platform_sources.delay" + ) + def test_console_platform_revocation_dispatches_cleanup_task( + self, mock_cleanup_task: MagicMock + ) -> None: + """Revoking console platforms dispatches the cleanup task with remaining platforms""" + staff_user = self.create_user(is_staff=True) + self.create_member(organization=self.organization, user=staff_user, role="owner") + self.login_as(user=staff_user, staff=True) + + self.organization.update_option( + "sentry:enabled_console_platforms", ["playstation", "nintendo-switch"] + ) + + data = {"enabledConsolePlatforms": ["playstation"]} + self.get_success_response(self.organization.slug, **data) + + mock_cleanup_task.assert_called_once_with(self.organization.id, ["playstation"]) + + @patch( + "sentry.tasks.console_platform_cleanup.remove_inaccessible_console_platform_sources.delay" + ) + def test_console_platform_addition_does_not_dispatch_cleanup_task( + self, mock_cleanup_task: MagicMock + ) -> None: + """Adding console platforms without revoking any does not dispatch the cleanup task""" + staff_user = self.create_user(is_staff=True) + self.create_member(organization=self.organization, user=staff_user, role="owner") + self.login_as(user=staff_user, staff=True) + + data = {"enabledConsolePlatforms": ["playstation", "xbox"]} + self.get_success_response(self.organization.slug, **data) + + mock_cleanup_task.assert_not_called() + + @patch( + "sentry.tasks.console_platform_cleanup.remove_inaccessible_console_platform_sources.delay" + ) + def test_console_platform_revoke_all_dispatches_cleanup_task( + self, mock_cleanup_task: MagicMock + ) -> None: + """Revoking all console platforms dispatches the cleanup task with empty list""" + staff_user = self.create_user(is_staff=True) + self.create_member(organization=self.organization, user=staff_user, role="owner") + self.login_as(user=staff_user, staff=True) + + self.organization.update_option( + "sentry:enabled_console_platforms", ["playstation", "nintendo-switch"] + ) + + data: dict[str, list[str]] = {"enabledConsolePlatforms": []} + self.get_success_response(self.organization.slug, **data) + + mock_cleanup_task.assert_called_once_with(self.organization.id, []) + def test_enable_seer_enhanced_alerts_default_true(self) -> None: response = self.get_success_response(self.organization.slug) assert response.data["enableSeerEnhancedAlerts"] is True diff --git a/tests/sentry/tasks/test_console_platform_cleanup.py b/tests/sentry/tasks/test_console_platform_cleanup.py new file mode 100644 index 000000000000..852d9ebc62b5 --- /dev/null +++ b/tests/sentry/tasks/test_console_platform_cleanup.py @@ -0,0 +1,161 @@ +from django.test import override_settings + +from sentry.models.options.project_option import ProjectOption +from sentry.tasks.console_platform_cleanup import remove_inaccessible_console_platform_sources +from sentry.testutils.cases import TestCase + +SENTRY_BUILTIN_SOURCES_TEST = { + "microsoft": { + "id": "sentry:microsoft", + "name": "Microsoft", + "type": "http", + "url": "https://msdl.microsoft.com/download/symbols/", + }, + "nintendo": { + "id": "sentry:nintendo", + "name": "Nintendo SDK", + "type": "s3", + "bucket": "nintendo-symbols", + "region": "us-east-1", + "access_key": "test-key", + "secret_key": "test-secret", + "layout": {"type": "native"}, + "platforms": ["nintendo-switch"], + }, + "playstation": { + "id": "sentry:playstation", + "name": "PlayStation SDK", + "type": "s3", + "bucket": "playstation-symbols", + "region": "us-east-1", + "access_key": "test-key", + "secret_key": "test-secret", + "layout": {"type": "native"}, + "platforms": ["playstation"], + }, + "multi-console": { + "id": "sentry:multi-console", + "name": "Multi Console SDK", + "type": "s3", + "bucket": "multi-console-symbols", + "region": "us-east-1", + "access_key": "test-key", + "secret_key": "test-secret", + "layout": {"type": "native"}, + "platforms": ["nintendo-switch", "playstation"], + }, +} + + +class RemoveInaccessibleConsolePlatformSourcesTest(TestCase): + @override_settings(SENTRY_BUILTIN_SOURCES=SENTRY_BUILTIN_SOURCES_TEST) + def test_removes_sources_for_revoked_platform(self) -> None: + """Revoking nintendo-switch access removes 'nintendo' from projects""" + project = self.create_project(organization=self.organization) + project.update_option("sentry:builtin_symbol_sources", ["microsoft", "nintendo"]) + + # current_platforms after revocation: no console access left + remove_inaccessible_console_platform_sources(self.organization.id, []) + + sources = ProjectOption.objects.get_value(project, "sentry:builtin_symbol_sources") + assert sources == ["microsoft"] + + @override_settings(SENTRY_BUILTIN_SOURCES=SENTRY_BUILTIN_SOURCES_TEST) + def test_preserves_sources_for_remaining_platforms(self) -> None: + """Only sources for inaccessible platforms are removed""" + project = self.create_project(organization=self.organization) + project.update_option( + "sentry:builtin_symbol_sources", ["microsoft", "nintendo", "playstation"] + ) + + # Org still has playstation, lost nintendo-switch + remove_inaccessible_console_platform_sources(self.organization.id, ["playstation"]) + + sources = ProjectOption.objects.get_value(project, "sentry:builtin_symbol_sources") + assert "microsoft" in sources + assert "playstation" in sources + assert "nintendo" not in sources + + @override_settings(SENTRY_BUILTIN_SOURCES=SENTRY_BUILTIN_SOURCES_TEST) + def test_no_change_when_project_lacks_source(self) -> None: + """Projects without the inaccessible source are not modified""" + project = self.create_project(organization=self.organization) + project.update_option("sentry:builtin_symbol_sources", ["microsoft"]) + + remove_inaccessible_console_platform_sources(self.organization.id, []) + + sources = ProjectOption.objects.get_value(project, "sentry:builtin_symbol_sources") + assert sources == ["microsoft"] + + @override_settings(SENTRY_BUILTIN_SOURCES=SENTRY_BUILTIN_SOURCES_TEST) + def test_other_org_projects_unaffected(self) -> None: + """Projects in other orgs are not modified""" + other_org = self.create_organization() + other_project = self.create_project(organization=other_org) + other_project.update_option("sentry:builtin_symbol_sources", ["microsoft", "nintendo"]) + + remove_inaccessible_console_platform_sources(self.organization.id, []) + + sources = ProjectOption.objects.get_value(other_project, "sentry:builtin_symbol_sources") + assert sources == ["microsoft", "nintendo"] + + @override_settings(SENTRY_BUILTIN_SOURCES=SENTRY_BUILTIN_SOURCES_TEST) + def test_multiple_projects_cleaned_up(self) -> None: + """All projects in the org are cleaned up""" + project1 = self.create_project(organization=self.organization) + project2 = self.create_project(organization=self.organization) + project1.update_option("sentry:builtin_symbol_sources", ["microsoft", "nintendo"]) + project2.update_option("sentry:builtin_symbol_sources", ["nintendo"]) + + remove_inaccessible_console_platform_sources(self.organization.id, []) + + sources1 = ProjectOption.objects.get_value(project1, "sentry:builtin_symbol_sources") + sources2 = ProjectOption.objects.get_value(project2, "sentry:builtin_symbol_sources") + assert sources1 == ["microsoft"] + assert sources2 == [] + + @override_settings(SENTRY_BUILTIN_SOURCES=SENTRY_BUILTIN_SOURCES_TEST) + def test_noop_when_all_platforms_still_accessible(self) -> None: + """No changes when org still has access to all platforms""" + project = self.create_project(organization=self.organization) + project.update_option("sentry:builtin_symbol_sources", ["microsoft", "nintendo"]) + + remove_inaccessible_console_platform_sources( + self.organization.id, ["nintendo-switch", "playstation"] + ) + + sources = ProjectOption.objects.get_value(project, "sentry:builtin_symbol_sources") + assert sources == ["microsoft", "nintendo"] + + @override_settings(SENTRY_BUILTIN_SOURCES=SENTRY_BUILTIN_SOURCES_TEST) + def test_multi_platform_source_removed_when_no_platforms_accessible(self) -> None: + """A multi-platform source is removed when the org has access to none + of its platforms.""" + project = self.create_project(organization=self.organization) + project.update_option("sentry:builtin_symbol_sources", ["microsoft", "multi-console"]) + + # Org has no console access at all + remove_inaccessible_console_platform_sources(self.organization.id, []) + + sources = ProjectOption.objects.get_value(project, "sentry:builtin_symbol_sources") + assert sources == ["microsoft"] + + @override_settings(SENTRY_BUILTIN_SOURCES=SENTRY_BUILTIN_SOURCES_TEST) + def test_multi_platform_source_kept_when_one_platform_still_accessible(self) -> None: + """A multi-platform source is preserved when the org still has access + to at least one of its platforms.""" + project = self.create_project(organization=self.organization) + project.update_option("sentry:builtin_symbol_sources", ["microsoft", "multi-console"]) + + # Org lost nintendo-switch but still has playstation + remove_inaccessible_console_platform_sources(self.organization.id, ["playstation"]) + + sources = ProjectOption.objects.get_value(project, "sentry:builtin_symbol_sources") + assert "microsoft" in sources + assert "multi-console" in sources + + @override_settings(SENTRY_BUILTIN_SOURCES=SENTRY_BUILTIN_SOURCES_TEST) + def test_nonexistent_organization(self) -> None: + """Task gracefully handles a nonexistent organization""" + remove_inaccessible_console_platform_sources(999999999, []) + # Should not raise From 22e78478e48925f14abeadd139e752e70ff02879 Mon Sep 17 00:00:00 2001 From: Ogi Date: Wed, 1 Apr 2026 14:40:51 +0200 Subject: [PATCH 03/57] feat(agents): Use CurrencyCell in Agent Monitoring dashboards and handle negative costs (#111986) --- .../widgetCard/visualizationWidget.tsx | 13 ++++-- .../tableCells/currencyCell.spec.tsx | 43 +++++++++++++++++ .../components/tableCells/currencyCell.tsx | 46 ++++++++++++++++++- .../pages/agents/components/modelsTable.tsx | 5 +- .../pages/agents/components/tracesTable.tsx | 8 +--- 5 files changed, 101 insertions(+), 14 deletions(-) create mode 100644 static/app/views/insights/common/components/tableCells/currencyCell.spec.tsx diff --git a/static/app/views/dashboards/widgetCard/visualizationWidget.tsx b/static/app/views/dashboards/widgetCard/visualizationWidget.tsx index d2abadf0cc43..ba83c983390c 100644 --- a/static/app/views/dashboards/widgetCard/visualizationWidget.tsx +++ b/static/app/views/dashboards/widgetCard/visualizationWidget.tsx @@ -48,6 +48,7 @@ import {Thresholds} from 'sentry/views/dashboards/widgets/timeSeriesWidget/plott import {TimeSeriesWidgetVisualization} from 'sentry/views/dashboards/widgets/timeSeriesWidget/timeSeriesWidgetVisualization'; import {Widget} from 'sentry/views/dashboards/widgets/widget/widget'; import {getExploreUrl} from 'sentry/views/explore/utils'; +import {NegativeCostWarning} from 'sentry/views/insights/common/components/tableCells/currencyCell'; import {TextAlignRight} from 'sentry/views/insights/common/components/textAlign'; import type {LoadableChartWidgetProps} from 'sentry/views/insights/common/components/widgets/types'; import {ModelName} from 'sentry/views/insights/pages/agents/components/modelName'; @@ -363,10 +364,14 @@ function VisualizationWidgetContent({ {labelContent} - {dataType === 'number' && - value !== null && - value > 0 && - value < NUMBER_MIN_VALUE ? ( + {dataType === 'currency' && value !== null && value < 0 ? ( + + {formatBreakdownLegendValue(value, dataType, dataUnit)} + + ) : dataType === 'number' && + value !== null && + value > 0 && + value < NUMBER_MIN_VALUE ? ( {formatBreakdownLegendValue(value, dataType, dataUnit)} diff --git a/static/app/views/insights/common/components/tableCells/currencyCell.spec.tsx b/static/app/views/insights/common/components/tableCells/currencyCell.spec.tsx new file mode 100644 index 000000000000..69b4d9025d22 --- /dev/null +++ b/static/app/views/insights/common/components/tableCells/currencyCell.spec.tsx @@ -0,0 +1,43 @@ +import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary'; + +import {CurrencyCell} from './currencyCell'; + +describe('CurrencyCell', () => { + it('renders em dash for null values', () => { + render(); + expect(screen.getByText('\u2014')).toBeInTheDocument(); + }); + + it('renders formatted dollar value', () => { + render(); + expect(screen.getByText('$17.5')).toBeInTheDocument(); + }); + + it('renders less than $0.01 for small positive values', () => { + render(); + expect(screen.getByText(/^<\$/)).toBeInTheDocument(); + }); + + it('renders $0 for zero value', () => { + render(); + expect(screen.getByText('$0')).toBeInTheDocument(); + }); + + it('renders negative value with warning icon', async () => { + render(); + + // The negative value should be rendered + expect(screen.getByText('$-5.5')).toBeInTheDocument(); + + // Warning icon should be present + const warningIcon = screen.getByRole('img', {hidden: true}); + expect(warningIcon).toBeInTheDocument(); + + // Hover over the wrapper to see tooltip + await userEvent.hover(warningIcon); + expect( + await screen.findByText(/Negative costs indicate an error/) + ).toBeInTheDocument(); + expect(screen.getByText('Follow this guide')).toBeInTheDocument(); + }); +}); diff --git a/static/app/views/insights/common/components/tableCells/currencyCell.tsx b/static/app/views/insights/common/components/tableCells/currencyCell.tsx index 3dd5ac7589d0..f91c2e547c4c 100644 --- a/static/app/views/insights/common/components/tableCells/currencyCell.tsx +++ b/static/app/views/insights/common/components/tableCells/currencyCell.tsx @@ -1,10 +1,54 @@ +import {Flex} from '@sentry/scraps/layout'; +import {Tooltip} from '@sentry/scraps/tooltip'; + +import {ExternalLink} from 'sentry/components/links/externalLink'; +import {IconWarning} from 'sentry/icons'; +import {tct} from 'sentry/locale'; import {NumberContainer} from 'sentry/utils/discover/styles'; import {formatDollars} from 'sentry/utils/formatters'; type Props = { - value: number; + value: number | null; }; +const NEGATIVE_COST_DOCS_URL = + 'https://docs.sentry.io/ai/monitoring/agents/costs/#troubleshooting'; + +export function NegativeCostWarning({children}: {children: React.ReactNode}) { + return ( + , + } + )} + skipWrapper + > + + + {children} + + + ); +} + export function CurrencyCell({value}: Props) { + if (value === null || value === undefined) { + return {'\u2014'}; + } + + if (value < 0) { + return ( + + {formatDollars(value)} + + ); + } + + if (value > 0 && value < 0.01) { + return {`<$${(0.01).toLocaleString()}`}; + } + return {formatDollars(value)}; } diff --git a/static/app/views/insights/pages/agents/components/modelsTable.tsx b/static/app/views/insights/pages/agents/components/modelsTable.tsx index 0b9b87b920bf..6a4017a8d5a1 100644 --- a/static/app/views/insights/pages/agents/components/modelsTable.tsx +++ b/static/app/views/insights/pages/agents/components/modelsTable.tsx @@ -19,7 +19,7 @@ import {useOrganization} from 'sentry/utils/useOrganization'; import {Mode} from 'sentry/views/explore/contexts/pageParamsContext/mode'; import {getExploreUrl} from 'sentry/views/explore/utils'; import {ChartType} from 'sentry/views/insights/common/components/chart'; -import {TextAlignRight} from 'sentry/views/insights/common/components/textAlign'; +import {CurrencyCell} from 'sentry/views/insights/common/components/tableCells/currencyCell'; import {useSpans} from 'sentry/views/insights/common/queries/useDiscover'; import { CellLink, @@ -34,7 +34,6 @@ import {ModelName} from 'sentry/views/insights/pages/agents/components/modelName import {useCombinedQuery} from 'sentry/views/insights/pages/agents/hooks/useCombinedQuery'; import {useTableCursor} from 'sentry/views/insights/pages/agents/hooks/useTableCursor'; import {ErrorCell} from 'sentry/views/insights/pages/agents/utils/cells'; -import {formatLLMCosts} from 'sentry/views/insights/pages/agents/utils/formatLLMCosts'; import {getAIGenerationsFilter} from 'sentry/views/insights/pages/agents/utils/query'; import {Referrer} from 'sentry/views/insights/pages/agents/utils/referrers'; import {DurationCell} from 'sentry/views/insights/pages/platform/shared/table/DurationCell'; @@ -263,7 +262,7 @@ const BodyCell = memo(function BodyCell({ case 'p95(span.duration)': return ; case 'sum(gen_ai.cost.total_tokens)': - return {formatLLMCosts(dataRow.cost)}; + return ; case 'count_if(span.status,equals,internal_error)': return ( ; } - return ( - - - - ); + return ; case 'timestamp': return ( From 94d9793fb3529417017a06d369f3853933f3ad7b Mon Sep 17 00:00:00 2001 From: Ogi Date: Wed, 1 Apr 2026 14:47:03 +0200 Subject: [PATCH 04/57] fix(agents): Clarify token counting in LLM onboarding instructions (#111989) --- .../pages/agents/llmOnboardingInstructions.tsx | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/static/app/views/insights/pages/agents/llmOnboardingInstructions.tsx b/static/app/views/insights/pages/agents/llmOnboardingInstructions.tsx index ca9ddf23f577..60688d1b177d 100644 --- a/static/app/views/insights/pages/agents/llmOnboardingInstructions.tsx +++ b/static/app/views/insights/pages/agents/llmOnboardingInstructions.tsx @@ -258,6 +258,23 @@ with sentry_sdk.start_span(op="gen_ai.execute_tool", name=f"execute_tool {tool_n span.set_data("gen_ai.tool.output", json.dumps(result)) \`\`\` +## Token Counting & Cost Calculation + +\`gen_ai.usage.input_tokens\` must be the **total** input tokens (cached + non-cached). Sentry computes cost as \`(input_tokens - cached_tokens) * price\`, so if \`input_tokens\` only contains non-cached tokens, costs go **negative**. Each \`gen_ai.request\` span should only report its own token usage, not an accumulation of tokens from previous spans in the conversation. + +\`\`\`python +# Correct — input_tokens includes cached +span.set_data("gen_ai.usage.input_tokens", 100) # total +span.set_data("gen_ai.usage.input_tokens.cached", 80) # cached subset +span.set_data("gen_ai.usage.output_tokens", 50) + +# Wrong — produces negative cost +span.set_data("gen_ai.usage.input_tokens", 20) # non-cached only +span.set_data("gen_ai.usage.input_tokens.cached", 80) # (20 - 80) * price → negative +\`\`\` + +See: https://docs.sentry.io/ai/monitoring/agents/costs/#troubleshooting + ## Key Rules 1. **Always set the agent name** — enables per-agent dashboards, trace grouping, and alerting @@ -266,4 +283,5 @@ with sentry_sdk.start_span(op="gen_ai.execute_tool", name=f"execute_tool {tool_n 4. **Nest spans correctly:** \`gen_ai.invoke_agent\` should contain \`gen_ai.request\` and \`gen_ai.execute_tool\` as children 5. **JS min version:** \`@sentry/node@10.28.0\` or later 6. **Enable PII:** \`sendDefaultPii: true\` (JS) / \`send_default_pii=True\` (Python) to capture inputs/outputs +7. **\`gen_ai.usage.input_tokens\` must include cached tokens** — otherwise cost calculations will be negative `; From 5def1463945328222cc411c4b887d3e878301209 Mon Sep 17 00:00:00 2001 From: Stefan Jandl Date: Wed, 1 Apr 2026 06:32:33 -0700 Subject: [PATCH 05/57] feat(onboarding): Added Metrics to Unity (#108118) --- .../onboarding/productSelection.tsx | 2 +- static/app/data/platformCategories.tsx | 1 + static/app/gettingStartedDocs/unity/index.tsx | 2 + .../gettingStartedDocs/unity/metrics.spec.tsx | 42 +++++++ .../app/gettingStartedDocs/unity/metrics.tsx | 106 ++++++++++++++++++ .../unity/onboarding.spec.tsx | 14 +++ .../gettingStartedDocs/unity/onboarding.tsx | 22 ++++ 7 files changed, 188 insertions(+), 1 deletion(-) create mode 100644 static/app/gettingStartedDocs/unity/metrics.spec.tsx create mode 100644 static/app/gettingStartedDocs/unity/metrics.tsx diff --git a/static/app/components/onboarding/productSelection.tsx b/static/app/components/onboarding/productSelection.tsx index 7cc88cc02306..f5431ba69416 100644 --- a/static/app/components/onboarding/productSelection.tsx +++ b/static/app/components/onboarding/productSelection.tsx @@ -489,7 +489,7 @@ export const platformProductAvailability = { ProductSolution.PROFILING, ProductSolution.LOGS, ], - unity: [ProductSolution.LOGS], + unity: [ProductSolution.LOGS, ProductSolution.METRICS], unreal: [ProductSolution.LOGS], } as Record; diff --git a/static/app/data/platformCategories.tsx b/static/app/data/platformCategories.tsx index 7ea6da29ccc3..5792f74e5673 100644 --- a/static/app/data/platformCategories.tsx +++ b/static/app/data/platformCategories.tsx @@ -492,6 +492,7 @@ export const withMetricsOnboarding = new Set([ 'ruby', 'ruby-rack', 'ruby-rails', + 'unity', ]); // List of platforms that do not have metrics support. We make use of this list in the product to not provide any Metrics diff --git a/static/app/gettingStartedDocs/unity/index.tsx b/static/app/gettingStartedDocs/unity/index.tsx index c52f302fd9e9..c6d03a345780 100644 --- a/static/app/gettingStartedDocs/unity/index.tsx +++ b/static/app/gettingStartedDocs/unity/index.tsx @@ -2,6 +2,7 @@ import type {Docs} from 'sentry/components/onboarding/gettingStartedDoc/types'; import {crashReport} from './crashReport'; import {logs} from './logs'; +import {metrics} from './metrics'; import {onboarding} from './onboarding'; export const docs: Docs = { @@ -9,4 +10,5 @@ export const docs: Docs = { feedbackOnboardingCrashApi: crashReport, crashReportOnboarding: crashReport, logsOnboarding: logs, + metricsOnboarding: metrics, }; diff --git a/static/app/gettingStartedDocs/unity/metrics.spec.tsx b/static/app/gettingStartedDocs/unity/metrics.spec.tsx new file mode 100644 index 000000000000..041c53830e41 --- /dev/null +++ b/static/app/gettingStartedDocs/unity/metrics.spec.tsx @@ -0,0 +1,42 @@ +import {ProjectFixture} from 'sentry-fixture/project'; + +import {renderWithOnboardingLayout} from 'sentry-test/onboarding/renderWithOnboardingLayout'; +import {screen} from 'sentry-test/reactTestingLibrary'; +import {textWithMarkupMatcher} from 'sentry-test/utils'; + +import {ProductSolution} from 'sentry/components/onboarding/gettingStartedDoc/types'; + +import {docs} from '.'; + +function renderMockRequests() { + MockApiClient.addMockResponse({ + url: '/projects/org-slug/project-slug/', + body: [ProjectFixture()], + }); +} + +describe('metrics', () => { + it('unity metrics onboarding docs', () => { + renderMockRequests(); + + renderWithOnboardingLayout(docs, { + selectedProducts: [ProductSolution.METRICS], + }); + + expect( + screen.getByText(textWithMarkupMatcher(/SentrySdk\.Metrics\.Increment/)) + ).toBeInTheDocument(); + }); + + it('does not render metrics configuration when metrics is not enabled', () => { + renderMockRequests(); + + renderWithOnboardingLayout(docs, { + selectedProducts: [], + }); + + expect( + screen.queryByText(textWithMarkupMatcher(/SentrySdk\.Metrics\.Increment/)) + ).not.toBeInTheDocument(); + }); +}); diff --git a/static/app/gettingStartedDocs/unity/metrics.tsx b/static/app/gettingStartedDocs/unity/metrics.tsx new file mode 100644 index 000000000000..9da762075eb9 --- /dev/null +++ b/static/app/gettingStartedDocs/unity/metrics.tsx @@ -0,0 +1,106 @@ +import {ExternalLink} from '@sentry/scraps/link'; + +import type { + ContentBlock, + DocsParams, + OnboardingConfig, +} from 'sentry/components/onboarding/gettingStartedDoc/types'; +import {StepType} from 'sentry/components/onboarding/gettingStartedDoc/types'; +import {t, tct} from 'sentry/locale'; + +export const metricsVerify = (params: DocsParams): ContentBlock => ({ + type: 'conditional', + condition: params.isMetricsSelected, + content: [ + { + type: 'text', + text: t( + 'Send test metrics from your app to verify metrics are arriving in Sentry.' + ), + }, + { + type: 'code', + language: 'csharp', + code: `using Sentry; + +SentrySdk.Metrics.Increment("player_interaction", + tags: new Dictionary {{"action", "jump"}, {"scene", "main_menu"}}); +SentrySdk.Metrics.Distribution("scene_load", 230, + unit: MeasurementUnit.Duration.Millisecond, + tags: new Dictionary {{"scene", "world_1"}}); +SentrySdk.Metrics.Gauge("active_players", 42, + tags: new Dictionary {{"server", "us-east-1"}});`, + }, + { + type: 'text', + text: tct('For more detailed information, see the [link:metrics documentation].', { + link: , + }), + }, + ], +}); + +export const metrics: OnboardingConfig = { + install: () => [ + { + type: StepType.INSTALL, + content: [ + { + type: 'text', + text: tct( + 'Metrics for Unity are supported in Sentry SDK version [code:4.1.0] and above.', + { + code: , + } + ), + }, + ], + }, + ], + configure: (params: DocsParams) => [ + { + type: StepType.CONFIGURE, + content: [ + { + type: 'text', + text: t( + 'To enable metrics in your Unity game, you need to configure the Sentry SDK with metrics enabled.' + ), + }, + { + type: 'text', + text: tct( + 'Open your project settings: [strong:Tools > Sentry > Advanced > Metrics] and check the [strong:Enable Metrics] option.', + { + strong: , + } + ), + }, + { + type: 'text', + text: t('Alternatively, you can enable metrics programmatically:'), + }, + { + type: 'code', + language: 'csharp', + code: `SentrySdk.Init(options => +{ + options.Dsn = "${params.dsn.public}"; + + // Enable metrics to be sent to Sentry + options.ExperimentalMetrics = new ExperimentalMetricsOptions + { + EnableCodeLocations = true + }; +});`, + }, + ], + }, + ], + verify: (params: DocsParams) => [ + { + type: StepType.VERIFY, + content: [metricsVerify(params)], + }, + ], +}; diff --git a/static/app/gettingStartedDocs/unity/onboarding.spec.tsx b/static/app/gettingStartedDocs/unity/onboarding.spec.tsx index 0ee5a30a6bfa..421d56c530be 100644 --- a/static/app/gettingStartedDocs/unity/onboarding.spec.tsx +++ b/static/app/gettingStartedDocs/unity/onboarding.spec.tsx @@ -4,6 +4,8 @@ import {renderWithOnboardingLayout} from 'sentry-test/onboarding/renderWithOnboa import {screen} from 'sentry-test/reactTestingLibrary'; import {textWithMarkupMatcher} from 'sentry-test/utils'; +import {ProductSolution} from 'sentry/components/onboarding/gettingStartedDoc/types'; + import {docs} from '.'; function renderMockRequests() { @@ -32,4 +34,16 @@ describe('unity onboarding docs', () => { ) ).toBeInTheDocument(); }); + + it('renders metrics snippet when metrics product is selected', () => { + renderMockRequests(); + + renderWithOnboardingLayout(docs, { + selectedProducts: [ProductSolution.METRICS], + }); + + expect( + screen.getByText(textWithMarkupMatcher(/SentrySdk\.Metrics\.Increment/)) + ).toBeInTheDocument(); + }); }); diff --git a/static/app/gettingStartedDocs/unity/onboarding.tsx b/static/app/gettingStartedDocs/unity/onboarding.tsx index 6ac25fed2f80..b3123a465194 100644 --- a/static/app/gettingStartedDocs/unity/onboarding.tsx +++ b/static/app/gettingStartedDocs/unity/onboarding.tsx @@ -11,6 +11,7 @@ import {getConsoleExtensions} from 'sentry/components/onboarding/gettingStartedD import {t, tct} from 'sentry/locale'; import {logsVerify} from './logs'; +import {metricsVerify} from './metrics'; const getVerifySnippet = () => ` using Sentry; // On the top of the script @@ -74,6 +75,19 @@ export const onboarding: OnboardingConfig = { }, ], }, + { + type: 'conditional', + condition: params.isMetricsSelected, + content: [ + { + type: 'text', + text: tct( + 'To enable metrics, navigate to [strong:Tools > Sentry > Advanced > Metrics] and check the [strong:Enable Metrics] option.', + {strong: } + ), + }, + ], + }, { type: 'text', text: tct( @@ -113,6 +127,14 @@ export const onboarding: OnboardingConfig = { }, ] satisfies OnboardingStep[]) : []), + ...(params.isMetricsSelected + ? ([ + { + title: t('Metrics'), + content: [metricsVerify(params)], + }, + ] satisfies OnboardingStep[]) + : []), { title: t('Troubleshooting'), content: [ From c7cafd25b42e2b7dd2b51c02c2b440593820a68b Mon Sep 17 00:00:00 2001 From: Ogi Date: Wed, 1 Apr 2026 15:40:51 +0200 Subject: [PATCH 06/57] fix(agents): Make negative cost tooltip hoverable for clickable link (#111991) --- .../views/insights/common/components/tableCells/currencyCell.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/static/app/views/insights/common/components/tableCells/currencyCell.tsx b/static/app/views/insights/common/components/tableCells/currencyCell.tsx index f91c2e547c4c..bac869de25de 100644 --- a/static/app/views/insights/common/components/tableCells/currencyCell.tsx +++ b/static/app/views/insights/common/components/tableCells/currencyCell.tsx @@ -24,6 +24,7 @@ export function NegativeCostWarning({children}: {children: React.ReactNode}) { } )} skipWrapper + isHoverable > From 994bd6df4c323fc1834af4f5ec853cb6a923fd6f Mon Sep 17 00:00:00 2001 From: Tony Xiao Date: Wed, 1 Apr 2026 10:14:00 -0400 Subject: [PATCH 07/57] feat(autofix): Remove empty file patches from autofix (#111961) Empty file patches are generated to overwrite previous file patches. These are rendered as empty changes. We only want to preserve the last file patch for a file and we need to ensure it is a non empty file patch. --- .../autofix/useExplorerAutofix.spec.tsx | 153 +++++++++++++++++- .../events/autofix/useExplorerAutofix.tsx | 41 +++++ .../events/autofix/v3/autofixCards.tsx | 22 +-- .../events/autofix/v3/autofixPreviews.tsx | 18 +-- 4 files changed, 205 insertions(+), 29 deletions(-) diff --git a/static/app/components/events/autofix/useExplorerAutofix.spec.tsx b/static/app/components/events/autofix/useExplorerAutofix.spec.tsx index c1a9fd7ef0cb..d8db6726e967 100644 --- a/static/app/components/events/autofix/useExplorerAutofix.spec.tsx +++ b/static/app/components/events/autofix/useExplorerAutofix.spec.tsx @@ -3,6 +3,7 @@ import {act, renderHookWithProviders, waitFor} from 'sentry-test/reactTestingLib import {addErrorMessage} from 'sentry/actionCreators/indicator'; import {DiffFileType, DiffLineType} from 'sentry/components/events/autofix/types'; import { + collectPatches, isCodeChangesArtifact, isCodingAgentsArtifact, isPullRequestsArtifact, @@ -12,7 +13,7 @@ import { type RootCauseArtifact, type SolutionArtifact, } from 'sentry/components/events/autofix/useExplorerAutofix'; -import type {Artifact} from 'sentry/views/seerExplorer/types'; +import type {Artifact, ExplorerFilePatch} from 'sentry/views/seerExplorer/types'; jest.mock('sentry/actionCreators/indicator'); @@ -240,6 +241,156 @@ describe('isCodingAgentsArtifact', () => { }); }); +describe('collectPatches', () => { + function makePatch( + overrides: Partial & {repo_name: string} + ): ExplorerFilePatch { + return { + diff: 'diff content', + patch: { + added: 1, + removed: 0, + path: 'file.py', + source_file: 'file.py', + target_file: 'file.py', + type: DiffFileType.MODIFIED, + hunks: [], + }, + ...overrides, + }; + } + + it('returns an empty map for empty input', () => { + expect(collectPatches([])).toEqual(new Map()); + }); + + it('returns a single patch grouped by repo', () => { + const patch = makePatch({repo_name: 'owner/repo'}); + const result = collectPatches([patch]); + + expect(result.size).toBe(1); + expect(result.get('owner/repo')).toEqual([patch]); + }); + + it('groups multiple patches in the same repo', () => { + const patch1 = makePatch({ + repo_name: 'owner/repo', + patch: { + added: 1, + removed: 0, + path: 'a.py', + source_file: 'a.py', + target_file: 'a.py', + type: DiffFileType.MODIFIED, + hunks: [], + }, + }); + const patch2 = makePatch({ + repo_name: 'owner/repo', + patch: { + added: 2, + removed: 1, + path: 'b.py', + source_file: 'b.py', + target_file: 'b.py', + type: DiffFileType.MODIFIED, + hunks: [], + }, + }); + + const result = collectPatches([patch1, patch2]); + + expect(result.size).toBe(1); + expect(result.get('owner/repo')).toEqual([patch1, patch2]); + }); + + it('separates patches into different repos', () => { + const patch1 = makePatch({repo_name: 'owner/repo-a'}); + const patch2 = makePatch({repo_name: 'owner/repo-b'}); + + const result = collectPatches([patch1, patch2]); + + expect(result.size).toBe(2); + expect(result.get('owner/repo-a')).toEqual([patch1]); + expect(result.get('owner/repo-b')).toEqual([patch2]); + }); + + it('deduplicates by file path keeping the last occurrence', () => { + const patchOld = makePatch({ + repo_name: 'owner/repo', + diff: 'old diff', + patch: { + added: 1, + removed: 0, + path: 'file.py', + source_file: 'file.py', + target_file: 'file.py', + type: DiffFileType.MODIFIED, + hunks: [], + }, + }); + const patchNew = makePatch({ + repo_name: 'owner/repo', + diff: 'new diff', + patch: { + added: 3, + removed: 2, + path: 'file.py', + source_file: 'file.py', + target_file: 'file.py', + type: DiffFileType.MODIFIED, + hunks: [], + }, + }); + + const result = collectPatches([patchOld, patchNew]); + + expect(result.get('owner/repo')).toHaveLength(1); + expect(result.get('owner/repo')![0]!.diff).toBe('new diff'); + }); + + it('filters out no-op patches with zero added and removed', () => { + const noOpPatch = makePatch({ + repo_name: 'owner/repo', + patch: { + added: 0, + removed: 0, + path: 'file.py', + source_file: 'file.py', + target_file: 'file.py', + type: DiffFileType.MODIFIED, + hunks: [], + }, + }); + + const result = collectPatches([noOpPatch]); + + expect(result.size).toBe(0); + }); + + it('removes repos that have only no-op patches', () => { + const noOp = makePatch({ + repo_name: 'owner/empty-repo', + patch: { + added: 0, + removed: 0, + path: 'file.py', + source_file: 'file.py', + target_file: 'file.py', + type: DiffFileType.MODIFIED, + hunks: [], + }, + }); + const real = makePatch({repo_name: 'owner/real-repo'}); + + const result = collectPatches([noOp, real]); + + expect(result.size).toBe(1); + expect(result.has('owner/empty-repo')).toBe(false); + expect(result.get('owner/real-repo')).toEqual([real]); + }); +}); + describe('useExplorerAutofix - createPR', () => { const GROUP_ID = '123'; const AUTOFIX_URL = `/organizations/org-slug/issues/${GROUP_ID}/autofix/`; diff --git a/static/app/components/events/autofix/useExplorerAutofix.tsx b/static/app/components/events/autofix/useExplorerAutofix.tsx index 1b0d37740618..5f81f93b13a5 100644 --- a/static/app/components/events/autofix/useExplorerAutofix.tsx +++ b/static/app/components/events/autofix/useExplorerAutofix.tsx @@ -795,3 +795,44 @@ export function useExplorerAutofix( triggerCodingAgentHandoff, }; } + +export function collectPatches( + patches: ExplorerFilePatch[] +): Map { + const patchesByRepo = new Map(); + + for (const patch of patches) { + const existing = patchesByRepo.get(patch.repo_name) || []; + existing.push(patch); + patchesByRepo.set(patch.repo_name, existing); + } + + for (const [repoName, repoPatches] of patchesByRepo) { + const cleanedPatches = cleanPatches(repoPatches); + if (cleanedPatches.length) { + patchesByRepo.set(repoName, cleanedPatches); + } else { + patchesByRepo.delete(repoName); + } + } + + return patchesByRepo; +} + +function cleanPatches(patches: ExplorerFilePatch[]): ExplorerFilePatch[] { + const cleanedPatches: ExplorerFilePatch[] = []; + + const patchedFiles = new Set(); + + patches.toReversed().forEach(patch => { + if (patchedFiles.has(patch.patch.path)) { + return; + } + patchedFiles.add(patch.patch.path); + cleanedPatches.push(patch); + }); + + return cleanedPatches.reverse().filter(patch => { + return patch.patch.added > 0 || patch.patch.removed > 0; + }); +} diff --git a/static/app/components/events/autofix/v3/autofixCards.tsx b/static/app/components/events/autofix/v3/autofixCards.tsx index a13d58f40222..f21bd17202f1 100644 --- a/static/app/components/events/autofix/v3/autofixCards.tsx +++ b/static/app/components/events/autofix/v3/autofixCards.tsx @@ -12,6 +12,7 @@ import { getResultButtonLabel, } from 'sentry/components/events/autofix/types'; import { + collectPatches, getAutofixArtifactFromSection, isCodeChangesArtifact, isCodingAgentsArtifact, @@ -33,7 +34,6 @@ import {IconPullRequest} from 'sentry/icons/iconPullRequest'; import {t, tct, tn} from 'sentry/locale'; import {defined} from 'sentry/utils'; import {FileDiffViewer} from 'sentry/views/seerExplorer/fileDiffViewer'; -import {type ExplorerFilePatch} from 'sentry/views/seerExplorer/types'; interface AutofixCardProps { autofix: ReturnType; @@ -167,22 +167,14 @@ export function CodeChangesCard({autofix, section}: AutofixCardProps) { return isCodeChangesArtifact(sectionArtifact) ? sectionArtifact : null; }, [section]); - const patchesForRepos = useMemo(() => { - const patchesByRepo = new Map(); - for (const patch of artifact ?? []) { - const existing = patchesByRepo.get(patch.repo_name) || []; - existing.push(patch); - patchesByRepo.set(patch.repo_name, existing); - } - return patchesByRepo; - }, [artifact]); + const patchesByRepo = useMemo(() => collectPatches(artifact ?? []), [artifact]); const summary = useMemo(() => { - const reposChanged = patchesForRepos.size; + const reposChanged = patchesByRepo.size; const filesChanged = new Set(); - for (const [repo, patchesForRepo] of patchesForRepos.entries()) { + for (const [repo, patchesForRepo] of patchesByRepo.entries()) { for (const patch of patchesForRepo) { filesChanged.add(`${repo}:${patch.patch.path}`); } @@ -197,7 +189,7 @@ export function CodeChangesCard({autofix, section}: AutofixCardProps) { } return t('%s files changed in %s repos', filesChanged.size, reposChanged); - }, [patchesForRepos]); + }, [patchesByRepo]); const {runState, startStep} = autofix; const runId = runState?.run_id; @@ -208,10 +200,10 @@ export function CodeChangesCard({autofix, section}: AutofixCardProps) { ) : ( - {patchesForRepos.size ? ( + {patchesByRepo.size ? ( {summary} - {[...patchesForRepos.entries()].map(([repo, patches]) => ( + {[...patchesByRepo.entries()].map(([repo, patches]) => ( {t('Repository:')} diff --git a/static/app/components/events/autofix/v3/autofixPreviews.tsx b/static/app/components/events/autofix/v3/autofixPreviews.tsx index 1ab9f0499f1f..f25cfa45d917 100644 --- a/static/app/components/events/autofix/v3/autofixPreviews.tsx +++ b/static/app/components/events/autofix/v3/autofixPreviews.tsx @@ -11,6 +11,7 @@ import { getCodingAgentName, } from 'sentry/components/events/autofix/types'; import { + collectPatches, getAutofixArtifactFromSection, isCodeChangesArtifact, isCodingAgentsArtifact, @@ -27,7 +28,6 @@ import {IconCode} from 'sentry/icons/iconCode'; import {IconList} from 'sentry/icons/iconList'; import {IconPullRequest} from 'sentry/icons/iconPullRequest'; import {t, tn} from 'sentry/locale'; -import {type ExplorerFilePatch} from 'sentry/views/seerExplorer/types'; interface ArtifactPreviewProps { section: AutofixSection; @@ -83,22 +83,14 @@ export function CodeChangesPreview({section}: ArtifactPreviewProps) { return isCodeChangesArtifact(sectionArtifact) ? sectionArtifact : []; }, [section]); - const patchesForRepos = useMemo(() => { - const patchesByRepo = new Map(); - for (const patch of artifact) { - const existing = patchesByRepo.get(patch.repo_name) || []; - existing.push(patch); - patchesByRepo.set(patch.repo_name, existing); - } - return patchesByRepo; - }, [artifact]); + const patchesByRepo = useMemo(() => collectPatches(artifact ?? []), [artifact]); const summary = useMemo(() => { - const reposChanged = patchesForRepos.size; + const reposChanged = patchesByRepo.size; const filesChanged = new Set(); - for (const [repo, patchesForRepo] of patchesForRepos.entries()) { + for (const [repo, patchesForRepo] of patchesByRepo.entries()) { for (const patch of patchesForRepo) { filesChanged.add(`${repo}:${patch.patch.path}`); } @@ -129,7 +121,7 @@ export function CodeChangesPreview({section}: ArtifactPreviewProps) { return ( {t('%s files changed in %s repos', filesChanged.size, reposChanged)} ); - }, [patchesForRepos]); + }, [patchesByRepo]); return ( } title={t('Code Changes')}> From ff98e897f94ea9adf696a9bd43fbf955f948f48d Mon Sep 17 00:00:00 2001 From: Giovanni Barillari Date: Wed, 1 Apr 2026 17:00:54 +0200 Subject: [PATCH 08/57] fix(apigateway): async/sync flow patches (#111993) SSIA Current impl does `sync_to_async` of `async_to_sync` when ASGI is enabled. This fixes that. --- .../apigateway_async/middleware.py | 49 +++---------------- tests/sentry/middleware/test_proxy.py | 25 ---------- .../endpoints/test_organization.py | 22 +++++---- ...test_organization_trace_item_attributes.py | 2 + 4 files changed, 22 insertions(+), 76 deletions(-) diff --git a/src/sentry/hybridcloud/apigateway_async/middleware.py b/src/sentry/hybridcloud/apigateway_async/middleware.py index 446210ee1836..963b3530a860 100644 --- a/src/sentry/hybridcloud/apigateway_async/middleware.py +++ b/src/sentry/hybridcloud/apigateway_async/middleware.py @@ -1,10 +1,9 @@ from __future__ import annotations -import asyncio from collections.abc import Callable from typing import Any -from asgiref.sync import async_to_sync, iscoroutinefunction, markcoroutinefunction +from asgiref.sync import iscoroutinefunction, markcoroutinefunction from django.http.response import HttpResponseBase from rest_framework.request import Request @@ -15,12 +14,15 @@ class ApiGatewayMiddleware: """Proxy requests intended for remote silos""" async_capable = True - sync_capable = True + sync_capable = False def __init__(self, get_response: Callable[[Request], HttpResponseBase]): self.get_response = get_response - if iscoroutinefunction(self.get_response): - markcoroutinefunction(self) + markcoroutinefunction(self) + # NOTE: this shouldn't be necessary, but for $REASONS Django's middleware + # sync-async code patches won't recognize `process_view` as async. + # The following line does. + markcoroutinefunction(self.process_view) def __call__(self, request: Request) -> Any: if iscoroutinefunction(self): @@ -30,42 +32,7 @@ def __call__(self, request: Request) -> Any: async def __acall__(self, request: Request) -> HttpResponseBase: return await self.get_response(request) # type: ignore[misc] - def process_view( - self, - request: Request, - view_func: Callable[..., HttpResponseBase], - view_args: tuple[str], - view_kwargs: dict[str, Any], - ) -> HttpResponseBase | None: - return self._process_view_match(request, view_func, view_args, view_kwargs) - - def _process_view_match( - self, - request: Request, - view_func: Callable[..., HttpResponseBase], - view_args: tuple[str], - view_kwargs: dict[str, Any], - ) -> Any: - #: we check if we're in an async or sync runtime once, then - # overwrite the method with the actual impl. - try: - asyncio.get_running_loop() - method = self._process_view_inner - except RuntimeError: - method = self._process_view_sync # type: ignore[assignment] - setattr(self, "_process_view_match", method) - return method(request, view_func, view_args, view_kwargs) - - def _process_view_sync( - self, - request: Request, - view_func: Callable[..., HttpResponseBase], - view_args: tuple[str], - view_kwargs: dict[str, Any], - ) -> HttpResponseBase | None: - return async_to_sync(self._process_view_inner)(request, view_func, view_args, view_kwargs) - - async def _process_view_inner( + async def process_view( self, request: Request, view_func: Callable[..., HttpResponseBase], diff --git a/tests/sentry/middleware/test_proxy.py b/tests/sentry/middleware/test_proxy.py index 3d1bcb8163f9..a607aff96455 100644 --- a/tests/sentry/middleware/test_proxy.py +++ b/tests/sentry/middleware/test_proxy.py @@ -1,8 +1,6 @@ from __future__ import annotations -import asyncio from functools import cached_property -from unittest.mock import patch from django.http import HttpRequest @@ -51,29 +49,6 @@ class FakedAPIProxyTest(APITestCase): endpoint = "sentry-api-0-organization-teams" method = "post" - def setUp(self) -> None: - super().setUp() - - from sentry.hybridcloud.apigateway_async.middleware import ApiGatewayMiddleware - - _original_middleware = ApiGatewayMiddleware._process_view_inner - - def _process_view_match(self, request, view_func, view_args, view_kwargs): - try: - asyncio.get_running_loop() - return self._process_view_inner(request, view_func, view_args, view_kwargs) - except RuntimeError: - return self._process_view_sync(request, view_func, view_args, view_kwargs) - - self._middleware_patch = patch.object( - ApiGatewayMiddleware, "_process_view_match", _process_view_match - ) - self._middleware_patch.start() - - def tearDown(self) -> None: - self._middleware_patch.stop() - super().tearDown() - def test_through_api_gateway(self) -> None: if SiloMode.get_current_mode() == SiloMode.MONOLITH: return diff --git a/tests/sentry/objectstore/endpoints/test_organization.py b/tests/sentry/objectstore/endpoints/test_organization.py index d327fc4406c1..7a586c095bd6 100644 --- a/tests/sentry/objectstore/endpoints/test_organization.py +++ b/tests/sentry/objectstore/endpoints/test_organization.py @@ -177,12 +177,6 @@ class OrganizationObjectstoreEndpointWithControlSiloTest(TransactionTestCase): def setUp(self) -> None: super().setUp() - self.login_as(user=self.user) - self.organization = self.create_organization(owner=self.user) - self.api_key = self.create_api_key( - organization=self.organization, - scope_list=["project:releases"], - ) #: some shenanigans to work around async/sync hell: # - use a "one shot" httpx client, so that we're not bound previous @@ -202,14 +196,15 @@ def build_request(self, *args, **kwargs): from sentry.hybridcloud.apigateway_async.middleware import ApiGatewayMiddleware - _original_middleware = ApiGatewayMiddleware._process_view_inner + _original_mw = ApiGatewayMiddleware.process_view - async def _eager_process_view_inner(mw_self, request, view_func, view_args, view_kwargs): - resp = await _original_middleware(mw_self, request, view_func, view_args, view_kwargs) + async def _eager_process_view(mw_self, request, view_func, view_args, view_kwargs): + resp = await _original_mw(mw_self, request, view_func, view_args, view_kwargs) if isinstance(resp, StreamingHttpResponse) and resp.is_async: body = b"" async for chunk in resp: body += chunk + await proxy_mod.proxy_client.aclose() sync_resp = HttpResponse( content=body, status=resp.status_code, @@ -223,11 +218,18 @@ async def _eager_process_view_inner(mw_self, request, view_func, view_args, view self._apigateway_patch = patch.object(proxy_mod, "proxy_client", HTTPXOneShotClient()) self._middleware_patch = patch.object( - ApiGatewayMiddleware, "_process_view_inner", _eager_process_view_inner + ApiGatewayMiddleware, "process_view", _eager_process_view ) self._apigateway_patch.start() self._middleware_patch.start() + self.login_as(user=self.user) + self.organization = self.create_organization(owner=self.user) + self.api_key = self.create_api_key( + organization=self.organization, + scope_list=["project:releases"], + ) + def tearDown(self) -> None: self._middleware_patch.stop() self._apigateway_patch.stop() diff --git a/tests/snuba/api/endpoints/test_organization_trace_item_attributes.py b/tests/snuba/api/endpoints/test_organization_trace_item_attributes.py index 91d70a23b29a..4b7dcbfc394c 100644 --- a/tests/snuba/api/endpoints/test_organization_trace_item_attributes.py +++ b/tests/snuba/api/endpoints/test_organization_trace_item_attributes.py @@ -2,6 +2,7 @@ from unittest import mock from uuid import uuid4 +import pytest from django.urls import reverse from rest_framework.exceptions import ErrorDetail @@ -2512,6 +2513,7 @@ def test_mixed_valid_and_invalid(self): assert attrs["nonexistent.tag"]["valid"] is False assert "error" in attrs["nonexistent.tag"] + @pytest.mark.xfail(reason="Flaky test - PR #111993") # noqa: F821 def test_stats_period_limits_time_range(self): self.store_segment( self.project.id, From 9da640556ab7f229f5ee937fe89eb0291b022bad Mon Sep 17 00:00:00 2001 From: Max Topolsky <30879163+mtopo27@users.noreply.github.com> Date: Wed, 1 Apr 2026 11:27:10 -0400 Subject: [PATCH 09/57] feat(preprod): Add triggered condition section for size analysis issues (#111978) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Display the triggered condition details on the issue detail page for `preprod_size_analysis` issues. New self-gating `SizeAnalysisTriggeredSection` component renders an `InterimSection` with a `KeyValueList` showing: - **Threshold Type** — absolute size, absolute diff, or relative diff - **Measurement** — platform-aware label derived from `head.artifact_type` event tag (e.g., "Install Size" for Apple, "Uncompressed Size" for Android) - **Query** — shown when a filter query is configured - **Condition** — human-readable threshold description (e.g., "Above 1 MB") - **Evaluated Value** — formatted with 2 decimal places and appropriate units Action buttons: - **Open Build** — always shown, links to the head build detail page - **Open Comparison** — shown for `absolute_diff` / `relative_diff` threshold types when a base artifact exists Also adds `getMetricLabelForArtifactType` to the preprod types utils, exports `formatComparisonValue` and `getConditionDescription` from mobileBuild/detect.tsx, and adds a space between values and units in condition descriptions. Diff Example CleanShot 2026-04-01 at 09 33 35@2x absolute example CleanShot 2026-04-01 at 09 34 23@2x Refs EME-981 --------- Co-authored-by: Claude Opus 4.6 --- .../forms/mobileBuild/detectSection.tsx | 3 +- .../groupEventDetailsContent.tsx | 4 + .../sizeAnalysisTriggeredSection.spec.tsx | 229 ++++++++++++++++++ .../sidebar/sizeAnalysisTriggeredSection.tsx | 187 ++++++++++++++ .../views/settings/project/preprod/types.ts | 13 + 5 files changed, 435 insertions(+), 1 deletion(-) create mode 100644 static/app/views/issueDetails/streamline/sidebar/sizeAnalysisTriggeredSection.spec.tsx create mode 100644 static/app/views/issueDetails/streamline/sidebar/sizeAnalysisTriggeredSection.tsx diff --git a/static/app/views/detectors/components/forms/mobileBuild/detectSection.tsx b/static/app/views/detectors/components/forms/mobileBuild/detectSection.tsx index 2279e853ddcd..1728d8ce9e50 100644 --- a/static/app/views/detectors/components/forms/mobileBuild/detectSection.tsx +++ b/static/app/views/detectors/components/forms/mobileBuild/detectSection.tsx @@ -24,6 +24,7 @@ import type {Platform} from 'sentry/views/preprod/types/sharedTypes'; import { getMetricLabelForPlatform, guessPlatformForProject, + isDiffThreshold, MEASUREMENT_OPTIONS, METRIC_OPTIONS, } from 'sentry/views/settings/project/preprod/types'; @@ -81,7 +82,7 @@ export function MobileBuildDetectSection() { flexibleControlStateSize preserveOnUnmount /> - {(thresholdType === 'absolute_diff' || thresholdType === 'relative_diff') && ( + {isDiffThreshold(thresholdType) && ( diff --git a/static/app/views/issueDetails/groupEventDetails/groupEventDetailsContent.tsx b/static/app/views/issueDetails/groupEventDetails/groupEventDetailsContent.tsx index 30e1dcb03457..fe07654eed57 100644 --- a/static/app/views/issueDetails/groupEventDetails/groupEventDetailsContent.tsx +++ b/static/app/views/issueDetails/groupEventDetails/groupEventDetailsContent.tsx @@ -90,6 +90,7 @@ import {useCopyIssueDetails} from 'sentry/views/issueDetails/streamline/hooks/us import {InstrumentationFixSection} from 'sentry/views/issueDetails/streamline/instrumentationFixSection'; import {InterimSection} from 'sentry/views/issueDetails/streamline/interimSection'; import {MetricDetectorTriggeredSection} from 'sentry/views/issueDetails/streamline/sidebar/metricDetectorTriggeredSection'; +import {SizeAnalysisTriggeredSection} from 'sentry/views/issueDetails/streamline/sidebar/sizeAnalysisTriggeredSection'; import {TraceDataSection} from 'sentry/views/issueDetails/traceDataSection'; import {useHasStreamlinedUI} from 'sentry/views/issueDetails/utils'; import {DEFAULT_TRACE_VIEW_PREFERENCES} from 'sentry/views/performance/newTraceDetails/traceState/tracePreferences'; @@ -386,6 +387,9 @@ export function EventDetailsContent({ null}> + null}> + + {defined(eventEntries[EntryType.HPKP]) && ( diff --git a/static/app/views/issueDetails/streamline/sidebar/sizeAnalysisTriggeredSection.spec.tsx b/static/app/views/issueDetails/streamline/sidebar/sizeAnalysisTriggeredSection.spec.tsx new file mode 100644 index 000000000000..4d2985a8b600 --- /dev/null +++ b/static/app/views/issueDetails/streamline/sidebar/sizeAnalysisTriggeredSection.spec.tsx @@ -0,0 +1,229 @@ +import type {ComponentProps} from 'react'; +import {EventFixture} from 'sentry-fixture/event'; +import {GroupFixture} from 'sentry-fixture/group'; + +import {render, screen} from 'sentry-test/reactTestingLibrary'; + +import {IssueCategory, IssueType} from 'sentry/types/group'; +import {DataConditionType} from 'sentry/types/workflowEngine/dataConditions'; +import type {MetricCondition} from 'sentry/types/workflowEngine/detectors'; +import {SizeAnalysisTriggeredSection} from 'sentry/views/issueDetails/streamline/sidebar/sizeAnalysisTriggeredSection'; + +describe('SizeAnalysisTriggeredSection', () => { + const condition: MetricCondition = { + id: 'cond-1', + type: DataConditionType.GREATER, + comparison: 1000000, + conditionResult: 75, + }; + + const defaultGroup = GroupFixture({ + issueType: IssueType.PREPROD_SIZE_ANALYSIS, + issueCategory: IssueCategory.PREPROD, + }); + + const defaultEvidenceData = { + detectorId: 8, + value: 4292608, + conditions: [condition], + config: { + measurement: 'install_size', + thresholdType: 'absolute_diff', + }, + headArtifactId: 100, + baseArtifactId: 99, + headSizeMetricId: 200, + baseSizeMetricId: 199, + }; + + const defaultEvent = EventFixture({ + id: 'event-1', + eventID: 'event-1', + occurrence: { + id: '1', + eventId: 'event-1', + fingerprint: ['fingerprint'], + issueTitle: 'Size regression', + subtitle: 'A preprod static analysis issue was detected', + resourceId: 'resource-1', + evidenceData: defaultEvidenceData, + evidenceDisplay: [], + type: 11003, + detectionTime: '2024-01-01T00:00:00Z', + }, + }); + + const defaultProps: ComponentProps = { + group: defaultGroup, + event: defaultEvent, + }; + + it('renders nothing when event has no occurrence', () => { + const event = EventFixture({occurrence: null}); + const {container} = render( + + ); + expect(container).toBeEmptyDOMElement(); + }); + + it('renders nothing when evidenceData has no config', () => { + const event = EventFixture({ + occurrence: { + id: '1', + eventId: 'event-1', + fingerprint: ['fingerprint'], + issueTitle: 'Test', + subtitle: '', + resourceId: 'resource-1', + evidenceData: {}, + evidenceDisplay: [], + type: 11003, + detectionTime: '2024-01-01T00:00:00Z', + }, + }); + const {container} = render( + + ); + expect(container).toBeEmptyDOMElement(); + }); + + it('renders triggered condition details for absolute_diff', () => { + render(); + + expect(screen.getByRole('region', {name: 'Triggered Condition'})).toBeInTheDocument(); + + expect(screen.getByRole('cell', {name: 'Threshold Type'})).toBeInTheDocument(); + expect(screen.getByRole('cell', {name: 'Absolute Diff'})).toBeInTheDocument(); + expect(screen.getByRole('cell', {name: 'Measurement'})).toBeInTheDocument(); + expect( + screen.getByRole('cell', {name: 'Install/Uncompressed Size'}) + ).toBeInTheDocument(); + expect(screen.getByRole('cell', {name: 'Condition'})).toBeInTheDocument(); + expect(screen.getByRole('cell', {name: 'Evaluated Value'})).toBeInTheDocument(); + }); + + it('formats condition as "measurement Diff > value" for diff threshold', () => { + render(); + + // absolute_diff with no artifact tag: "Install/Uncompressed Size Diff > 1 MB" + expect( + screen.getByRole('cell', {name: 'Install/Uncompressed Size Diff > 1 MB'}) + ).toBeInTheDocument(); + }); + + it('shows platform-specific measurement label from artifact type tag', () => { + const event = EventFixture({ + ...defaultEvent, + tags: [{key: 'head.artifact_type', value: 'xcarchive'}], + }); + + render(); + + expect(screen.getByRole('cell', {name: 'Install Size'})).toBeInTheDocument(); + expect( + screen.getByRole('cell', {name: 'Install Size Diff > 1 MB'}) + ).toBeInTheDocument(); + }); + + it('shows Android measurement label for aab artifact type', () => { + const event = EventFixture({ + ...defaultEvent, + tags: [{key: 'head.artifact_type', value: 'aab'}], + }); + + render(); + + expect(screen.getByRole('cell', {name: 'Uncompressed Size'})).toBeInTheDocument(); + }); + + it('shows both Open Build and Open Comparison for diff threshold types', () => { + render(); + + expect(screen.getByRole('button', {name: 'Open Build'})).toBeInTheDocument(); + expect(screen.getByRole('button', {name: 'Open Comparison'})).toBeInTheDocument(); + }); + + it('shows comparison link with correct path', () => { + render(); + + expect(screen.getByRole('button', {name: 'Open Comparison'})).toHaveAttribute( + 'href', + '/organizations/org-slug/preprod/size/compare/100/99/' + ); + }); + + it('does not show Open Comparison for absolute threshold type', () => { + const event = EventFixture({ + ...defaultEvent, + occurrence: { + ...defaultEvent.occurrence!, + evidenceData: { + ...defaultEvidenceData, + config: {measurement: 'install_size', thresholdType: 'absolute'}, + baseArtifactId: undefined, + }, + }, + }); + + render(); + + expect(screen.getByRole('button', {name: 'Open Build'})).toBeInTheDocument(); + expect( + screen.queryByRole('button', {name: 'Open Comparison'}) + ).not.toBeInTheDocument(); + }); + + it('renders relative_diff values with percentage', () => { + const event = EventFixture({ + ...defaultEvent, + occurrence: { + ...defaultEvent.occurrence!, + evidenceData: { + ...defaultEvidenceData, + value: 15, + config: {measurement: 'install_size', thresholdType: 'relative_diff'}, + }, + }, + }); + + render(); + + expect(screen.getByRole('cell', {name: '+15%'})).toBeInTheDocument(); + }); + + it('renders absolute_diff values in MB', () => { + render(); + + // 4292608 bytes = +4.29 MB (capped at 2 decimals, + prefix for diff) + expect(screen.getByRole('cell', {name: '+4.29 MB'})).toBeInTheDocument(); + }); + + it('renders query field when present in config', () => { + const event = EventFixture({ + ...defaultEvent, + occurrence: { + ...defaultEvent.occurrence!, + evidenceData: { + ...defaultEvidenceData, + config: { + ...defaultEvidenceData.config, + query: 'app_id:com.example.app', + }, + }, + }, + }); + + render(); + + expect(screen.getByRole('cell', {name: 'Query'})).toBeInTheDocument(); + expect( + screen.getByRole('cell', {name: 'app_id:com.example.app'}) + ).toBeInTheDocument(); + }); + + it('does not render query field when not present', () => { + render(); + + expect(screen.queryByRole('cell', {name: 'Query'})).not.toBeInTheDocument(); + }); +}); diff --git a/static/app/views/issueDetails/streamline/sidebar/sizeAnalysisTriggeredSection.tsx b/static/app/views/issueDetails/streamline/sidebar/sizeAnalysisTriggeredSection.tsx new file mode 100644 index 000000000000..cf68dcef8719 --- /dev/null +++ b/static/app/views/issueDetails/streamline/sidebar/sizeAnalysisTriggeredSection.tsx @@ -0,0 +1,187 @@ +import {LinkButton} from '@sentry/scraps/button'; +import {Flex} from '@sentry/scraps/layout'; + +import {KeyValueList} from 'sentry/components/events/interfaces/keyValueList'; +import {t} from 'sentry/locale'; +import type {Event, EventOccurrence} from 'sentry/types/event'; +import type {Group} from 'sentry/types/group'; +import type {MetricCondition} from 'sentry/types/workflowEngine/detectors'; +import {defined} from 'sentry/utils'; +import {useOrganization} from 'sentry/utils/useOrganization'; +import {InterimSection} from 'sentry/views/issueDetails/streamline/interimSection'; +import {getCompareBuildPath} from 'sentry/views/preprod/utils/buildLinkUtils'; +import { + bytesToMB, + getDisplayUnit, + getMeasurementLabel, + getMetricLabelForArtifactType, + isDiffThreshold, +} from 'sentry/views/settings/project/preprod/types'; +import type { + MeasurementType, + MetricType, +} from 'sentry/views/settings/project/preprod/types'; + +interface SizeAnalysisEvidenceData { + conditions: MetricCondition[]; + config: { + measurement: MetricType; + thresholdType: MeasurementType; + query?: string; + }; + detectorId: number; + headArtifactId: number; + value: number; + baseArtifactId?: number; + baseSizeMetricId?: number; + headSizeMetricId?: number; +} + +function isSizeAnalysisEvidenceData( + evidenceData?: EventOccurrence['evidenceData'] +): evidenceData is SizeAnalysisEvidenceData { + if (!defined(evidenceData) || !('config' in evidenceData)) { + return false; + } + + const {config} = evidenceData; + return ( + defined(config) && + typeof config === 'object' && + 'thresholdType' in config && + 'measurement' in config && + 'headArtifactId' in evidenceData + ); +} + +function formatRawValueWithUnit(value: number, thresholdType: MeasurementType): string { + if (thresholdType === 'relative_diff') { + return `${value}%`; + } + const mb = parseFloat(bytesToMB(value).toFixed(2)); + return `${mb} ${getDisplayUnit(thresholdType)}`; +} + +function formatEvaluatedValue(value: number, thresholdType: MeasurementType): string { + const prefix = isDiffThreshold(thresholdType) ? '+' : ''; + return `${prefix}${formatRawValueWithUnit(value, thresholdType)}`; +} + +function formatCondition({ + condition, + thresholdType, + measurementLabel, +}: { + condition: MetricCondition; + measurementLabel: string; + thresholdType: MeasurementType; +}): string { + const label = isDiffThreshold(thresholdType) + ? `${measurementLabel} Diff` + : measurementLabel; + const comparisonValue = + typeof condition.comparison === 'number' + ? formatRawValueWithUnit(condition.comparison, thresholdType) + : ''; + return `${label} > ${comparisonValue}`; +} + +interface SizeAnalysisTriggeredSectionProps { + event: Event; + group: Group; +} + +export function SizeAnalysisTriggeredSection({event}: SizeAnalysisTriggeredSectionProps) { + const organization = useOrganization(); + const evidenceData = event.occurrence?.evidenceData; + + if (!isSizeAnalysisEvidenceData(evidenceData)) { + return null; + } + + const {conditions, config, value, headArtifactId, baseArtifactId} = evidenceData; + const triggeredCondition = conditions[0]; + const artifactType = event.tags?.find(({key}) => key === 'head.artifact_type')?.value; + const measurementLabel = getMetricLabelForArtifactType( + config.measurement, + artifactType + ); + const hasDiffThreshold = isDiffThreshold(config.thresholdType); + + const headBuildPath = `/organizations/${organization.slug}/preprod/size/${headArtifactId}/`; + + const compareBuildPath = + hasDiffThreshold && defined(baseArtifactId) + ? getCompareBuildPath({ + organizationSlug: organization.slug, + headArtifactId: String(headArtifactId), + baseArtifactId: String(baseArtifactId), + }) + : null; + + return ( + + + {t('Open Build')} + + {compareBuildPath && ( + + {t('Open Comparison')} + + )} + + } + > + + {formatCondition({ + condition: triggeredCondition, + thresholdType: config.thresholdType, + measurementLabel, + })} + + ), + subject: t('Condition'), + }, + ] + : []), + { + key: 'value', + value: formatEvaluatedValue(value, config.thresholdType), + subject: t('Evaluated Value'), + }, + ]} + /> + + ); +} diff --git a/static/app/views/settings/project/preprod/types.ts b/static/app/views/settings/project/preprod/types.ts index fa3ed53cdd76..cad5394777d9 100644 --- a/static/app/views/settings/project/preprod/types.ts +++ b/static/app/views/settings/project/preprod/types.ts @@ -132,6 +132,15 @@ export function getMetricLabelForPlatform( return getMetricLabel(metric); } +export function getMetricLabelForArtifactType( + metric: MetricType, + artifactType: string | undefined +): string { + const platform = + artifactType === 'xcarchive' ? 'apple' : artifactType ? 'android' : undefined; + return getMetricLabelForPlatform(metric, platform); +} + export function getMeasurementLabel(measurement: MeasurementType): string { return MEASUREMENT_LABELS[measurement]; } @@ -176,3 +185,7 @@ export function bytesToMB(bytes: number): number { export function mbToBytes(mb: number): number { return mb * 1000 * 1000; } + +export function isDiffThreshold(thresholdType: MeasurementType): boolean { + return thresholdType === 'absolute_diff' || thresholdType === 'relative_diff'; +} From 8820b701a21676127b637892c6752ea801eca9ea Mon Sep 17 00:00:00 2001 From: Max Topolsky <30879163+mtopo27@users.noreply.github.com> Date: Wed, 1 Apr 2026 11:27:51 -0400 Subject: [PATCH 10/57] feat(preprod): Add app identifier to size analysis alert notifications (#111994) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds app name, platform, and bundle ID as a prefix to the evidence display text in size analysis Slack/Jira notifications. This gives recipients immediate context about which app triggered the alert without clicking through. **Before:** `Install Size, Absolute Diff > 1.0 MB (+1.0 MB)` **After:** `MyApp android (com.example.app) — Install Size, Absolute Diff > 1.0 MB (+1.0 MB)` When metadata is unavailable (e.g. no artifact info), the output is unchanged. Builds on #111660. --------- Co-authored-by: Claude Opus 4.6 --- src/sentry/preprod/size_analysis/grouptype.py | 33 ++++++++++++++++--- .../preprod/size_analysis/test_grouptype.py | 16 +++++++-- 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/src/sentry/preprod/size_analysis/grouptype.py b/src/sentry/preprod/size_analysis/grouptype.py index 8e0b30126029..c3a28e6fc679 100644 --- a/src/sentry/preprod/size_analysis/grouptype.py +++ b/src/sentry/preprod/size_analysis/grouptype.py @@ -81,19 +81,43 @@ def _get_measurement_label(measurement: str, platform: str) -> str: return measurement.replace("_", " ").title() +def _build_identifier_prefix(metadata: SizeAnalysisMetadata | None) -> str: + """Build an app identifier prefix like 'MyApp android (com.example.app) — '. + + Returns empty string if no metadata is available. + """ + if metadata is None: + return "" + + head_artifact = metadata["head_artifact"] + parts: list[str] = [] + + mobile_app_info = head_artifact.get_mobile_app_info() + if mobile_app_info is not None and mobile_app_info.app_name: + parts.append(mobile_app_info.app_name) + + parts.append(metadata["platform"]) + + if head_artifact.app_id: + parts.append(f"({head_artifact.app_id})") + + return " ".join(parts) + " — " + + def _build_evidence_text( detector_config: dict[str, Any], evaluation_result: ProcessedDataConditionGroup, data_packet: SizeAnalysisDataPacket, platform: str, ) -> str: - """Build a single-line evidence string for Slack/Jira notifications. + """Build evidence string for Slack/Jira notifications. - Format: {measurement}, {threshold_type} > {threshold_value} ({actual_value}) - Example: Install Size, Absolute Diff > 1.0 MB (+1.0 MB) + Format: {app_name}, {platform} ({bundle}) — {measurement}, {threshold_type} > {threshold} ({actual_value}) + Example: MyApp, android (com.example.app) — Install Size, Absolute Diff > 1.0 MB (+1.0 MB) """ from sentry.preprod.utils import format_bytes_base10 + metadata = data_packet.packet.get("metadata") measurement = detector_config["measurement"] threshold_type = detector_config["threshold_type"] measurement_label = _get_measurement_label(measurement, platform) @@ -134,7 +158,8 @@ def _build_evidence_text( sign = "+" if delta >= 0 else "-" actual_value = f"{sign}{delta_formatted}" - return f"{measurement_label}{threshold_part} ({actual_value})" + identifier = _build_identifier_prefix(metadata) + return f"{identifier}{measurement_label}{threshold_part} ({actual_value})" class SizeAnalysisMetadata(TypedDict): diff --git a/tests/sentry/preprod/size_analysis/test_grouptype.py b/tests/sentry/preprod/size_analysis/test_grouptype.py index fe2ae8ed8bd8..1224c6de9161 100644 --- a/tests/sentry/preprod/size_analysis/test_grouptype.py +++ b/tests/sentry/preprod/size_analysis/test_grouptype.py @@ -954,16 +954,20 @@ def test_evidence_relative_diff_download_size(self) -> None: evidence = occurrence.evidence_display[0] assert evidence.value == "Download Size, Relative Diff > 5% (+20.0%)" - def _make_metadata(self, platform: str, artifact_type: int) -> dict[str, Any]: + def _make_metadata( + self, platform: str, artifact_type: int, app_name: str = "MyApp" + ) -> dict[str, Any]: head_artifact = self.create_preprod_artifact( project=self.project, app_id="com.example.app", artifact_type=artifact_type, + app_name=app_name, ) base_artifact = self.create_preprod_artifact( project=self.project, app_id="com.example.app", artifact_type=artifact_type, + app_name=app_name, ) head_artifact = PreprodArtifact.objects.select_related( "mobile_app_info", "commit_comparison" @@ -992,7 +996,10 @@ def test_evidence_android_shows_uncompressed_size(self) -> None: metadata=metadata, ) evidence = occurrence.evidence_display[0] - assert evidence.value == "Uncompressed Size, Absolute Size > 1.0 MB (5.0 MB)" + assert ( + evidence.value + == "MyApp android (com.example.app) — Uncompressed Size, Absolute Size > 1.0 MB (5.0 MB)" + ) def test_evidence_apple_shows_install_size(self) -> None: self._create_condition(Condition.GREATER, 1000000) @@ -1005,4 +1012,7 @@ def test_evidence_apple_shows_install_size(self) -> None: metadata=metadata, ) evidence = occurrence.evidence_display[0] - assert evidence.value == "Install Size, Absolute Size > 1.0 MB (5.0 MB)" + assert ( + evidence.value + == "MyApp apple (com.example.app) — Install Size, Absolute Size > 1.0 MB (5.0 MB)" + ) From ca3c36d119bbbe49ce551b9a6e4b2f8085a7778e Mon Sep 17 00:00:00 2001 From: William Mak Date: Wed, 1 Apr 2026 11:33:04 -0400 Subject: [PATCH 11/57] feat(wmak): Use non strings in group by (#111753) - Allow the use of numbers in the group by (ie. numbers) so we're now returning the actual events in the groupby instead --- src/sentry/api/endpoints/timeseries.py | 2 +- src/sentry/snuba/discover.py | 6 ++---- .../test_organization_events_timeseries_spans.py | 16 ++++++++-------- 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/src/sentry/api/endpoints/timeseries.py b/src/sentry/api/endpoints/timeseries.py index 190ffd93ea83..20aeae732171 100644 --- a/src/sentry/api/endpoints/timeseries.py +++ b/src/sentry/api/endpoints/timeseries.py @@ -33,7 +33,7 @@ class SeriesMeta(TypedDict): class GroupBy(TypedDict): key: str - value: str | None + value: str | float | None class TimeSeries(TypedDict): diff --git a/src/sentry/snuba/discover.py b/src/sentry/snuba/discover.py index 7c026f16f6e7..8efa6fa8885f 100644 --- a/src/sentry/snuba/discover.py +++ b/src/sentry/snuba/discover.py @@ -438,10 +438,8 @@ def create_groupby_dict( value = f"[{','.join(filtered_value)}]" else: value = "" - if stringify_none: - values.append(GroupBy(key=field, value=str(value))) - else: - values.append(GroupBy(key=field, value=str(value) if value is not None else None)) + none_value = "None" if stringify_none else None + values.append(GroupBy(key=field, value=value if value is not None else none_value)) return values diff --git a/tests/snuba/api/endpoints/test_organization_events_timeseries_spans.py b/tests/snuba/api/endpoints/test_organization_events_timeseries_spans.py index 7219a6f7ca54..056afdb5ec04 100644 --- a/tests/snuba/api/endpoints/test_organization_events_timeseries_spans.py +++ b/tests/snuba/api/endpoints/test_organization_events_timeseries_spans.py @@ -1096,7 +1096,7 @@ def test_top_events_with_project_and_project_id(self) -> None: ) assert timeseries["groupBy"] == [ {"key": "project", "value": projects[0].slug}, - {"key": "project.id", "value": str(projects[0].id)}, + {"key": "project.id", "value": projects[0].id}, ] assert timeseries["meta"] == { "dataScanned": "full", @@ -1115,7 +1115,7 @@ def test_top_events_with_project_and_project_id(self) -> None: ) assert timeseries["groupBy"] == [ {"key": "project", "value": projects[1].slug}, - {"key": "project.id", "value": str(projects[1].id)}, + {"key": "project.id", "value": projects[1].id}, ] assert timeseries["meta"] == { "dataScanned": "full", @@ -2635,8 +2635,8 @@ def test_group_by_project_id(self) -> None: for ts in response.data["timeSeries"] if ts["groupBy"] is not None } - assert time_series_by_project_id[str(self.project.id)]["groupBy"] == [ - {"key": "project.id", "value": str(self.project.id)}, + assert time_series_by_project_id[self.project.id]["groupBy"] == [ + {"key": "project.id", "value": self.project.id}, {"key": "project.name", "value": self.project.slug}, ] @@ -2683,9 +2683,9 @@ def test_group_by_http_response_status_code(self) -> None: for ts in response.data["timeSeries"] if ts["groupBy"] is not None } - assert time_series_by_status["200"]["groupBy"] == [ - {"key": "http.response_status_code", "value": "200"} + assert time_series_by_status[200]["groupBy"] == [ + {"key": "http.response_status_code", "value": 200} ] - assert time_series_by_status["404"]["groupBy"] == [ - {"key": "http.response_status_code", "value": "404"} + assert time_series_by_status[404]["groupBy"] == [ + {"key": "http.response_status_code", "value": 404} ] From 1248f94516871d8618ed4dce09fccb61eacb3e68 Mon Sep 17 00:00:00 2001 From: William Mak Date: Wed, 1 Apr 2026 11:33:26 -0400 Subject: [PATCH 12/57] feat(metrics): Add a metrics if-combinator (#111837) - This adds an -if combinator to all the metric functions so that we can construct equations out of them in the new ui - Going to port this to the other datasets so we can do more interesting functions in the long term - The way this is parsed you can't do a query within a query (see the last test that checks a 400) which is intentional to keep this functionality simple --- src/sentry/search/eap/columns.py | 147 ++++++++++++++---- src/sentry/search/eap/resolver.py | 10 +- .../search/eap/trace_metrics/aggregates.py | 41 +++++ src/sentry/search/events/fields.py | 14 +- tests/sentry/search/events/test_fields.py | 16 ++ .../test_organization_events_trace_metrics.py | 90 +++++++++++ 6 files changed, 283 insertions(+), 35 deletions(-) diff --git a/src/sentry/search/eap/columns.py b/src/sentry/search/eap/columns.py index 7648645f6fe7..c53a264dd99c 100644 --- a/src/sentry/search/eap/columns.py +++ b/src/sentry/search/eap/columns.py @@ -17,7 +17,7 @@ Function, VirtualColumnContext, ) -from sentry_protos.snuba.v1.trace_item_filter_pb2 import TraceItemFilter +from sentry_protos.snuba.v1.trace_item_filter_pb2 import AndFilter, TraceItemFilter from sentry.api.event_search import SearchFilter from sentry.exceptions import InvalidSearchQuery @@ -27,7 +27,7 @@ from sentry.search.eap.types import EAPResponse, SearchResolverConfig from sentry.search.events.types import SnubaParams -ResolvedArgument: TypeAlias = AttributeKey | str | int | float +ResolvedArgument: TypeAlias = AttributeKey | str | int | float | TraceItemFilter ResolvedArguments: TypeAlias = list[ResolvedArgument] ProtoDefinition: TypeAlias = ( LiteralValue @@ -144,7 +144,7 @@ class BaseArgumentDefinition: @dataclass class ValueArgumentDefinition(BaseArgumentDefinition): # the type of the argument itself, if the type is a non-string you should ensure an appropriate validator is provided to avoid conversion errors - argument_types: set[Literal["integer", "string", "number"]] | None = None + argument_types: set[Literal["integer", "string", "number", "query"]] | None = None @dataclass @@ -270,13 +270,38 @@ def proto_definition(self) -> AttributeAggregation | AttributeConditionalAggrega ) +@dataclass(frozen=True, kw_only=True) +class ResolvedConditionalTraceMetricAggregate(ResolvedTraceMetricAggregate): + trace_filter: TraceItemFilter + + @property + def proto_definition(self) -> AttributeAggregation | AttributeConditionalAggregation: + if self.trace_metric is None: + return AttributeConditionalAggregation( + aggregate=self.internal_name, + key=self.key, + filter=self.trace_filter, + label=self.public_alias, + extrapolation_mode=self.extrapolation_mode, + ) + return AttributeConditionalAggregation( + aggregate=self.internal_name, + key=self.key, + filter=TraceItemFilter( + and_filter=AndFilter(filters=[self.trace_metric.get_filter(), self.trace_filter]) + ), + label=self.public_alias, + extrapolation_mode=self.extrapolation_mode, + ) + + @dataclass(frozen=True, kw_only=True) class ResolvedConditionalAggregate(ResolvedFunction): # The internal rpc alias for this column internal_name: Function.ValueType extrapolation_mode: ExtrapolationMode.ValueType # The condition to filter on - filter: TraceItemFilter + trace_filter: TraceItemFilter # The attribute to conditionally aggregate on key: AttributeKey @@ -288,7 +313,7 @@ def proto_definition(self) -> AttributeConditionalAggregation: return AttributeConditionalAggregation( aggregate=self.internal_name, key=self.key, - filter=self.filter, + filter=self.trace_filter, label=self.public_alias, extrapolation_mode=self.extrapolation_mode, ) @@ -395,8 +420,11 @@ def resolve( @dataclass(kw_only=True) class TraceMetricAggregateDefinition(AggregateDefinition): + offset = 0 + expected_args = 4 + def __post_init__(self) -> None: - validate_trace_metric_aggregate_arguments(self.arguments) + validate_trace_metric_aggregate_arguments(self.arguments, self.offset, self.expected_args) def resolve( self, @@ -407,16 +435,39 @@ def resolve( query_result_cache: dict[str, EAPResponse], search_config: SearchResolverConfig, ) -> ResolvedFunction: - if not isinstance(resolved_arguments[0], AttributeKey): + return self._resolve( + alias, + search_type, + resolved_arguments, + snuba_params, + query_result_cache, + search_config, + ResolvedTraceMetricAggregate, + ) + + def _resolve( + self, + alias: str, + search_type: constants.SearchType, + resolved_arguments: ResolvedArguments, + snuba_params: SnubaParams, + query_result_cache: dict[str, EAPResponse], + search_config: SearchResolverConfig, + aggregate: type[ResolvedConditionalTraceMetricAggregate] + | type[ResolvedTraceMetricAggregate], + ) -> ResolvedFunction: + if not isinstance(resolved_arguments[self.offset], AttributeKey): raise InvalidSearchQuery( - "Trace metric aggregates expect argument 0 to be of type AttributeArgumentDefinition" + f"Trace metric aggregates expect argument {self.offset} to be of type AttributeArgumentDefinition" ) - resolved_attribute = resolved_arguments[0] + resolved_attribute = resolved_arguments[self.offset] if self.attribute_resolver is not None: resolved_attribute = self.attribute_resolver(resolved_attribute) - trace_metric = extract_trace_metric_aggregate_arguments(resolved_arguments) + trace_metric = extract_trace_metric_aggregate_arguments( + resolved_arguments, offset=self.offset + ) # The search type for the first argument is always going to be 'number', but # the real unit for the metric is stored in the metric_unit field of the trace metric @@ -432,17 +483,46 @@ def resolve( ): resolved_search_type = cast(constants.SearchType, trace_metric.metric_unit) - return ResolvedTraceMetricAggregate( - public_alias=alias, - internal_name=self.internal_function, - search_type=resolved_search_type, - internal_type=self.internal_type, - processor=self.processor, - extrapolation_mode=resolve_extrapolation_mode( + kwargs = { + "public_alias": alias, + "internal_name": self.internal_function, + "search_type": resolved_search_type, + "internal_type": self.internal_type, + "processor": self.processor, + "extrapolation_mode": resolve_extrapolation_mode( search_config, self.extrapolation_mode_override ), - key=resolved_attribute, - trace_metric=trace_metric, + "key": resolved_attribute, + "trace_metric": trace_metric, + } + if aggregate == ResolvedConditionalTraceMetricAggregate: + kwargs["trace_filter"] = resolved_arguments[0] + + return aggregate(**kwargs) # type: ignore[arg-type] + + +@dataclass(kw_only=True) +class ConditionalTraceMetricAggregateDefinition(TraceMetricAggregateDefinition): + offset = 1 + expected_args = 5 + + def resolve( + self, + alias: str, + search_type: constants.SearchType, + resolved_arguments: ResolvedArguments, + snuba_params: SnubaParams, + query_result_cache: dict[str, EAPResponse], + search_config: SearchResolverConfig, + ) -> ResolvedFunction: + return self._resolve( + alias, + search_type, + resolved_arguments, + snuba_params, + query_result_cache, + search_config, + ResolvedConditionalTraceMetricAggregate, ) @@ -473,7 +553,7 @@ def resolve( internal_name=self.internal_function, search_type=search_type, internal_type=self.internal_type, - filter=aggregate_filter, + trace_filter=aggregate_filter, key=key, processor=self.processor, extrapolation_mode=resolve_extrapolation_mode( @@ -655,18 +735,20 @@ def count_argument_resolver(resolved_argument: ResolvedArgument) -> AttributeKey def validate_trace_metric_aggregate_arguments( arguments: list[ValueArgumentDefinition | AttributeArgumentDefinition], + offset: int = 0, + expected_args: int = 4, ) -> None: - if len(arguments) != 4: + if len(arguments) != expected_args: raise InvalidSearchQuery( - f"Trace metric aggregates expects exactly 4 arguments to be defined, got {len(arguments)}" + f"Trace metric aggregates expects exactly {expected_args} arguments to be defined, got {len(arguments)}" ) - if not isinstance(arguments[0], AttributeArgumentDefinition): + if not isinstance(arguments[offset], AttributeArgumentDefinition): raise InvalidSearchQuery( - "Trace metric aggregates expect argument 0 to be of type AttributeArgumentDefinition" + f"Trace metric aggregates expect argument {offset} to be of type AttributeArgumentDefinition" ) - for i in range(1, 4): + for i in range(offset + 1, expected_args): if not isinstance(arguments[i], ValueArgumentDefinition): raise InvalidSearchQuery( f"Trace metric aggregates expects argument {i} to be of type ValueArgumentDefinition" @@ -675,21 +757,24 @@ def validate_trace_metric_aggregate_arguments( def extract_trace_metric_aggregate_arguments( resolved_arguments: ResolvedArguments, + offset: int = 0, ) -> TraceMetric | None: if all( isinstance(resolved_argument, str) and resolved_argument != "" - for resolved_argument in resolved_arguments[1:] + for resolved_argument in resolved_arguments[offset + 1 :] ): # a metric was passed return TraceMetric( - metric_name=cast(str, resolved_arguments[1]), - metric_type=cast(TraceMetricType, resolved_arguments[2]), - metric_unit=None if resolved_arguments[3] == "-" else cast(str, resolved_arguments[3]), + metric_name=cast(str, resolved_arguments[offset + 1]), + metric_type=cast(TraceMetricType, resolved_arguments[offset + 2]), + metric_unit=None + if resolved_arguments[offset + 3] == "-" + else cast(str, resolved_arguments[offset + 3]), ) - elif all(resolved_argument == "" for resolved_argument in resolved_arguments[1:]): + elif all(resolved_argument == "" for resolved_argument in resolved_arguments[offset + 1 :]): # no metrics were specified, assume we query all metrics return None raise InvalidSearchQuery( - f"Trace metric aggregates expect the full metric to be specified, got name:{resolved_arguments[1]} type:{resolved_arguments[2]} unit:{resolved_arguments[3]}" + f"Trace metric aggregates expect the full metric to be specified, got name:{resolved_arguments[offset + 1]} type:{resolved_arguments[offset + 2]} unit:{resolved_arguments[offset + 3]}" ) diff --git a/src/sentry/search/eap/resolver.py b/src/sentry/search/eap/resolver.py index 53822371291a..7970d9027f02 100644 --- a/src/sentry/search/eap/resolver.py +++ b/src/sentry/search/eap/resolver.py @@ -1107,7 +1107,7 @@ def resolve_function( if function_definition.private and function_name not in self.config.fields_acl.functions: raise InvalidSearchQuery(f"The function {function_name} is not allowed for this query") - parsed_args: list[ResolvedAttribute | str | int | float] = [] + parsed_args: list[ResolvedAttribute | str | int | float | TraceItemFilter] = [] # Parse the arguments arguments = fields.parse_arguments(function_name, columns) @@ -1154,6 +1154,14 @@ def resolve_function( parsed_args.append(int(argument)) elif arg_type == "number": parsed_args.append(float(argument)) + elif arg_type == "query": + # Only TraceItemFilter currently supported + trace_item_filters = self.resolve_query(argument[1:-1])[0] + if trace_item_filters is None: + raise InvalidSearchQuery( + "The if combinator requires non-aggregate filters" + ) + parsed_args.append(trace_item_filters) else: parsed_args.append(argument) continue diff --git a/src/sentry/search/eap/trace_metrics/aggregates.py b/src/sentry/search/eap/trace_metrics/aggregates.py index 3a22e3a14d38..7df48923658a 100644 --- a/src/sentry/search/eap/trace_metrics/aggregates.py +++ b/src/sentry/search/eap/trace_metrics/aggregates.py @@ -1,3 +1,5 @@ +from typing import Callable + from sentry_protos.snuba.v1.trace_item_attribute_pb2 import AttributeKey, Function from sentry.search.eap import constants @@ -5,6 +7,7 @@ from sentry.search.eap.columns import ( AggregateDefinition, AttributeArgumentDefinition, + ConditionalTraceMetricAggregateDefinition, TraceMetricAggregateDefinition, ValueArgumentDefinition, count_argument_resolver_optimized, @@ -363,3 +366,41 @@ ], ), } + + +def if_query_validator(term: str) -> bool: + if not term.startswith("`") or not term.endswith("`"): + return False + return True + + +def if_combinator(definition: AggregateDefinition) -> AggregateDefinition: + # Copy the TraceMetricAggregateDefinition but make it Conditional + if_definition = ConditionalTraceMetricAggregateDefinition( + attribute_resolver=definition.attribute_resolver, + default_search_type=definition.default_search_type, + extrapolation_mode_override=definition.extrapolation_mode_override, + infer_search_type_from_arguments=definition.infer_search_type_from_arguments, + internal_function=definition.internal_function, + internal_type=definition.internal_type, + processor=definition.processor, + private=definition.private, + arguments=[ + ValueArgumentDefinition(argument_types={"query"}, validator=if_query_validator), + *definition.arguments, + ], + ) + return if_definition + + +TRACE_METRICS_COMBINATORS: dict[str, Callable[[AggregateDefinition], AggregateDefinition]] = { + "if": if_combinator, +} + + +combinator_aggregate_definitions: dict[str, AggregateDefinition] = {} +for combinator, apply_combinator in TRACE_METRICS_COMBINATORS.items(): + for function, definition in TRACE_METRICS_AGGREGATE_DEFINITIONS.items(): + combinator_aggregate_definitions[f"{function}_{combinator}"] = apply_combinator(definition) + +TRACE_METRICS_AGGREGATE_DEFINITIONS.update(combinator_aggregate_definitions) diff --git a/src/sentry/search/events/fields.py b/src/sentry/search/events/fields.py index 94f06647734b..2fe6427e94b7 100644 --- a/src/sentry/search/events/fields.py +++ b/src/sentry/search/events/fields.py @@ -484,18 +484,23 @@ def parse_arguments(_function: str, columns: str) -> list[str]: quoted = False in_tag = False escaped = False + in_filter = False i, j = 0, 0 while j < len(columns): - if not in_tag and i == j and columns[j] == '"': + if not in_filter and not in_tag and i == j and columns[j] == '"': # when we see a quote at the beginning of # an argument, then this is a quoted string quoted = True - elif not quoted and columns[j] == "[" and _lookback(columns, j, "tags"): + elif not in_filter and not quoted and columns[j] == "[" and _lookback(columns, j, "tags"): # when the argument begins with tags[, # then this is the beginning of the tag that may contain commas in_tag = True + elif not quoted and i == j and columns[j] == "`": + # When the argument starts with ` + # then its a filter that may contain any number of characters + in_filter = True elif i == j and columns[j] == " ": # argument has leading spaces, skip over them i += 1 @@ -511,6 +516,9 @@ def parse_arguments(_function: str, columns: str) -> list[str]: # when we see a non-escaped quote while inside # of a quoted string, we should end it in_tag = False + elif in_filter and columns[j] == "`": + # When we see a ` while we're in a filter end it + in_filter = False elif quoted and escaped: # when we are inside a quoted string and have # begun an escape character, we should end it @@ -520,7 +528,7 @@ def parse_arguments(_function: str, columns: str) -> list[str]: # a comma, it should not be considered an # argument separator pass - elif columns[j] == ",": + elif not in_filter and columns[j] == ",": # when we see a comma outside of a quoted string # it is an argument separator args.append(columns[i:j].strip()) diff --git a/tests/sentry/search/events/test_fields.py b/tests/sentry/search/events/test_fields.py index f0e844e60b53..98b8fc388da2 100644 --- a/tests/sentry/search/events/test_fields.py +++ b/tests/sentry/search/events/test_fields.py @@ -173,6 +173,22 @@ def test_get_json_meta_type(field_alias, snuba_type, function, expected) -> None ("to_other", ["release", r'"asdf @ \"qwer: (3,2)"'], None), ), ("identity(sessions)", ("identity", ["sessions"], None)), + ( + r'to_other(release,"asdf @ \"`qwer: (3,2)")', + ("to_other", ["release", r'"asdf @ \"`qwer: (3,2)"'], None), + ), + ( + "count_if(`p50(test, test, test):>test`, sessions)", + ("count_if", ["`p50(test, test, test):>test`", "sessions"], None), + ), + ( + "count_if(`p50(test, test, test):>test thing:\"blah\" tags[foo]:'bar'`, sessions)", + ( + "count_if", + ["`p50(test, test, test):>test thing:\"blah\" tags[foo]:'bar'`", "sessions"], + None, + ), + ), ], ) def test_parse_function(function, expected) -> None: diff --git a/tests/snuba/api/endpoints/test_organization_events_trace_metrics.py b/tests/snuba/api/endpoints/test_organization_events_trace_metrics.py index 8a639bcc8ccb..a317afcd2b38 100644 --- a/tests/snuba/api/endpoints/test_organization_events_trace_metrics.py +++ b/tests/snuba/api/endpoints/test_organization_events_trace_metrics.py @@ -749,3 +749,93 @@ def test_aggregation_with_none_includes_items_with_none_explicitly_set(self): assert len(data) == 1 assert data[0]["count(value,request_duration,distribution,none)"] == 1 + + def test_count_if_with_arbitrary_search_term(self): + trace_metrics = [ + self.create_trace_metric("request_duration", 200.0, "distribution", metric_unit="none"), + self.create_trace_metric( + "request_duration", + 200.0, + "distribution", + metric_unit="none", + attributes={"release": "abcdef"}, + ), + ] + self.store_eap_items(trace_metrics) + + response = self.do_request( + { + "field": ["count_if(`release:abcdef`,value,request_duration,distribution,none)"], + "project": self.project.id, + "dataset": self.dataset, + "statsPeriod": "10m", + } + ) + assert response.status_code == 200, response.content + data = response.data["data"] + + assert len(data) == 1 + assert data[0]["count_if(`release:abcdef`,value,request_duration,distribution,none)"] == 1 + + def test_p50_if_with_arbitrary_search_term(self): + trace_metrics = [ + self.create_trace_metric( + "request_duration", 1000000.0, "distribution", metric_unit="none" + ), + self.create_trace_metric( + "request_duration", + 200.0, + "distribution", + metric_unit="none", + attributes={"release": "abcdef", "foobar": "barfoo"}, + ), + ] + self.store_eap_items(trace_metrics) + + response = self.do_request( + { + "field": [ + "p50_if(`release:abcdef and foobar:barfoo`,value,request_duration,distribution,none)" + ], + "project": self.project.id, + "dataset": self.dataset, + "statsPeriod": "10m", + } + ) + assert response.status_code == 200, response.content + data = response.data["data"] + + assert len(data) == 1 + assert ( + data[0][ + "p50_if(`release:abcdef and foobar:barfoo`,value,request_duration,distribution,none)" + ] + == 200 + ) + + def test_errors_if_query_in_query(self): + trace_metrics = [ + self.create_trace_metric( + "request_duration", 1000000.0, "distribution", metric_unit="none" + ), + self.create_trace_metric( + "request_duration", + 200.0, + "distribution", + metric_unit="none", + attributes={"release": "abcdef", "foobar": "barfoo"}, + ), + ] + self.store_eap_items(trace_metrics) + + response = self.do_request( + { + "field": [ + "p50_if(`release:abcdef and foobar:barfoo p50(`test`, test, test)`,value,request_duration,distribution,none)" + ], + "project": self.project.id, + "dataset": self.dataset, + "statsPeriod": "10m", + } + ) + assert response.status_code == 400, response.content From 0d47368799d441a967928cdc80dc5f7dfbb5131d Mon Sep 17 00:00:00 2001 From: Evan Purkhiser Date: Wed, 1 Apr 2026 11:50:30 -0400 Subject: [PATCH 13/57] feat(gitlab): Add API-driven GitLab integration setup (#111999) Add API pipeline steps for GitLab integration installation: InstallationConfigApiStep collects instance URL, group path, OAuth credentials, and SSL preferences. The OAuth step is late-bound via OAuth2ApiStep using config from the previous step's state. build_integration now reads token data from either the legacy state["identity"]["data"] path or the new state["oauth_data"] path so both flows work during migration. Refs VDY-39 --- src/sentry/integrations/gitlab/integration.py | 107 ++++++++- .../integrations/gitlab/test_integration.py | 214 +++++++++++++++++- 2 files changed, 318 insertions(+), 3 deletions(-) diff --git a/src/sentry/integrations/gitlab/integration.py b/src/sentry/integrations/gitlab/integration.py index 292670e79e5b..5febeeeb7d10 100644 --- a/src/sentry/integrations/gitlab/integration.py +++ b/src/sentry/integrations/gitlab/integration.py @@ -9,9 +9,12 @@ from django.http.request import HttpRequest from django.http.response import HttpResponseBase from django.utils.translation import gettext_lazy as _ +from rest_framework.fields import BooleanField, CharField, URLField from sentry import features +from sentry.api.serializers.rest_framework.base import CamelSnakeSerializer from sentry.identity.gitlab.provider import GitlabIdentityProvider, get_oauth_data, get_user_info +from sentry.identity.oauth2 import OAuth2ApiStep from sentry.identity.pipeline import IdentityPipeline from sentry.integrations.base import ( FeatureDescription, @@ -37,7 +40,8 @@ from sentry.models.pullrequest import PullRequest from sentry.models.repository import Repository from sentry.organizations.services.organization import organization_service -from sentry.pipeline.views.base import PipelineView +from sentry.pipeline.types import PipelineStepResult +from sentry.pipeline.views.base import ApiPipelineSteps, PipelineView from sentry.pipeline.views.nested import NestedPipelineView from sentry.shared_integrations.exceptions import ( ApiError, @@ -393,6 +397,78 @@ def get_comment_data( } +class InstallationConfigSerializer(CamelSnakeSerializer): + url = URLField(required=False, default="https://gitlab.com") + group = CharField(required=False, allow_blank=True, default="") + include_subgroups = BooleanField(required=False, default=False) + verify_ssl = BooleanField(required=False, default=True) + client_id = CharField(required=True) + client_secret = CharField(required=True) + + +class InstallationConfigApiStep: + """ + Collects GitLab instance configuration: URL, group path, OAuth + credentials, and SSL/subgroup preferences. + + On POST, validates the form data, binds ``installation_data`` and + ``oauth_config_information`` to pipeline state, then advances. + """ + + step_name = "installation_config" + + def get_step_data(self, pipeline: IntegrationPipeline, request: HttpRequest) -> dict[str, Any]: + return { + "defaults": { + "verifySsl": True, + "includeSubgroups": False, + }, + "setupValues": [ + {"label": "Name", "value": "Sentry"}, + { + "label": "Redirect URI", + "value": absolute_uri("/extensions/gitlab/setup/"), + }, + {"label": "Scopes", "value": "api"}, + ], + } + + def get_serializer_cls(self) -> type: + return InstallationConfigSerializer + + def handle_post( + self, + validated_data: dict[str, Any], + pipeline: IntegrationPipeline, + request: HttpRequest, + ) -> PipelineStepResult: + # Strip trailing slash from URL (same as InstallationForm.clean_url) + validated_data["url"] = validated_data["url"].rstrip("/") + + pipeline.bind_state("installation_data", validated_data) + + pipeline.bind_state( + "oauth_config_information", + { + "access_token_url": f"{validated_data['url']}/oauth/token", + "authorize_url": f"{validated_data['url']}/oauth/authorize", + "client_id": validated_data["client_id"], + "client_secret": validated_data["client_secret"], + "verify_ssl": validated_data["verify_ssl"], + }, + ) + + pipeline.get_logger().info( + "gitlab.setup.installation-config-api-step.success", + extra={ + "base_url": validated_data["url"], + "client_id": validated_data["client_id"], + "verify_ssl": validated_data["verify_ssl"], + }, + ) + return PipelineStepResult.advance() + + class InstallationForm(forms.Form): url = forms.CharField( label=_("GitLab URL"), @@ -604,8 +680,35 @@ def get_pipeline_views( lambda: self._make_identity_pipeline_view(), ) + def _make_oauth_api_step(self) -> OAuth2ApiStep: + oauth_info = self.pipeline._fetch_state("oauth_config_information") + if oauth_info is None: + raise AssertionError("pipeline called out of order") + return OAuth2ApiStep( + authorize_url=oauth_info["authorize_url"], + client_id=oauth_info["client_id"], + client_secret=oauth_info["client_secret"], + access_token_url=oauth_info["access_token_url"], + scope=" ".join(sorted(GitlabIdentityProvider.oauth_scopes)), + redirect_url=absolute_uri("/extensions/gitlab/setup/"), + verify_ssl=oauth_info.get("verify_ssl", True), + bind_key="oauth_data", + ) + + def get_pipeline_api_steps(self) -> ApiPipelineSteps[IntegrationPipeline]: + return [ + InstallationConfigApiStep(), + lambda: self._make_oauth_api_step(), + ] + def build_integration(self, state: Mapping[str, Any]) -> IntegrationData: - data = state["identity"]["data"] + # TODO: legacy views write token data to state["identity"]["data"] via + # NestedPipelineView. API steps write directly to state["oauth_data"]. + # Remove the legacy path once the old views are retired. + if "oauth_data" in state: + data = state["oauth_data"] + else: + data = state["identity"]["data"] # Gitlab requires the client_id and client_secret for refreshing the access tokens oauth_config = state.get("oauth_config_information", {}) diff --git a/tests/sentry/integrations/gitlab/test_integration.py b/tests/sentry/integrations/gitlab/test_integration.py index c1cae71ad3b7..9b46a1a931b9 100644 --- a/tests/sentry/integrations/gitlab/test_integration.py +++ b/tests/sentry/integrations/gitlab/test_integration.py @@ -1,5 +1,8 @@ +from __future__ import annotations + from dataclasses import asdict from datetime import datetime, timezone +from typing import Any from unittest.mock import MagicMock, Mock, patch from urllib.parse import parse_qs, quote, urlencode, urlparse @@ -8,6 +11,7 @@ import responses from django.core.cache import cache from django.test import override_settings +from django.urls import reverse from fixtures.gitlab import GET_COMMIT_RESPONSE, GitLabTestCase from sentry.integrations.gitlab.blame import GitLabCommitResponse, GitLabFileBlameResponseItem @@ -16,6 +20,7 @@ from sentry.integrations.gitlab.integration import GitlabIntegration, GitlabIntegrationProvider from sentry.integrations.models.integration import Integration from sentry.integrations.models.organization_integration import OrganizationIntegration +from sentry.integrations.pipeline import IntegrationPipeline from sentry.integrations.services.integration import integration_service from sentry.integrations.source_code_management.commit_context import ( CommitInfo, @@ -27,7 +32,7 @@ from sentry.shared_integrations.exceptions import ApiForbiddenError, IntegrationConfigurationError from sentry.silo.base import SiloMode from sentry.silo.util import PROXY_BASE_PATH, PROXY_OI_HEADER, PROXY_SIGNATURE_HEADER -from sentry.testutils.cases import IntegrationTestCase +from sentry.testutils.cases import APITestCase, IntegrationTestCase from sentry.testutils.helpers.features import with_feature from sentry.testutils.helpers.integrations import get_installation_of_type from sentry.testutils.silo import assume_test_silo_mode, control_silo_test @@ -1357,3 +1362,210 @@ def test_update_organization_config_preserves_other_config_values(self) -> None: assert org_integration.config["existing_key"] == "existing_value" assert org_integration.config["sync_forward_assignment"] is True assert org_integration.config["sync_comments"] is True + + +@control_silo_test +class GitLabIntegrationApiPipelineTest(APITestCase): + endpoint = "sentry-api-0-organization-pipeline" + method = "post" + + gitlab_url = "https://gitlab.example.com" + + def setUp(self) -> None: + super().setUp() + self.login_as(self.user) + self.client_id = "app-id-abc123" + self.client_secret = "secret-xyz789" + + def tearDown(self) -> None: + responses.reset() + super().tearDown() + + def _get_pipeline_url(self) -> str: + return reverse( + self.endpoint, + args=[self.organization.slug, IntegrationPipeline.pipeline_name], + ) + + def _initialize_pipeline(self) -> Any: + return self.client.post( + self._get_pipeline_url(), + data={"action": "initialize", "provider": "gitlab"}, + format="json", + ) + + def _get_step_info(self) -> Any: + return self.client.get(self._get_pipeline_url()) + + def _advance_step(self, data: dict[str, Any]) -> Any: + return self.client.post(self._get_pipeline_url(), data=data, format="json") + + def _stub_gitlab_oauth(self) -> None: + responses.add( + responses.POST, + f"{self.gitlab_url}/oauth/token", + json={ + "access_token": "test-access-token", + "token_type": "bearer", + "refresh_token": "test-refresh-token", + "created_at": 1536798907, + "scope": "api", + }, + ) + + def _stub_gitlab_user(self) -> None: + responses.add( + responses.GET, + f"{self.gitlab_url}/api/v4/user", + json={ + "id": 42, + "username": "testuser", + "email": "test@example.com", + "name": "Test User", + }, + ) + + def _stub_gitlab_group(self) -> None: + responses.add( + responses.GET, + f"{self.gitlab_url}/api/v4/groups/my-group", + json={ + "id": 1, + "full_name": "My Group", + "full_path": "my-group", + "avatar_url": "https://gitlab.example.com/avatar.png", + "web_url": "https://gitlab.example.com/my-group", + }, + ) + + def _submit_config(self, **overrides: Any) -> Any: + data = { + "url": self.gitlab_url, + "group": "my-group", + "includeSubgroups": False, + "verifySsl": True, + "clientId": self.client_id, + "clientSecret": self.client_secret, + } + data.update(overrides) + return self._advance_step(data) + + def _get_pipeline_signature(self, resp: Any) -> str: + return resp.data["data"]["oauthUrl"].split("state=")[1].split("&")[0] + + @responses.activate + def test_initialize_pipeline(self) -> None: + resp = self._initialize_pipeline() + assert resp.status_code == 200 + assert resp.data["step"] == "installation_config" + assert resp.data["stepIndex"] == 0 + assert resp.data["totalSteps"] == 2 + assert resp.data["provider"] == "gitlab" + + @responses.activate + def test_config_step_data(self) -> None: + resp = self._initialize_pipeline() + data = resp.data["data"] + defaults = data["defaults"] + assert defaults["verifySsl"] is True + assert defaults["includeSubgroups"] is False + setup_values = data["setupValues"] + labels = [v["label"] for v in setup_values] + assert "Name" in labels + assert "Redirect URI" in labels + assert "Scopes" in labels + + @responses.activate + def test_config_step_validation_missing_required_fields(self) -> None: + self._initialize_pipeline() + resp = self._advance_step({"url": self.gitlab_url}) + assert resp.status_code == 400 + assert resp.data["clientId"] == ["This field is required."] + assert resp.data["clientSecret"] == ["This field is required."] + + @responses.activate + def test_config_step_advance_to_oauth(self) -> None: + self._initialize_pipeline() + resp = self._submit_config() + assert resp.status_code == 200 + assert resp.data["status"] == "advance" + assert resp.data["step"] == "oauth_login" + assert resp.data["stepIndex"] == 1 + assert "oauthUrl" in resp.data["data"] + oauth_url = resp.data["data"]["oauthUrl"] + assert self.gitlab_url in oauth_url + assert "client_id=" in oauth_url + + @responses.activate + def test_oauth_step_invalid_state(self) -> None: + self._initialize_pipeline() + self._submit_config() + resp = self._advance_step({"code": "abc123", "state": "wrong-state"}) + assert resp.status_code == 400 + assert resp.data["status"] == "error" + + @responses.activate + def test_oauth_step_missing_code(self) -> None: + self._initialize_pipeline() + self._submit_config() + resp = self._advance_step({}) + assert resp.status_code == 400 + assert resp.data["code"] == ["This field is required."] + assert resp.data["state"] == ["This field is required."] + + @responses.activate + def test_full_pipeline_flow(self) -> None: + self._stub_gitlab_oauth() + self._stub_gitlab_user() + self._stub_gitlab_group() + + resp = self._initialize_pipeline() + assert resp.data["step"] == "installation_config" + + resp = self._submit_config() + assert resp.data["step"] == "oauth_login" + pipeline_signature = self._get_pipeline_signature(resp) + + resp = self._advance_step({"code": "gitlab-auth-code", "state": pipeline_signature}) + assert resp.status_code == 200 + assert resp.data["status"] == "complete" + assert "data" in resp.data + + integration = Integration.objects.get(provider="gitlab") + assert integration.metadata["base_url"] == self.gitlab_url + assert integration.metadata["group_id"] == 1 + + assert OrganizationIntegration.objects.filter( + organization_id=self.organization.id, + integration=integration, + ).exists() + + @responses.activate + def test_full_pipeline_flow_no_group(self) -> None: + self._stub_gitlab_oauth() + self._stub_gitlab_user() + + self._initialize_pipeline() + resp = self._submit_config(group="") + pipeline_signature = self._get_pipeline_signature(resp) + + resp = self._advance_step({"code": "gitlab-auth-code", "state": pipeline_signature}) + assert resp.status_code == 200 + assert resp.data["status"] == "complete" + + integration = Integration.objects.get(provider="gitlab") + assert integration.metadata["group_id"] is None + assert integration.metadata["include_subgroups"] is False + + @responses.activate + def test_config_strips_trailing_slash(self) -> None: + self._stub_gitlab_oauth() + self._stub_gitlab_user() + self._stub_gitlab_group() + + self._initialize_pipeline() + resp = self._submit_config(url=f"{self.gitlab_url}///") + assert resp.data["status"] == "advance" + oauth_url = resp.data["data"]["oauthUrl"] + assert "gitlab.example.com/oauth/authorize" in oauth_url + assert "///" not in oauth_url From 0447b6fd0846715ca04b2c0f541e1e681e7d5d98 Mon Sep 17 00:00:00 2001 From: Kyle Consalus Date: Wed, 1 Apr 2026 09:01:03 -0700 Subject: [PATCH 14/57] feat(workflow): Add index on GroupOpenPeriodActivity.date_added (#111968) We want to clean this up by date, and we need this index to do it efficiently. --- migrations_lockfile.txt | 2 +- ...roupopenperiodactivity_date_added_index.py | 32 +++++++++++++++++++ src/sentry/models/groupopenperiodactivity.py | 1 + 3 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 src/sentry/migrations/1059_add_groupopenperiodactivity_date_added_index.py diff --git a/migrations_lockfile.txt b/migrations_lockfile.txt index f1d2336b2949..5b76081914f0 100644 --- a/migrations_lockfile.txt +++ b/migrations_lockfile.txt @@ -31,7 +31,7 @@ replays: 0007_organizationmember_replay_access seer: 0005_delete_seerorganizationsettings -sentry: 1058_change_code_mapping_unique_constraint +sentry: 1059_add_groupopenperiodactivity_date_added_index social_auth: 0003_social_auth_json_field diff --git a/src/sentry/migrations/1059_add_groupopenperiodactivity_date_added_index.py b/src/sentry/migrations/1059_add_groupopenperiodactivity_date_added_index.py new file mode 100644 index 000000000000..d68abbcee833 --- /dev/null +++ b/src/sentry/migrations/1059_add_groupopenperiodactivity_date_added_index.py @@ -0,0 +1,32 @@ +# Generated by Django 5.2.12 on 2026-03-31 22:06 + +from django.db import migrations, models + +from sentry.new_migrations.migrations import CheckedMigration + + +class Migration(CheckedMigration): + # This flag is used to mark that a migration shouldn't be automatically run in production. + # This should only be used for operations where it's safe to run the migration after your + # code has deployed. So this should not be used for most operations that alter the schema + # of a table. + # Here are some things that make sense to mark as post deployment: + # - Large data migrations. Typically we want these to be run manually so that they can be + # monitored and not block the deploy for a long period of time while they run. + # - Adding indexes to large tables. Since this can take a long time, we'd generally prefer to + # run this outside deployments so that we don't block them. Note that while adding an index + # is a schema change, it's completely safe to run the operation after the code has deployed. + # Once deployed, run these manually via: https://develop.sentry.dev/database-migrations/#migration-deployment + + is_post_deployment = True + + dependencies = [ + ("sentry", "1058_change_code_mapping_unique_constraint"), + ] + + operations = [ + migrations.AddIndex( + model_name="groupopenperiodactivity", + index=models.Index(fields=["date_added"], name="sentry_grou_date_ad_e242e5_idx"), + ), + ] diff --git a/src/sentry/models/groupopenperiodactivity.py b/src/sentry/models/groupopenperiodactivity.py index ddb5830a55bb..c9ec89e19073 100644 --- a/src/sentry/models/groupopenperiodactivity.py +++ b/src/sentry/models/groupopenperiodactivity.py @@ -48,4 +48,5 @@ class Meta: indexes = [ models.Index(fields=["group_open_period", "type", "event_id"]), models.Index(fields=["event_id"]), + models.Index(fields=["date_added"]), ] From ef811d3100012bdc703d55b75e53f63d8fa443c4 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Wed, 1 Apr 2026 18:14:10 +0200 Subject: [PATCH 15/57] fix(preprod): Exclude snapshot artifacts from size status check (EME-962) (#111981) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What Filter snapshot-only artifacts out of the size analysis status check so they no longer show as permanently "processing." ## Why When a commit comparison has both a size analysis artifact and a snapshot artifact, the size status check counts the snapshot artifact as "1 component processing" — even though snapshots never go through the size analysis pipeline and stay in `UPLOADED` state forever. This makes the GitHub check look stuck. Fixes EME-962. ## How Added a filter in the size status check task (after building `size_metrics_map`, before the existing SKIPPED filter) that keeps only artifacts with `artifact_type` set or with size metrics present. Snapshot artifacts have neither — they have no `artifact_type` and no `PreprodArtifactSizeMetrics` rows. This mirrors the pattern already used by the snapshot status check task (`snapshots/tasks.py:138`), which filters to only artifacts with `PreprodSnapshotMetrics`. Existing tests were updated to create `PreprodArtifactSizeMetrics` for UPLOADING/UPLOADED/FAILED artifacts, matching the production flow where `create_preprod_artifact` always creates size metrics at artifact creation time. --- .../preprod/vcs/status_checks/size/tasks.py | 4 + .../size/test_status_checks_tasks.py | 128 +++++++++++++++++- 2 files changed, 130 insertions(+), 2 deletions(-) diff --git a/src/sentry/preprod/vcs/status_checks/size/tasks.py b/src/sentry/preprod/vcs/status_checks/size/tasks.py index 9a0c32fd8b3e..0bacfccabd91 100644 --- a/src/sentry/preprod/vcs/status_checks/size/tasks.py +++ b/src/sentry/preprod/vcs/status_checks/size/tasks.py @@ -250,6 +250,10 @@ def create_preprod_status_check_task( for approval in approval_qs: approvals_map[approval.preprod_artifact_id] = approval + # Filter out artifacts not in the size analysis pipeline (e.g., snapshot-only artifacts). + # Symmetric with snapshots/tasks.py which filters to snapshot-only artifacts. + all_artifacts = [a for a in all_artifacts if a.id in size_metrics_map] + # Filter out SKIPPED artifacts (user didn't request size analysis) all_artifacts = [ a for a in all_artifacts if not _is_artifact_size_skipped(size_metrics_map.get(a.id, [])) diff --git a/tests/sentry/preprod/vcs/status_checks/size/test_status_checks_tasks.py b/tests/sentry/preprod/vcs/status_checks/size/test_status_checks_tasks.py index bb2b420b456d..24ca025c019f 100644 --- a/tests/sentry/preprod/vcs/status_checks/size/test_status_checks_tasks.py +++ b/tests/sentry/preprod/vcs/status_checks/size/test_status_checks_tasks.py @@ -275,6 +275,13 @@ def test_create_preprod_status_check_task_all_artifact_states(self) -> None: min_install_size=2 * 1024 * 1024, max_install_size=2 * 1024 * 1024, ) + else: + # In production, size metrics are created at artifact creation time + PreprodArtifactSizeMetrics.objects.create( + preprod_artifact=preprod_artifact, + metrics_artifact_type=PreprodArtifactSizeMetrics.MetricsArtifactType.MAIN_ARTIFACT, + state=PreprodArtifactSizeMetrics.SizeAnalysisState.PENDING, + ) _, mock_provider, client_patch, provider_patch = ( self._create_working_status_check_setup(preprod_artifact) @@ -458,20 +465,31 @@ def test_create_preprod_status_check_task_mixed_states_monorepo(self) -> None: max_install_size=2 * 1024 * 1024, ) - _ = PreprodArtifact.objects.create( + uploading_artifact = PreprodArtifact.objects.create( project=self.project, state=PreprodArtifact.ArtifactState.UPLOADING, app_id="com.example.uploading", commit_comparison=commit_comparison, ) + # In production, size metrics are created at artifact creation time + PreprodArtifactSizeMetrics.objects.create( + preprod_artifact=uploading_artifact, + metrics_artifact_type=PreprodArtifactSizeMetrics.MetricsArtifactType.MAIN_ARTIFACT, + state=PreprodArtifactSizeMetrics.SizeAnalysisState.PENDING, + ) - _ = PreprodArtifact.objects.create( + failed_artifact = PreprodArtifact.objects.create( project=self.project, state=PreprodArtifact.ArtifactState.FAILED, app_id="com.example.failed", error_message="Upload timeout", commit_comparison=commit_comparison, ) + PreprodArtifactSizeMetrics.objects.create( + preprod_artifact=failed_artifact, + metrics_artifact_type=PreprodArtifactSizeMetrics.MetricsArtifactType.MAIN_ARTIFACT, + state=PreprodArtifactSizeMetrics.SizeAnalysisState.PENDING, + ) _, mock_provider, client_patch, provider_patch = self._create_working_status_check_setup( processed_artifact @@ -795,6 +813,18 @@ def test_create_preprod_status_check_task_truncates_long_summary(self) -> None: ) artifacts.append(artifact) + # In production, size metrics are created at artifact creation time + PreprodArtifactSizeMetrics.objects.bulk_create( + [ + PreprodArtifactSizeMetrics( + preprod_artifact=a, + metrics_artifact_type=PreprodArtifactSizeMetrics.MetricsArtifactType.MAIN_ARTIFACT, + state=PreprodArtifactSizeMetrics.SizeAnalysisState.PENDING, + ) + for a in artifacts + ] + ) + integration = self.create_integration( organization=self.organization, external_id="test-truncation", @@ -1366,6 +1396,13 @@ def test_with_rules_configured_preserves_actual_status(self) -> None: min_install_size=2 * 1024 * 1024, max_install_size=2 * 1024 * 1024, ) + else: + # In production, size metrics are created at artifact creation time + PreprodArtifactSizeMetrics.objects.create( + preprod_artifact=preprod_artifact, + metrics_artifact_type=PreprodArtifactSizeMetrics.MetricsArtifactType.MAIN_ARTIFACT, + state=PreprodArtifactSizeMetrics.SizeAnalysisState.PENDING, + ) _, mock_provider, client_patch, provider_patch = ( self._create_working_status_check_setup(preprod_artifact) @@ -1532,3 +1569,90 @@ def test_skipped_triggering_artifact_uses_sibling_url(self) -> None: kwargs = mock_provider.create_status_check.call_args.kwargs assert f"/size/{valid.id}" in kwargs["target_url"] assert f"/size/{skipped.id}" not in kwargs["target_url"] + + def test_snapshot_only_artifact_not_counted_as_processing(self) -> None: + """Snapshot-only artifacts (no artifact_type, no size metrics) should be excluded.""" + commit_comparison = CommitComparison.objects.create( + organization_id=self.organization.id, + head_sha="a" * 40, + base_sha="b" * 40, + provider="github", + head_repo_name="owner/repo", + base_repo_name="owner/repo", + head_ref="feature/test", + base_ref="main", + ) + + size_artifact = Factories.create_preprod_artifact( + project=self.project, + state=PreprodArtifact.ArtifactState.PROCESSED, + app_id="com.example.app", + commit_comparison=commit_comparison, + ) + PreprodArtifactSizeMetrics.objects.create( + preprod_artifact=size_artifact, + metrics_artifact_type=PreprodArtifactSizeMetrics.MetricsArtifactType.MAIN_ARTIFACT, + state=PreprodArtifactSizeMetrics.SizeAnalysisState.COMPLETED, + max_download_size=1024, + max_install_size=2048, + ) + + # Snapshot artifact: UPLOADED state, no artifact_type, no size metrics + PreprodArtifact.objects.create( + project=self.project, + state=PreprodArtifact.ArtifactState.UPLOADED, + app_id="com.example.snapshot", + commit_comparison=commit_comparison, + ) + + _, mock_provider, client_patch, provider_patch = self._create_working_status_check_setup( + size_artifact + ) + + with client_patch, provider_patch: + with self.tasks(): + create_preprod_status_check_task(size_artifact.id) + + mock_provider.create_status_check.assert_called_once() + kwargs = mock_provider.create_status_check.call_args.kwargs + assert "1 component analyzed" in kwargs["subtitle"] + assert "processing" not in kwargs["subtitle"] + + def test_uploading_artifact_with_size_metrics_not_excluded(self) -> None: + """An UPLOADING artifact with size metrics should still be counted (not filtered out).""" + commit_comparison = CommitComparison.objects.create( + organization_id=self.organization.id, + head_sha="a" * 40, + base_sha="b" * 40, + provider="github", + head_repo_name="owner/repo", + base_repo_name="owner/repo", + head_ref="feature/test", + base_ref="main", + ) + + # Normal artifact still in UPLOADING state but with size metrics already created + uploading_artifact = PreprodArtifact.objects.create( + project=self.project, + state=PreprodArtifact.ArtifactState.UPLOADING, + app_id="com.example.uploading", + commit_comparison=commit_comparison, + ) + PreprodArtifactSizeMetrics.objects.create( + preprod_artifact=uploading_artifact, + metrics_artifact_type=PreprodArtifactSizeMetrics.MetricsArtifactType.MAIN_ARTIFACT, + state=PreprodArtifactSizeMetrics.SizeAnalysisState.PENDING, + ) + + _, mock_provider, client_patch, provider_patch = self._create_working_status_check_setup( + uploading_artifact + ) + + with client_patch, provider_patch: + with self.tasks(): + create_preprod_status_check_task(uploading_artifact.id) + + mock_provider.create_status_check.assert_called_once() + kwargs = mock_provider.create_status_check.call_args.kwargs + # The artifact is still UPLOADING, so should show as processing + assert "processing" in kwargs["subtitle"] From 7f772f0205a218c42422994738e54fb5f4161c8b Mon Sep 17 00:00:00 2001 From: Jonas Date: Wed, 1 Apr 2026 09:25:23 -0700 Subject: [PATCH 16/57] ref(layout) align page paddings (#111823) Audit pages for layout use so that menus, headers and page contents are aligned horizontally with each other. Since many layouts use bespoke styling, this is only the first pass of pages that are easily visible by going through the list of some top level routes CleanShot 2026-03-30 at 13 45 27 --------- Co-authored-by: Claude Co-authored-by: Priscila Oliveira Co-authored-by: Cursor Agent Co-authored-by: Jonas --- static/app/components/layouts/thirds.tsx | 50 ++++++++++--------- static/app/views/issueList/overview.tsx | 21 ++++---- static/app/views/navigation/topBar.tsx | 8 ++- .../newTraceDetails/traceHeader/styles.tsx | 32 ++++++++---- static/app/views/replays/details.tsx | 47 ++++++++++------- .../settings/components/settingsHeader.tsx | 2 +- .../settings/components/settingsLayout.tsx | 27 ++++------ .../components/subscriptionPageContainer.tsx | 6 +-- 8 files changed, 105 insertions(+), 88 deletions(-) diff --git a/static/app/components/layouts/thirds.tsx b/static/app/components/layouts/thirds.tsx index 7ecbfaa0acdf..0225ac06d8a0 100644 --- a/static/app/components/layouts/thirds.tsx +++ b/static/app/components/layouts/thirds.tsx @@ -33,7 +33,6 @@ export function Page(props: FlexProps<'main'> & {withPadding?: boolean}) { primaryNavigation.layout === 'sidebar' && secondaryNavigation?.view === 'expanded' } - padding={props.withPadding ? '2xl 3xl' : undefined} radius={ secondaryNavigation?.view === 'expanded' ? primaryNavigation.layout === 'sidebar' @@ -83,8 +82,18 @@ const StyledPageFrameStack = styled(Stack)<{roundedCorner: boolean}>` */ export const Header = styled((props: ContainerProps<'header'>) => { const hasPageFrame = useHasPageFrameFeature(); + return ( - + ); })<{ borderStyle?: 'dashed' | 'solid'; @@ -100,8 +109,6 @@ export const Header = styled((props: ContainerProps<'header'>) => { grid-template-columns: ${p => p.noActionWrap ? 'minmax(0, 1fr) auto' : 'minmax(0, 1fr)'}; - padding: ${p => p.theme.space.xl} ${p => p.theme.space.xl} 0 ${p => p.theme.space.xl}; - ${p => !p.unified && css` @@ -109,8 +116,6 @@ export const Header = styled((props: ContainerProps<'header'>) => { `} @media (min-width: ${p => p.theme.breakpoints.md}) { - padding: ${p => p.theme.space.xl} ${p => p.theme.space['3xl']} 0 - ${p => p.theme.space['3xl']}; grid-template-columns: minmax(0, 1fr) auto; } `; @@ -123,14 +128,8 @@ export const HeaderContent = styled('div')<{unified?: boolean}>` display: flex; flex-direction: column; justify-content: normal; - margin-bottom: ${p => p.theme.space.md}; + margin-bottom: ${p => (p.unified ? 0 : p.theme.space.md)}; max-width: 100%; - - ${p => - p.unified && - css` - margin-bottom: 0; - `} `; /** @@ -191,19 +190,22 @@ export const HeaderTabs = styled(Tabs)` /** * Base container for 66/33 containers. */ -export const Body = styled('div')<{noRowGap?: boolean}>` - padding: ${p => p.theme.space.xl}; - margin: 0; - background-color: ${p => p.theme.tokens.background.primary}; +export const Body = styled((props: ContainerProps<'div'> & {noRowGap?: boolean}) => { + const hasPageFrame = useHasPageFrameFeature(); + return ( + + ); +})<{noRowGap?: boolean}>` flex-grow: 1; - @media (min-width: ${p => p.theme.breakpoints.md}) { - padding: ${p => - p.noRowGap - ? `${p.theme.space.xl} ${p.theme.space['3xl']}` - : `${p.theme.space['2xl']} ${p.theme.space['3xl']}`}; - } - @media (min-width: ${p => p.theme.breakpoints.lg}) { display: grid; grid-template-columns: minmax(100px, auto) 325px; diff --git a/static/app/views/issueList/overview.tsx b/static/app/views/issueList/overview.tsx index 008349d65a7d..f709d8dd7c5b 100644 --- a/static/app/views/issueList/overview.tsx +++ b/static/app/views/issueList/overview.tsx @@ -9,7 +9,7 @@ import omit from 'lodash/omit'; import pickBy from 'lodash/pickBy'; import * as qs from 'query-string'; -import {Stack} from '@sentry/scraps/layout'; +import {Grid, Stack} from '@sentry/scraps/layout'; import {addMessage} from 'sentry/actionCreators/indicator'; import {fetchOrgMembers, indexMembersByProject} from 'sentry/actionCreators/members'; @@ -51,6 +51,7 @@ import {IssuesDataConsentBanner} from 'sentry/views/issueList/issuesDataConsentB import {IssueViewsHeader} from 'sentry/views/issueList/issueViewsHeader'; import type {IssueUpdateData} from 'sentry/views/issueList/types'; import {parseIssuePrioritySearch} from 'sentry/views/issueList/utils/parseIssuePrioritySearch'; +import {useHasPageFrameFeature} from 'sentry/views/navigation/useHasPageFrameFeature'; import {IssueListFilters} from './filters'; import { @@ -873,6 +874,8 @@ function IssueListOverview({ const {numPreviousIssues, numIssuesOnPage} = getPageCounts(); + const hasPageFrame = useHasPageFrameFeature(); + return ( - + - + ); @@ -944,12 +950,3 @@ const StyledBody = styled('div')` background-color: ${p => p.theme.tokens.background.primary}; flex: 1; `; - -const StyledMain = styled('section')` - grid-area: content; - padding: ${p => p.theme.space.xl}; - - @media (min-width: ${p => p.theme.breakpoints.md}) { - padding: ${p => p.theme.space['2xl']} ${p => p.theme.space['3xl']}; - } -`; diff --git a/static/app/views/navigation/topBar.tsx b/static/app/views/navigation/topBar.tsx index eb0d03a26731..836c81ac1d45 100644 --- a/static/app/views/navigation/topBar.tsx +++ b/static/app/views/navigation/topBar.tsx @@ -15,6 +15,7 @@ import {useHasPageFrameFeature} from 'sentry/views/navigation/useHasPageFrameFea import {useExplorerPanel} from 'sentry/views/seerExplorer/useExplorerPanel'; import {isSeerExplorerEnabled} from 'sentry/views/seerExplorer/utils'; +import {NAVIGATION_MOBILE_TOPBAR_HEIGHT_WITH_PAGE_FRAME} from './constants'; import {PRIMARY_HEADER_HEIGHT} from './constants'; export function TopBar() { @@ -72,11 +73,14 @@ export function TopBar() { return ( p.theme.tokens.background.primary}; - padding: ${p => p.theme.space.md} ${p => p.theme.space['2xl']} ${p => p.theme.space.md} - ${p => p.theme.space['2xl']}; - border-bottom: 1px solid ${p => p.theme.tokens.border.primary}; - flex-shrink: 0; - min-height: 150px; -`; +import {useHasPageFrameFeature} from 'sentry/views/navigation/useHasPageFrameFeature'; + +const HeaderLayout = styled((props: ContainerProps<'div'>) => { + const hasPageFrame = useHasPageFrameFeature(); + return ( + + ); +})``; const HeaderRow = styled('div')` display: flex; diff --git a/static/app/views/replays/details.tsx b/static/app/views/replays/details.tsx index 8f24dc9a20fe..82e6ff5c242a 100644 --- a/static/app/views/replays/details.tsx +++ b/static/app/views/replays/details.tsx @@ -1,5 +1,4 @@ import {Fragment} from 'react'; -import styled from '@emotion/styled'; import invariant from 'invariant'; import {Flex, Stack} from '@sentry/scraps/layout'; @@ -31,14 +30,23 @@ import {ReplayDetailsUserBadge} from 'sentry/views/replays/detail/header/replayD import {ReplayDetailsPage} from 'sentry/views/replays/detail/page'; export default function ReplayDetails() { + const hasPageFrame = useHasPageFrameFeature(); + return ( - + {t('Replay Details')} - + @@ -86,15 +94,27 @@ function ReplayDetailsContent() { const content = ( - - + + - - + + - + @@ -119,14 +139,3 @@ function ReplayDetailsContent() { ); } - -const TopHeader = styled(Flex)` - padding: ${p => p.theme.space.sm} ${p => p.theme.space.lg}; - border-bottom: 1px solid ${p => p.theme.tokens.border.secondary}; - flex-wrap: wrap; -`; - -const BottonHeader = styled(Flex)` - padding: ${p => p.theme.space.md} ${p => p.theme.space.lg}; - border-bottom: 1px solid ${p => p.theme.tokens.border.secondary}; -`; diff --git a/static/app/views/settings/components/settingsHeader.tsx b/static/app/views/settings/components/settingsHeader.tsx index c4cc4f40d818..8773599bc6c6 100644 --- a/static/app/views/settings/components/settingsHeader.tsx +++ b/static/app/views/settings/components/settingsHeader.tsx @@ -22,7 +22,7 @@ export const SettingsHeader = styled((props: ContainerProps<'div'>) => { borderBottom="primary" background="primary" style={{zIndex: theme.zIndex.header + HEADER_Z_INDEX_OFFSET}} - padding="xl 3xl" + padding={hasPageFrame ? {sm: 'sm lg', md: 'md xl'} : 'xl 3xl'} radius={hasPageFrame ? 'lg 0 0 0' : undefined} {...props} /> diff --git a/static/app/views/settings/components/settingsLayout.tsx b/static/app/views/settings/components/settingsLayout.tsx index dcd5e9789464..b744736b6af4 100644 --- a/static/app/views/settings/components/settingsLayout.tsx +++ b/static/app/views/settings/components/settingsLayout.tsx @@ -1,9 +1,10 @@ import styled from '@emotion/styled'; -import {Flex} from '@sentry/scraps/layout'; +import {Container, Flex} from '@sentry/scraps/layout'; import {useParams} from 'sentry/utils/useParams'; import {useRoutes} from 'sentry/utils/useRoutes'; +import {useHasPageFrameFeature} from 'sentry/views/navigation/useHasPageFrameFeature'; import {SettingsBreadcrumb} from './settingsBreadcrumb'; import {SettingsHeader} from './settingsHeader'; @@ -17,6 +18,8 @@ export function SettingsLayout({children}: Props) { const params = useParams(); const routes = useRoutes(); + const hasPageFrame = useHasPageFrameFeature(); + return ( @@ -27,7 +30,13 @@ export function SettingsLayout({children}: Props) { - {children} + + {children} + ); @@ -46,17 +55,3 @@ const SettingsColumn = styled('div')` const StyledSettingsBreadcrumb = styled(SettingsBreadcrumb)` flex: 1; `; - -/** - * Note: `overflow: hidden` will cause some buttons in `SettingsPageHeader` to be cut off because it has negative margin. - * Will also cut off tooltips. - */ -const Content = styled('div')` - flex: 1; - padding: ${p => p.theme.space['3xl']}; - min-width: 0; /* keep children from stretching container */ - - @media (max-width: ${p => p.theme.breakpoints.md}) { - padding: ${p => p.theme.space.xl}; - } -`; diff --git a/static/gsApp/views/subscriptionPage/components/subscriptionPageContainer.tsx b/static/gsApp/views/subscriptionPage/components/subscriptionPageContainer.tsx index c11b66af9e6f..624b36fddedf 100644 --- a/static/gsApp/views/subscriptionPage/components/subscriptionPageContainer.tsx +++ b/static/gsApp/views/subscriptionPage/components/subscriptionPageContainer.tsx @@ -21,11 +21,9 @@ export function SubscriptionPageContainer({ return ( {children} From 4e0a42bae27f1b30e3b1fa79d33235e1b22f17d4 Mon Sep 17 00:00:00 2001 From: Hector Dearman Date: Wed, 1 Apr 2026 17:32:36 +0100 Subject: [PATCH 17/57] ref(autofix): Seer -> Seer Autofix (#111985) Users are sometimes confused which Seer feature is which. This makes it hard to look up the feature in the docs / get the correct support etc. This changes the sidebar to label the section with 'Seer Autofix' to make it clearer which feature this is. Before: CleanShot 2026-04-01 at 10 20 54@2x After: CleanShot 2026-04-01 at 10 21 16@2x --- .../issueDetails/streamline/sidebar/autofixSection.spec.tsx | 6 +++--- .../issueDetails/streamline/sidebar/autofixSection.tsx | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/static/app/views/issueDetails/streamline/sidebar/autofixSection.spec.tsx b/static/app/views/issueDetails/streamline/sidebar/autofixSection.spec.tsx index 6beb152354ac..203d3ba2cfe8 100644 --- a/static/app/views/issueDetails/streamline/sidebar/autofixSection.spec.tsx +++ b/static/app/views/issueDetails/streamline/sidebar/autofixSection.spec.tsx @@ -74,7 +74,7 @@ describe('AutofixSection', () => { organization, }); - expect(screen.getByText('Seer')).toBeInTheDocument(); + expect(screen.getByText('Seer Autofix')).toBeInTheDocument(); }); it('renders Resources section when AI features are disabled', () => { @@ -381,7 +381,7 @@ describe('AutofixSection', () => { }); // The Seer title should still render - expect(screen.getByText('Seer')).toBeInTheDocument(); + expect(screen.getByText('Seer Autofix')).toBeInTheDocument(); expect(await screen.findByTestId('loading-placeholder')).toBeInTheDocument(); }); @@ -397,7 +397,7 @@ describe('AutofixSection', () => { }); // The Seer title should still render - expect(screen.getByText('Seer')).toBeInTheDocument(); + expect(screen.getByText('Seer Autofix')).toBeInTheDocument(); expect(await screen.findByText('Have Seer...')).toBeInTheDocument(); }); diff --git a/static/app/views/issueDetails/streamline/sidebar/autofixSection.tsx b/static/app/views/issueDetails/streamline/sidebar/autofixSection.tsx index 93566027385d..39346bb46089 100644 --- a/static/app/views/issueDetails/streamline/sidebar/autofixSection.tsx +++ b/static/app/views/issueDetails/streamline/sidebar/autofixSection.tsx @@ -92,7 +92,7 @@ export function AutofixSection({group, project, event}: AutofixSectionProps) { - {t('Seer')} + {t('Seer Autofix')} } From 57acfd4663abf30cfa88559b63f396dc7e3609b1 Mon Sep 17 00:00:00 2001 From: Jonas Date: Wed, 1 Apr 2026 09:45:34 -0700 Subject: [PATCH 18/57] ref(pageframe) removed border radius (#112008) Removes border radius from page frame and aligns border CleanShot 2026-04-01 at 09 30 24@2x CleanShot 2026-04-01 at 09 30 15@2x --------- Co-authored-by: Cursor Agent Co-authored-by: Jonas --- static/app/components/layouts/thirds.tsx | 26 +------------ static/app/views/navigation/topBar.tsx | 48 +----------------------- 2 files changed, 2 insertions(+), 72 deletions(-) diff --git a/static/app/components/layouts/thirds.tsx b/static/app/components/layouts/thirds.tsx index 0225ac06d8a0..65da2f90be5a 100644 --- a/static/app/components/layouts/thirds.tsx +++ b/static/app/components/layouts/thirds.tsx @@ -26,27 +26,9 @@ export function Page(props: FlexProps<'main'> & {withPadding?: boolean}) { if (hasPageFrame) { return ( - & {withPadding?: boolean}) { ); } -const StyledPageFrameStack = styled(Stack)<{roundedCorner: boolean}>` - > :first-child { - border-top-left-radius: ${p => (p.roundedCorner ? p.theme.radius.lg : undefined)}; - } -`; - /** * Header container for header content and header actions. * diff --git a/static/app/views/navigation/topBar.tsx b/static/app/views/navigation/topBar.tsx index 836c81ac1d45..49f2d82012a2 100644 --- a/static/app/views/navigation/topBar.tsx +++ b/static/app/views/navigation/topBar.tsx @@ -1,4 +1,3 @@ -import {useContext, useEffect, useRef} from 'react'; import {useTheme} from '@emotion/react'; import {Button} from '@sentry/scraps/button'; @@ -9,8 +8,6 @@ import {FeedbackButton} from 'sentry/components/feedbackButton/feedbackButton'; import {IconSeer} from 'sentry/icons'; import {t} from 'sentry/locale'; import {useOrganization} from 'sentry/utils/useOrganization'; -import {usePrimaryNavigation} from 'sentry/views/navigation/primaryNavigationContext'; -import {SecondaryNavigationContext} from 'sentry/views/navigation/secondaryNavigationContext'; import {useHasPageFrameFeature} from 'sentry/views/navigation/useHasPageFrameFeature'; import {useExplorerPanel} from 'sentry/views/seerExplorer/useExplorerPanel'; import {isSeerExplorerEnabled} from 'sentry/views/seerExplorer/utils'; @@ -20,50 +17,9 @@ import {PRIMARY_HEADER_HEIGHT} from './constants'; export function TopBar() { const theme = useTheme(); - const flexRef = useRef(null); const organization = useOrganization({allowNull: true}); - const primaryNavigation = usePrimaryNavigation(); - const secondaryNavigation = useContext(SecondaryNavigationContext); const hasPageFrame = useHasPageFrameFeature(); - useEffect(() => { - if (!flexRef.current) { - return undefined; - } - - if (primaryNavigation.layout === 'mobile') { - return undefined; - } - - if (secondaryNavigation?.view !== 'expanded') { - flexRef.current.style.borderBottom = `1px solid ${theme.tokens.border.primary}`; - return undefined; - } - - const handleScroll = () => { - if (!flexRef.current) { - return; - } - - if (primaryNavigation.layout === 'mobile') { - return; - } - - // @TODO(JonasBadalic): For the nicest transition possible, we should probably lerp the - // alpha color channel of the border color betweeen 0 and border radius distance. This would make the - // two blend nicely together without requiring us to approximate it usign the transition duration. - flexRef.current.style.borderBottom = - window.scrollY > 0 - ? `1px solid ${theme.tokens.border.primary}` - : '1px solid transparent'; - }; - - // Set initial state based on current scroll position - handleScroll(); - window.addEventListener('scroll', handleScroll, {passive: true}); - return () => window.removeEventListener('scroll', handleScroll); - }, [theme.tokens.border.primary, secondaryNavigation?.view, primaryNavigation.layout]); - const {openExplorerPanel} = useExplorerPanel(); if (!hasPageFrame) { @@ -72,7 +28,6 @@ export function TopBar() { return ( From 0672c10cdea41bb177ff9bed358ad27af1259b5a Mon Sep 17 00:00:00 2001 From: Sehr <58871345+sehr-m@users.noreply.github.com> Date: Wed, 1 Apr 2026 09:47:15 -0700 Subject: [PATCH 19/57] fix(coding integrations): add catching for integration not found error (#111691) When a coding agent integration (Cursor, Claude Code) is deleted or marked PENDING_DELETION, Seer's stored automation_handoff preference still references the old integration_id. On every subsequent automated autofix run, Seer calls Sentry's trigger_coding_agent_launch RPC with the stale ID. Sentry validates the integration, finds it gone, and raises NotFound. There are two paths that affect this, one is through the seer side root cause step and the other is after root cause completed first. To fix the first, I pulled out the NotFound error when returning to Seer and used an error code to identify it. When this is caught we clear the handoff preferences. For the second, there has been a matching change for the calling path in a separate [Seer PR](https://github.com/getsentry/seer/pull/5504). Tests were added and I ran it locally to make sure it works. --- src/sentry/seer/autofix/coding_agent.py | 28 +++- src/sentry/seer/autofix/on_completion_hook.py | 50 ++++++- src/sentry/seer/endpoints/seer_rpc.py | 60 ++++++++- tests/sentry/seer/autofix/__init__.py | 0 .../test_autofix_on_completion_hook.py | 48 +++++++ .../seer/autofix/test_on_completion_hook.py | 71 ++++++++++ tests/sentry/seer/endpoints/test_seer_rpc.py | 127 ++++++++++++++++++ tests/sentry/seer/explorer/__init__.py | 0 8 files changed, 374 insertions(+), 10 deletions(-) create mode 100644 tests/sentry/seer/autofix/__init__.py create mode 100644 tests/sentry/seer/autofix/test_on_completion_hook.py create mode 100644 tests/sentry/seer/explorer/__init__.py diff --git a/src/sentry/seer/autofix/coding_agent.py b/src/sentry/seer/autofix/coding_agent.py index ad9e9fd3cca7..5d61bdb065e3 100644 --- a/src/sentry/seer/autofix/coding_agent.py +++ b/src/sentry/seer/autofix/coding_agent.py @@ -12,6 +12,24 @@ from rest_framework.exceptions import APIException, NotFound, PermissionDenied, ValidationError from sentry import features + + +class IntegrationNotFound(NotFound): + pass + + +class OrganizationNotFound(NotFound): + pass + + +class AutofixStateNotFound(NotFound): + pass + + +class StateReposNotFound(NotFound): + pass + + from sentry.constants import ENABLE_SEER_CODING_DEFAULT, ObjectStatus from sentry.integrations.claude_code.integration import ( ClaudeCodeIntegrationMetadata, @@ -118,7 +136,7 @@ def _validate_and_get_integration(organization, integration_id: int): ) if not org_integration or org_integration.status != ObjectStatus.ACTIVE: - raise NotFound("Integration not found") + raise IntegrationNotFound("Integration not found") integration = integration_service.get_integration( organization_integration_id=org_integration.id, @@ -126,7 +144,7 @@ def _validate_and_get_integration(organization, integration_id: int): ) if not integration: - raise NotFound("Integration not found") + raise IntegrationNotFound("Integration not found") # Verify it's a coding agent integration if integration.provider not in get_coding_agent_providers(): @@ -252,7 +270,7 @@ def _launch_agents_for_repos( repos_to_launch = validated_repos or autofix_state_repos if not repos_to_launch: - raise NotFound( + raise StateReposNotFound( "There are no repos in the Seer state to launch coding agents with, make sure you have repos connected to Seer and rerun this Issue Fix." ) @@ -426,7 +444,7 @@ def launch_coding_agents_for_run( try: organization = Organization.objects.get(id=organization_id) except Organization.DoesNotExist: - raise NotFound("Organization not found") + raise OrganizationNotFound("Organization not found") if not organization.get_option("sentry:enable_seer_coding", default=ENABLE_SEER_CODING_DEFAULT): raise PermissionDenied("Code generation is disabled for this organization") @@ -457,7 +475,7 @@ def launch_coding_agents_for_run( autofix_state = _get_autofix_state(run_id, organization) if autofix_state is None: - raise NotFound("Autofix state not found") + raise AutofixStateNotFound("Autofix state not found") logger.info( "coding_agent.launch_request", diff --git a/src/sentry/seer/autofix/on_completion_hook.py b/src/sentry/seer/autofix/on_completion_hook.py index 84b6df842f4e..0f86910bb207 100644 --- a/src/sentry/seer/autofix/on_completion_hook.py +++ b/src/sentry/seer/autofix/on_completion_hook.py @@ -8,14 +8,22 @@ from sentry import features from sentry.models.group import Group from sentry.models.organization import Organization +from sentry.models.project import Project from sentry.seer.autofix.autofix_agent import ( AutofixStep, trigger_autofix_explorer, trigger_coding_agent_handoff, trigger_push_changes, ) +from sentry.seer.autofix.coding_agent import IntegrationNotFound from sentry.seer.autofix.constants import AutofixReferrer -from sentry.seer.autofix.utils import AutofixStoppingPoint, get_project_seer_preferences +from sentry.seer.autofix.utils import ( + AutofixStoppingPoint, + get_project_seer_preferences, + resolve_repository_ids, + set_project_seer_preference, + write_preference_to_sentry_db, +) from sentry.seer.entrypoints.operator import SeerAutofixOperator, process_autofix_updates from sentry.seer.explorer.client_models import Artifact from sentry.seer.explorer.client_utils import fetch_run_status @@ -25,6 +33,7 @@ SeerApiResponseValidationError, SeerAutomationHandoffConfiguration, ) +from sentry.seer.models.seer_api_models import SeerProjectPreference from sentry.seer.supergroups.embeddings import trigger_supergroups_embedding from sentry.sentry_apps.metrics import SentryAppEventType from sentry.sentry_apps.tasks.sentry_apps import broadcast_webhooks_for_organization @@ -472,6 +481,35 @@ def _get_handoff_config_if_applicable( return handoff_config + @classmethod + def _clear_handoff_preference( + cls, project: Project, run_id: int, organization: Organization + ) -> None: + """Clear automation_handoff from project preferences after integration is not found.""" + try: + preference_response = get_project_seer_preferences(project.id) + if preference_response and preference_response.preference: + updated_preference = preference_response.preference.copy( + update={"automation_handoff": None} + ) + set_project_seer_preference(updated_preference) + + if features.has("organizations:seer-project-settings-dual-write", organization): + try: + validated_pref = SeerProjectPreference.validate(updated_preference) + resolved_pref = resolve_repository_ids(organization.id, [validated_pref]) + write_preference_to_sentry_db(project, resolved_pref[0]) + except Exception: + logger.exception( + "seer.write_preferences.failed", + extra={"project_id": project.id, "organization_id": organization.id}, + ) + except (SeerApiError, SeerApiResponseValidationError): + logger.exception( + "autofix.on_completion_hook.clear_handoff_preference_failed", + extra={"run_id": run_id, "organization_id": organization.id}, + ) + @classmethod def _trigger_coding_agent_handoff( cls, @@ -508,6 +546,16 @@ def _trigger_coding_agent_handoff( "failures": len(result.get("failures", [])), }, ) + except IntegrationNotFound: + logger.exception( + "autofix.on_completion_hook.coding_agent_handoff_integration_not_found", + extra={ + "run_id": run_id, + "organization_id": organization.id, + "integration_id": handoff_config.integration_id, + }, + ) + cls._clear_handoff_preference(group.project, run_id, organization) except Exception: logger.exception( "autofix.on_completion_hook.coding_agent_handoff_failed", diff --git a/src/sentry/seer/endpoints/seer_rpc.py b/src/sentry/seer/endpoints/seer_rpc.py index f6c034d673e2..cdabc2867d0b 100644 --- a/src/sentry/seer/endpoints/seer_rpc.py +++ b/src/sentry/seer/endpoints/seer_rpc.py @@ -36,6 +36,7 @@ from sentry_protos.snuba.v1.trace_item_attribute_pb2 import AttributeKey, AttributeValue, StrArray from sentry_protos.snuba.v1.trace_item_filter_pb2 import ComparisonFilter, TraceItemFilter +from sentry import features from sentry.api.api_owners import ApiOwner from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.authentication import AuthenticationSiloLimit, StandardAuthentication @@ -73,8 +74,19 @@ get_attribute_values_with_substring, ) from sentry.seer.autofix.autofix_tools import get_error_event_details, get_profile_details -from sentry.seer.autofix.coding_agent import launch_coding_agents_for_run -from sentry.seer.autofix.utils import AutofixTriggerSource +from sentry.seer.autofix.coding_agent import ( + AutofixStateNotFound, + IntegrationNotFound, + OrganizationNotFound, + StateReposNotFound, + launch_coding_agents_for_run, +) +from sentry.seer.autofix.utils import ( + AutofixTriggerSource, + get_project_seer_preferences, + resolve_repository_ids, + write_preference_to_sentry_db, +) from sentry.seer.constants import SEER_SUPPORTED_SCM_PROVIDERS, SeerSCMProvider from sentry.seer.entrypoints.operator import SeerAutofixOperator, process_autofix_updates from sentry.seer.explorer.custom_tool_utils import call_custom_tool @@ -104,6 +116,7 @@ ) from sentry.seer.fetch_issues import by_error_type, by_function_name, by_text_query, utils from sentry.seer.issue_detection import create_issue_occurrence +from sentry.seer.models.seer_api_models import SeerProjectPreference from sentry.seer.utils import filter_repo_by_provider from sentry.sentry_apps.metrics import SentryAppEventType from sentry.sentry_apps.tasks.sentry_apps import broadcast_webhooks_for_organization @@ -565,6 +578,7 @@ def send_seer_webhook(*, event_name: str, organization_id: int, payload: dict) - def trigger_coding_agent_launch( *, organization_id: int, + project_id: int | None = None, integration_id: int, run_id: int, trigger_source: str = "solution", @@ -579,7 +593,7 @@ def trigger_coding_agent_launch( trigger_source: Either "root_cause" or "solution" (default: "solution") Returns: - dict: {"success": bool} + dict: {"success": bool, "error_code": str | None} """ try: launch_coding_agents_for_run( @@ -589,7 +603,45 @@ def trigger_coding_agent_launch( trigger_source=AutofixTriggerSource(trigger_source), ) return {"success": True} - except (NotFound, PermissionDenied, ValidationError, APIException): + except IntegrationNotFound: + logger.exception( + "coding_agent.rpc_launch_error", + extra={ + "organization_id": organization_id, + "integration_id": integration_id, + "run_id": run_id, + }, + ) + try: + project = Project.objects.get_from_cache(id=project_id) + organization = Organization.objects.get_from_cache(id=organization_id) + if features.has("organizations:seer-project-settings-dual-write", organization): + preference_response = get_project_seer_preferences(project.id) + if preference_response and preference_response.preference: + updated_preference = preference_response.preference.copy( + update={"automation_handoff": None} + ) + validated_pref = SeerProjectPreference.validate(updated_preference) + resolved_pref = resolve_repository_ids(organization.id, [validated_pref]) + write_preference_to_sentry_db(project, resolved_pref[0]) + except Exception: + logger.exception( + "coding_agent.clear_handoff_preference_failed", + extra={ + "project_id": project_id, + "organization_id": organization_id, + "run_id": run_id, + }, + ) + return {"success": False, "error_code": "integration_not_found"} + except ( + OrganizationNotFound, + AutofixStateNotFound, + StateReposNotFound, + PermissionDenied, + ValidationError, + APIException, + ): logger.exception( "coding_agent.rpc_launch_error", extra={ diff --git a/tests/sentry/seer/autofix/__init__.py b/tests/sentry/seer/autofix/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/tests/sentry/seer/autofix/test_autofix_on_completion_hook.py b/tests/sentry/seer/autofix/test_autofix_on_completion_hook.py index aa55052f2253..327669d82ec0 100644 --- a/tests/sentry/seer/autofix/test_autofix_on_completion_hook.py +++ b/tests/sentry/seer/autofix/test_autofix_on_completion_hook.py @@ -608,6 +608,54 @@ def test_maybe_continue_pipeline_triggers_handoff_when_configured( mock_trigger_handoff.assert_called_once() + @patch("sentry.seer.autofix.on_completion_hook.set_project_seer_preference") + @patch("sentry.seer.autofix.on_completion_hook.get_project_seer_preferences") + @patch("sentry.seer.autofix.on_completion_hook.trigger_coding_agent_handoff") + def test_trigger_coding_agent_handoff_clears_preference_on_not_found( + self, mock_trigger, mock_get_prefs, mock_set_pref + ): + """When IntegrationNotFound is raised, automation_handoff is cleared from preferences.""" + from sentry.seer.autofix.coding_agent import IntegrationNotFound + + mock_trigger.side_effect = IntegrationNotFound() + handoff_config = self._make_handoff_config() + mock_get_prefs.return_value = self._make_preference_response(handoff_config=handoff_config) + + AutofixOnCompletionHook._trigger_coding_agent_handoff( + organization=self.organization, + run_id=123, + group=self.group, + handoff_config=handoff_config, + ) + + mock_set_pref.assert_called_once() + updated = mock_set_pref.call_args.args[0] + assert updated.automation_handoff is None + + @patch("sentry.seer.autofix.on_completion_hook.set_project_seer_preference") + @patch("sentry.seer.autofix.on_completion_hook.get_project_seer_preferences") + @patch("sentry.seer.autofix.on_completion_hook.trigger_coding_agent_handoff") + def test_trigger_coding_agent_handoff_not_found_seer_api_error_does_not_raise( + self, mock_trigger, mock_get_prefs, mock_set_pref + ): + """A SeerApiError during preference-clearing after IntegrationNotFound should not propagate.""" + from sentry.seer.autofix.coding_agent import IntegrationNotFound + from sentry.seer.models import SeerApiError + + mock_trigger.side_effect = IntegrationNotFound() + mock_get_prefs.side_effect = SeerApiError("seer unavailable", 503) + handoff_config = self._make_handoff_config() + + # Should not raise + AutofixOnCompletionHook._trigger_coding_agent_handoff( + organization=self.organization, + run_id=123, + group=self.group, + handoff_config=handoff_config, + ) + + mock_set_pref.assert_not_called() + @patch("sentry.seer.autofix.on_completion_hook.trigger_coding_agent_handoff") def test_trigger_coding_agent_handoff_calls_function(self, mock_trigger): """Test _trigger_coding_agent_handoff calls the trigger function correctly.""" diff --git a/tests/sentry/seer/autofix/test_on_completion_hook.py b/tests/sentry/seer/autofix/test_on_completion_hook.py new file mode 100644 index 000000000000..7fc51b0eb87c --- /dev/null +++ b/tests/sentry/seer/autofix/test_on_completion_hook.py @@ -0,0 +1,71 @@ +from __future__ import annotations + +from unittest.mock import MagicMock, patch + +from sentry.seer.autofix.coding_agent import IntegrationNotFound +from sentry.seer.autofix.utils import CodingAgentProviderType +from sentry.seer.models.seer_api_models import SeerAutomationHandoffConfiguration +from sentry.testutils.cases import TestCase + + +class TestTriggerCodingAgentHandoff(TestCase): + def setUp(self) -> None: + super().setUp() + self.organization = self.create_organization() + self.project = self.create_project(organization=self.organization) + self.group = self.create_group(project=self.project) + + def _make_handoff_config(self, integration_id: int = 789) -> SeerAutomationHandoffConfiguration: + return SeerAutomationHandoffConfiguration( + handoff_point="root_cause", + target=CodingAgentProviderType.CURSOR_BACKGROUND_AGENT, + integration_id=integration_id, + ) + + @patch("sentry.seer.autofix.on_completion_hook.set_project_seer_preference") + @patch("sentry.seer.autofix.on_completion_hook.get_project_seer_preferences") + @patch("sentry.seer.autofix.on_completion_hook.trigger_coding_agent_handoff") + def test_not_found_clears_automation_handoff( + self, mock_trigger, mock_get_prefs, mock_set_pref + ) -> None: + from sentry.seer.autofix.on_completion_hook import AutofixOnCompletionHook + + mock_trigger.side_effect = IntegrationNotFound("Integration not found") + + mock_pref = MagicMock() + mock_pref.automation_handoff = self._make_handoff_config() + mock_pref.copy.return_value = mock_pref + mock_get_prefs.return_value = MagicMock(preference=mock_pref) + + handoff_config = self._make_handoff_config() + + AutofixOnCompletionHook._trigger_coding_agent_handoff( + organization=self.organization, + run_id=1, + group=self.group, + handoff_config=handoff_config, + ) + + mock_get_prefs.assert_called_once_with(self.group.project_id) + mock_pref.copy.assert_called_once_with(update={"automation_handoff": None}) + mock_set_pref.assert_called_once_with(mock_pref) + + @patch("sentry.seer.autofix.on_completion_hook.set_project_seer_preference") + @patch("sentry.seer.autofix.on_completion_hook.get_project_seer_preferences") + @patch("sentry.seer.autofix.on_completion_hook.trigger_coding_agent_handoff") + def test_not_found_no_preference_response_does_not_call_set( + self, mock_trigger, mock_get_prefs, mock_set_pref + ) -> None: + from sentry.seer.autofix.on_completion_hook import AutofixOnCompletionHook + + mock_trigger.side_effect = IntegrationNotFound("Integration not found") + mock_get_prefs.return_value = None + + AutofixOnCompletionHook._trigger_coding_agent_handoff( + organization=self.organization, + run_id=1, + group=self.group, + handoff_config=self._make_handoff_config(), + ) + + mock_set_pref.assert_not_called() diff --git a/tests/sentry/seer/endpoints/test_seer_rpc.py b/tests/sentry/seer/endpoints/test_seer_rpc.py index f7b133775e5a..80e6e3e9c369 100644 --- a/tests/sentry/seer/endpoints/test_seer_rpc.py +++ b/tests/sentry/seer/endpoints/test_seer_rpc.py @@ -22,6 +22,7 @@ get_github_enterprise_integration_config, get_repo_installation_id, has_repo_code_mappings, + trigger_coding_agent_launch, validate_repo, ) from sentry.seer.explorer.tools import get_trace_item_attributes @@ -1479,3 +1480,129 @@ def test_get_repo_installation_id_integration_not_found(self) -> None: ) assert result == {"error": "integration_not_found"} + + +class TestTriggerCodingAgentLaunch: + @patch("sentry.seer.endpoints.seer_rpc.launch_coding_agents_for_run") + def test_not_found_returns_integration_not_found_error_code(self, mock_launch): + from sentry.seer.autofix.coding_agent import IntegrationNotFound + + mock_launch.side_effect = IntegrationNotFound() + + result = trigger_coding_agent_launch( + organization_id=1, + project_id=4, + integration_id=2, + run_id=3, + ) + + assert result == {"success": False, "error_code": "integration_not_found"} + + @patch("sentry.seer.endpoints.seer_rpc.launch_coding_agents_for_run") + def test_organization_not_found_does_not_return_integration_error_code(self, mock_launch): + from sentry.seer.autofix.coding_agent import OrganizationNotFound + + mock_launch.side_effect = OrganizationNotFound() + + result = trigger_coding_agent_launch( + organization_id=1, + project_id=4, + integration_id=2, + run_id=3, + ) + + assert result == {"success": False} + assert result.get("error_code") != "integration_not_found" + + @patch("sentry.seer.endpoints.seer_rpc.launch_coding_agents_for_run") + def test_autofix_state_not_found_does_not_return_integration_error_code(self, mock_launch): + from sentry.seer.autofix.coding_agent import AutofixStateNotFound + + mock_launch.side_effect = AutofixStateNotFound() + + result = trigger_coding_agent_launch( + organization_id=1, + project_id=4, + integration_id=2, + run_id=3, + ) + + assert result == {"success": False} + assert result.get("error_code") != "integration_not_found" + + +class TestTriggerCodingAgentLaunchClearsHandoff(APITestCase): + def _make_preference_response(self): + from sentry.seer.models.seer_api_models import ( + AutofixHandoffPoint, + SeerAutomationHandoffConfiguration, + SeerProjectPreference, + SeerRawPreferenceResponse, + ) + + return SeerRawPreferenceResponse( + preference=SeerProjectPreference( + organization_id=self.organization.id, + project_id=self.project.id, + repositories=[], + automation_handoff=SeerAutomationHandoffConfiguration( + handoff_point=AutofixHandoffPoint.ROOT_CAUSE, + target="cursor_background_agent", + integration_id=42, + ), + ) + ) + + @patch("sentry.seer.endpoints.seer_rpc.get_project_seer_preferences") + @patch("sentry.seer.endpoints.seer_rpc.launch_coding_agents_for_run") + def test_integration_not_found_clears_handoff_project_options( + self, mock_launch, mock_get_prefs + ): + from sentry.seer.autofix.coding_agent import IntegrationNotFound + + mock_launch.side_effect = IntegrationNotFound() + mock_get_prefs.return_value = self._make_preference_response() + + self.project.update_option("sentry:seer_automation_handoff_point", "root_cause") + self.project.update_option( + "sentry:seer_automation_handoff_target", "cursor_background_agent" + ) + self.project.update_option("sentry:seer_automation_handoff_integration_id", 42) + self.project.update_option("sentry:seer_automation_handoff_auto_create_pr", True) + + with self.feature("organizations:seer-project-settings-dual-write"): + result = trigger_coding_agent_launch( + organization_id=self.organization.id, + project_id=self.project.id, + integration_id=42, + run_id=99, + ) + + assert result == {"success": False, "error_code": "integration_not_found"} + assert self.project.get_option("sentry:seer_automation_handoff_point") is None + assert self.project.get_option("sentry:seer_automation_handoff_target") is None + assert self.project.get_option("sentry:seer_automation_handoff_integration_id") is None + assert self.project.get_option("sentry:seer_automation_handoff_auto_create_pr") is False + + @patch("sentry.seer.endpoints.seer_rpc.get_project_seer_preferences") + @patch("sentry.seer.endpoints.seer_rpc.launch_coding_agents_for_run") + def test_integration_not_found_skips_clear_without_feature_flag( + self, mock_launch, mock_get_prefs + ): + from sentry.seer.autofix.coding_agent import IntegrationNotFound + + mock_launch.side_effect = IntegrationNotFound() + mock_get_prefs.return_value = self._make_preference_response() + + self.project.update_option("sentry:seer_automation_handoff_point", "root_cause") + + result = trigger_coding_agent_launch( + organization_id=self.organization.id, + project_id=self.project.id, + integration_id=42, + run_id=99, + ) + + assert result == {"success": False, "error_code": "integration_not_found"} + assert self.project.get_option("sentry:seer_automation_handoff_point") == "root_cause" + mock_get_prefs.assert_not_called() diff --git a/tests/sentry/seer/explorer/__init__.py b/tests/sentry/seer/explorer/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 From 1bf6e265a707520b44bc4b85d42d65bb83367dfc Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Wed, 1 Apr 2026 18:56:37 +0200 Subject: [PATCH 20/57] feat(attachments): Add a `date_expires` column (#111881) Adds a new column `date_expires` to event attachments that allows us to clean them up depending on organization-specific retention settings. Existing rows default to a sentinel value that will allow us to backfill. New rows are set to 30 days from now. In a follow-up, we will fill this with the retention value passed from Relay. Note: An index on this new column is added as a dedicated post-deploy migration, since this can take a long time. --- migrations_lockfile.txt | 2 +- .../1060_eventattachment_date_expires.py | 37 +++++++++++++++++++ ...1061_eventattachment_date_expires_index.py | 37 +++++++++++++++++++ src/sentry/models/eventattachment.py | 20 +++++++++- 4 files changed, 93 insertions(+), 3 deletions(-) create mode 100644 src/sentry/migrations/1060_eventattachment_date_expires.py create mode 100644 src/sentry/migrations/1061_eventattachment_date_expires_index.py diff --git a/migrations_lockfile.txt b/migrations_lockfile.txt index 5b76081914f0..6ade19ffd258 100644 --- a/migrations_lockfile.txt +++ b/migrations_lockfile.txt @@ -31,7 +31,7 @@ replays: 0007_organizationmember_replay_access seer: 0005_delete_seerorganizationsettings -sentry: 1059_add_groupopenperiodactivity_date_added_index +sentry: 1061_eventattachment_date_expires_index social_auth: 0003_social_auth_json_field diff --git a/src/sentry/migrations/1060_eventattachment_date_expires.py b/src/sentry/migrations/1060_eventattachment_date_expires.py new file mode 100644 index 000000000000..e238ab8a268a --- /dev/null +++ b/src/sentry/migrations/1060_eventattachment_date_expires.py @@ -0,0 +1,37 @@ +# Generated by Django 5.2.12 on 2026-04-01 07:22 + +import datetime + +from django.db import migrations, models + +from sentry.new_migrations.migrations import CheckedMigration + + +class Migration(CheckedMigration): + # This flag is used to mark that a migration shouldn't be automatically run in production. + # This should only be used for operations where it's safe to run the migration after your + # code has deployed. So this should not be used for most operations that alter the schema + # of a table. + # Here are some things that make sense to mark as post deployment: + # - Large data migrations. Typically we want these to be run manually so that they can be + # monitored and not block the deploy for a long period of time while they run. + # - Adding indexes to large tables. Since this can take a long time, we'd generally prefer to + # run this outside deployments so that we don't block them. Note that while adding an index + # is a schema change, it's completely safe to run the operation after the code has deployed. + # Once deployed, run these manually via: https://develop.sentry.dev/database-migrations/#migration-deployment + + is_post_deployment = False + + dependencies = [ + ("sentry", "1059_add_groupopenperiodactivity_date_added_index"), + ] + + operations = [ + migrations.AddField( + model_name="eventattachment", + name="date_expires", + field=models.DateTimeField( + db_default=datetime.datetime(1970, 1, 1, 0, 0, tzinfo=datetime.timezone.utc), + ), + ), + ] diff --git a/src/sentry/migrations/1061_eventattachment_date_expires_index.py b/src/sentry/migrations/1061_eventattachment_date_expires_index.py new file mode 100644 index 000000000000..72d7de02889f --- /dev/null +++ b/src/sentry/migrations/1061_eventattachment_date_expires_index.py @@ -0,0 +1,37 @@ +# Generated by Django 5.2.12 on 2026-04-01 12:13 + +import datetime +from django.db import migrations, models + +from sentry.new_migrations.migrations import CheckedMigration + + +class Migration(CheckedMigration): + # This flag is used to mark that a migration shouldn't be automatically run in production. + # This should only be used for operations where it's safe to run the migration after your + # code has deployed. So this should not be used for most operations that alter the schema + # of a table. + # Here are some things that make sense to mark as post deployment: + # - Large data migrations. Typically we want these to be run manually so that they can be + # monitored and not block the deploy for a long period of time while they run. + # - Adding indexes to large tables. Since this can take a long time, we'd generally prefer to + # run this outside deployments so that we don't block them. Note that while adding an index + # is a schema change, it's completely safe to run the operation after the code has deployed. + # Once deployed, run these manually via: https://develop.sentry.dev/database-migrations/#migration-deployment + + is_post_deployment = True + + dependencies = [ + ("sentry", "1060_eventattachment_date_expires"), + ] + + operations = [ + migrations.AlterField( + model_name="eventattachment", + name="date_expires", + field=models.DateTimeField( + db_default=datetime.datetime(1970, 1, 1, 0, 0, tzinfo=datetime.timezone.utc), + db_index=True, + ), + ), + ] diff --git a/src/sentry/models/eventattachment.py b/src/sentry/models/eventattachment.py index abb1f402b721..d556750ede0b 100644 --- a/src/sentry/models/eventattachment.py +++ b/src/sentry/models/eventattachment.py @@ -3,7 +3,8 @@ import mimetypes import os from dataclasses import dataclass -from datetime import timedelta +from datetime import datetime, timedelta +from datetime import timezone as dt_timezone from hashlib import sha1 from io import BytesIO from typing import IO, Any @@ -11,6 +12,7 @@ import zstandard from django.core.cache import cache from django.db import models +from django.db.models.expressions import DatabaseDefault from django.utils import timezone from objectstore_client import TimeToLive @@ -20,10 +22,13 @@ from sentry.db.models.fields.bounded import BoundedIntegerField from sentry.db.models.manager.base_query_set import BaseQuerySet from sentry.models.files.utils import get_size_and_checksum, get_storage -from sentry.objectstore import get_attachments_session +from sentry.objectstore import default_attachment_retention, get_attachments_session from sentry.objectstore.metrics import measure_storage_operation from sentry.options.rollout import in_random_rollout +# Sentinel value stored in `date_expires` to mean "no explicit expiry — use default retention". +DATE_EXPIRES_SENTINEL = datetime(1970, 1, 1, 0, 0, 0, tzinfo=dt_timezone.utc) + # Attachment file types that are considered a crash report (PII relevant) CRASH_REPORT_TYPES = ("event.minidump", "event.applecrashreport") @@ -100,6 +105,10 @@ class EventAttachment(Model): sha1 = models.CharField(max_length=40, null=True) date_added = models.DateTimeField(default=timezone.now, db_index=True) + date_expires = models.DateTimeField( + db_default=DATE_EXPIRES_SENTINEL, + db_index=True, + ) # storage: blob_path = models.TextField(null=True) @@ -114,6 +123,13 @@ class Meta: __repr__ = sane_repr("event_id", "name") + def save(self, *args: Any, **kwargs: Any) -> None: + # Computed here rather than as a field default to avoid freezing a callable + # reference into migrations, which would break if the function is ever renamed. + if self.date_expires is None or isinstance(self.date_expires, DatabaseDefault): # type: ignore[unreachable] + self.date_expires = timezone.now() + timedelta(days=default_attachment_retention()) # type: ignore[unreachable] + super().save(*args, **kwargs) + def delete(self, *args: Any, **kwargs: Any) -> tuple[int, dict[str, int]]: rv = super().delete(*args, **kwargs) From 3fcbc54c5757e8ec1b83df36f8a18b52abe78d15 Mon Sep 17 00:00:00 2001 From: Tony Xiao Date: Wed, 1 Apr 2026 13:01:01 -0400 Subject: [PATCH 21/57] fix(autofix): Restarting root cause from error should use new run (#111995) If root cause ever fail to produce an artifact, there's no reason to keep the conversation for a re-run. So remove the run id to start a fresh run. --- static/app/components/events/autofix/v3/autofixCards.tsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/static/app/components/events/autofix/v3/autofixCards.tsx b/static/app/components/events/autofix/v3/autofixCards.tsx index f21bd17202f1..f32622f6b99b 100644 --- a/static/app/components/events/autofix/v3/autofixCards.tsx +++ b/static/app/components/events/autofix/v3/autofixCards.tsx @@ -46,8 +46,7 @@ export function RootCauseCard({autofix, section}: AutofixCardProps) { return isRootCauseArtifact(sectionArtifact) ? sectionArtifact : null; }, [section]); - const {runState, startStep} = autofix; - const runId = runState?.run_id; + const {startStep} = autofix; return ( } title={t('Root Cause')}> @@ -94,7 +93,7 @@ export function RootCauseCard({autofix, section}: AutofixCardProps) { From 0448b18dc676c99f3a04688ea77aaf3d4a278ee6 Mon Sep 17 00:00:00 2001 From: Lyn Nagara <1779792+lynnagara@users.noreply.github.com> Date: Wed, 1 Apr 2026 10:06:19 -0700 Subject: [PATCH 22/57] ref(cells): Update callers to convert_to_async_discord_response and convert_to_async_slack_response (#111930) Follow up to https://github.com/getsentry/sentry/pull/111858, now workers are fully updated it is now safe to update callers to pass the new parameter --- .../middleware/integrations/parsers/discord.py | 2 +- .../middleware/integrations/parsers/slack.py | 2 +- .../integrations/parsers/test_discord.py | 10 +++++----- .../middleware/integrations/parsers/test_slack.py | 2 +- tests/sentry/middleware/integrations/test_tasks.py | 14 +++++++------- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/sentry/middleware/integrations/parsers/discord.py b/src/sentry/middleware/integrations/parsers/discord.py index 6d3d8eb9721f..390e84ff8c1d 100644 --- a/src/sentry/middleware/integrations/parsers/discord.py +++ b/src/sentry/middleware/integrations/parsers/discord.py @@ -64,7 +64,7 @@ def get_async_cell_response(self, cells: Sequence[Cell]) -> HttpResponse: if self.discord_request: convert_to_async_discord_response.apply_async( kwargs={ - "region_names": [c.name for c in cells], + "cell_names": [c.name for c in cells], "payload": create_async_request_payload(self.request), "response_url": self.discord_request.response_url, } diff --git a/src/sentry/middleware/integrations/parsers/slack.py b/src/sentry/middleware/integrations/parsers/slack.py index 38c88ff61775..06f26baf8273 100644 --- a/src/sentry/middleware/integrations/parsers/slack.py +++ b/src/sentry/middleware/integrations/parsers/slack.py @@ -180,7 +180,7 @@ def get_async_cell_response(self, cells: Sequence[Cell]) -> HttpResponseBase: convert_to_async_slack_response.apply_async( kwargs={ - "region_names": [r.name for r in cells], + "cell_names": [r.name for r in cells], "payload": create_async_request_payload(self.request), "response_url": self.response_url, } diff --git a/tests/sentry/middleware/integrations/parsers/test_discord.py b/tests/sentry/middleware/integrations/parsers/test_discord.py index 0bf5e61b4a81..ed55a408c001 100644 --- a/tests/sentry/middleware/integrations/parsers/test_discord.py +++ b/tests/sentry/middleware/integrations/parsers/test_discord.py @@ -141,13 +141,13 @@ def test_interactions_endpoint_routing_command(self, mock_verify_signature: Magi responses.POST, "http://us.testserver/extensions/discord/interactions/", status=202, - body=b"region_response", + body=b"cell_response", ) response = parser.get_response() assert isinstance(response, HttpResponse) assert response.status_code == status.HTTP_202_ACCEPTED - assert response.content == b"region_response" + assert response.content == b"cell_response" assert len(responses.calls) == 1 assert_no_webhook_payloads() @@ -186,13 +186,13 @@ def test_interactions_endpoint_routing_message_component( responses.POST, "http://us.testserver/extensions/discord/interactions/", status=201, - body=b"region_response", + body=b"cell_response", ) response = parser.get_response() assert isinstance(response, HttpResponse) assert response.status_code == status.HTTP_201_CREATED - assert response.content == b"region_response" + assert response.content == b"cell_response" assert len(responses.calls) == 1 assert_no_webhook_payloads() @@ -239,7 +239,7 @@ def test_triggers_async_response( payload = create_async_request_payload(self.request) mock_discord_task.apply_async.assert_called_once_with( kwargs={ - "region_names": ["us"], + "cell_names": ["us"], "payload": payload, "response_url": response_url, } diff --git a/tests/sentry/middleware/integrations/parsers/test_slack.py b/tests/sentry/middleware/integrations/parsers/test_slack.py index c5c8d3495f5c..b2946f1df2ea 100644 --- a/tests/sentry/middleware/integrations/parsers/test_slack.py +++ b/tests/sentry/middleware/integrations/parsers/test_slack.py @@ -129,7 +129,7 @@ def test_triggers_async_response( response = parser.get_response() mock_slack_task.apply_async.assert_called_once_with( kwargs={ - "region_names": ["us"], + "cell_names": ["us"], "payload": create_async_request_payload(request), "response_url": response_url, } diff --git a/tests/sentry/middleware/integrations/test_tasks.py b/tests/sentry/middleware/integrations/test_tasks.py index 1c51a73efbe5..dd39b876df2f 100644 --- a/tests/sentry/middleware/integrations/test_tasks.py +++ b/tests/sentry/middleware/integrations/test_tasks.py @@ -55,7 +55,7 @@ def test_convert_to_async_slack_response_all_success(self) -> None: status=200, ) convert_to_async_slack_response( - region_names=["eu", "us"], + cell_names=["eu", "us"], payload=self.payload, response_url=self.response_url, ) @@ -84,7 +84,7 @@ def test_convert_to_async_slack_response_mixed_success(self) -> None: match=[matchers.json_params_matcher({"ok": True, "region": "eu"})], ) convert_to_async_slack_response( - region_names=["us", "eu"], + cell_names=["us", "eu"], payload=self.payload, response_url=self.response_url, ) @@ -111,7 +111,7 @@ def test_convert_to_async_slack_response_no_success(self) -> None: status=200, ) convert_to_async_slack_response( - region_names=["us", "eu"], + cell_names=["us", "eu"], payload=self.payload, response_url=self.response_url, ) @@ -137,7 +137,7 @@ def test_empty_request_bdoy(self, mock_logger_info: MagicMock) -> None: status=200, ) convert_to_async_slack_response( - region_names=["us", "eu"], + cell_names=["us", "eu"], payload=self.payload, response_url=self.response_url, ) @@ -194,7 +194,7 @@ def test_convert_to_async_discord_response_all_success(self) -> None: status=200, ) convert_to_async_discord_response( - region_names=["eu", "us"], + cell_names=["eu", "us"], payload=self.payload, response_url=self.response_url, ) @@ -223,7 +223,7 @@ def test_convert_to_async_discord_response_mixed_success(self) -> None: match=[matchers.json_params_matcher({"ok": True, "region": "eu"})], ) convert_to_async_discord_response( - region_names=["eu", "us"], + cell_names=["eu", "us"], payload=self.payload, response_url=self.response_url, ) @@ -250,7 +250,7 @@ def test_convert_to_async_discord_response_no_success(self) -> None: status=200, ) convert_to_async_discord_response( - region_names=["eu", "us"], + cell_names=["eu", "us"], payload=self.payload, response_url=self.response_url, ) From a0835dde9ae39ac1b611d449964ef6d55e06f16d Mon Sep 17 00:00:00 2001 From: Evan Purkhiser Date: Wed, 1 Apr 2026 13:08:28 -0400 Subject: [PATCH 23/57] ci(frontend): Increase Node memory limit to 8GB (#112010) Bump max-old-space-size from 4096 to 8192 in frontend CI workflows to give the TypeScript checker more headroom. --- .github/workflows/frontend-optional.yml | 2 +- .github/workflows/frontend.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/frontend-optional.yml b/.github/workflows/frontend-optional.yml index e018750130c2..0ec51df7b67d 100644 --- a/.github/workflows/frontend-optional.yml +++ b/.github/workflows/frontend-optional.yml @@ -12,7 +12,7 @@ concurrency: # hack for https://github.com/actions/cache/issues/810#issuecomment-1222550359 env: SEGMENT_DOWNLOAD_TIMEOUT_MINS: 3 - NODE_OPTIONS: '--max-old-space-size=4096' + NODE_OPTIONS: '--max-old-space-size=8192' jobs: files-changed: diff --git a/.github/workflows/frontend.yml b/.github/workflows/frontend.yml index c0cf8b6737f8..5ed8b2bb60e2 100644 --- a/.github/workflows/frontend.yml +++ b/.github/workflows/frontend.yml @@ -15,7 +15,7 @@ concurrency: # hack for https://github.com/actions/cache/issues/810#issuecomment-1222550359 env: SEGMENT_DOWNLOAD_TIMEOUT_MINS: 3 - NODE_OPTIONS: '--max-old-space-size=4096' + NODE_OPTIONS: '--max-old-space-size=8192' jobs: files-changed: From 214c06c59a7ce9f57d3c6a2774473197aab590f0 Mon Sep 17 00:00:00 2001 From: Dan Fuller Date: Wed, 1 Apr 2026 10:11:33 -0700 Subject: [PATCH 24/57] fix(processing-errors): Fix bug in `produce_processing_errors_to_eap` (#112012) Fix a bug in eap processing error ingestion. Fixes SENTRY-5KRQ --- src/sentry/processing_errors/eap/producer.py | 3 ++- .../processing_errors/eap/test_producer.py | 26 +++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/sentry/processing_errors/eap/producer.py b/src/sentry/processing_errors/eap/producer.py index adc56b9d6db6..0d3793115aa5 100644 --- a/src/sentry/processing_errors/eap/producer.py +++ b/src/sentry/processing_errors/eap/producer.py @@ -19,6 +19,7 @@ from sentry.utils.arroyo_producer import SingletonProducer, get_arroyo_producer from sentry.utils.eap import hex_to_item_id from sentry.utils.kafka_config import get_topic_definition +from sentry.utils.safe import get_path if TYPE_CHECKING: from sentry.models.project import Project @@ -51,7 +52,7 @@ def produce_processing_errors_to_eap( stored as attributes. This enables querying processing errors in EAP for configuration issue detection. """ - trace_id = event_data.get("contexts", {}).get("trace", {}).get("trace_id") + trace_id = get_path(event_data, "contexts", "trace", "trace_id") if trace_id is None: logger.debug("Skipping EAP processing error production: missing trace_id") return diff --git a/tests/sentry/processing_errors/eap/test_producer.py b/tests/sentry/processing_errors/eap/test_producer.py index 174dec7a810d..2f552628b144 100644 --- a/tests/sentry/processing_errors/eap/test_producer.py +++ b/tests/sentry/processing_errors/eap/test_producer.py @@ -167,6 +167,32 @@ def test_skips_when_no_contexts(self, mock_producer, mock_logger): "Skipping EAP processing error production: missing trace_id" ) + @patch("sentry.processing_errors.eap.producer.logger") + @patch("sentry.processing_errors.eap.producer._eap_producer") + def test_skips_when_contexts_is_none(self, mock_producer, mock_logger): + event_data = self._make_event_data(contexts=None) + errors = [{"type": "js_no_source"}] + + produce_processing_errors_to_eap(self.project, event_data, errors) + + mock_producer.produce.assert_not_called() + mock_logger.debug.assert_called_once_with( + "Skipping EAP processing error production: missing trace_id" + ) + + @patch("sentry.processing_errors.eap.producer.logger") + @patch("sentry.processing_errors.eap.producer._eap_producer") + def test_skips_when_trace_context_is_none(self, mock_producer, mock_logger): + event_data = self._make_event_data(contexts={"trace": None}) + errors = [{"type": "js_no_source"}] + + produce_processing_errors_to_eap(self.project, event_data, errors) + + mock_producer.produce.assert_not_called() + mock_logger.debug.assert_called_once_with( + "Skipping EAP processing error production: missing trace_id" + ) + @patch("sentry.processing_errors.eap.producer._eap_producer") @patch("sentry.processing_errors.eap.producer.logger") def test_error_handling_does_not_raise(self, mock_logger, mock_producer): From 4b58d6f6f9c80d2f9b80697dfa3f40034475dd42 Mon Sep 17 00:00:00 2001 From: "sentry[bot]" <39604003+sentry[bot]@users.noreply.github.com> Date: Wed, 1 Apr 2026 10:11:47 -0700 Subject: [PATCH 25/57] fix(autofix): Handle NoneType project preferences from Seer API (#112009) The `TypeError: 'NoneType' object is not a mapping` occurred in the `OrganizationAutofixAutomationSettingsEndpoint.post` method when attempting to update autofix automation settings. The root cause was that `bulk_get_project_preferences()` could return `None` for projects that did not have existing preferences in the Seer API. When `existing_preferences.get(project_id_str, {})` was called for such a project, it would correctly retrieve the `None` value (as the key existed, but its value was `None`), rather than the default `{}`. Subsequently, attempting to unpack this `None` value using the `**` operator (`**existing_pref`) resulted in the `TypeError`. This fix changes the problematic line from `existing_pref = existing_preferences.get(project_id_str, {})` to `existing_pref = existing_preferences.get(project_id_str) or {}`. This ensures that if `existing_preferences.get(project_id_str)` returns `None`, `existing_pref` will be assigned an empty dictionary `{}`, allowing the dictionary unpacking to proceed safely without error. This aligns with the defensive pattern already used elsewhere in the codebase for similar scenarios. ### Legal Boilerplate Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms. Fixes SENTRY-5FNP --------- Co-authored-by: sentry[bot] <39604003+sentry[bot]@users.noreply.github.com> Co-authored-by: srest2021 --- ...rganization_autofix_automation_settings.py | 2 +- ...rganization_autofix_automation_settings.py | 31 +++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/sentry/seer/endpoints/organization_autofix_automation_settings.py b/src/sentry/seer/endpoints/organization_autofix_automation_settings.py index dd025bf950b1..70991078d393 100644 --- a/src/sentry/seer/endpoints/organization_autofix_automation_settings.py +++ b/src/sentry/seer/endpoints/organization_autofix_automation_settings.py @@ -304,7 +304,7 @@ def post(self, request: Request, organization: Organization) -> Response: continue project_id_str = str(proj_id) - existing_pref = existing_preferences.get(project_id_str, {}) + existing_pref = existing_preferences.get(project_id_str) or {} pref_update: dict[str, Any] = { **default_seer_project_preference(project).dict(), diff --git a/tests/sentry/seer/endpoints/test_organization_autofix_automation_settings.py b/tests/sentry/seer/endpoints/test_organization_autofix_automation_settings.py index ed0f3c74e4c5..1473e2580c12 100644 --- a/tests/sentry/seer/endpoints/test_organization_autofix_automation_settings.py +++ b/tests/sentry/seer/endpoints/test_organization_autofix_automation_settings.py @@ -934,6 +934,37 @@ def test_post_creates_seer_project_repository( assert seer_repo.repository_id == repo.id assert project.get_option("sentry:seer_automated_run_stopping_point") == "open_pr" + @patch( + "sentry.seer.endpoints.organization_autofix_automation_settings.bulk_set_project_preferences" + ) + @patch( + "sentry.seer.endpoints.organization_autofix_automation_settings.bulk_get_project_preferences" + ) + def test_post_handles_null_existing_preference( + self, mock_bulk_get_preferences, mock_bulk_set_preferences + ): + project = self.create_project(organization=self.organization) + + mock_bulk_get_preferences.return_value = { + str(project.id): None, + } + + response = self.client.post( + self.url, + { + "projectIds": [project.id], + "automatedRunStoppingPoint": AutofixStoppingPoint.OPEN_PR.value, + }, + ) + assert response.status_code == 204 + + mock_bulk_set_preferences.assert_called_once() + call_args = mock_bulk_set_preferences.call_args + preferences = call_args[0][1] + assert len(preferences) == 1 + assert preferences[0]["project_id"] == project.id + assert preferences[0]["automated_run_stopping_point"] == AutofixStoppingPoint.OPEN_PR.value + @patch( "sentry.seer.endpoints.organization_autofix_automation_settings.bulk_set_project_preferences" ) From 421b82f0cdfc1b8f8dee90acbea9e203c40829f0 Mon Sep 17 00:00:00 2001 From: Dominik Buszowiecki <44422760+DominikB2014@users.noreply.github.com> Date: Wed, 1 Apr 2026 13:12:48 -0400 Subject: [PATCH 26/57] feat(admin): Add gsAdmin action to adjust dashboard parallel query limit (#111889) Add a modal in the `/_admin/` customer details page that allows staff to change the `dashboardsAsyncQueueParallelLimit` org option, which controls how many dashboard widget queries can run in parallel per org. This option was previously only settable via the API directly (`PUT /api/0/organizations/{org}/`). The new action appears in the admin customer details actions dropdown alongside existing options like "Toggle Console Platforms" and "Update Retentions". image image --------- Co-authored-by: Claude Opus 4.6 --- .github/CODEOWNERS | 1 + static/app/types/organization.tsx | 1 + .../changeDashboardsParallelLimitModal.tsx | 123 ++++++++++++++++++ static/gsAdmin/views/customerDetails.tsx | 13 ++ 4 files changed, 138 insertions(+) create mode 100644 static/gsAdmin/components/changeDashboardsParallelLimitModal.tsx diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index eb964798463d..34561a6b0c9d 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -764,6 +764,7 @@ tests/sentry/api/endpoints/test_organization_attribute_mappings.py @get ## gsAdmin # /static/gsAdmin/* unowned +/static/gsAdmin/components/changeDashboardsParallelLimitModal.tsx @getsentry/data-browsing /static/gsAdmin/components/forkCustomer.tsx @getsentry/hybrid-cloud /static/gsAdmin/components/relocation* @getsentry/hybrid-cloud /static/gsAdmin/views/relocation* @getsentry/hybrid-cloud diff --git a/static/app/types/organization.tsx b/static/app/types/organization.tsx index 1123f243ba52..5cb3ffea2c0f 100644 --- a/static/app/types/organization.tsx +++ b/static/app/types/organization.tsx @@ -101,6 +101,7 @@ export interface Organization extends OrganizationSummary { teamRoleList: TeamRole[]; trustedRelays: Relay[]; consoleSdkInviteQuota?: number; + dashboardsAsyncQueueParallelLimit?: number; defaultAutofixAutomationTuning?: | 'off' | 'super_low' diff --git a/static/gsAdmin/components/changeDashboardsParallelLimitModal.tsx b/static/gsAdmin/components/changeDashboardsParallelLimitModal.tsx new file mode 100644 index 000000000000..d2b44b45d0b2 --- /dev/null +++ b/static/gsAdmin/components/changeDashboardsParallelLimitModal.tsx @@ -0,0 +1,123 @@ +import {Fragment} from 'react'; + +import {Heading, Text} from '@sentry/scraps/text'; + +import {addErrorMessage, addSuccessMessage} from 'sentry/actionCreators/indicator'; +import type {ModalRenderProps} from 'sentry/actionCreators/modal'; +import {openModal} from 'sentry/actionCreators/modal'; +import {NumberField} from 'sentry/components/forms/fields/numberField'; +import {Form, type FormProps} from 'sentry/components/forms/form'; +import type {Organization} from 'sentry/types/organization'; +import {fetchMutation, useMutation} from 'sentry/utils/queryClient'; +import type {RequestError} from 'sentry/utils/requestError/requestError'; + +const DEFAULT_PARALLEL_LIMIT = 20; + +type OnSubmitArgs = Parameters>; +interface MutationVariables { + limit: number; + onSubmitError: OnSubmitArgs[2]; + onSubmitSuccess: OnSubmitArgs[1]; +} + +interface ChangeDashboardsParallelLimitModalProps extends ModalRenderProps { + onSuccess: () => void; + organization: Organization; +} + +function ChangeDashboardsParallelLimitModal({ + Header, + Body, + closeModal, + organization, + onSuccess, +}: ChangeDashboardsParallelLimitModalProps) { + const currentLimit = + organization.dashboardsAsyncQueueParallelLimit ?? DEFAULT_PARALLEL_LIMIT; + + const {mutate, isPending} = useMutation< + Record, + RequestError, + MutationVariables + >({ + mutationFn: ({limit}) => + fetchMutation({ + method: 'PUT', + url: `/organizations/${organization.slug}/`, + data: {dashboardsAsyncQueueParallelLimit: limit}, + }), + onSuccess: (response, {onSubmitSuccess}) => { + onSubmitSuccess?.(response); + addSuccessMessage('Dashboard parallel query limit updated.'); + onSuccess(); + closeModal(); + }, + onError: (error, {onSubmitError}) => { + onSubmitError?.({responseJSON: error?.responseJSON}); + addErrorMessage('Failed to update dashboard parallel query limit.'); + }, + }); + + const onSubmit: NonNullable = ( + data, + onSubmitSuccess, + onSubmitError + ) => { + const limit = Number(data.dashboardsAsyncQueueParallelLimit); + + if (!limit || limit < 1 || isPending) { + return; + } + + mutate({limit, onSubmitSuccess, onSubmitError}); + }; + + return ( + +
+ Change Dashboard Parallel Query Limit +
+ +

+ Current value: + {currentLimit} +

+
+ + + +
+ ); +} + +export function openChangeDashboardsParallelLimitModal({ + organization, + onSuccess, +}: { + onSuccess: () => void; + organization: Organization; +}) { + openModal(modalProps => ( + + )); +} diff --git a/static/gsAdmin/views/customerDetails.tsx b/static/gsAdmin/views/customerDetails.tsx index 0396f5bb8a85..0ac8adfe10f3 100644 --- a/static/gsAdmin/views/customerDetails.tsx +++ b/static/gsAdmin/views/customerDetails.tsx @@ -39,6 +39,7 @@ import {AddGiftEventsAction} from 'admin/components/addGiftEventsAction'; import {triggerAddToStartupProgramModal} from 'admin/components/addToStartupProgramAction'; import {CancelSubscriptionAction} from 'admin/components/cancelSubscriptionAction'; import {triggerChangeBalanceModal} from 'admin/components/changeBalanceAction'; +import {openChangeDashboardsParallelLimitModal} from 'admin/components/changeDashboardsParallelLimitModal'; import {triggerChangeDatesModal} from 'admin/components/changeDatesAction'; import {triggerGoogleDomainModal} from 'admin/components/changeGoogleDomainAction'; import {triggerChangePlanAction} from 'admin/components/changePlanAction'; @@ -859,6 +860,18 @@ export function CustomerDetails() { }); }, }, + { + key: 'changeDashboardsParallelLimit', + name: 'Change Dashboard Parallel Query Limit', + help: 'Adjust how many dashboard widget queries can run in parallel for this organization.', + skipConfirmModal: true, + onAction: () => { + openChangeDashboardsParallelLimitModal({ + organization, + onSuccess: reloadData, + }); + }, + }, ]} sections={[ { From ff058979faa41a8c390d91b3e1efbcb48c7621d8 Mon Sep 17 00:00:00 2001 From: Charlie Luo Date: Wed, 1 Apr 2026 13:24:01 -0400 Subject: [PATCH 27/57] ref(api): move relevant endpoint ownership from enterprise -> ecosystem (#111174) Breaking up https://github.com/getsentry/sentry/pull/111002. Move ownership for ecosystem-related endpoints from enterprise. Co-authored-by: Claude --- src/sentry/api/endpoints/api_application_rotate_secret.py | 2 +- src/sentry/api/endpoints/api_authorizations.py | 2 +- src/sentry/api/endpoints/organization_auth_token_details.py | 2 +- src/sentry/api/endpoints/organization_auth_tokens.py | 2 +- src/sentry/data_secrecy/api/waive_data_secrecy.py | 2 +- .../sentry_apps/api/endpoints/sentry_app_rotate_secret.py | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/sentry/api/endpoints/api_application_rotate_secret.py b/src/sentry/api/endpoints/api_application_rotate_secret.py index 5dd2f189e7a6..79d392bafc8a 100644 --- a/src/sentry/api/endpoints/api_application_rotate_secret.py +++ b/src/sentry/api/endpoints/api_application_rotate_secret.py @@ -16,7 +16,7 @@ class ApiApplicationRotateSecretEndpoint(ApiApplicationEndpoint): publish_status = { "POST": ApiPublishStatus.PRIVATE, } - owner = ApiOwner.ENTERPRISE + owner = ApiOwner.ECOSYSTEM authentication_classes = (SessionAuthentication,) permission_classes = (SentryIsAuthenticated,) diff --git a/src/sentry/api/endpoints/api_authorizations.py b/src/sentry/api/endpoints/api_authorizations.py index 9711451d69d2..cc9b75f3562a 100644 --- a/src/sentry/api/endpoints/api_authorizations.py +++ b/src/sentry/api/endpoints/api_authorizations.py @@ -22,7 +22,7 @@ class ApiAuthorizationsEndpoint(Endpoint): "DELETE": ApiPublishStatus.PRIVATE, "GET": ApiPublishStatus.PRIVATE, } - owner = ApiOwner.ENTERPRISE + owner = ApiOwner.ECOSYSTEM authentication_classes = (SessionAuthentication,) permission_classes = (SentryIsAuthenticated,) diff --git a/src/sentry/api/endpoints/organization_auth_token_details.py b/src/sentry/api/endpoints/organization_auth_token_details.py index dad86cbb76e4..69d8ad37ae69 100644 --- a/src/sentry/api/endpoints/organization_auth_token_details.py +++ b/src/sentry/api/endpoints/organization_auth_token_details.py @@ -23,7 +23,7 @@ class OrganizationAuthTokenDetailsEndpoint(ControlSiloOrganizationEndpoint): "GET": ApiPublishStatus.PRIVATE, "PUT": ApiPublishStatus.PRIVATE, } - owner = ApiOwner.ENTERPRISE + owner = ApiOwner.ECOSYSTEM authentication_classes = (SessionNoAuthTokenAuthentication,) permission_classes = (OrgAuthTokenPermission, DisallowImpersonatedTokenCreation) diff --git a/src/sentry/api/endpoints/organization_auth_tokens.py b/src/sentry/api/endpoints/organization_auth_tokens.py index a44c2dca7f1b..69df1892110d 100644 --- a/src/sentry/api/endpoints/organization_auth_tokens.py +++ b/src/sentry/api/endpoints/organization_auth_tokens.py @@ -42,7 +42,7 @@ class OrganizationAuthTokensEndpoint(ControlSiloOrganizationEndpoint): "GET": ApiPublishStatus.PRIVATE, "POST": ApiPublishStatus.PRIVATE, } - owner = ApiOwner.ENTERPRISE + owner = ApiOwner.ECOSYSTEM authentication_classes = (SessionNoAuthTokenAuthentication,) permission_classes = (OrgAuthTokenPermission, DisallowImpersonatedTokenCreation) diff --git a/src/sentry/data_secrecy/api/waive_data_secrecy.py b/src/sentry/data_secrecy/api/waive_data_secrecy.py index c33ce0fcadf5..b56c296d407a 100644 --- a/src/sentry/data_secrecy/api/waive_data_secrecy.py +++ b/src/sentry/data_secrecy/api/waive_data_secrecy.py @@ -53,7 +53,7 @@ class WaiveDataSecrecyEndpoint(ControlSiloOrganizationEndpoint): "POST": ApiPublishStatus.PRIVATE, "DELETE": ApiPublishStatus.PRIVATE, } - owner = ApiOwner.ENTERPRISE + owner = ApiOwner.ECOSYSTEM def get( self, diff --git a/src/sentry/sentry_apps/api/endpoints/sentry_app_rotate_secret.py b/src/sentry/sentry_apps/api/endpoints/sentry_app_rotate_secret.py index fc951688ffe5..af4da3b4eda3 100644 --- a/src/sentry/sentry_apps/api/endpoints/sentry_app_rotate_secret.py +++ b/src/sentry/sentry_apps/api/endpoints/sentry_app_rotate_secret.py @@ -77,7 +77,7 @@ class SentryAppRotateSecretEndpoint(SentryAppBaseEndpoint): publish_status = { "POST": ApiPublishStatus.PRIVATE, } - owner = ApiOwner.ENTERPRISE + owner = ApiOwner.ECOSYSTEM permission_classes = (SentryAppRotateSecretPermission,) def post(self, request: Request, sentry_app: SentryApp) -> Response: From 609e62815a84920f1e73a4bcd3df844db459633f Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 1 Apr 2026 10:30:21 -0700 Subject: [PATCH 28/57] fix(grouping): Fix and expand hex parameterization (#111969) This makes a few different changes to our hex parameterization: - Include the negative sign as part of the hex number. Also make dashes only count as negative signs if they're not in the middle of something alphanumeric. (This matches our recent int parameterization fix.) - Allow hex values to be as short as 4 characters as long as they include both a hex letter and a number. There's a small chance for false positives here, but right now we're missing a lot of parameterizations where longer hex values are broken up by dashes or dots into shorter ones, so it seems worth it to make this change. (There's also definitely a chance of false negatives - where values which should be `--` instead end up as `--` or `--face` - but unless we see examples in the wild, it's probably not worth it to try to be smarter about when `face`, etc. should count as a word and when it should count as hex.) - Stop requiring a number in longer (8+-character) hex strings. The chance of false positives here is _extremely_ low (Claude could find literally exactly one 8+-letter word consisting of only hex letters in any common language using the roman alphabet, and it's the latin name for a plant family), and though _most_ hex strings have numbers in them, the chances of seeing some without still seems much higher than the chance of encountering the `fabaceae` family of legumes in an error message. --- src/sentry/grouping/parameterization.py | 57 ++++++++++++------- .../sentry/grouping/test_parameterization.py | 25 ++++---- 2 files changed, 48 insertions(+), 34 deletions(-) diff --git a/src/sentry/grouping/parameterization.py b/src/sentry/grouping/parameterization.py index dca27064f4e0..603d6b6ae177 100644 --- a/src/sentry/grouping/parameterization.py +++ b/src/sentry/grouping/parameterization.py @@ -207,23 +207,40 @@ def _get_pattern(self, raw_pattern: str) -> str: # the prefix pretty much guarantees it's hex). (\b0[xX][0-9a-fA-F]+\b) | - # Hex value without `0x/0X` prefix (between 8 and 128 digits, including a number, and - # either all uppercase or all lowercase - we're more conservative here on all three - # scores in order to reduce false positives). - # - # Note: We use a lookahead for `0-9` but don't need one for `a-f/A-F` since if - # a) the value consists of nothing but potential hex digits, but - # b) none of those potential hex digits is a letter - # then the pattern would already have caught it. Given that we're here, it didn't, - # so the only thing we need the lookahead to guard against is it being all letters. - # - # Each regex consists of two parts, the lookahead and the hex characters themselves. For - # example, for the lowercase pattern we have: - # (?=[a-f]*[0-9]) The lookahead - there must be a digit, which may or may not be - # preceded by some number of hex letters - # [0-9a-f]{8,128} The matcher itself - between 8 and 128 hex characters - (\b(?=[a-f]*[0-9])[0-9a-f]{8,128}\b) | - (\b(?=[A-F]*[0-9])[0-9A-F]{8,128}\b) + # Hex value without `0x/0X` prefix (at least 4 characters long, containing both a letter + # and number if shorter than 8 characters, and either all uppercase or all lowercase - + # we're more conservative here in order to reduce false positives). + ( + # Regular word boundary (for positive values) + \b + | + # Alphanumeric negative lookbehind before the dash in negative values to ensure it's + # only considered a minus sign if it doesn't connect two alphanumeric strings. (No + # word boundary here because the dash serves as the word boundary, since it's not a + # word character.) + (? str: # Regular word boundary for positive ints \b | - # Alphanumeric negative lookbehind for negative ints to ensure a dash is only - # considered a minus sign if it doesn't connect two alphanumeric strings. (No word - # boundary here because the dash serves as the word boundary, since it's not a word - # character.) + # Alphanumeric negative lookbehind before the dash in negative ints (logic the same + # as in the hex pattern above) (?-", + "--", ), ("uuid", "7c1811ed-e98f-4c9c-a9f9-58c757ff494f", ""), ( @@ -141,8 +141,10 @@ ("hex with prefix - uppercase, 24 digits", "0x9AF8C3BE3A1231FE1121ACB1", ""), ("hex with prefix - lowercase, no numbers", "0xdeadbeef", ""), ("hex with prefix - uppercase, no numbers", "0xDEADBEEF", ""), - ("hex without prefix - lowercase, 4 digits", "9af8", "9af8"), - ("hex without prefix - uppercase, 4 digits", "9AF8", "9AF8"), + ("hex without prefix - lowercase, < 4 digits", "9af", "9af"), + ("hex without prefix - uppercase, < 4 digits", "9AF", "9AF"), + ("hex without prefix - lowercase, 4 digits", "9af8", ""), + ("hex without prefix - uppercase, 4 digits", "9AF8", ""), ("hex without prefix - lowercase, 8 digits", "9af8c3be", ""), ("hex without prefix - uppercase, 8 digits", "9AF8C3BE", ""), ("hex without prefix - lowercase, 10 digits", "9af8c3be3a", ""), @@ -153,13 +155,16 @@ ("hex without prefix - uppercase, 24 digits", "9AF8C3BE3A1231FE1121ACB1", ""), ("hex without prefix - lowercase, 128 digits", "b0" * 64, ""), ("hex without prefix - uppercase, 128 digits", "B0" * 64, ""), - ("hex without prefix - lowercase, no numbers", "deadbeef", "deadbeef"), - ("hex without prefix - uppercase, no numbers", "DEADBEEF", "DEADBEEF"), - ("hex without prefix - lowercase, no numbers until later", "deadbeef 123", "deadbeef "), - ("hex without prefix - uppercase, no numbers until later", "DEADBEEF 123", "DEADBEEF "), + ("hex without prefix - lowercase, no numbers, < 8 digits", "deadbee", "deadbee"), + ("hex without prefix - uppercase, no numbers, < 8 digits", "DEADBEE", "DEADBEE"), + ("hex without prefix - lowercase, no numbers, 8 digits", "deadbeef", ""), + ("hex without prefix - uppercase, no numbers, 8 digits", "DEADBEEF", ""), + ("hex without prefix - lowercase, no numbers until later", "cafe 123", "cafe "), + ("hex without prefix - uppercase, no numbers until later", "CAFE 123", "CAFE "), ("hex without prefix - no letters, < 8 digits, positive", "1234567", ""), ("hex without prefix - no letters, < 8 digits, negative", "-1234567", ""), ("hex without prefix - no letters, 8+ digits, positive", "12345678", ""), + ("hex without prefix - no letters, 8+ digits, negative", "-12345678", ""), ("git sha", "commit a93c7d2", "commit "), ("git sha - all letters", "commit cabcafe", "commit cabcafe"), ("git sha - all numbers", "commit 4150908", "commit "), @@ -240,12 +245,6 @@ def test_experimental_parameterization(name: str, input: str, expected: str) -> # parameterization. (Remember to remove the last item in each tuple for the cases you fix.) incorrect_cases = [ # ("name", "input", "desired", "actual") - ( - "hex without prefix - no letters, 8+ digits, negative", - "-12345678", - "", - "-", - ), ( "int - number in word", "Encoding: utf-8", From e7097cc0ba857d5c53a8ca702c94855d3e62ce32 Mon Sep 17 00:00:00 2001 From: Alberto Leal Date: Wed, 1 Apr 2026 13:33:39 -0400 Subject: [PATCH 29/57] feat(billing): Implement ClickHouse backend for UsageService (#111799) --- .github/CODEOWNERS | 1 + .../services/usage/_category_mapping.py | 50 ++ .../services/usage/_outcomes_query.py | 264 ++++++++++ .../platform/services/usage/service.py | 5 +- src/sentry/snuba/referrer.py | 1 + .../services/usage/test_outcomes_query.py | 365 ++++++++++++++ tests/snuba/billing/__init__.py | 0 tests/snuba/billing/platform/__init__.py | 0 .../billing/platform/services/__init__.py | 0 .../platform/services/usage/__init__.py | 0 .../usage/test_outcomes_integration.py | 454 ++++++++++++++++++ 11 files changed, 1137 insertions(+), 3 deletions(-) create mode 100644 src/sentry/billing/platform/services/usage/_category_mapping.py create mode 100644 src/sentry/billing/platform/services/usage/_outcomes_query.py create mode 100644 tests/sentry/billing/platform/services/usage/test_outcomes_query.py create mode 100644 tests/snuba/billing/__init__.py create mode 100644 tests/snuba/billing/platform/__init__.py create mode 100644 tests/snuba/billing/platform/services/__init__.py create mode 100644 tests/snuba/billing/platform/services/usage/__init__.py create mode 100644 tests/snuba/billing/platform/services/usage/test_outcomes_integration.py diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 34561a6b0c9d..eaf26e9de5c5 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -745,6 +745,7 @@ tests/sentry/api/endpoints/test_organization_attribute_mappings.py @get /src/sentry/api/endpoints/check_am2_compatibility.py @getsentry/revenue /tests/js/getsentry-test/ @getsentry/revenue /src/sentry/billing/ @getsentry/revenue +/tests/sentry/billing/ @getsentry/revenue /src/sentry/audit_log/ @getsentry/revenue ## gsApp diff --git a/src/sentry/billing/platform/services/usage/_category_mapping.py b/src/sentry/billing/platform/services/usage/_category_mapping.py new file mode 100644 index 000000000000..8bbe76019934 --- /dev/null +++ b/src/sentry/billing/platform/services/usage/_category_mapping.py @@ -0,0 +1,50 @@ +from __future__ import annotations + +import logging + +from sentry_protos.billing.v1.data_category_pb2 import DataCategory as ProtoDataCategory + +from sentry.constants import DataCategory +from sentry.utils import metrics + +logger = logging.getLogger(__name__) + +# Proto DataCategory enum uses different int values from Relay/Sentry DataCategory. +# e.g., Proto ATTACHMENT=3 vs Relay ATTACHMENT=4. +# ClickHouse stores Relay ints. The proto request carries proto ints. +# This mapping converts between the two. +PROTO_TO_RELAY_CATEGORY: dict[int, int] = { + ProtoDataCategory.DATA_CATEGORY_ERROR: int(DataCategory.ERROR), + ProtoDataCategory.DATA_CATEGORY_TRANSACTION: int(DataCategory.TRANSACTION), + ProtoDataCategory.DATA_CATEGORY_ATTACHMENT: int(DataCategory.ATTACHMENT), + ProtoDataCategory.DATA_CATEGORY_PROFILE: int(DataCategory.PROFILE), + ProtoDataCategory.DATA_CATEGORY_REPLAY: int(DataCategory.REPLAY), + ProtoDataCategory.DATA_CATEGORY_MONITOR: int(DataCategory.MONITOR), + ProtoDataCategory.DATA_CATEGORY_SPAN: int(DataCategory.SPAN), + ProtoDataCategory.DATA_CATEGORY_USER_REPORT_V2: int(DataCategory.USER_REPORT_V2), + ProtoDataCategory.DATA_CATEGORY_PROFILE_DURATION: int(DataCategory.PROFILE_DURATION), + ProtoDataCategory.DATA_CATEGORY_LOG_BYTE: int(DataCategory.LOG_BYTE), + ProtoDataCategory.DATA_CATEGORY_PROFILE_DURATION_UI: int(DataCategory.PROFILE_DURATION_UI), + ProtoDataCategory.DATA_CATEGORY_SEER_AUTOFIX: int(DataCategory.SEER_AUTOFIX), + ProtoDataCategory.DATA_CATEGORY_SEER_SCANNER: int(DataCategory.SEER_SCANNER), + ProtoDataCategory.DATA_CATEGORY_SIZE_ANALYSIS: int(DataCategory.SIZE_ANALYSIS), + ProtoDataCategory.DATA_CATEGORY_INSTALLABLE_BUILD: int(DataCategory.INSTALLABLE_BUILD), + ProtoDataCategory.DATA_CATEGORY_TRACE_METRIC: int(DataCategory.TRACE_METRIC), + ProtoDataCategory.DATA_CATEGORY_DEFAULT: int(DataCategory.DEFAULT), + ProtoDataCategory.DATA_CATEGORY_SECURITY: int(DataCategory.SECURITY), + ProtoDataCategory.DATA_CATEGORY_PROFILE_CHUNK: int(DataCategory.PROFILE_CHUNK), + ProtoDataCategory.DATA_CATEGORY_PROFILE_CHUNK_UI: int(DataCategory.PROFILE_CHUNK_UI), +} + + +def proto_to_relay_category(proto_category: int) -> int: + """Convert a proto DataCategory int to the Relay/Sentry int used in ClickHouse.""" + result = PROTO_TO_RELAY_CATEGORY.get(proto_category) + if result is None: + metrics.incr( + "billing.proto_category_mapping.unmapped", + tags={"proto_category": str(proto_category)}, + sample_rate=1.0, + ) + return proto_category + return result diff --git a/src/sentry/billing/platform/services/usage/_outcomes_query.py b/src/sentry/billing/platform/services/usage/_outcomes_query.py new file mode 100644 index 000000000000..5e16ef64bab1 --- /dev/null +++ b/src/sentry/billing/platform/services/usage/_outcomes_query.py @@ -0,0 +1,264 @@ +from __future__ import annotations + +import logging +from collections import defaultdict +from collections.abc import Sequence +from datetime import datetime, timedelta, timezone + +from google.protobuf.timestamp_pb2 import Timestamp +from sentry_protos.billing.v1.date_pb2 import Date +from sentry_protos.billing.v1.services.usage.v1.endpoint_usage_pb2 import ( + CategoryUsage, + DailyUsage, + GetUsageRequest, + GetUsageResponse, +) +from sentry_protos.billing.v1.usage_data_pb2 import UsageData +from snuba_sdk import ( + Column, + Condition, + Entity, + Function, + Granularity, + Limit, + Op, + OrderBy, + Query, + Request, +) +from snuba_sdk.orderby import Direction + +from sentry.billing.platform.services.usage._category_mapping import proto_to_relay_category +from sentry.snuba.referrer import Referrer +from sentry.utils import metrics +from sentry.utils.outcomes import Outcome +from sentry.utils.snuba import raw_snql_query + +logger = logging.getLogger(__name__) + +_REFERRER = Referrer.BILLING_USAGE_SERVICE_CLICKHOUSE.value +_APP_ID = "billing" +_DATASET = "outcomes" +_DAILY_GRANULARITY = 86400 +_QUERY_LIMIT = 10000 + +# Outcomes stored in PG BillingMetricUsage (getsentry outcomes consumer +# filters to these three at ingest). The CH outcomes table also has +# INVALID, ABUSE, CLIENT_DISCARD, and CARDINALITY_LIMITED. +_BILLABLE_OUTCOMES = [Outcome.ACCEPTED, Outcome.FILTERED, Outcome.RATE_LIMITED] + + +def query_outcomes_usage(request: GetUsageRequest) -> GetUsageResponse: + org_id = request.organization_id + start = _timestamp_to_datetime(request.start) + # The proto contract defines `end` as inclusive (midnight of the last + # included day). Snuba queries use a half-open interval [start, end), + # so we add one day to convert inclusive→exclusive. Without this, all + # hourly rows on the last day would be excluded. + end = _timestamp_to_datetime(request.end) + timedelta(days=1) + # Proto categories use different int values from Relay/ClickHouse + # (e.g., proto ATTACHMENT=3 vs Relay ATTACHMENT=4). Convert before querying. + categories = [proto_to_relay_category(c) for c in request.categories] + + snuba_request = _build_query(org_id, start, end, categories, total_outcomes=_BILLABLE_OUTCOMES) + result = raw_snql_query(snuba_request, referrer=_REFERRER) + rows = result["data"] + + if len(rows) >= _QUERY_LIMIT: + logger.warning( + "billing.usage_query.truncated", + extra={"org_id": org_id, "row_count": len(rows)}, + ) + metrics.incr( + "billing.usage_query.truncated", + tags={"org_id": str(org_id)}, + sample_rate=1.0, + ) + + return _build_response(rows) + + +def _build_query( + org_id: int, + start: datetime, + end: datetime, + categories: Sequence[int], + *, + total_outcomes: Sequence[int] | None = None, +) -> Request: + # Half-open interval [start, end) — standard sentry.snuba.outcomes convention. + # `end` has already been shifted +1 day in query_outcomes_usage() to convert + # the proto's inclusive end into the exclusive boundary Snuba expects. + where = [ + Condition(Column("org_id"), Op.EQ, org_id), + Condition(Column("timestamp"), Op.GTE, start), + Condition(Column("timestamp"), Op.LT, end), + ] + if categories: + where.append(Condition(Column("category"), Op.IN, categories)) + + query = Query( + match=Entity("outcomes"), + select=[ + Column("category"), + Column("time"), + _total_function(total_outcomes), + Function( + "sumIf", + [Column("quantity"), Function("equals", [Column("outcome"), Outcome.ACCEPTED])], + "accepted", + ), + Function( + "sumIf", + [ + Column("quantity"), + Function("equals", [Column("outcome"), Outcome.RATE_LIMITED]), + ], + "dropped", + ), + Function( + "sumIf", + [Column("quantity"), Function("equals", [Column("outcome"), Outcome.FILTERED])], + "filtered", + ), + Function("sumIf", [Column("quantity"), _over_quota_condition()], "over_quota"), + Function( + "sumIf", + [ + Column("quantity"), + Function( + "and", + [ + Function("equals", [Column("outcome"), Outcome.RATE_LIMITED]), + Function("equals", [Column("reason"), "smart_rate_limit"]), + ], + ), + ], + "spike_protection", + ), + Function( + "sumIf", + [ + Column("quantity"), + Function( + "and", + [ + Function("equals", [Column("outcome"), Outcome.FILTERED]), + Function("startsWith", [Column("reason"), "Sampled:"]), + ], + ), + ], + "dynamic_sampling", + ), + ], + groupby=[Column("category"), Column("time")], + where=where, + orderby=[OrderBy(Column("time"), Direction.ASC)], + granularity=Granularity(_DAILY_GRANULARITY), + limit=Limit(_QUERY_LIMIT), + ) + return Request( + dataset=_DATASET, + app_id=_APP_ID, + query=query, + tenant_ids={"organization_id": org_id}, + ) + + +def _build_response(rows: list[dict]) -> GetUsageResponse: + # Two-level accumulator: days_map[day_str][category_id] -> usage fields. + # Each row already contains all 7 sumIf-aggregated fields from ClickHouse. + # + # NOTE: CategoryUsage.category carries Relay/Sentry int values (not proto + # DataCategory ints). The proto field is typed as DataCategory but every + # existing consumer (getsentry postgres backend, shadow comparison, + # UsagePricerService, customer_usage, projection, etc.) interprets it as a + # Relay int. Converting to proto ints here would break all consumers and + # the shadow comparison. See the TODO in getsentry's + # usage_pricer/service.py for the planned migration. + days_map: defaultdict[str, dict[int, dict[str, int]]] = defaultdict(dict) + + for row in rows: + day = row["time"] + category = int(row["category"]) + days_map[day][category] = { + "total": int(row["total"]), + "accepted": int(row["accepted"]), + "dropped": int(row["dropped"]), + "filtered": int(row["filtered"]), + "over_quota": int(row["over_quota"]), + "spike_protection": int(row["spike_protection"]), + "dynamic_sampling": int(row["dynamic_sampling"]), + } + + days = [] + for day_str in sorted(days_map): + date = _parse_day(day_str) + usage = [ + CategoryUsage(category=cat, data=UsageData(**fields)) # type: ignore[arg-type] + for cat, fields in sorted(days_map[day_str].items()) + ] + days.append(DailyUsage(date=date, usage=usage)) + + return GetUsageResponse(days=days, seats=[]) + + +def _total_function(outcomes: Sequence[int] | None) -> Function: + """Build the ``total`` aggregate. + + When *outcomes* is provided, only those outcome types are counted + (billing callers pass ``_BILLABLE_OUTCOMES``). When ``None``, every + outcome is counted (useful for general-purpose usage queries). + """ + if outcomes is None: + return Function("sum", [Column("quantity")], "total") + return Function( + "sumIf", + [ + Column("quantity"), + Function( + "in", + [ + Column("outcome"), + Function("tuple", list(outcomes)), + ], + ), + ], + "total", + ) + + +def _over_quota_condition() -> Function: + """ClickHouse condition for over-quota rate limiting. + + Matches: outcome=RATE_LIMITED AND (reason ends with "_usage_exceeded" + OR reason="usage_exceeded" OR reason="grace_period"). + """ + return Function( + "and", + [ + Function("equals", [Column("outcome"), Outcome.RATE_LIMITED]), + Function( + "or", + [ + Function("endsWith", [Column("reason"), "_usage_exceeded"]), + Function( + "or", + [ + Function("equals", [Column("reason"), "usage_exceeded"]), + Function("equals", [Column("reason"), "grace_period"]), + ], + ), + ], + ), + ], + ) + + +def _timestamp_to_datetime(ts: Timestamp) -> datetime: + return ts.ToDatetime(tzinfo=timezone.utc) + + +def _parse_day(value: str) -> Date: + dt = datetime.fromisoformat(value) + return Date(year=dt.year, month=dt.month, day=dt.day) diff --git a/src/sentry/billing/platform/services/usage/service.py b/src/sentry/billing/platform/services/usage/service.py index ba4f158c2127..cf2452deda1d 100644 --- a/src/sentry/billing/platform/services/usage/service.py +++ b/src/sentry/billing/platform/services/usage/service.py @@ -6,6 +6,7 @@ ) from sentry.billing.platform.core import BillingService, service_method +from sentry.billing.platform.services.usage._outcomes_query import query_outcomes_usage class UsageService(BillingService): @@ -18,6 +19,4 @@ def get_usage(self, request: GetUsageRequest) -> GetUsageResponse: accepted, dropped, filtered, over_quota, spike_protection, and dynamic_sampling. """ - # Default implementation returns empty response. - # GetSentry overrides this with Postgres/ClickHouse backends. - return GetUsageResponse() + return query_outcomes_usage(request) diff --git a/src/sentry/snuba/referrer.py b/src/sentry/snuba/referrer.py index 8996e68a9748..06892fc7c490 100644 --- a/src/sentry/snuba/referrer.py +++ b/src/sentry/snuba/referrer.py @@ -675,6 +675,7 @@ class Referrer(StrEnum): INSIGHTS_TIME_SPENT_TOTAL_TIME = "insights.time_spent.total_time" METRIC_EXTRACTION_CARDINALITY_CHECK = "metric_extraction.cardinality_check" + BILLING_USAGE_SERVICE_CLICKHOUSE = "billing.usage_service.clickhouse" OUTCOMES_TIMESERIES = "outcomes.timeseries" OUTCOMES_TOTALS = "outcomes.totals" PREVIEW_GET_EVENTS = "preview.get_events" diff --git a/tests/sentry/billing/platform/services/usage/test_outcomes_query.py b/tests/sentry/billing/platform/services/usage/test_outcomes_query.py new file mode 100644 index 000000000000..697a2128ea57 --- /dev/null +++ b/tests/sentry/billing/platform/services/usage/test_outcomes_query.py @@ -0,0 +1,365 @@ +from __future__ import annotations + +from datetime import datetime, timedelta, timezone +from unittest.mock import patch + +from google.protobuf.timestamp_pb2 import Timestamp +from sentry_protos.billing.v1.date_pb2 import Date +from sentry_protos.billing.v1.services.usage.v1.endpoint_usage_pb2 import ( + GetUsageRequest, + GetUsageResponse, +) +from snuba_sdk import Column, Function, Op + +from sentry.billing.platform.services.usage._outcomes_query import ( + _BILLABLE_OUTCOMES, + _build_query, + _build_response, + _over_quota_condition, + _total_function, + query_outcomes_usage, +) +from sentry.utils.outcomes import Outcome + + +def _make_timestamp(dt: datetime) -> Timestamp: + ts = Timestamp() + ts.FromDatetime(dt) + return ts + + +def _make_row( + *, + time: str = "2025-03-15T00:00:00+00:00", + category: int = 1, + total: int = 0, + accepted: int = 0, + dropped: int = 0, + filtered: int = 0, + over_quota: int = 0, + spike_protection: int = 0, + dynamic_sampling: int = 0, +) -> dict: + return { + "time": time, + "category": category, + "total": total, + "accepted": accepted, + "dropped": dropped, + "filtered": filtered, + "over_quota": over_quota, + "spike_protection": spike_protection, + "dynamic_sampling": dynamic_sampling, + } + + +class TestOverQuotaCondition: + def test_returns_and_function(self): + cond = _over_quota_condition() + assert isinstance(cond, Function) + assert cond.function == "and" + + def test_outer_condition_checks_rate_limited(self): + cond = _over_quota_condition() + outcome_check = cond.parameters[0] + assert outcome_check.function == "equals" + assert isinstance(outcome_check.parameters[0], Column) + assert outcome_check.parameters[0].name == "outcome" + assert outcome_check.parameters[1] == Outcome.RATE_LIMITED + + def test_inner_or_includes_ends_with(self): + cond = _over_quota_condition() + or_clause = cond.parameters[1] + assert or_clause.function == "or" + ends_with = or_clause.parameters[0] + assert ends_with.function == "endsWith" + assert ends_with.parameters[1] == "_usage_exceeded" + + def test_inner_or_includes_usage_exceeded_and_grace_period(self): + cond = _over_quota_condition() + inner_or = cond.parameters[1].parameters[1] + assert inner_or.function == "or" + equals_fns = [inner_or.parameters[0], inner_or.parameters[1]] + values = {fn.parameters[1] for fn in equals_fns} + assert values == {"usage_exceeded", "grace_period"} + + +class TestBuildResponse: + def test_build_response_empty(self): + response = _build_response([]) + + assert isinstance(response, GetUsageResponse) + assert list(response.days) == [] + + def test_build_response_multi_day(self): + rows = [ + _make_row(time="2025-03-17T00:00:00+00:00", accepted=50, total=50), + _make_row(time="2025-03-15T00:00:00+00:00", accepted=100, total=100), + _make_row(time="2025-03-16T00:00:00+00:00", accepted=75, total=75), + ] + response = _build_response(rows) + + assert len(response.days) == 3 + assert response.days[0].date == Date(year=2025, month=3, day=15) + assert response.days[1].date == Date(year=2025, month=3, day=16) + assert response.days[2].date == Date(year=2025, month=3, day=17) + + def test_build_response_multi_category(self): + rows = [ + _make_row(time="2025-03-15T00:00:00+00:00", category=2, accepted=50, total=50), + _make_row(time="2025-03-15T00:00:00+00:00", category=1, accepted=100, total=100), + _make_row(time="2025-03-15T00:00:00+00:00", category=9, accepted=25, total=25), + ] + response = _build_response(rows) + + assert len(response.days) == 1 + day = response.days[0] + assert len(day.usage) == 3 + assert day.usage[0].category == 1 + assert day.usage[1].category == 2 + assert day.usage[2].category == 9 + + assert day.usage[0].data.accepted == 100 + assert day.usage[1].data.accepted == 50 + assert day.usage[2].data.accepted == 25 + + def test_build_response_preserves_overlapping_semantics(self): + """dropped >= over_quota + spike_protection — all values preserved as-is from query.""" + rows = [ + _make_row( + total=175, + accepted=0, + dropped=175, + filtered=0, + over_quota=100, + spike_protection=50, + dynamic_sampling=0, + ) + ] + response = _build_response(rows) + + data = response.days[0].usage[0].data + assert data.dropped == 175 + assert data.over_quota == 100 + assert data.spike_protection == 50 + assert data.dropped >= data.over_quota + data.spike_protection + + def test_build_response_all_fields(self): + rows = [ + _make_row( + time="2025-03-15T00:00:00+00:00", + category=1, + total=100, + accepted=60, + dropped=40, + filtered=0, + over_quota=30, + spike_protection=10, + dynamic_sampling=0, + ), + ] + response = _build_response(rows) + + assert len(response.days) == 1 + data = response.days[0].usage[0].data + assert data.total == 100 + assert data.accepted == 60 + assert data.dropped == 40 + assert data.over_quota == 30 + assert data.spike_protection == 10 + assert data.filtered == 0 + assert data.dynamic_sampling == 0 + + +class TestBuildQuery: + def test_build_query_with_categories(self): + start = datetime(2025, 3, 1, tzinfo=timezone.utc) + end = datetime(2025, 3, 31, tzinfo=timezone.utc) + + snuba_request = _build_query(org_id=1, start=start, end=end, categories=[1, 2]) + + query = snuba_request.query + category_conditions = [ + c for c in query.where if hasattr(c, "lhs") and c.lhs.name == "category" + ] + assert len(category_conditions) == 1 + assert category_conditions[0].op == Op.IN + assert category_conditions[0].rhs == [1, 2] + + def test_build_query_no_categories(self): + start = datetime(2025, 3, 1, tzinfo=timezone.utc) + end = datetime(2025, 3, 31, tzinfo=timezone.utc) + + snuba_request = _build_query(org_id=1, start=start, end=end, categories=[]) + + query = snuba_request.query + category_conditions = [ + c for c in query.where if hasattr(c, "lhs") and c.lhs.name == "category" + ] + assert len(category_conditions) == 0 + + def test_build_query_basic_structure(self): + start = datetime(2025, 3, 1, tzinfo=timezone.utc) + end = datetime(2025, 3, 31, tzinfo=timezone.utc) + + snuba_request = _build_query(org_id=42, start=start, end=end, categories=[]) + + assert snuba_request.dataset == "outcomes" + assert snuba_request.app_id == "billing" + assert snuba_request.tenant_ids == {"organization_id": 42} + + query = snuba_request.query + org_conditions = [c for c in query.where if hasattr(c, "lhs") and c.lhs.name == "org_id"] + assert len(org_conditions) == 1 + assert org_conditions[0].rhs == 42 + + def test_build_query_groups_by_category_and_time_only(self): + start = datetime(2025, 3, 1, tzinfo=timezone.utc) + end = datetime(2025, 3, 31, tzinfo=timezone.utc) + + snuba_request = _build_query(org_id=1, start=start, end=end, categories=[]) + + groupby_names = [col.name for col in snuba_request.query.groupby] + assert groupby_names == ["category", "time"] + + def test_build_query_total_filters_billable_outcomes(self): + start = datetime(2025, 3, 1, tzinfo=timezone.utc) + end = datetime(2025, 3, 31, tzinfo=timezone.utc) + + snuba_request = _build_query( + org_id=1, start=start, end=end, categories=[], total_outcomes=_BILLABLE_OUTCOMES + ) + + select = snuba_request.query.select + total_fn = next(f for f in select if isinstance(f, Function) and f.alias == "total") + # total must use sumIf, not bare sum — only billable outcomes + assert total_fn.function == "sumIf" + # The condition should filter to ACCEPTED, FILTERED, RATE_LIMITED via in(outcome, tuple(...)) + condition = total_fn.parameters[1] + assert condition.function == "in" + outcome_col = condition.parameters[0] + assert isinstance(outcome_col, Column) + assert outcome_col.name == "outcome" + tuple_fn = condition.parameters[1] + assert tuple_fn.function == "tuple" + assert set(tuple_fn.parameters) == { + Outcome.ACCEPTED, + Outcome.FILTERED, + Outcome.RATE_LIMITED, + } + + def test_build_query_total_all_outcomes_when_none(self): + start = datetime(2025, 3, 1, tzinfo=timezone.utc) + end = datetime(2025, 3, 31, tzinfo=timezone.utc) + + snuba_request = _build_query( + org_id=1, start=start, end=end, categories=[], total_outcomes=None + ) + + select = snuba_request.query.select + total_fn = next(f for f in select if isinstance(f, Function) and f.alias == "total") + # No outcome filter — bare sum(quantity) for all outcomes + assert total_fn.function == "sum" + + def test_total_function_with_outcomes(self): + fn = _total_function(_BILLABLE_OUTCOMES) + assert fn.function == "sumIf" + assert fn.alias == "total" + + def test_total_function_without_outcomes(self): + fn = _total_function(None) + assert fn.function == "sum" + assert fn.alias == "total" + + def test_build_query_select_has_sumif_columns(self): + start = datetime(2025, 3, 1, tzinfo=timezone.utc) + end = datetime(2025, 3, 31, tzinfo=timezone.utc) + + snuba_request = _build_query(org_id=1, start=start, end=end, categories=[]) + + select = snuba_request.query.select + aliases = [] + for item in select: + if isinstance(item, Column): + aliases.append(item.name) + elif isinstance(item, Function): + aliases.append(item.alias) + assert aliases == [ + "category", + "time", + "total", + "accepted", + "dropped", + "filtered", + "over_quota", + "spike_protection", + "dynamic_sampling", + ] + + +class TestQueryOutcomesUsage: + @patch("sentry.billing.platform.services.usage._outcomes_query.raw_snql_query") + def test_query_uses_billing_referrer(self, mock_query): + mock_query.return_value = {"data": []} + + start = _make_timestamp(datetime(2025, 3, 1, tzinfo=timezone.utc)) + end = _make_timestamp(datetime(2025, 3, 31, tzinfo=timezone.utc)) + request = GetUsageRequest(organization_id=1, start=start, end=end) + + query_outcomes_usage(request) + + mock_query.assert_called_once() + _, kwargs = mock_query.call_args + assert kwargs["referrer"] == "billing.usage_service.clickhouse" + + @patch("sentry.billing.platform.services.usage._outcomes_query.raw_snql_query") + def test_end_date_shifted_plus_one_day(self, mock_query): + """Proto end is inclusive (last included day). The query shifts +1 day for half-open.""" + mock_query.return_value = {"data": []} + + start = _make_timestamp(datetime(2025, 3, 1, tzinfo=timezone.utc)) + end = _make_timestamp(datetime(2025, 3, 31, tzinfo=timezone.utc)) + request = GetUsageRequest(organization_id=1, start=start, end=end) + + query_outcomes_usage(request) + + snuba_request = mock_query.call_args[0][0] + timestamp_conditions = { + c.op: c.rhs + for c in snuba_request.query.where + if hasattr(c, "lhs") and c.lhs.name == "timestamp" + } + assert timestamp_conditions[Op.GTE] == datetime(2025, 3, 1, tzinfo=timezone.utc) + # end=March 31 (inclusive) → query uses < April 1 (exclusive) + assert timestamp_conditions[Op.LT] == datetime( + 2025, 3, 31, tzinfo=timezone.utc + ) + timedelta(days=1) + + @patch("sentry.billing.platform.services.usage._outcomes_query.raw_snql_query") + def test_query_returns_response(self, mock_query): + mock_query.return_value = { + "data": [ + { + "time": "2025-03-15T00:00:00+00:00", + "category": 1, + "total": 200, + "accepted": 200, + "dropped": 0, + "filtered": 0, + "over_quota": 0, + "spike_protection": 0, + "dynamic_sampling": 0, + } + ] + } + + start = _make_timestamp(datetime(2025, 3, 1, tzinfo=timezone.utc)) + end = _make_timestamp(datetime(2025, 3, 31, tzinfo=timezone.utc)) + request = GetUsageRequest(organization_id=1, start=start, end=end) + + response = query_outcomes_usage(request) + + assert isinstance(response, GetUsageResponse) + assert len(response.days) == 1 + assert response.days[0].date == Date(year=2025, month=3, day=15) + assert response.days[0].usage[0].data.accepted == 200 diff --git a/tests/snuba/billing/__init__.py b/tests/snuba/billing/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/tests/snuba/billing/platform/__init__.py b/tests/snuba/billing/platform/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/tests/snuba/billing/platform/services/__init__.py b/tests/snuba/billing/platform/services/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/tests/snuba/billing/platform/services/usage/__init__.py b/tests/snuba/billing/platform/services/usage/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/tests/snuba/billing/platform/services/usage/test_outcomes_integration.py b/tests/snuba/billing/platform/services/usage/test_outcomes_integration.py new file mode 100644 index 000000000000..21f258f3d864 --- /dev/null +++ b/tests/snuba/billing/platform/services/usage/test_outcomes_integration.py @@ -0,0 +1,454 @@ +from __future__ import annotations + +from collections.abc import Sequence +from datetime import UTC, datetime, timedelta + +import pytest +from google.protobuf.timestamp_pb2 import Timestamp +from sentry_protos.billing.v1.data_category_pb2 import DataCategory as ProtoDataCategory +from sentry_protos.billing.v1.services.usage.v1.endpoint_usage_pb2 import GetUsageRequest + +from sentry.billing.platform.services.usage._outcomes_query import query_outcomes_usage +from sentry.billing.platform.services.usage.service import UsageService +from sentry.constants import DataCategory +from sentry.testutils.cases import OutcomesSnubaTest, TestCase +from sentry.utils.outcomes import Outcome + +_now = datetime.now(UTC).replace(hour=12, minute=27, second=28, microsecond=0) + + +def _make_timestamp(dt: datetime) -> Timestamp: + ts = Timestamp() + ts.FromDatetime(dt) + return ts + + +def _make_request( + org_id: int, + start: datetime, + end: datetime, + categories: Sequence[int] | None = None, +) -> GetUsageRequest: + return GetUsageRequest( + organization_id=org_id, + start=_make_timestamp(start), + end=_make_timestamp(end), + categories=categories or [], # type: ignore[arg-type] + ) + + +@pytest.mark.snuba +class TestOutcomesIntegration(OutcomesSnubaTest, TestCase): + def test_accepted_errors(self): + self.store_outcomes( + { + "org_id": self.organization.id, + "project_id": self.project.id, + "timestamp": _now - timedelta(hours=1), + "outcome": Outcome.ACCEPTED, + "reason": "none", + "category": DataCategory.ERROR, + "quantity": 10, + }, + num_times=3, + ) + + request = _make_request( + org_id=self.organization.id, + start=_now - timedelta(days=1), + end=_now + timedelta(days=1), + ) + response = query_outcomes_usage(request) + + assert len(response.days) == 1 + day = response.days[0] + assert len(day.usage) == 1 + + usage = day.usage[0] + assert usage.category == DataCategory.ERROR + assert usage.data.accepted == 30 + assert usage.data.total == 30 + assert usage.data.dropped == 0 + assert usage.data.filtered == 0 + assert usage.data.over_quota == 0 + assert usage.data.spike_protection == 0 + assert usage.data.dynamic_sampling == 0 + + def test_rate_limited_over_quota(self): + self.store_outcomes( + { + "org_id": self.organization.id, + "project_id": self.project.id, + "timestamp": _now - timedelta(hours=1), + "outcome": Outcome.RATE_LIMITED, + "reason": "usage_exceeded", + "category": DataCategory.ERROR, + "quantity": 50, + }, + ) + + request = _make_request( + org_id=self.organization.id, + start=_now - timedelta(days=1), + end=_now + timedelta(days=1), + ) + response = query_outcomes_usage(request) + + assert len(response.days) == 1 + usage = response.days[0].usage[0] + assert usage.category == DataCategory.ERROR + assert usage.data.total == 50 + assert usage.data.accepted == 0 + assert usage.data.dropped == 50 + assert usage.data.over_quota == 50 + assert usage.data.spike_protection == 0 + + def test_rate_limited_spike_protection(self): + self.store_outcomes( + { + "org_id": self.organization.id, + "project_id": self.project.id, + "timestamp": _now - timedelta(hours=1), + "outcome": Outcome.RATE_LIMITED, + "reason": "smart_rate_limit", + "category": DataCategory.TRANSACTION, + "quantity": 25, + }, + ) + + request = _make_request( + org_id=self.organization.id, + start=_now - timedelta(days=1), + end=_now + timedelta(days=1), + ) + response = query_outcomes_usage(request) + + assert len(response.days) == 1 + usage = response.days[0].usage[0] + assert usage.category == DataCategory.TRANSACTION + assert usage.data.total == 25 + assert usage.data.dropped == 25 + assert usage.data.spike_protection == 25 + assert usage.data.over_quota == 0 + + def test_filtered_dynamic_sampling(self): + self.store_outcomes( + { + "org_id": self.organization.id, + "project_id": self.project.id, + "timestamp": _now - timedelta(hours=1), + "outcome": Outcome.FILTERED, + "reason": "Sampled:80", + "category": DataCategory.TRANSACTION, + "quantity": 100, + }, + ) + + request = _make_request( + org_id=self.organization.id, + start=_now - timedelta(days=1), + end=_now + timedelta(days=1), + ) + response = query_outcomes_usage(request) + + assert len(response.days) == 1 + usage = response.days[0].usage[0] + assert usage.category == DataCategory.TRANSACTION + assert usage.data.total == 100 + assert usage.data.filtered == 100 + assert usage.data.dynamic_sampling == 100 + assert usage.data.accepted == 0 + assert usage.data.dropped == 0 + + def test_multiple_days_and_categories(self): + today = _now - timedelta(hours=1) + yesterday = _now - timedelta(days=1, hours=1) + + # Errors today + self.store_outcomes( + { + "org_id": self.organization.id, + "project_id": self.project.id, + "timestamp": today, + "outcome": Outcome.ACCEPTED, + "reason": "none", + "category": DataCategory.ERROR, + "quantity": 10, + }, + ) + # Transactions today + self.store_outcomes( + { + "org_id": self.organization.id, + "project_id": self.project.id, + "timestamp": today, + "outcome": Outcome.ACCEPTED, + "reason": "none", + "category": DataCategory.TRANSACTION, + "quantity": 20, + }, + ) + # Errors yesterday + self.store_outcomes( + { + "org_id": self.organization.id, + "project_id": self.project.id, + "timestamp": yesterday, + "outcome": Outcome.ACCEPTED, + "reason": "none", + "category": DataCategory.ERROR, + "quantity": 5, + }, + ) + + request = _make_request( + org_id=self.organization.id, + start=_now - timedelta(days=2), + end=_now + timedelta(days=1), + ) + response = query_outcomes_usage(request) + + assert len(response.days) == 2 + + # Days should be sorted ascending + day0_date = response.days[0].date + day1_date = response.days[1].date + assert (day0_date.year, day0_date.month, day0_date.day) < ( + day1_date.year, + day1_date.month, + day1_date.day, + ) + + # Yesterday: only errors + yesterday_usage = {int(u.category): u.data for u in response.days[0].usage} + assert DataCategory.ERROR in yesterday_usage + assert yesterday_usage[DataCategory.ERROR].accepted == 5 + + # Today: errors and transactions + today_usage = {int(u.category): u.data for u in response.days[1].usage} + assert DataCategory.ERROR in today_usage + assert DataCategory.TRANSACTION in today_usage + assert today_usage[DataCategory.ERROR].accepted == 10 + assert today_usage[DataCategory.TRANSACTION].accepted == 20 + + def test_empty_org(self): + other_org = self.create_organization() + + request = _make_request( + org_id=other_org.id, + start=_now - timedelta(days=1), + end=_now + timedelta(days=1), + ) + response = query_outcomes_usage(request) + + assert len(response.days) == 0 + + def test_end_date_inclusive(self): + """Data on the last included day (end date) must be returned. + + The proto end field is inclusive — midnight of the last day to include. + Without the +1 day conversion in the CH backend, this day would be + excluded because Snuba uses half-open intervals. + """ + # Store data at 6am on _now's date + data_time = _now.replace(hour=6, minute=0, second=0, microsecond=0) + self.store_outcomes( + { + "org_id": self.organization.id, + "project_id": self.project.id, + "timestamp": data_time, + "outcome": Outcome.ACCEPTED, + "reason": "none", + "category": DataCategory.ERROR, + "quantity": 77, + }, + ) + + # Pass end = midnight of the SAME day as the data. + # This is how getsentry callers construct the request (inclusive end). + day_midnight = data_time.replace(hour=0, minute=0, second=0, microsecond=0) + request = _make_request( + org_id=self.organization.id, + start=day_midnight - timedelta(days=1), + end=day_midnight, # inclusive: midnight of the data day + ) + response = query_outcomes_usage(request) + + # The data on this day MUST be included + assert len(response.days) == 1 + assert response.days[0].usage[0].data.accepted == 77 + + def test_category_filter(self): + self.store_outcomes( + { + "org_id": self.organization.id, + "project_id": self.project.id, + "timestamp": _now - timedelta(hours=1), + "outcome": Outcome.ACCEPTED, + "reason": "none", + "category": DataCategory.ERROR, + "quantity": 10, + }, + ) + self.store_outcomes( + { + "org_id": self.organization.id, + "project_id": self.project.id, + "timestamp": _now - timedelta(hours=1), + "outcome": Outcome.ACCEPTED, + "reason": "none", + "category": DataCategory.TRANSACTION, + "quantity": 20, + }, + ) + + # Filter to only errors + request = _make_request( + org_id=self.organization.id, + start=_now - timedelta(days=1), + end=_now + timedelta(days=1), + categories=[DataCategory.ERROR], + ) + response = query_outcomes_usage(request) + + assert len(response.days) == 1 + assert len(response.days[0].usage) == 1 + assert response.days[0].usage[0].category == DataCategory.ERROR + assert response.days[0].usage[0].data.accepted == 10 + + def test_category_filter_proto_to_relay_conversion(self): + """Proto ATTACHMENT=3 must map to Relay ATTACHMENT=4 when filtering CH. + + Proto and Relay use different int values for some categories. + The request carries proto ints, but CH stores Relay ints. + """ + self.store_outcomes( + { + "org_id": self.organization.id, + "project_id": self.project.id, + "timestamp": _now - timedelta(hours=1), + "outcome": Outcome.ACCEPTED, + "reason": "none", + "category": DataCategory.ATTACHMENT, # Relay int = 4 + "quantity": 512, + }, + ) + self.store_outcomes( + { + "org_id": self.organization.id, + "project_id": self.project.id, + "timestamp": _now - timedelta(hours=1), + "outcome": Outcome.ACCEPTED, + "reason": "none", + "category": DataCategory.ERROR, # Relay int = 1 + "quantity": 10, + }, + ) + + # Filter using proto ATTACHMENT enum value (3), not Relay (4) + request = _make_request( + org_id=self.organization.id, + start=_now - timedelta(days=1), + end=_now + timedelta(days=1), + categories=[ProtoDataCategory.DATA_CATEGORY_ATTACHMENT], + ) + response = query_outcomes_usage(request) + + # Should find the ATTACHMENT data (Relay category 4), not errors + assert len(response.days) == 1 + assert len(response.days[0].usage) == 1 + assert response.days[0].usage[0].category == DataCategory.ATTACHMENT + assert response.days[0].usage[0].data.accepted == 512 + + def test_overlapping_semantics(self): + """Verify dropped >= over_quota + spike_protection with real data.""" + # over_quota + self.store_outcomes( + { + "org_id": self.organization.id, + "project_id": self.project.id, + "timestamp": _now - timedelta(hours=1), + "outcome": Outcome.RATE_LIMITED, + "reason": "usage_exceeded", + "category": DataCategory.ERROR, + "quantity": 30, + }, + ) + # spike_protection + self.store_outcomes( + { + "org_id": self.organization.id, + "project_id": self.project.id, + "timestamp": _now - timedelta(hours=1), + "outcome": Outcome.RATE_LIMITED, + "reason": "smart_rate_limit", + "category": DataCategory.ERROR, + "quantity": 20, + }, + ) + # other rate-limited reason (neither over_quota nor spike) + self.store_outcomes( + { + "org_id": self.organization.id, + "project_id": self.project.id, + "timestamp": _now - timedelta(hours=1), + "outcome": Outcome.RATE_LIMITED, + "reason": "some_other_reason", + "category": DataCategory.ERROR, + "quantity": 10, + }, + ) + + request = _make_request( + org_id=self.organization.id, + start=_now - timedelta(days=1), + end=_now + timedelta(days=1), + ) + response = query_outcomes_usage(request) + + assert len(response.days) == 1 + data = response.days[0].usage[0].data + + assert data.dropped == 60 # 30 + 20 + 10 + assert data.over_quota == 30 + assert data.spike_protection == 20 + assert data.dropped >= data.over_quota + data.spike_protection + + def test_full_usage_service_e2e(self): + """End-to-end test via UsageService().get_usage().""" + self.store_outcomes( + { + "org_id": self.organization.id, + "project_id": self.project.id, + "timestamp": _now - timedelta(hours=1), + "outcome": Outcome.ACCEPTED, + "reason": "none", + "category": DataCategory.ERROR, + "quantity": 42, + }, + ) + self.store_outcomes( + { + "org_id": self.organization.id, + "project_id": self.project.id, + "timestamp": _now - timedelta(hours=1), + "outcome": Outcome.RATE_LIMITED, + "reason": "usage_exceeded", + "category": DataCategory.ERROR, + "quantity": 8, + }, + ) + + request = _make_request( + org_id=self.organization.id, + start=_now - timedelta(days=1), + end=_now + timedelta(days=1), + ) + response = UsageService().get_usage(request) + + assert len(response.days) == 1 + data = response.days[0].usage[0].data + assert data.total == 50 + assert data.accepted == 42 + assert data.dropped == 8 + assert data.over_quota == 8 From 6da1d9d9d33948fb7f945f0e68c37f0cb77e705d Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 1 Apr 2026 10:54:41 -0700 Subject: [PATCH 30/57] feat(grouping): Add replacement callback option to parameterizer (#111352) This adds the ability to specify a replacement callback when defining a regex for the parameterizer. For example, a regex can now be defined like this: ParameterizationRegex( name="dog", raw_pattern=r"some pattern", replacement_callback=lambda orig_value: "" if some_condition else orig_value, ) When the parameterizer finds a matching value, instead of just reflexively replacing it with the regex's name, it'll pass the original value to the callback, and let it decide what the replacement value will be. (This is going to be necessary for us to fix our IPv6 parameterization.) --- src/sentry/grouping/parameterization.py | 31 ++++++++++++++++--- .../sentry/grouping/test_parameterization.py | 21 +++++++++++++ 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/src/sentry/grouping/parameterization.py b/src/sentry/grouping/parameterization.py index 603d6b6ae177..1c1b1b7d84aa 100644 --- a/src/sentry/grouping/parameterization.py +++ b/src/sentry/grouping/parameterization.py @@ -2,9 +2,15 @@ import re from collections import defaultdict from collections.abc import Sequence +from typing import Callable from sentry.utils import metrics +# Function parameterization regexes can specify to provide a customized replacement string. Can also +# be used to do conditional replacement, by returning the original value in cases where replacement +# shouldn't happen. +ParameterizationReplacementFunction = Callable[[str], str] + @dataclasses.dataclass class ParameterizationRegex: @@ -13,6 +19,8 @@ class ParameterizationRegex: raw_pattern_experimental: str | None = None lookbehind: str | None = None # positive lookbehind prefix if needed lookahead: str | None = None # positive lookahead postfix if needed + # Function which takes the matched value and returns the replacement value. + replacement_callback: ParameterizationReplacementFunction | None = None # These need to be used with `(?x)`, to tell the regex compiler to ignore comments # and unescaped whitespace, so we can use newlines and indentation for better legibility. @@ -300,6 +308,7 @@ def __init__( # List of `ParameterizationRegex` objects defining the regexes to use. If nothing is passed, # the default set will be used. regexes: Sequence[ParameterizationRegex] = DEFAULT_PARAMETERIZATION_REGEXES, + *, # List of `ParameterizationRegex.name` values, used to selectively enable pattern types. To # use all available parameterization, omit this argument. regex_keys: Sequence[str] | None = None, @@ -334,6 +343,11 @@ def __init__( rf"(?x){'|'.join(pattern_strings)}" ) + # Collect replacement callbacks, if any + self.replacement_functions = { + r.name: r.replacement_callback for r in regexes if r.replacement_callback + } + def parameterize(self, input_str: str) -> str: """ Replace all regex matches in the input string with placeholder strings, using the regexes @@ -342,7 +356,7 @@ def parameterize(self, input_str: str) -> str: For example, turn "Error with order #1231" into "Error with order #". """ - matches_counter: defaultdict[str, int] = defaultdict(int) + replacement_counts: defaultdict[str, int] = defaultdict(int) def _handle_regex_match(match: re.Match[str]) -> str: # Since @@ -359,8 +373,17 @@ def _handle_regex_match(match: re.Match[str]) -> str: if not matched_key or not orig_value: # Insurance - shouldn't happen IRL return "" - matches_counter[matched_key] += 1 - return f"<{matched_key}>" + replacement_callback = self.replacement_functions.get(matched_key) + replacement_string = ( + replacement_callback(orig_value) if replacement_callback else f"<{matched_key}>" + ) + + # The replacement callback might return the original value, if it determines it's not + # something which should be replaced, and we don't want to count that + if replacement_string != orig_value: + replacement_counts[matched_key] += 1 + + return replacement_string with metrics.timer( "grouping.parameterize", tags={"experimental": self.is_experimental} @@ -368,7 +391,7 @@ def _handle_regex_match(match: re.Match[str]) -> str: parameterized = self.combined_regex.sub(_handle_regex_match, input_str) metric_tags["changed"] = parameterized != input_str - for regex_key, count in matches_counter.items(): + for regex_key, count in replacement_counts.items(): # Track the kinds of replacements being made metrics.incr("grouping.value_parameterized", amount=count, tags={"key": regex_key}) diff --git a/tests/sentry/grouping/test_parameterization.py b/tests/sentry/grouping/test_parameterization.py index 09eed1e08c05..9ca27f087475 100644 --- a/tests/sentry/grouping/test_parameterization.py +++ b/tests/sentry/grouping/test_parameterization.py @@ -11,6 +11,8 @@ ) from sentry.grouping.context import GroupingContext from sentry.grouping.parameterization import ( + ParameterizationRegex, + Parameterizer, experimental_parameterizer, parameterizer, ) @@ -590,3 +592,22 @@ def test_runs_parameterizer_on_fingerprint_constant_matching_chained_error_messa "That's ball number that Charlie hasn't brought back!", "Dogs are great!", ] + + +def test_uses_callback_for_replacement_value() -> None: + input_str = "Dog number 1, #1 dog" + callback_parameterizer = Parameterizer( + [ + ParameterizationRegex( + name="int", + raw_pattern=r"""-\d+\b | \b\d+\b""", + replacement_callback=lambda _: "", + ) + ] + ) + + assert parameterizer.parameterize(input_str) == "Dog number , # dog" + assert ( + callback_parameterizer.parameterize(input_str) + == "Dog number , # dog" # Callback function was used + ) From 3bf579fc2d47711a9d7f42a8089b2b0b7aaf4895 Mon Sep 17 00:00:00 2001 From: Scott Cooper Date: Wed, 1 Apr 2026 10:56:26 -0700 Subject: [PATCH 31/57] feat(supergroups): Smarter group loading and match highlighting (#111927) Replace GroupList in SupergroupDetailDrawer with a custom paginated list that shows already-loaded groups from GroupStore immediately while fetching remaining groups from the API. Matched groups (from the current search) are sorted to the top and highlighted with an accent border. --------- Co-authored-by: Claude Opus 4.6 --- .../app/components/stream/supergroupRow.tsx | 27 ++- static/app/views/issueList/groupListBody.tsx | 3 +- .../app/views/issueList/pages/supergroups.tsx | 11 +- .../supergroups/supergroupDrawer.tsx | 227 ++++++++++++++++-- 4 files changed, 239 insertions(+), 29 deletions(-) diff --git a/static/app/components/stream/supergroupRow.tsx b/static/app/components/stream/supergroupRow.tsx index 6f07d00c579e..dff3ee828bbb 100644 --- a/static/app/components/stream/supergroupRow.tsx +++ b/static/app/components/stream/supergroupRow.tsx @@ -4,6 +4,7 @@ import styled from '@emotion/styled'; import InteractionStateLayer from '@sentry/scraps/interactionStateLayer'; import {Text} from '@sentry/scraps/text'; +import type {IndexedMembersByProject} from 'sentry/actionCreators/members'; import {GroupStatusChart} from 'sentry/components/charts/groupStatusChart'; import {Count} from 'sentry/components/count'; import {useDrawer} from 'sentry/components/globalDrawer'; @@ -18,25 +19,37 @@ import {SupergroupDetailDrawer} from 'sentry/views/issueList/supergroups/supergr import type {SupergroupDetail} from 'sentry/views/issueList/supergroups/types'; interface SupergroupRowProps { - matchedCount: number; + matchedGroupIds: string[]; supergroup: SupergroupDetail; aggregatedStats?: AggregatedSupergroupStats | null; + memberList?: IndexedMembersByProject; } export function SupergroupRow({ supergroup, - matchedCount, + matchedGroupIds, aggregatedStats, + memberList, }: SupergroupRowProps) { + const matchedCount = matchedGroupIds.length; const {openDrawer, isDrawerOpen} = useDrawer(); const [isActive, setIsActive] = useState(false); const handleClick = () => { setIsActive(true); - openDrawer(() => , { - ariaLabel: t('Supergroup details'), - drawerKey: 'supergroup-drawer', - onClose: () => setIsActive(false), - }); + openDrawer( + () => ( + + ), + { + ariaLabel: t('Supergroup details'), + drawerKey: 'supergroup-drawer', + onClose: () => setIsActive(false), + } + ); }; const highlighted = isActive && isDrawerOpen; diff --git a/static/app/views/issueList/groupListBody.tsx b/static/app/views/issueList/groupListBody.tsx index 8690b712f67d..1154ad5502b0 100644 --- a/static/app/views/issueList/groupListBody.tsx +++ b/static/app/views/issueList/groupListBody.tsx @@ -237,8 +237,9 @@ function GroupList({ ); })} diff --git a/static/app/views/issueList/pages/supergroups.tsx b/static/app/views/issueList/pages/supergroups.tsx index 03868ea3009a..cbdd890146f4 100644 --- a/static/app/views/issueList/pages/supergroups.tsx +++ b/static/app/views/issueList/pages/supergroups.tsx @@ -109,10 +109,13 @@ function Supergroups() { const supergroups = (response?.data ?? []).filter(sg => sg.group_ids.length > 1); const handleSupergroupClick = (supergroup: SupergroupDetail) => { - openDrawer(() => , { - ariaLabel: t('Supergroup details'), - drawerKey: 'supergroup-drawer', - }); + openDrawer( + () => , + { + ariaLabel: t('Supergroup details'), + drawerKey: 'supergroup-drawer', + } + ); }; if (!hasTopIssuesUI) { diff --git a/static/app/views/issueList/supergroups/supergroupDrawer.tsx b/static/app/views/issueList/supergroups/supergroupDrawer.tsx index 66f378c2bbe6..6db963cbdd62 100644 --- a/static/app/views/issueList/supergroups/supergroupDrawer.tsx +++ b/static/app/views/issueList/supergroups/supergroupDrawer.tsx @@ -1,28 +1,61 @@ -import {Fragment} from 'react'; +import {Fragment, useMemo, useState} from 'react'; +import {css} from '@emotion/react'; import styled from '@emotion/styled'; import {Badge} from '@sentry/scraps/badge'; +import {Button} from '@sentry/scraps/button'; import {Container, Flex, Stack} from '@sentry/scraps/layout'; import {Heading, Text} from '@sentry/scraps/text'; +import type {IndexedMembersByProject} from 'sentry/actionCreators/members'; import { CrumbContainer, NavigationCrumbs, ShortId, } from 'sentry/components/events/eventDrawer'; import {DrawerBody, DrawerHeader} from 'sentry/components/globalDrawer/components'; -import {GroupList} from 'sentry/components/issues/groupList'; +import type {GroupListColumn} from 'sentry/components/issues/groupList'; +import {GroupListHeader} from 'sentry/components/issues/groupListHeader'; import {ALL_ACCESS_PROJECTS} from 'sentry/components/pageFilters/constants'; -import {IconFocus} from 'sentry/icons'; +import {Panel} from 'sentry/components/panels/panel'; +import {PanelBody} from 'sentry/components/panels/panelBody'; +import {Placeholder} from 'sentry/components/placeholder'; +import { + DEFAULT_STREAM_GROUP_STATS_PERIOD, + StreamGroup, +} from 'sentry/components/stream/group'; +import {IconChevron, IconFocus} from 'sentry/icons'; import {t} from 'sentry/locale'; +import {GroupStore} from 'sentry/stores/groupStore'; +import type {Group} from 'sentry/types/group'; +import {getApiUrl} from 'sentry/utils/api/getApiUrl'; +import {useApiQuery} from 'sentry/utils/queryClient'; +import {useOrganization} from 'sentry/utils/useOrganization'; import {StyledMarkedText} from 'sentry/views/issueList/pages/supergroups'; import {SupergroupFeedback} from 'sentry/views/issueList/supergroups/supergroupFeedback'; import type {SupergroupDetail} from 'sentry/views/issueList/supergroups/types'; -export function SupergroupDetailDrawer({supergroup}: {supergroup: SupergroupDetail}) { - const placeholderRows = Math.min(supergroup.group_ids.length, 10); - const issueIdQuery = `issue.id:[${supergroup.group_ids.join(',')}]`; +const PAGE_SIZE = 20; + +const DRAWER_COLUMNS: GroupListColumn[] = [ + 'event', + 'users', + 'assignee', + 'firstSeen', + 'lastSeen', +]; +interface SupergroupDetailDrawerProps { + matchedGroupIds: string[]; + supergroup: SupergroupDetail; + memberList?: IndexedMembersByProject; +} + +export function SupergroupDetailDrawer({ + supergroup, + matchedGroupIds, + memberList, +}: SupergroupDetailDrawerProps) { return ( @@ -91,17 +124,10 @@ export function SupergroupDetailDrawer({supergroup}: {supergroup: SupergroupDeta {supergroup.group_ids.length > 0 && ( - )} @@ -110,6 +136,173 @@ export function SupergroupDetailDrawer({supergroup}: {supergroup: SupergroupDeta ); } +function SupergroupIssueList({ + groupIds, + matchedGroupIds, + memberList, +}: { + groupIds: number[]; + matchedGroupIds: string[]; + memberList?: IndexedMembersByProject; +}) { + const organization = useOrganization(); + const [page, setPage] = useState(0); + + // Sort: matched first, then other loaded groups, then unloaded + const {sortedGroupIds, loadedIds} = useMemo(() => { + const matched: number[] = []; + const loaded: number[] = []; + const cachedIds = new Set(); + const unloaded: number[] = []; + + for (const id of groupIds) { + const strId = String(id); + if (GroupStore.get(strId)) { + cachedIds.add(strId); + if (matchedGroupIds.includes(strId)) { + matched.push(id); + } else { + loaded.push(id); + } + } else { + unloaded.push(id); + } + } + + return {sortedGroupIds: [...matched, ...loaded, ...unloaded], loadedIds: cachedIds}; + }, [groupIds, matchedGroupIds]); + + const totalPages = Math.ceil(sortedGroupIds.length / PAGE_SIZE); + const pageGroupIds = sortedGroupIds.slice(page * PAGE_SIZE, (page + 1) * PAGE_SIZE); + const pageUnloadedIds = pageGroupIds.filter(id => !loadedIds.has(String(id))); + + const {data: fetchedGroups, isPending} = useApiQuery( + [ + getApiUrl('/organizations/$organizationIdOrSlug/issues/', { + path: {organizationIdOrSlug: organization.slug}, + }), + { + query: { + group: pageUnloadedIds.map(String), + project: ALL_ACCESS_PROJECTS, + }, + }, + ], + { + staleTime: 30_000, + enabled: pageUnloadedIds.length > 0, + } + ); + + return ( + + {matchedGroupIds.length > 0 && ( + + + + {t('Visible in current results')} + + + )} + + + + {pageGroupIds.map(id => { + const strId = String(id); + const group = + (GroupStore.get(strId) as Group | undefined) ?? + fetchedGroups?.find(g => g.id === strId); + + if (group) { + const members = memberList?.[group.project?.slug] + ? memberList[group.project.slug] + : undefined; + return ( + + + + ); + } + + if (isPending) { + return ( + + + + ); + } + + return null; + })} + + + {totalPages > 1 && ( + + + {`${page * PAGE_SIZE + 1}-${Math.min((page + 1) * PAGE_SIZE, sortedGroupIds.length)} of ${sortedGroupIds.length}`} + + +