From 7592d6ba95c7a11029b0b3b7a5f99716c3353ae9 Mon Sep 17 00:00:00 2001 From: Matt Duncan <14761+mrduncan@users.noreply.github.com> Date: Fri, 29 May 2026 15:03:43 -0700 Subject: [PATCH 01/11] ref(snuba): Use metrics.timer instead of bespoke timer helper (#115279) Replace the local timer context manager in `sentry.utils.snuba` with `metrics.timer` from `sentry.utils.metrics` so timing in this file matches the rest of the codebase. Metric names are preserved as `snuba.client.`; each timing now also carries the standard `result=success|failure` tag emitted by `metrics.timer`. --- src/sentry/utils/snuba.py | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/src/sentry/utils/snuba.py b/src/sentry/utils/snuba.py index da452f68597e..14873df8c087 100644 --- a/src/sentry/utils/snuba.py +++ b/src/sentry/utils/snuba.py @@ -6,7 +6,6 @@ import math import os import re -import time from collections import namedtuple from collections.abc import Callable, Collection, Mapping, MutableMapping, Sequence from contextlib import contextmanager @@ -481,15 +480,6 @@ class QueryOutsideGroupActivityError(Exception): SnubaTSResult = namedtuple("SnubaTSResult", ("data", "start", "end", "rollup")) -@contextmanager -def timer(name, prefix="snuba.client"): - t = time.time() - try: - yield - finally: - metrics.timing(f"{prefix}.{name}", time.time() - t) - - @contextmanager def options_override(overrides): """\ @@ -716,7 +706,7 @@ def get_query_params_to_update_for_projects( project_ids = list(set(query_params.filter_keys["project_id"])) elif query_params.filter_keys: # Otherwise infer the project_ids from any related models - with timer("get_related_project_ids"): + with metrics.timer("snuba.client.get_related_project_ids"): project_ids = infer_project_ids_from_related_models(query_params.filter_keys) elif query_params.conditions: project_ids = [] @@ -1469,7 +1459,7 @@ def _raw_delete_query( ) # Enter hub such that http spans are properly nested - with timer("delete_query"): + with metrics.timer("snuba.client.delete_query"): referrer = headers.get("referer", "unknown") with sentry_sdk.start_span(op="snuba_delete.validation", name=referrer) as span: span.set_tag("snuba.referrer", referrer) @@ -1484,7 +1474,7 @@ def _raw_delete_query( def _raw_mql_query(request: Request, headers: Mapping[str, str]) -> urllib3.response.HTTPResponse: # Enter hub such that http spans are properly nested - with timer("mql_query"): + with metrics.timer("snuba.client.mql_query"): referrer = headers.get("referer", "unknown") # TODO: This can be changed back to just `serialize` after we remove SnQL support for MetricsQuery @@ -1502,7 +1492,7 @@ def _raw_mql_query(request: Request, headers: Mapping[str, str]) -> urllib3.resp def _raw_snql_query(request: Request, headers: Mapping[str, str]) -> urllib3.response.HTTPResponse: # Enter hub such that http spans are properly nested - with timer("snql_query"): + with metrics.timer("snuba.client.snql_query"): referrer = headers.get("referer", "") serialized_req = request.serialize() @@ -1570,7 +1560,7 @@ def query( assert expected_cols == got_cols, f"expected {expected_cols}, got {got_cols}" - with timer("process_result"): + with metrics.timer("snuba.client.process_result"): if totals: return ( nest_groups(body["data"], groupby, aggregate_names + selected_names), From 2cde52fe9e43bc35110874244d7981fcac8b3727 Mon Sep 17 00:00:00 2001 From: Charlie Luo Date: Fri, 29 May 2026 18:11:42 -0400 Subject: [PATCH 02/11] ref(trace-waterfall): drop deprecated aliases from trace meta endpoint (#116514) Remove deprecated fields from this endpoint from https://github.com/getsentry/sentry/pull/115107. This makes publishing it a little cleaner: https://github.com/getsentry/sentry/pull/116445. Frontend already only uses the non-deprecated versions. Co-authored-by: Claude --- .../api/endpoints/organization_trace_meta.py | 25 ++---------- .../endpoints/test_organization_trace_meta.py | 40 +++++++++---------- 2 files changed, 23 insertions(+), 42 deletions(-) diff --git a/src/sentry/api/endpoints/organization_trace_meta.py b/src/sentry/api/endpoints/organization_trace_meta.py index e20c0b3e00c8..af711ac22bf5 100644 --- a/src/sentry/api/endpoints/organization_trace_meta.py +++ b/src/sentry/api/endpoints/organization_trace_meta.py @@ -44,15 +44,6 @@ class SerializedResponse(TypedDict, total=False): transactionChildCountMap: SnubaData spansCountMap: dict[str, int] - # These are deprecated - logs: int - errors: int - performance_issues: int - span_count: int - transaction_child_count_map: SnubaData - span_count_map: dict[str, int] - uptime_checks: int # Only present when include_uptime is True - def extract_uptime_count(uptime_result: list[TraceItemTableResponse]) -> int: """Safely extract uptime count from query result.""" @@ -282,31 +273,21 @@ def serialize( "spansCountMap": { row["span.op"]: row["count()"] for row in results["spans_op_count"]["data"] }, - # these are deprecated - "logs": results["logs_meta"]["data"][0].get("count()") or 0, - "errors": errors_count, - "performance_issues": perf_issues, - "span_count": results["spans_meta"]["data"][0].get("count()") or 0, - "transaction_child_count_map": results["transaction_children"]["data"], - "span_count_map": { - row["span.op"]: row["count()"] for row in results["spans_op_count"]["data"] - }, } sentry_sdk.metrics.distribution( "performance.trace.logs.count", - response["logs"], + response["logsCount"], ) sentry_sdk.metrics.distribution( "performance.trace.span.count", - response["span_count"], + response["spansCount"], ) sentry_sdk.metrics.distribution( "performance.trace.errors.count", - response["errors"], + response["errorsCount"], ) if uptime_count is not None: - response["uptime_checks"] = uptime_count response["uptimeCount"] = uptime_count return response diff --git a/tests/snuba/api/endpoints/test_organization_trace_meta.py b/tests/snuba/api/endpoints/test_organization_trace_meta.py index ba0aa6922762..defc0078bcbc 100644 --- a/tests/snuba/api/endpoints/test_organization_trace_meta.py +++ b/tests/snuba/api/endpoints/test_organization_trace_meta.py @@ -69,7 +69,7 @@ def test_bad_ids(self) -> None: assert data["spansCount"] == 0 assert data["spansCountMap"] == {} assert data["metricsCount"] == 0 - assert "uptime_checks" not in data # Should not be present without include_uptime param + assert "uptimeCount" not in data # Should not be present without include_uptime param # Invalid trace id with pytest.raises(NoReverseMatch): @@ -309,7 +309,7 @@ def create_uptime_check(self, trace_id=None, **kwargs): return self.create_eap_uptime_result(**defaults) def test_trace_meta_without_uptime_param(self) -> None: - """Test that uptime_checks field is NOT present when include_uptime is not set""" + """Test that uptimeCount field is NOT present when include_uptime is not set""" self.load_trace() uptime_result = self.create_uptime_check() self.store_eap_items([uptime_result]) @@ -322,13 +322,13 @@ def test_trace_meta_without_uptime_param(self) -> None: assert response.status_code == 200 data = response.data - assert "uptime_checks" not in data - assert data["errors"] == 0 - assert data["performance_issues"] == 2 - assert data["span_count"] == 19 + assert "uptimeCount" not in data + assert data["errorsCount"] == 0 + assert data["performanceIssuesCount"] == 2 + assert data["spansCount"] == 19 def test_trace_meta_with_uptime_param(self) -> None: - """Test that uptime_checks shows correct count when include_uptime=1""" + """Test that uptimeCount shows correct count when include_uptime=1""" self.load_trace() uptime_results = [ @@ -347,14 +347,14 @@ def test_trace_meta_with_uptime_param(self) -> None: assert response.status_code == 200 data = response.data - assert "uptime_checks" in data - assert data["uptime_checks"] == 3 - assert data["errors"] == 0 - assert data["performance_issues"] == 2 - assert data["span_count"] == 19 + assert "uptimeCount" in data + assert data["uptimeCount"] == 3 + assert data["errorsCount"] == 0 + assert data["performanceIssuesCount"] == 2 + assert data["spansCount"] == 19 def test_trace_meta_no_uptime_results(self) -> None: - """Test that uptime_checks is 0 when there are no uptime results""" + """Test that uptimeCount is 0 when there are no uptime results""" self.load_trace() with self.feature(self.FEATURES): @@ -366,11 +366,11 @@ def test_trace_meta_no_uptime_results(self) -> None: assert response.status_code == 200 data = response.data - assert "uptime_checks" in data - assert data["uptime_checks"] == 0 - assert data["errors"] == 0 - assert data["performance_issues"] == 2 - assert data["span_count"] == 19 + assert "uptimeCount" in data + assert data["uptimeCount"] == 0 + assert data["errorsCount"] == 0 + assert data["performanceIssuesCount"] == 2 + assert data["spansCount"] == 19 def test_trace_meta_different_trace_id(self) -> None: """Test that uptime results from different traces are not counted""" @@ -387,5 +387,5 @@ def test_trace_meta_different_trace_id(self) -> None: ) assert response.status_code == 200 data = response.data - assert "uptime_checks" in data - assert data["uptime_checks"] == 0 + assert "uptimeCount" in data + assert data["uptimeCount"] == 0 From 53c5dd6b634850c790d3f71c403f632a9b74621d Mon Sep 17 00:00:00 2001 From: Lyn <1779792+lynnagara@users.noreply.github.com> Date: Fri, 29 May 2026 15:16:39 -0700 Subject: [PATCH 03/11] feat(cells): Implement owner=1 on control silo org listing (#116439) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the `owner=1` → 400 stub in `OrganizationIndexEndpoint._get_from_control` with a real implementation that queries `OrganizationMemberMapping` for the user's owner-role orgs and computes `singleOwner` from an aggregated count of active co-owners. Returns the same `[{organization, singleOwner}]` shape as the cell-side path, so existing frontend callers (account close + security enrolment) can migrate to this endpoint without major frontend changes. Also fixes an N+1 query in the existing cell-side path. The cell version runs `has_single_owner()` and a full feature-flag evaluation (`features.batch_has` + per-feature `features.has` + onboarding/option queries) per org. Control issues the same queries regardless of org count and skips feature evaluation entirely since org features are no longer returned from control. --- .../core/endpoints/organization_index.py | 58 +++++++++++++++++-- .../core/endpoints/test_organization_index.py | 27 ++++++--- 2 files changed, 73 insertions(+), 12 deletions(-) diff --git a/src/sentry/core/endpoints/organization_index.py b/src/sentry/core/endpoints/organization_index.py index 4b93ec21270c..b2b7bafb5ef3 100644 --- a/src/sentry/core/endpoints/organization_index.py +++ b/src/sentry/core/endpoints/organization_index.py @@ -6,10 +6,11 @@ from django.db.models.functions import Coalesce from drf_spectacular.utils import extend_schema from rest_framework import serializers, status +from rest_framework.exceptions import PermissionDenied from rest_framework.request import Request from rest_framework.response import Response -from sentry import features, options +from sentry import features, options, roles from sentry import ratelimits as ratelimiter from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import Endpoint, all_silo_endpoint @@ -253,11 +254,14 @@ def _get_from_control(self, request: Request) -> Response: owner_only = request.GET.get("owner") in ("1", "true") + # owner=1 (used by the account-close flow) means "orgs I own", which is + # defined by the user's membership. A userless token (org auth token or + # DSN) passes the permission check but has no membership to resolve, so + # reject it rather than falling through to the general token-scoped listing. if owner_only: - return Response( - {"detail": "The control-silo organizations endpoint does not support owner=1."}, - status=status.HTTP_400_BAD_REQUEST, - ) + if not request.user.is_authenticated: + raise PermissionDenied + return self._get_owned_from_control(request) queryset = OrganizationMapping.objects.distinct() @@ -359,6 +363,50 @@ def _get_from_control(self, request: Request) -> Response: paginator_cls=paginator_cls, ) + def _get_owned_from_control(self, request: Request) -> Response: + assert request.user.id is not None + owner_role = roles.get_top_dog().id + + owner_org_ids = OrganizationMemberMapping.objects.filter( + user_id=request.user.id, + role=owner_role, + ).values_list("organization_id", flat=True) + + org_mappings = list( + OrganizationMapping.objects.filter( + organization_id__in=owner_org_ids, + status=OrganizationStatus.ACTIVE, + ).order_by("name") + ) + + owner_counts = ( + OrganizationMemberMapping.objects.filter( + organization_id__in=[m.organization_id for m in org_mappings], + role=owner_role, + user_id__isnull=False, + user__is_active=True, + ) + .values("organization_id") + .annotate(count=Count("id")) + ) + single_owner_org_ids = {row["organization_id"] for row in owner_counts if row["count"] == 1} + + serialized = serialize( + org_mappings, + request.user, + serializer=ControlSiloOrganizationMappingSerializer(), + ) + + return Response( + [ + { + "organization": data, + "singleOwner": mapping.organization_id in single_owner_org_ids, + } + for mapping, data in zip(org_mappings, serialized) + ] + ) + def post(self, request: Request) -> Response: """ Create a New Organization diff --git a/tests/sentry/core/endpoints/test_organization_index.py b/tests/sentry/core/endpoints/test_organization_index.py index 95b55deb1419..c95c89f08025 100644 --- a/tests/sentry/core/endpoints/test_organization_index.py +++ b/tests/sentry/core/endpoints/test_organization_index.py @@ -176,15 +176,28 @@ def test_show_only_token_organization(self) -> None: assert len(response.data) == 1 assert response.data[0]["id"] == str(org1.id) - def test_owner_not_supported(self) -> None: - self.create_organization(cell="us", owner=self.user) + def test_ownership_across_cells(self) -> None: + org_a = self.create_organization(cell="us", name="A", owner=self.user) + org_b = self.create_organization(cell="de", name="B", owner=self.user) - response = self.get_error_response(status_code=400, owner="1") + user2 = self.create_user(email="user2@example.com") + org_c = self.create_organization(cell="us", name="C", owner=user2) + self.create_organization(cell="de", name="D", owner=user2) + org_e = self.create_organization(cell="us", name="E", owner=user2) - assert ( - response.data["detail"] - == "The control-silo organizations endpoint does not support owner=1." - ) + self.create_member(user=user2, organization=org_b, role="owner") + self.create_member(user=self.user, organization=org_c, role="owner") + self.create_member(user=self.user, organization=org_e, role="member") + + response = self.get_success_response(qs_params={"owner": 1}) + + assert len(response.data) == 3 + assert response.data[0]["organization"]["id"] == str(org_a.id) + assert response.data[0]["singleOwner"] is True + assert response.data[1]["organization"]["id"] == str(org_b.id) + assert response.data[1]["singleOwner"] is False + assert response.data[2]["organization"]["id"] == str(org_c.id) + assert response.data[2]["singleOwner"] is False def test_sort_by_members(self) -> None: smaller_org = self.create_organization( From bd70f1563f6687084ba7e7d3276d805268db41c1 Mon Sep 17 00:00:00 2001 From: Sentry Bot Date: Fri, 29 May 2026 15:26:16 -0700 Subject: [PATCH 04/11] ref: bump taskbroker-client to 0.17.1 (#116535) Co-Authored-By: untitaker <837573+untitaker@users.noreply.github.com> Co-authored-by: getsentry-bot <10587625+getsentry-bot@users.noreply.github.com> Co-authored-by: untitaker <837573+untitaker@users.noreply.github.com> --- pyproject.toml | 2 +- uv.lock | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index dcd98a19c7b8..cf937986b83c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -111,7 +111,7 @@ dependencies = [ "statsd>=3.3.0", "structlog>=22.1.0", "symbolic>=12.14.1", - "taskbroker-client>=0.17.0", + "taskbroker-client>=0.17.1", "tiktoken>=0.8.0", "tokenizers>=0.22.0", "tldextract>=5.1.2", diff --git a/uv.lock b/uv.lock index 199e6fae844c..86936b919c53 100644 --- a/uv.lock +++ b/uv.lock @@ -2454,7 +2454,7 @@ requires-dist = [ { name = "stripe", specifier = ">=6.7.0" }, { name = "structlog", specifier = ">=22.1.0" }, { name = "symbolic", specifier = ">=12.14.1" }, - { name = "taskbroker-client", specifier = ">=0.17.0" }, + { name = "taskbroker-client", specifier = ">=0.17.1" }, { name = "tiktoken", specifier = ">=0.8.0" }, { name = "tldextract", specifier = ">=5.1.2" }, { name = "tokenizers", specifier = ">=0.22.0" }, @@ -2848,7 +2848,7 @@ wheels = [ [[package]] name = "taskbroker-client" -version = "0.17.0" +version = "0.17.1" source = { registry = "https://pypi.devinfra.sentry.io/simple" } dependencies = [ { name = "confluent-kafka", marker = "sys_platform == 'darwin' or sys_platform == 'linux'" }, @@ -2866,7 +2866,7 @@ dependencies = [ { name = "zstandard", marker = "sys_platform == 'darwin' or sys_platform == 'linux'" }, ] wheels = [ - { url = "https://pypi.devinfra.sentry.io/wheels/taskbroker_client-0.17.0-py3-none-any.whl", hash = "sha256:a52f5ff5914a50dbea50f07777a3c46080b467e04c7ec06895e7d3dceea16dc3" }, + { url = "https://pypi.devinfra.sentry.io/wheels/taskbroker_client-0.17.1-py3-none-any.whl", hash = "sha256:101f138a8e152e7d8dc5818e4366d07075caf05b813221aeb3474edda051e6bd" }, ] [[package]] From d06b45000cd7c1ab8d1193df2d2444c766e82c95 Mon Sep 17 00:00:00 2001 From: Charlie Luo Date: Fri, 29 May 2026 18:43:41 -0400 Subject: [PATCH 05/11] ref(api): remove unused events-trace-light endpoint (#116519) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Endpoint is private with zero external usage and no live frontend or backend callers — superseded by the events-trace and EAP trace endpoints. Remove the endpoint, its URL route, and its test class. --------- Co-authored-by: Claude Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com> --- .../endpoints/organization_events_trace.py | 202 +------ src/sentry/api/urls.py | 6 - .../utils/api/knownSentryApiUrls.generated.ts | 2 +- .../test_organization_events_trace.py | 495 ------------------ 4 files changed, 4 insertions(+), 701 deletions(-) diff --git a/src/sentry/api/endpoints/organization_events_trace.py b/src/sentry/api/endpoints/organization_events_trace.py index c8001c93147c..12cf0e382cca 100644 --- a/src/sentry/api/endpoints/organization_events_trace.py +++ b/src/sentry/api/endpoints/organization_events_trace.py @@ -3,13 +3,13 @@ import abc import logging from collections import defaultdict, deque -from collections.abc import Callable, Iterable, Mapping, Sequence +from collections.abc import Mapping, Sequence from concurrent.futures import as_completed from datetime import datetime, timedelta -from typing import Any, Optional, TypedDict, TypeVar, cast +from typing import Any, Optional, TypedDict, cast import sentry_sdk -from django.http import Http404, HttpRequest, HttpResponse +from django.http import HttpRequest, HttpResponse from rest_framework.exceptions import ParseError from rest_framework.request import Request from rest_framework.response import Response @@ -48,7 +48,6 @@ MAX_TRACE_SIZE: int = 100 -_T = TypeVar("_T") NodeSpans = list[dict[str, Any]] SnubaSpan = TypedDict( "SnubaSpan", @@ -530,14 +529,6 @@ def find_timestamp_params(transactions: Sequence[SnubaTransaction]) -> dict[str, } -def find_event( - items: Iterable[_T | None], - function: Callable[[_T | None], Any], - default: _T | None = None, -) -> _T | None: - return next(filter(function, items), default) - - def is_root(item: SnubaTransaction) -> bool: return item.get("root", "0") == "1" @@ -1078,193 +1069,6 @@ def serialize( raise NotImplementedError -@cell_silo_endpoint -class OrganizationEventsTraceLightEndpoint(OrganizationEventsTraceEndpointBase): - publish_status = { - "GET": ApiPublishStatus.PRIVATE, - } - - @staticmethod - def get_current_transaction( - transactions: Sequence[SnubaTransaction], - errors: Sequence[SnubaError], - event_id: str, - ) -> tuple[SnubaTransaction | None, Event | GroupEvent | None]: - """Given an event_id return the related transaction event - - The event_id could be for an error, since we show the quick-trace - for both event types - We occasionally have to get the nodestore data, so this function returns - the nodestore event as well so that we're doing that in one location. - """ - transaction_event = find_event( - transactions, lambda item: item is not None and item["id"] == event_id - ) - if transaction_event is not None: - return transaction_event, eventstore.backend.get_event_by_id( - transaction_event["project.id"], transaction_event["id"] - ) - - # The event couldn't be found, it might be an error - error_event = find_event(errors, lambda item: item is not None and item["id"] == event_id) - # Alright so we're looking at an error, time to see if we can find its transaction - if error_event is not None: - # Unfortunately the only association from an event back to its transaction is name & span_id - # First maybe we got lucky and the error happened on the transaction's "span" - error_span = error_event["trace.span"] - transaction_event = find_event( - transactions, lambda item: item is not None and item["trace.span"] == error_span - ) - if transaction_event is not None: - return transaction_event, eventstore.backend.get_event_by_id( - transaction_event["project.id"], transaction_event["id"] - ) - # We didn't get lucky, time to talk to nodestore... - for transaction_event in transactions: - if transaction_event["transaction"] != error_event["transaction"]: - continue - - nodestore_event = eventstore.backend.get_event_by_id( - transaction_event["project.id"], transaction_event["id"] - ) - if nodestore_event is None: - return None, None - transaction_spans: NodeSpans = nodestore_event.data.get("spans", []) - for span in transaction_spans: - if span["span_id"] == error_event["trace.span"]: - return transaction_event, nodestore_event - - return None, None - - def serialize( - self, - limit: int, - transactions: Sequence[SnubaTransaction], - errors: Sequence[SnubaError], - roots: Sequence[SnubaTransaction], - warning_extra: dict[str, str], - event_id: str | None, - detailed: bool = False, - query_source: QuerySource | None = None, - ) -> dict[str, list[LightResponse | TraceError]]: - """Because the light endpoint could potentially have gaps between root and event we return a flattened list""" - if event_id is None: - raise ParseError(detail="An event_id is required for the light trace") - snuba_event, nodestore_event = self.get_current_transaction(transactions, errors, event_id) - parent_map = self.construct_parent_map(transactions) - error_map = self.construct_error_map(errors) - trace_results: list[TraceEvent] = [] - current_generation: int | None = None - root_id: str | None = None - - with sentry_sdk.start_span(op="building.trace", name="light trace"): - # Check if the event is an orphan_error - if not snuba_event or not nodestore_event: - orphan_error = find_event( - errors, lambda item: item is not None and item["id"] == event_id - ) - if orphan_error: - return { - "transactions": [], - "orphan_errors": [self.serialize_error(orphan_error)], - } - else: - # The current event couldn't be found in errors or transactions - raise Http404() - - # Going to nodestore is more expensive than looping twice so check if we're on the root first - for root in roots: - if root["id"] == snuba_event["id"]: - current_generation = 0 - break - - snuba_params = self.get_snuba_params(self.request, self.request.organization) - if current_generation is None: - for root in roots: - # We might not be necessarily connected to the root if we're on an orphan event - if root["id"] != snuba_event["id"]: - # Get the root event and see if the current event's span is in the root event - root_event = eventstore.backend.get_event_by_id( - root["project.id"], root["id"] - ) - if root_event is None: - root_spans: NodeSpans = [] - else: - root_spans = root_event.data.get("spans", []) - root_span = find_event( - root_spans, - lambda item: item is not None - and item["span_id"] == snuba_event["trace.parent_span"], - ) - - # We only know to add the root if its the direct parent - if root_span is not None: - # For the light response, the parent will be unknown unless it is a direct descendent of the root - root_id = root["id"] - trace_results.append( - TraceEvent( - root, - None, - 0, - True, - snuba_params=snuba_params, - query_source=query_source, - ) - ) - current_generation = 1 - break - - current_event = TraceEvent( - snuba_event, - root_id, - current_generation, - True, - snuba_params=snuba_params, - query_source=query_source, - ) - trace_results.append(current_event) - - spans: NodeSpans = nodestore_event.data.get("spans", []) - # Need to include the transaction as a span as well - # - # Important that we left pad the span id with 0s because - # the span id is stored as an UInt64 and converted into - # a hex string when quering. However, the conversion does - # not ensure that the final span id is 16 chars long since - # it's a naive base 10 to base 16 conversion. - spans.append({"span_id": snuba_event["trace.span"].rjust(16, "0")}) - - for span in spans: - if span["span_id"] in error_map: - current_event.errors.extend( - [self.serialize_error(error) for error in error_map.pop(span["span_id"])] - ) - if span["span_id"] in parent_map: - child_events = parent_map.pop(span["span_id"]) - trace_results.extend( - [ - TraceEvent( - child_event, - snuba_event["id"], - ( - current_event.generation + 1 - if current_event.generation is not None - else None - ), - True, - snuba_params=snuba_params, - query_source=query_source, - ) - for child_event in child_events - ] - ) - - return { - "transactions": [result.to_dict() for result in trace_results], - "orphan_errors": [], - } - - @cell_silo_endpoint class OrganizationEventsTraceEndpoint(OrganizationEventsTraceEndpointBase): @staticmethod diff --git a/src/sentry/api/urls.py b/src/sentry/api/urls.py index e560e1939c23..11f2edb2cb2a 100644 --- a/src/sentry/api/urls.py +++ b/src/sentry/api/urls.py @@ -757,7 +757,6 @@ from .endpoints.organization_events_timeseries import OrganizationEventsTimeseriesEndpoint from .endpoints.organization_events_trace import ( OrganizationEventsTraceEndpoint, - OrganizationEventsTraceLightEndpoint, OrganizationEventsTraceMetaEndpoint, ) from .endpoints.organization_events_trends import ( @@ -1900,11 +1899,6 @@ def create_group_urls(name_prefix: str) -> list[URLPattern | URLResolver]: OrganizationEventsNewTrendsStatsEndpoint.as_view(), name="sentry-api-0-organization-events-trends-statsv2", ), - re_path( - r"^(?P[^/]+)/events-trace-light/(?P(?:\d+|[A-Fa-f0-9-]{32,36}))/$", - OrganizationEventsTraceLightEndpoint.as_view(), - name="sentry-api-0-organization-events-trace-light", - ), re_path( r"^(?P[^/]+)/events-trace/(?P(?:\d+|[A-Fa-f0-9-]{32,36}))/$", OrganizationEventsTraceEndpoint.as_view(), diff --git a/static/app/utils/api/knownSentryApiUrls.generated.ts b/static/app/utils/api/knownSentryApiUrls.generated.ts index e29eb3401a0f..3981258bd4d5 100644 --- a/static/app/utils/api/knownSentryApiUrls.generated.ts +++ b/static/app/utils/api/knownSentryApiUrls.generated.ts @@ -253,7 +253,6 @@ export type KnownSentryApiUrls = | '/organizations/$organizationIdOrSlug/events-sql/' | '/organizations/$organizationIdOrSlug/events-stats/' | '/organizations/$organizationIdOrSlug/events-timeseries/' - | '/organizations/$organizationIdOrSlug/events-trace-light/$traceId/' | '/organizations/$organizationIdOrSlug/events-trace-meta/$traceId/' | '/organizations/$organizationIdOrSlug/events-trace/$traceId/' | '/organizations/$organizationIdOrSlug/events-trends-stats/' @@ -263,6 +262,7 @@ export type KnownSentryApiUrls = | '/organizations/$organizationIdOrSlug/events/' | '/organizations/$organizationIdOrSlug/events/$projectIdOrSlug:$eventId/' | '/organizations/$organizationIdOrSlug/events/anomalies/' + | '/organizations/$organizationIdOrSlug/experimental/projects/' | '/organizations/$organizationIdOrSlug/explore/saved/' | '/organizations/$organizationIdOrSlug/explore/saved/$id/' | '/organizations/$organizationIdOrSlug/explore/saved/$id/starred/' diff --git a/tests/snuba/api/endpoints/test_organization_events_trace.py b/tests/snuba/api/endpoints/test_organization_events_trace.py index c8ddf4dad646..7cb38e93cabc 100644 --- a/tests/snuba/api/endpoints/test_organization_events_trace.py +++ b/tests/snuba/api/endpoints/test_organization_events_trace.py @@ -154,501 +154,6 @@ def load_trace(self): ) -class OrganizationEventsTraceLightEndpointTest(OrganizationEventsTraceEndpointBase): - url_name = "sentry-api-0-organization-events-trace-light" - - def test_no_projects(self) -> None: - user = self.create_user() - org = self.create_organization(owner=user) - self.login_as(user=user) - - url = reverse( - self.url_name, - kwargs={"organization_id_or_slug": org.slug, "trace_id": uuid4().hex}, - ) - - with self.feature(self.FEATURES): - response = self.client.get( - url, - format="json", - ) - - assert response.status_code == 404, response.content - - def test_bad_ids(self) -> None: - # Fake event id - with self.feature(self.FEATURES): - response = self.client.get( - self.url, - data={"event_id": uuid4().hex}, - format="json", - ) - - assert response.status_code == 404, response.content - - # Invalid event id - with self.feature(self.FEATURES): - response = self.client.get( - self.url, - data={"event_id": "not-a-event"}, - format="json", - ) - - assert response.status_code == 400, response.content - - # Fake trace id - self.url = reverse( - "sentry-api-0-organization-events-trace-light", - kwargs={ - "organization_id_or_slug": self.project.organization.slug, - "trace_id": uuid4().hex, - }, - ) - - with self.feature(self.FEATURES): - response = self.client.get( - self.url, - data={"event_id": "a" * 32}, - format="json", - ) - - assert response.status_code == 404, response.content - - # Invalid trace id - with pytest.raises(NoReverseMatch): - self.url = reverse( - "sentry-api-0-organization-events-trace-light", - kwargs={ - "organization_id_or_slug": self.project.organization.slug, - "trace_id": "not-a-trace", - }, - ) - - def test_no_roots(self) -> None: - """Even when there's no root, we return the current event""" - self.load_trace() - no_root_trace = uuid4().hex - parent_span_id = uuid4().hex[:16] - no_root_event = self.create_event( - trace_id=no_root_trace, - transaction="/not_root/but_only_transaction", - spans=[], - parent_span_id=parent_span_id, - project_id=self.project.id, - ) - url = reverse( - "sentry-api-0-organization-events-trace-light", - kwargs={ - "organization_id_or_slug": self.project.organization.slug, - "trace_id": no_root_trace, - }, - ) - - with self.feature(self.FEATURES): - response = self.client.get( - url, - data={"event_id": no_root_event.event_id}, - format="json", - ) - - assert response.status_code == 200, response.content - assert len(response.data["transactions"]) == 1 - - event = response.data["transactions"][0] - # Basically know nothing about this event - assert event["generation"] is None - assert event["parent_event_id"] is None - assert event["parent_span_id"] == parent_span_id - assert event["event_id"] == no_root_event.event_id - - def test_multiple_roots(self) -> None: - self.load_trace() - second_root = self.create_event( - trace_id=self.trace_id, - transaction="/second_root", - spans=[], - parent_span_id=None, - project_id=self.project.id, - ) - with self.feature(self.FEATURES): - data: dict[str, str | int] = {"event_id": second_root.event_id, "project": -1} - response = self.client.get(self.url, data=data, format="json") - - assert response.status_code == 200, response.content - assert len(response.data["transactions"]) == 1 - - event = response.data["transactions"][0] - assert event["generation"] == 0 - assert event["parent_event_id"] is None - assert event["parent_span_id"] is None - - def test_root_event(self) -> None: - self.load_trace() - root_event_id = self.root_event.event_id - with self.feature(self.FEATURES): - data: dict[str, str | int] = {"event_id": root_event_id, "project": -1} - response = self.client.get(self.url, data=data, format="json") - - assert response.status_code == 200, response.content - assert len(response.data["transactions"]) == 4 - events = {item["event_id"]: item for item in response.data["transactions"]} - - assert root_event_id in events - event = events[root_event_id] - assert event["generation"] == 0 - assert event["parent_event_id"] is None - assert event["parent_span_id"] is None - - for i, child_event in enumerate(self.gen1_events): - child_event_id = child_event.event_id - assert child_event_id in events - event = events[child_event_id] - assert event["generation"] == 1 - assert event["parent_event_id"] == root_event_id - assert event["parent_span_id"] == self.root_span_ids[i] - - def test_root_with_multiple_roots(self) -> None: - self.load_trace() - root_event_id = self.root_event.event_id - self.create_event( - trace_id=self.trace_id, - transaction="/second_root", - spans=[], - parent_span_id=None, - project_id=self.project.id, - ) - with self.feature(self.FEATURES): - response = self.client.get( - self.url, - data={"event_id": self.root_event.event_id}, - format="json", - ) - - assert response.status_code == 200, response.content - - assert len(response.data["transactions"]) == 4 - events = {item["event_id"]: item for item in response.data["transactions"]} - - assert root_event_id in events - event = events[root_event_id] - assert event["generation"] == 0 - assert event["parent_event_id"] is None - assert event["parent_span_id"] is None - - for i, child_event in enumerate(self.gen1_events): - child_event_id = child_event.event_id - assert child_event_id in events - event = events[child_event_id] - assert event["generation"] == 1 - assert event["parent_event_id"] == root_event_id - assert event["parent_span_id"] == self.root_span_ids[i] - - def test_direct_parent_with_children(self) -> None: - self.load_trace() - root_event_id = self.root_event.event_id - current_event = self.gen1_events[0].event_id - child_event_id = self.gen2_events[0].event_id - - with self.feature(self.FEATURES): - data: dict[str, str | int] = {"event_id": current_event, "project": -1} - response = self.client.get(self.url, data=data, format="json") - - assert response.status_code == 200, response.content - - assert len(response.data["transactions"]) == 3 - events = {item["event_id"]: item for item in response.data["transactions"]} - - assert root_event_id in events - event = events[root_event_id] - assert event["generation"] == 0 - assert event["parent_event_id"] is None - assert event["parent_span_id"] is None - - assert current_event in events - event = events[current_event] - assert event["generation"] == 1 - assert event["parent_event_id"] == root_event_id - assert event["parent_span_id"] == self.root_span_ids[0] - - assert child_event_id in events - event = events[child_event_id] - assert event["generation"] == 2 - assert event["parent_event_id"] == current_event - assert event["parent_span_id"] == self.gen1_span_ids[0] - - def test_direct_parent_with_children_and_multiple_root(self) -> None: - self.load_trace() - root_event_id = self.root_event.event_id - current_event = self.gen1_events[0].event_id - child_event_id = self.gen2_events[0].event_id - self.create_event( - trace_id=self.trace_id, - transaction="/second_root", - spans=[], - parent_span_id=None, - project_id=self.project.id, - ) - - with self.feature(self.FEATURES): - data: dict[str, str | int] = {"event_id": current_event, "project": -1} - response = self.client.get(self.url, data=data, format="json") - - assert response.status_code == 200, response.content - - assert len(response.data["transactions"]) == 3 - events = {item["event_id"]: item for item in response.data["transactions"]} - - assert root_event_id in events - event = events[root_event_id] - assert event["generation"] == 0 - assert event["parent_event_id"] is None - assert event["parent_span_id"] is None - - assert current_event in events - event = events[current_event] - assert event["generation"] == 1 - assert event["parent_event_id"] == root_event_id - assert event["parent_span_id"] == self.root_span_ids[0] - - assert child_event_id in events - event = events[child_event_id] - assert event["generation"] == 2 - assert event["parent_event_id"] == current_event - assert event["parent_span_id"] == self.gen1_span_ids[0] - - def test_second_generation_with_children(self) -> None: - self.load_trace() - current_event = self.gen2_events[0].event_id - child_event_id = self.gen3_event.event_id - - with self.feature(self.FEATURES): - data: dict[str, str | int] = {"event_id": current_event, "project": -1} - response = self.client.get(self.url, data=data, format="json") - - assert response.status_code == 200, response.content - - assert len(response.data["transactions"]) == 2 - events = {item["event_id"]: item for item in response.data["transactions"]} - - assert current_event in events - event = events[current_event] - # Parent/generation is unknown in this case - assert event["generation"] is None - assert event["parent_event_id"] is None - # But we still know the parent_span - assert event["parent_span_id"] == self.gen1_span_ids[0] - - assert child_event_id in events - event = events[child_event_id] - assert event["generation"] is None - assert event["parent_event_id"] == current_event - assert event["parent_span_id"] == self.gen2_span_id - - def test_third_generation_no_children(self) -> None: - self.load_trace() - current_event = self.gen3_event.event_id - - with self.feature(self.FEATURES): - data: dict[str, str | int] = {"event_id": current_event, "project": -1} - response = self.client.get(self.url, data=data, format="json") - - assert response.status_code == 200, response.content - - assert len(response.data["transactions"]) == 1 - - event = response.data["transactions"][0] - assert event["generation"] is None - # Parent is unknown in this case - assert event["parent_event_id"] is None - # But we still know the parent_span - assert event["parent_span_id"] == self.gen2_span_id - - def test_sibling_transactions(self) -> None: - """More than one transaction can share a parent_span_id""" - self.load_trace() - gen3_event_siblings = [ - self.create_event( - trace_id=self.trace_id, - transaction="/transaction/gen3-1", - spans=[], - project_id=self.create_project(organization=self.organization).id, - parent_span_id=self.gen2_span_ids[1], - milliseconds=500, - ).event_id, - self.create_event( - trace_id=self.trace_id, - transaction="/transaction/gen3-2", - spans=[], - project_id=self.create_project(organization=self.organization).id, - parent_span_id=self.gen2_span_ids[1], - milliseconds=1500, - ).event_id, - ] - - current_event = self.gen2_events[1].event_id - - with self.feature(self.FEATURES): - data: dict[str, str | int] = {"event_id": current_event, "project": -1} - response = self.client.get(self.url, data=data, format="json") - - assert len(response.data["transactions"]) == 3 - events = {item["event_id"]: item for item in response.data["transactions"]} - - for child_event_id in gen3_event_siblings: - assert child_event_id in events - event = events[child_event_id] - assert event["generation"] is None - assert event["parent_event_id"] == current_event - assert event["parent_span_id"] == self.gen2_span_ids[1] - - def test_with_error_event(self) -> None: - self.load_trace() - root_event_id = self.root_event.event_id - current_transaction_event = self.gen1_events[0].event_id - - start, _ = self.get_start_end_from_day_ago(1000) - error_data = load_data( - "javascript", - timestamp=start, - ) - error_data["contexts"]["trace"] = { - "type": "trace", - "trace_id": self.trace_id, - "span_id": self.gen1_span_ids[0], - } - error_data["tags"] = [["transaction", "/transaction/gen1-0"]] - error = self.store_event(error_data, project_id=self.gen1_project.id) - - def assertions(response): - assert response.status_code == 200, response.content - assert len(response.data["transactions"]) == 3 - events = {item["event_id"]: item for item in response.data["transactions"]} - - assert root_event_id in events - event = events[root_event_id] - assert event["generation"] == 0 - assert event["parent_event_id"] is None - assert event["parent_span_id"] is None - assert len(event["errors"]) == 0 - - assert current_transaction_event in events - event = events[current_transaction_event] - assert event["generation"] == 1 - assert event["parent_event_id"] == root_event_id - assert event["parent_span_id"] == self.root_span_ids[0] - assert len(event["errors"]) == 1 - assert event["errors"][0]["event_id"] == error.event_id - assert event["errors"][0]["issue_id"] == error.group_id - assert event["errors"][0]["message"] == error.search_message - - with self.feature(self.FEATURES): - response = self.client.get( - self.url, - data={"event_id": error.event_id, "project": -1}, - format="json", - ) - - assertions(response) - - with self.feature(self.FEATURES): - data: dict[str, str | int] = {"event_id": current_transaction_event, "project": -1} - response = self.client.get(self.url, data=data, format="json") - - assertions(response) - - def assert_orphan_error_response(self, response, error, span_id): - assert response.status_code == 200, response.content - assert response.data["transactions"] == [] - assert len(response.data["orphan_errors"]) == 1 - assert { - "event_id": error.event_id, - "issue_id": error.group_id, - "span": span_id, - "project_id": self.project.id, - "project_slug": self.project.slug, - "level": "fatal", - "title": error.title, - "timestamp": datetime.fromisoformat(error.timestamp).timestamp(), - "generation": 0, - "event_type": "error", - "message": error.search_message, - } == response.data["orphan_errors"][0] - - def test_with_one_orphan_error(self) -> None: - self.load_trace() - span_id = uuid4().hex[:16] - start, _ = self.get_start_end_from_day_ago(1000) - - error_data = load_data( - "javascript", - timestamp=start, - ) - error_data["contexts"]["trace"] = { - "type": "trace", - "trace_id": self.trace_id, - "span_id": span_id, - } - error_data["level"] = "fatal" - error = self.store_event(error_data, project_id=self.project.id) - - with self.feature([*self.FEATURES]): - response = self.client.get( - self.url, - data={"event_id": error.event_id, "project": -1}, - format="json", - ) - - self.assert_orphan_error_response(response, error, span_id) - - def test_with_multiple_orphan_errors(self) -> None: - self.load_trace() - span_id = uuid4().hex[:16] - start, end = self.get_start_end_from_day_ago(1000) - - error_data = load_data( - "javascript", - timestamp=start, - ) - error_data["contexts"]["trace"] = { - "type": "trace", - "trace_id": self.trace_id, - "span_id": span_id, - } - error_data["level"] = "fatal" - error = self.store_event(error_data, project_id=self.project.id) - - error_data1 = load_data( - "javascript", - timestamp=end, - ) - error_data1["contexts"]["trace"] = { - "type": "trace", - "trace_id": self.trace_id, - "span_id": span_id, - } - error_data1["level"] = "warning" - self.store_event(error_data1, project_id=self.project.id) - - with self.feature([*self.FEATURES]): - response = self.client.get( - self.url, - data={"event_id": error.event_id, "project": -1}, - format="json", - ) - - self.assert_orphan_error_response(response, error, span_id) - - def test_with_unknown_event(self) -> None: - with self.feature([*self.FEATURES]): - response = self.client.get( - self.url, - data={"event_id": "766758c00ff54d8ab865369ecab53ae6", "project": "-1"}, - format="json", - ) - - assert response.status_code == 404 - - class OrganizationEventsTraceEndpointTest(OrganizationEventsTraceEndpointBase, OccurrenceTestCase): url_name = "sentry-api-0-organization-events-trace" check_generation = True From f1a5adedc8f8a8151aa88056ff1b61d9a2964bcf Mon Sep 17 00:00:00 2001 From: Charlie Luo Date: Fri, 29 May 2026 18:43:50 -0400 Subject: [PATCH 06/11] ref(traces): remove stale events-trace-light frontend references (#116523) Follow-up to https://github.com/getsentry/sentry/pull/116515. Remove references to deprecated endpoint. Co-authored-by: Claude --- .../groupEventDetails/groupEventDetails.spec.tsx | 7 ------- .../views/performance/newTraceDetails/traceApi/types.tsx | 2 +- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/static/app/views/issueDetails/groupEventDetails/groupEventDetails.spec.tsx b/static/app/views/issueDetails/groupEventDetails/groupEventDetails.spec.tsx index 946f76d1d9e2..7e5aa973f226 100644 --- a/static/app/views/issueDetails/groupEventDetails/groupEventDetails.spec.tsx +++ b/static/app/views/issueDetails/groupEventDetails/groupEventDetails.spec.tsx @@ -212,13 +212,6 @@ const mockGroupApis = ( : {transactions: [], orphan_errors: []}, }); - MockApiClient.addMockResponse({ - url: `/organizations/${organization.slug}/events-trace-light/${TRACE_ID}/`, - body: trace - ? {transactions: [trace], orphan_errors: []} - : {transactions: [], orphan_errors: []}, - }); - MockApiClient.addMockResponse({ url: `/organizations/${organization.slug}/issues/${group.id}/integrations/`, body: [], diff --git a/static/app/views/performance/newTraceDetails/traceApi/types.tsx b/static/app/views/performance/newTraceDetails/traceApi/types.tsx index 0739e8a7bbf8..5c02584297f3 100644 --- a/static/app/views/performance/newTraceDetails/traceApi/types.tsx +++ b/static/app/views/performance/newTraceDetails/traceApi/types.tsx @@ -3,7 +3,7 @@ import type {TraceTree} from 'sentry/views/performance/newTraceDetails/traceMode /** * `EventLite` represents the type of a simplified event from - * the `events-trace` and `events-trace-light` endpoints. + * the `events-trace` endpoint. */ type EventLite = { event_id: string; From eeceb05a09e03c8444d22dbc65a71751f7930721 Mon Sep 17 00:00:00 2001 From: Brendan Hy Date: Fri, 29 May 2026 16:12:28 -0700 Subject: [PATCH 07/11] chore(billing-platform): Bump sentry-protos 0.21.0 (#116539) --- pyproject.toml | 2 +- uv.lock | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index cf937986b83c..e3731299519f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -96,7 +96,7 @@ dependencies = [ "sentry-ophio>=1.1.3", # sentry-options is only used in getsentry for now "sentry-options>=1.0.13", - "sentry-protos>=0.17.0", + "sentry-protos>=0.21.0", "sentry-redis-tools>=0.5.0", "sentry-relay>=0.9.27", "sentry-scm==0.20.0", diff --git a/uv.lock b/uv.lock index 86936b919c53..7d67b86b9d69 100644 --- a/uv.lock +++ b/uv.lock @@ -2439,7 +2439,7 @@ requires-dist = [ { name = "sentry-kafka-schemas", specifier = ">=2.1.27" }, { name = "sentry-ophio", specifier = ">=1.1.3" }, { name = "sentry-options", specifier = ">=1.0.13" }, - { name = "sentry-protos", specifier = ">=0.17.0" }, + { name = "sentry-protos", specifier = ">=0.21.0" }, { name = "sentry-redis-tools", specifier = ">=0.5.0" }, { name = "sentry-relay", specifier = ">=0.9.27" }, { name = "sentry-scm", specifier = "==0.20.0" }, @@ -2623,7 +2623,7 @@ wheels = [ [[package]] name = "sentry-protos" -version = "0.17.0" +version = "0.21.0" source = { registry = "https://pypi.devinfra.sentry.io/simple" } dependencies = [ { name = "grpc-stubs", marker = "sys_platform == 'darwin' or sys_platform == 'linux'" }, @@ -2631,7 +2631,7 @@ dependencies = [ { name = "protobuf", marker = "sys_platform == 'darwin' or sys_platform == 'linux'" }, ] wheels = [ - { url = "https://pypi.devinfra.sentry.io/wheels/sentry_protos-0.17.0-py3-none-any.whl", hash = "sha256:e5a7c320678a6204cfa6e8a8981952e8dd1080131de076c5cec8702a24f62d7d" }, + { url = "https://pypi.devinfra.sentry.io/wheels/sentry_protos-0.21.0-py3-none-any.whl", hash = "sha256:ef3bc29f77af8f8fa17fc1a58fe96ae2aa22b2c2201e532047bce4889074a121" }, ] [[package]] From b69202c2d988786cafdf2283b396bafc2e7bb334 Mon Sep 17 00:00:00 2001 From: Christinarlong <60594860+Christinarlong@users.noreply.github.com> Date: Fri, 29 May 2026 16:25:51 -0700 Subject: [PATCH 08/11] chore(webhooks): Remove raise that short circuit url sending (#116534) --- src/sentry/plugins/sentry_webhooks/plugin.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/sentry/plugins/sentry_webhooks/plugin.py b/src/sentry/plugins/sentry_webhooks/plugin.py index 0b2c248ec36e..de6b886da454 100644 --- a/src/sentry/plugins/sentry_webhooks/plugin.py +++ b/src/sentry/plugins/sentry_webhooks/plugin.py @@ -144,7 +144,6 @@ def notify_users(self, group, event, triggering_rules) -> None: tags={"outcome": LegacyWebhookOutcome.ERROR}, sample_rate=1.0, ) - raise except (ConnectionError, ReadTimeout): metrics.incr( "legacy_webhook.plugin.send", From 61bc60a3e716563ca3bdc1dc0cd0f17114666408 Mon Sep 17 00:00:00 2001 From: Kyle Consalus Date: Fri, 29 May 2026 16:29:11 -0700 Subject: [PATCH 09/11] fix(workflows): Rule deletion shouldn't automatically result in Workflow deletion (#116537) It's broken for project deletion to result in org-scoped workflow deletion, and for teams with legacy alerting models, this is a real risk. It's not too troubling if the project was the only user of the workflow, and arguably expected, but removing org-scoped data when no longer used anywhere is a choice we haven't yet made, so best to default to the safest path. The legacy Rule endpoint still will delete the Workflow associated, which is troublesome in sharing cases, but will be addressed elsewhere. This is spun off of https://github.com/getsentry/sentry/pull/116532. --- src/sentry/deletions/defaults/rule.py | 37 +++---------------- tests/sentry/deletions/test_rule.py | 3 ++ .../test_issue_alert_dual_write.py | 17 +++++++-- 3 files changed, 22 insertions(+), 35 deletions(-) diff --git a/src/sentry/deletions/defaults/rule.py b/src/sentry/deletions/defaults/rule.py index c0ee27b5badc..4f4a317cb344 100644 --- a/src/sentry/deletions/defaults/rule.py +++ b/src/sentry/deletions/defaults/rule.py @@ -1,12 +1,8 @@ -import logging from collections.abc import Sequence from sentry.deletions.base import BaseRelation, ModelDeletionTask, ModelRelation from sentry.deletions.defaults.alertrule import AlertRuleDetectorDeletionTask from sentry.models.rule import Rule -from sentry.workflow_engine.models import Workflow - -logger = logging.getLogger(__name__) class RuleDeletionTask(ModelDeletionTask[Rule]): @@ -15,7 +11,11 @@ def get_child_relations(self, instance: Rule) -> list[BaseRelation]: from sentry.models.rule import RuleActivity from sentry.workflow_engine.models import AlertRuleDetector, AlertRuleWorkflow - model_relations: list[BaseRelation] = [ + # Workflows are org-scoped and must not be deleted when a project-scoped + # Rule is deleted. Workflow cleanup happens via the API (which schedules + # Workflow deletion explicitly) or OrganizationDeletionTask. We only + # clean up the link rows (AlertRuleWorkflow, AlertRuleDetector) here. + return [ ModelRelation(GroupRuleStatus, {"rule_id": instance.id}), ModelRelation(RuleActivity, {"rule_id": instance.id}), ModelRelation( @@ -23,34 +23,9 @@ def get_child_relations(self, instance: Rule) -> list[BaseRelation]: {"rule_id": instance.id}, task=AlertRuleDetectorDeletionTask, ), + ModelRelation(AlertRuleWorkflow, {"rule_id": instance.id}), ] - # AlertRuleWorkflow must be deleted before Workflow so the link rows - # are gone by the time WorkflowDeletionTask runs — otherwise it would - # cascade back to this Rule and loop infinitely. - workflow_ids = list( - AlertRuleWorkflow.objects.filter(rule_id=instance.id).values_list( - "workflow_id", flat=True - ) - ) - if workflow_ids: - model_relations.append(ModelRelation(AlertRuleWorkflow, {"rule_id": instance.id})) - model_relations.append( - ModelRelation( - Workflow, - { - "id__in": workflow_ids, - "organization_id": instance.project.organization_id, - }, - ) - ) - else: - logger.info( - "No AlertRuleWorkflow found for rule, skipping", extra={"rule_id": instance.id} - ) - - return model_relations - def mark_deletion_in_progress(self, instance_list: Sequence[Rule]) -> None: from sentry.constants import ObjectStatus diff --git a/tests/sentry/deletions/test_rule.py b/tests/sentry/deletions/test_rule.py index 0fbdefb8b749..a14508b63e5d 100644 --- a/tests/sentry/deletions/test_rule.py +++ b/tests/sentry/deletions/test_rule.py @@ -64,5 +64,8 @@ def test_simple(self) -> None: ).exists() assert not GroupRuleStatus.objects.filter(id=group_rule_status.id).exists() assert not RuleActivity.objects.filter(id=rule_activity.id).exists() + # Link rows are cleaned up assert not AlertRuleDetector.objects.filter(rule_id=rule.id).exists() assert not AlertRuleWorkflow.objects.filter(rule_id=rule.id).exists() + # Org-scoped Workflow survives Rule deletion + assert Workflow.objects.filter(id=workflow.id).exists() diff --git a/tests/sentry/workflow_engine/migration_helpers/test_issue_alert_dual_write.py b/tests/sentry/workflow_engine/migration_helpers/test_issue_alert_dual_write.py index 1ce77a833815..2c08be4c286e 100644 --- a/tests/sentry/workflow_engine/migration_helpers/test_issue_alert_dual_write.py +++ b/tests/sentry/workflow_engine/migration_helpers/test_issue_alert_dual_write.py @@ -4,6 +4,7 @@ from sentry.constants import ObjectStatus from sentry.deletions.models.scheduleddeletion import CellScheduledDeletion from sentry.deletions.tasks.scheduled import run_scheduled_deletions +from sentry.models.rule import Rule from sentry.models.rulesnooze import RuleSnooze from sentry.rules.age import AgeComparisonType from sentry.rules.conditions.event_frequency import ( @@ -354,7 +355,13 @@ def setUp(self) -> None: self.when_dcg: DataConditionGroup = when_dcg self.if_dcg: DataConditionGroup = if_dcg - def assert_issue_alert_deleted( + def assert_rule_deleted_workflow_survives(self, workflow: Workflow) -> None: + """Rule and link rows are deleted, but org-scoped Workflow survives.""" + assert not Rule.objects.filter(id=self.issue_alert.id).exists() + assert not AlertRuleWorkflow.objects.filter(rule_id=self.issue_alert.id).exists() + assert Workflow.objects.filter(id=workflow.id).exists() + + def assert_everything_deleted( self, workflow: Workflow, when_dcg: DataConditionGroup, if_dcg: DataConditionGroup ) -> None: assert not AlertRuleWorkflow.objects.filter(rule_id=self.issue_alert.id).exists() @@ -373,7 +380,7 @@ def test_delete_issue_alert__rule_deletion_task(self) -> None: with self.tasks(): run_scheduled_deletions() - self.assert_issue_alert_deleted(self.workflow, self.when_dcg, self.if_dcg) + self.assert_rule_deleted_workflow_survives(self.workflow) def test_delete_issue_alert__project_deletion_task(self) -> None: self.project.update(status=ObjectStatus.PENDING_DELETION) @@ -382,7 +389,9 @@ def test_delete_issue_alert__project_deletion_task(self) -> None: with self.tasks(): run_scheduled_deletions() - self.assert_issue_alert_deleted(self.workflow, self.when_dcg, self.if_dcg) + # Workflows are org-scoped, not project-scoped, so they survive + # project deletion. Only OrganizationDeletionTask cleans them up. + self.assert_rule_deleted_workflow_survives(self.workflow) def test_delete_issue_alert__org_deletion_task(self) -> None: self.organization.update(status=ObjectStatus.PENDING_DELETION) @@ -391,4 +400,4 @@ def test_delete_issue_alert__org_deletion_task(self) -> None: with self.tasks(): run_scheduled_deletions() - self.assert_issue_alert_deleted(self.workflow, self.when_dcg, self.if_dcg) + self.assert_everything_deleted(self.workflow, self.when_dcg, self.if_dcg) From f05b40ad88340d409f43057c6ba2076dc6fde2de Mon Sep 17 00:00:00 2001 From: Kyle Consalus Date: Fri, 29 May 2026 17:41:06 -0700 Subject: [PATCH 10/11] fix(aci): Forbid unsafe legacy interactions with shared Workflows/Detectors (#116532) For reasons valid and otherwise, it's possible to have multiple legacy models (Rule, AlertRule) mapped to the same Workflow or Detector. For read purposes, this can be convenient for preserving the old interface, but mutations in this case are confusing at best. Moreover, our deletion code had assumed no sharing, which meant that deleting a Rule can result in a workflow used by many other projects being deleted, which is almost certainly not the intended behavior. To address this, here we: 1. Remove the ability for Rules to delete Workflows. They only delete their link to them. Rules are vestigial; they should be removable without impacting Workflows. 2. Serve a 400 when a user attempts to modify a Rule/AlertRule backed by a shared Workflow or Detector, providing an error that indicates that the new API should be used. This is less convenient, as some operations may be totally fine, but allowing changing one Rule to update another Rule with a different id via shared backend is wrong. At this point they're all aliases I think (at least, for Rules) but there's no way via the legacy API to know that. Thus, we ask them to use the new API in this case. The second part in particular may prove annoying, but we can fix it by un-merging (in some cases) or they can use new APIs. --- src/sentry/api/bases/rule.py | 22 +++++++++++++++ src/sentry/incidents/endpoints/bases.py | 27 ++++++++++++++++++- .../endpoints/test_project_rule_details.py | 21 +++++++++++++++ .../test_organization_alert_rule_details.py | 21 +++++++++++++++ 4 files changed, 90 insertions(+), 1 deletion(-) diff --git a/src/sentry/api/bases/rule.py b/src/sentry/api/bases/rule.py index 68278e74dcc9..4c6889e1cd8d 100644 --- a/src/sentry/api/bases/rule.py +++ b/src/sentry/api/bases/rule.py @@ -1,5 +1,7 @@ from typing import Any +from rest_framework import status +from rest_framework.exceptions import APIException from rest_framework.request import Request from sentry import features @@ -14,6 +16,16 @@ from sentry.workflow_engine.utils.legacy_metric_tracking import report_used_legacy_models +class SharedWorkflowError(APIException): + status_code = status.HTTP_400_BAD_REQUEST + + def __init__(self, workflow_id: int) -> None: + detail = ( + f"Workflow {workflow_id} is shared with other rules. Use the workflow API to manage it." + ) + super().__init__(detail=detail) + + class RuleEndpoint(ProjectEndpoint): owner = ApiOwner.ISSUES permission_classes = (ProjectAlertRulePermission,) @@ -66,6 +78,16 @@ def convert_args( workflow__organization=project.organization, workflow__status=ObjectStatus.ACTIVE, ) + # For mutating requests via a legacy Rule ID, reject if the + # Workflow is shared with other Rules or AlertRules. + if request.method in ("PUT", "DELETE"): + has_other_links = ( + AlertRuleWorkflow.objects.filter(workflow_id=arw.workflow_id) + .exclude(id=arw.id) + .exists() + ) + if has_other_links: + raise SharedWorkflowError(arw.workflow_id) kwargs["rule"] = arw.workflow except AlertRuleWorkflow.DoesNotExist: # XXX: this means the workflow was single written and has no ARW or related Rule object diff --git a/src/sentry/incidents/endpoints/bases.py b/src/sentry/incidents/endpoints/bases.py index 7e9c70aaf975..17df173ad1e3 100644 --- a/src/sentry/incidents/endpoints/bases.py +++ b/src/sentry/incidents/endpoints/bases.py @@ -1,6 +1,7 @@ from typing import Any -from rest_framework.exceptions import PermissionDenied +from rest_framework import status +from rest_framework.exceptions import APIException, PermissionDenied from rest_framework.request import Request from sentry import features @@ -16,6 +17,28 @@ from sentry.workflow_engine.models.detector import Detector +class SharedDetectorError(APIException): + status_code = status.HTTP_400_BAD_REQUEST + + def __init__(self, detector_id: int) -> None: + detail = ( + f"Detector {detector_id} is shared with other rules. Use the detector API to manage it." + ) + super().__init__(detail=detail) + + +def _check_shared_detector(request: Request, ard: AlertRuleDetector) -> None: + """Reject PUT/DELETE on a Detector that is shared with other rules.""" + if request.method in ("PUT", "DELETE"): + has_other_links = ( + AlertRuleDetector.objects.filter(detector_id=ard.detector_id) + .exclude(id=ard.id) + .exists() + ) + if has_other_links: + raise SharedDetectorError(ard.detector_id) + + class OrganizationAlertRuleBaseEndpoint(OrganizationEndpoint): """ Base endpoint for organization-scoped alert rule creation. @@ -131,6 +154,7 @@ def convert_args( alert_rule_id=validated_alert_rule_id, detector__project=project, ) + _check_shared_detector(request, ard) kwargs["alert_rule"] = ard.detector except AlertRuleDetector.DoesNotExist: # XXX: this means the detector was single written and has no ARD or related AlertRule object @@ -179,6 +203,7 @@ def convert_args( alert_rule_id=validated_alert_rule_id, detector__project__organization=organization, ) + _check_shared_detector(request, ard) kwargs["alert_rule"] = ard.detector except AlertRuleDetector.DoesNotExist: # XXX: this means the detector was single written and has no ARD or related AlertRule object diff --git a/tests/sentry/api/endpoints/test_project_rule_details.py b/tests/sentry/api/endpoints/test_project_rule_details.py index ffd9c7ac194d..7cf287499ffc 100644 --- a/tests/sentry/api/endpoints/test_project_rule_details.py +++ b/tests/sentry/api/endpoints/test_project_rule_details.py @@ -1473,6 +1473,27 @@ def test_delete_does_not_cascade_to_cross_org_rule(self) -> None: other_rule.refresh_from_db() assert other_rule.status == ObjectStatus.ACTIVE + def test_delete_shared_workflow_returns_400(self) -> None: + rule = self.create_project_rule(self.project) + arw = AlertRuleWorkflow.objects.get(rule_id=rule.id) + workflow = arw.workflow + + # Create another Rule linked to the same Workflow. + other_rule = self.create_project_rule(self.project) + other_arw = AlertRuleWorkflow.objects.get(rule_id=other_rule.id) + other_arw.update(workflow_id=workflow.id) + + response = self.get_error_response( + self.organization.slug, self.project.slug, rule.id, status_code=400 + ) + assert "Workflow" in response.data["detail"] + assert str(workflow.id) in response.data["detail"] + + # Neither rule nor workflow were deleted. + rule.refresh_from_db() + assert rule.status == ObjectStatus.ACTIVE + assert Workflow.objects.filter(id=workflow.id).exists() + class GetProjectRuleDetailsDeltaTest(ProjectRuleDetailsBaseTestCase): def test_dual_written_rule_parity(self) -> None: diff --git a/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py b/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py index a8666ec55804..16fc712af98e 100644 --- a/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py +++ b/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py @@ -2757,3 +2757,24 @@ def test_dual_delete_detector_id_passed(self) -> None: assert not AlertRule.objects_with_snapshots.filter(name=self.alert_rule.name).exists() assert not AlertRule.objects_with_snapshots.filter(id=self.alert_rule.id).exists() assert not Detector.objects.filter(id=ard.detector_id).exists() + + @with_feature("organizations:workflow-engine-rule-serializers") + def test_delete_shared_detector_returns_400(self) -> None: + self.create_member( + user=self.user, organization=self.organization, role="owner", teams=[self.team] + ) + self.login_as(self.user) + + ard = AlertRuleDetector.objects.get(alert_rule_id=self.alert_rule.id) + # Create another Rule linked to the same Detector. + AlertRuleDetector.objects.create(rule_id=999999, detector_id=ard.detector_id) + + with self.feature("organizations:incidents"): + resp = self.get_error_response( + self.organization.slug, self.alert_rule.id, status_code=400 + ) + + assert "Detector" in resp.data["detail"] + assert str(ard.detector_id) in resp.data["detail"] + # AlertRule was not deleted. + assert AlertRule.objects.filter(id=self.alert_rule.id).exists() From aa8dbc01891fbe666a176dff5e2b39b45ebb7f02 Mon Sep 17 00:00:00 2001 From: getsentry-bot Date: Sat, 30 May 2026 00:47:01 +0000 Subject: [PATCH 11/11] Revert "fix(aci): Forbid unsafe legacy interactions with shared Workflows/Detectors (#116532)" This reverts commit f05b40ad88340d409f43057c6ba2076dc6fde2de. Co-authored-by: kcons <6990351+kcons@users.noreply.github.com> --- src/sentry/api/bases/rule.py | 22 --------------- src/sentry/incidents/endpoints/bases.py | 27 +------------------ .../endpoints/test_project_rule_details.py | 21 --------------- .../test_organization_alert_rule_details.py | 21 --------------- 4 files changed, 1 insertion(+), 90 deletions(-) diff --git a/src/sentry/api/bases/rule.py b/src/sentry/api/bases/rule.py index 4c6889e1cd8d..68278e74dcc9 100644 --- a/src/sentry/api/bases/rule.py +++ b/src/sentry/api/bases/rule.py @@ -1,7 +1,5 @@ from typing import Any -from rest_framework import status -from rest_framework.exceptions import APIException from rest_framework.request import Request from sentry import features @@ -16,16 +14,6 @@ from sentry.workflow_engine.utils.legacy_metric_tracking import report_used_legacy_models -class SharedWorkflowError(APIException): - status_code = status.HTTP_400_BAD_REQUEST - - def __init__(self, workflow_id: int) -> None: - detail = ( - f"Workflow {workflow_id} is shared with other rules. Use the workflow API to manage it." - ) - super().__init__(detail=detail) - - class RuleEndpoint(ProjectEndpoint): owner = ApiOwner.ISSUES permission_classes = (ProjectAlertRulePermission,) @@ -78,16 +66,6 @@ def convert_args( workflow__organization=project.organization, workflow__status=ObjectStatus.ACTIVE, ) - # For mutating requests via a legacy Rule ID, reject if the - # Workflow is shared with other Rules or AlertRules. - if request.method in ("PUT", "DELETE"): - has_other_links = ( - AlertRuleWorkflow.objects.filter(workflow_id=arw.workflow_id) - .exclude(id=arw.id) - .exists() - ) - if has_other_links: - raise SharedWorkflowError(arw.workflow_id) kwargs["rule"] = arw.workflow except AlertRuleWorkflow.DoesNotExist: # XXX: this means the workflow was single written and has no ARW or related Rule object diff --git a/src/sentry/incidents/endpoints/bases.py b/src/sentry/incidents/endpoints/bases.py index 17df173ad1e3..7e9c70aaf975 100644 --- a/src/sentry/incidents/endpoints/bases.py +++ b/src/sentry/incidents/endpoints/bases.py @@ -1,7 +1,6 @@ from typing import Any -from rest_framework import status -from rest_framework.exceptions import APIException, PermissionDenied +from rest_framework.exceptions import PermissionDenied from rest_framework.request import Request from sentry import features @@ -17,28 +16,6 @@ from sentry.workflow_engine.models.detector import Detector -class SharedDetectorError(APIException): - status_code = status.HTTP_400_BAD_REQUEST - - def __init__(self, detector_id: int) -> None: - detail = ( - f"Detector {detector_id} is shared with other rules. Use the detector API to manage it." - ) - super().__init__(detail=detail) - - -def _check_shared_detector(request: Request, ard: AlertRuleDetector) -> None: - """Reject PUT/DELETE on a Detector that is shared with other rules.""" - if request.method in ("PUT", "DELETE"): - has_other_links = ( - AlertRuleDetector.objects.filter(detector_id=ard.detector_id) - .exclude(id=ard.id) - .exists() - ) - if has_other_links: - raise SharedDetectorError(ard.detector_id) - - class OrganizationAlertRuleBaseEndpoint(OrganizationEndpoint): """ Base endpoint for organization-scoped alert rule creation. @@ -154,7 +131,6 @@ def convert_args( alert_rule_id=validated_alert_rule_id, detector__project=project, ) - _check_shared_detector(request, ard) kwargs["alert_rule"] = ard.detector except AlertRuleDetector.DoesNotExist: # XXX: this means the detector was single written and has no ARD or related AlertRule object @@ -203,7 +179,6 @@ def convert_args( alert_rule_id=validated_alert_rule_id, detector__project__organization=organization, ) - _check_shared_detector(request, ard) kwargs["alert_rule"] = ard.detector except AlertRuleDetector.DoesNotExist: # XXX: this means the detector was single written and has no ARD or related AlertRule object diff --git a/tests/sentry/api/endpoints/test_project_rule_details.py b/tests/sentry/api/endpoints/test_project_rule_details.py index 7cf287499ffc..ffd9c7ac194d 100644 --- a/tests/sentry/api/endpoints/test_project_rule_details.py +++ b/tests/sentry/api/endpoints/test_project_rule_details.py @@ -1473,27 +1473,6 @@ def test_delete_does_not_cascade_to_cross_org_rule(self) -> None: other_rule.refresh_from_db() assert other_rule.status == ObjectStatus.ACTIVE - def test_delete_shared_workflow_returns_400(self) -> None: - rule = self.create_project_rule(self.project) - arw = AlertRuleWorkflow.objects.get(rule_id=rule.id) - workflow = arw.workflow - - # Create another Rule linked to the same Workflow. - other_rule = self.create_project_rule(self.project) - other_arw = AlertRuleWorkflow.objects.get(rule_id=other_rule.id) - other_arw.update(workflow_id=workflow.id) - - response = self.get_error_response( - self.organization.slug, self.project.slug, rule.id, status_code=400 - ) - assert "Workflow" in response.data["detail"] - assert str(workflow.id) in response.data["detail"] - - # Neither rule nor workflow were deleted. - rule.refresh_from_db() - assert rule.status == ObjectStatus.ACTIVE - assert Workflow.objects.filter(id=workflow.id).exists() - class GetProjectRuleDetailsDeltaTest(ProjectRuleDetailsBaseTestCase): def test_dual_written_rule_parity(self) -> None: diff --git a/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py b/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py index 16fc712af98e..a8666ec55804 100644 --- a/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py +++ b/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py @@ -2757,24 +2757,3 @@ def test_dual_delete_detector_id_passed(self) -> None: assert not AlertRule.objects_with_snapshots.filter(name=self.alert_rule.name).exists() assert not AlertRule.objects_with_snapshots.filter(id=self.alert_rule.id).exists() assert not Detector.objects.filter(id=ard.detector_id).exists() - - @with_feature("organizations:workflow-engine-rule-serializers") - def test_delete_shared_detector_returns_400(self) -> None: - self.create_member( - user=self.user, organization=self.organization, role="owner", teams=[self.team] - ) - self.login_as(self.user) - - ard = AlertRuleDetector.objects.get(alert_rule_id=self.alert_rule.id) - # Create another Rule linked to the same Detector. - AlertRuleDetector.objects.create(rule_id=999999, detector_id=ard.detector_id) - - with self.feature("organizations:incidents"): - resp = self.get_error_response( - self.organization.slug, self.alert_rule.id, status_code=400 - ) - - assert "Detector" in resp.data["detail"] - assert str(ard.detector_id) in resp.data["detail"] - # AlertRule was not deleted. - assert AlertRule.objects.filter(id=self.alert_rule.id).exists()