diff --git a/.agents/skills/hybrid-cloud-outboxes/references/debugging.md b/.agents/skills/hybrid-cloud-outboxes/references/debugging.md index 96e54c23ecd0..bfeb512577fc 100644 --- a/.agents/skills/hybrid-cloud-outboxes/references/debugging.md +++ b/.agents/skills/hybrid-cloud-outboxes/references/debugging.md @@ -6,7 +6,7 @@ Understanding the pipeline helps locate where things break: 1. **Model save/delete** writes outbox row inside `outbox_context(transaction.atomic(...))` 2. **On commit**: if `flush=True`, `drain_shard()` runs synchronously for that shard -3. **Periodic task**: `enqueue_outbox_jobs` (region) / `enqueue_outbox_jobs_control` (control) runs on a cron schedule +3. **Periodic task**: `enqueue_outbox_jobs` (cell) / `enqueue_outbox_jobs_control` (control) runs on a cron schedule 4. **`schedule_batch`** partitions the ID range into `CONCURRENCY=5` chunks and spawns `drain_outbox_shards` tasks 5. **`drain_outbox_shards`** calls `process_outbox_batch` which: - Calls `find_scheduled_shards(lo, hi)` to find shards with `scheduled_for <= now` @@ -71,9 +71,9 @@ Both tables share these columns: `sentry_controloutbox` has one additional column: -| Column | Type | Description | -| ------------- | ------- | ----------------------------- | -| `region_name` | varchar | Target region for this outbox | +| Column | Type | Description | +| ------------- | ------- | --------------------------- | +| `region_name` | varchar | Target cell for this outbox | ### Resolving Enum Values @@ -92,10 +92,10 @@ When generating SQL for a developer, **print the query to the terminal** so they 2. Comments mapping integer values to their enum names 3. Reasonable `LIMIT` clauses to avoid overwhelming output -#### Find stuck shards (region) +#### Find stuck shards (cell) ```sql --- Find region outbox shards stuck in backoff +-- Find cell outbox shards stuck in backoff -- shard_scope: 0 = ORGANIZATION_SCOPE, 1 = USER_SCOPE, etc. -- category: see OutboxCategory enum in category.py SELECT @@ -199,7 +199,7 @@ LIMIT 10; When a developer asks you to debug stuck outboxes: -1. **Determine the table**: Ask which model or direction is involved, or infer from context. Use `sentry_regionoutbox` for region models, `sentry_controloutbox` for control models. +1. **Determine the table**: Ask which model or direction is involved, or infer from context. Use `sentry_regionoutbox` for cell models, `sentry_controloutbox` for control models. 2. **Resolve enum values**: Read `src/sentry/hybridcloud/outbox/category.py` to get the integer values for the relevant `OutboxCategory` and `OutboxScope`. 3. **Construct the query**: Use the templates above, substituting resolved values. Always add comments with the human-readable enum names. 4. **Print to terminal**: Output the final SQL so the developer can copy it. Do NOT attempt to run it — you don't have production database access. @@ -212,10 +212,10 @@ When a developer asks you to debug stuck outboxes: The `should_skip_shard()` method checks these options: ```python -# Skip specific organization shards (region outboxes) +# Skip specific organization shards (cell outboxes) "hybrid_cloud.authentication.disabled_organization_shards": [org_id_1, org_id_2] -# Skip specific user shards (region/control outboxes) +# Skip specific user shards (cell/control outboxes) "hybrid_cloud.authentication.disabled_user_shards": [user_id_1, user_id_2] ``` @@ -252,7 +252,7 @@ For local debugging or in a Django shell: ```python from sentry.hybridcloud.models.outbox import CellOutbox, ControlOutbox -# Top 10 deepest region shards +# Top 10 deepest cell shards for shard in CellOutbox.get_shard_depths_descending(limit=10): print(f"Scope={shard['shard_scope']} ID={shard['shard_identifier']} Depth={shard['depth']}") ``` diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 13781d0ce9ff..a2f08ede5ed7 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -489,6 +489,11 @@ tests/sentry/api/endpoints/test_organization_attribute_mappings.py @get *sentryapp*.py @getsentry/product-owners-settings-integrations @getsentry/ecosystem *doc_integration*.py @getsentry/ecosystem +/src/sentry/runner/commands/createproject.py @getsentry/ecosystem +/src/sentry/runner/commands/createorg.py @getsentry/ecosystem +/tests/sentry/runner/commands/test_createproject.py @getsentry/ecosystem +/tests/sentry/runner/commands/test_createorg.py @getsentry/ecosystem + ## End of Integrations @@ -673,12 +678,14 @@ tests/sentry/api/endpoints/test_organization_attribute_mappings.py @get /static/app/components/events/highlights/ @getsentry/issue-workflow /static/app/components/issues/ @getsentry/issue-workflow /static/app/components/stackTrace/ @getsentry/issue-workflow +/static/app/components/stream/supergroupRow.tsx @getsentry/issue-detection-frontend /static/app/views/issueList/ @getsentry/issue-workflow /static/app/views/issueList/issueListSeerComboBox.tsx @getsentry/issue-workflow @getsentry/machine-learning-ai /static/app/views/issueList/pages/supergroups.tsx @getsentry/issue-detection-frontend /static/app/views/issueList/supergroups/ @getsentry/issue-detection-frontend /static/app/views/issueDetails/ @getsentry/issue-workflow /static/app/views/nav/secondary/sections/issues/ @getsentry/issue-workflow +/static/app/views/sharedGroupDetails/ @getsentry/issue-workflow /static/app/components/events/interfaces/performance/spanEvidenceKeyValueList.tsx @getsentry/issue-detection-frontend /static/app/components/events/interfaces/crashContent/exception/actionableItems.tsx @getsentry/issue-workflow /tests/sentry/deletions/test_group.py @getsentry/issue-detection-backend diff --git a/.github/codeowners-coverage-baseline.txt b/.github/codeowners-coverage-baseline.txt index 156f74abe3fb..c7b63d380a87 100644 --- a/.github/codeowners-coverage-baseline.txt +++ b/.github/codeowners-coverage-baseline.txt @@ -184,6 +184,7 @@ src/sentry/runner/commands/cleanup.py src/sentry/runner/commands/config.py src/sentry/runner/commands/configoptions.py src/sentry/runner/commands/createflag.py +src/sentry/runner/commands/createorg.py src/sentry/runner/commands/createuser.py src/sentry/runner/commands/devserver.py src/sentry/runner/commands/devservices.py @@ -225,9 +226,6 @@ src/sentry/scripts/quotas/is_rate_limited.lua src/sentry/scripts/ratelimits/api_limiter.lua src/sentry/scripts/ratelimits/leaky_bucket.lua src/sentry/scripts/similarity/index.lua -src/sentry/scripts/spans/add-buffer.lua -src/sentry/scripts/spans/done-flush-segment-data.lua -src/sentry/scripts/spans/done-flush-segment.lua src/sentry/scripts/tsdb/cmsketch.lua src/sentry/scripts/utils/locking/delete_lock.lua src/sentry/scripts/utils/sadd_capped.lua @@ -552,7 +550,6 @@ static/app/components/discover/transactionsList.spec.tsx static/app/components/discover/transactionsList.tsx static/app/components/discover/transactionsTable.tsx static/app/components/discoverButton.tsx -static/app/components/dnd/dragReorderButton.tsx static/app/components/dropdownButton.tsx static/app/components/dropdownMenu/footer.tsx static/app/components/dropdownMenu/index.spec.tsx @@ -605,14 +602,9 @@ static/app/components/events/eventAttachmentActions.tsx static/app/components/events/eventAttachments.spec.tsx static/app/components/events/eventAttachments.tsx static/app/components/events/eventAttachmentsCrashReportsNotice.tsx -static/app/components/events/eventBreadcrumbsSection.spec.tsx -static/app/components/events/eventBreadcrumbsSection.tsx static/app/components/events/eventCustomPerformanceMetrics.tsx static/app/components/events/eventDataSection.tsx static/app/components/events/eventDrawer.tsx -static/app/components/events/eventEntries.spec.tsx -static/app/components/events/eventEntries.tsx -static/app/components/events/eventEntry.tsx static/app/components/events/eventEvidence.spec.tsx static/app/components/events/eventEvidence.tsx static/app/components/events/eventExtraData/getEventExtraDataKnownDataDetails.tsx @@ -1023,18 +1015,6 @@ static/app/components/replaysOnboarding/replayConfigToggle.tsx static/app/components/replaysOnboarding/replayOnboardingLayout.tsx static/app/components/replaysOnboarding/sidebar.tsx static/app/components/replaysOnboarding/utils.tsx -static/app/components/repositories/getRepoStatusLabel.tsx -static/app/components/repositories/repoProviderIcon.tsx -static/app/components/repositories/scmIntegrationTree/providerConfigLink.tsx -static/app/components/repositories/scmIntegrationTree/scmIntegrationTree.tsx -static/app/components/repositories/scmIntegrationTree/scmIntegrationTreeNodes.ts -static/app/components/repositories/scmIntegrationTree/scmIntegrationTreeRow.tsx -static/app/components/repositories/scmIntegrationTree/scmTreeFilters.tsx -static/app/components/repositories/scmIntegrationTree/types.ts -static/app/components/repositories/scmIntegrationTree/useScmIntegrationTreeData.spec.tsx -static/app/components/repositories/scmIntegrationTree/useScmIntegrationTreeData.ts -static/app/components/repositories/scmIntegrationTree/useScmTreeFilters.tsx -static/app/components/repositories/scmRepoTreeModal.tsx static/app/components/repositoryRow.spec.tsx static/app/components/repositoryRow.tsx static/app/components/resolutionBox.spec.tsx @@ -1489,9 +1469,6 @@ static/app/views/setupWizard/utils/useOrganizationsWithRegion.tsx static/app/views/setupWizard/utils/useUpdateWizardCache.tsx static/app/views/setupWizard/waitingForWizardToConnect.tsx static/app/views/setupWizard/wizardProjectSelection.tsx -static/app/views/sharedGroupDetails/index.spec.tsx -static/app/views/sharedGroupDetails/index.tsx -static/app/views/sharedGroupDetails/sharedGroupHeader.tsx static/app/views/unsubscribe/issue.spec.tsx static/app/views/unsubscribe/issue.tsx static/app/views/unsubscribe/project.spec.tsx @@ -2318,6 +2295,7 @@ tests/sentry/runner/commands/test_cleanup.py tests/sentry/runner/commands/test_config.py tests/sentry/runner/commands/test_configoptions.py tests/sentry/runner/commands/test_createflag.py +tests/sentry/runner/commands/test_createorg.py tests/sentry/runner/commands/test_createuser.py tests/sentry/runner/commands/test_init.py tests/sentry/runner/commands/test_migrations.py diff --git a/.github/workflows/backend-selective.yml b/.github/workflows/backend-selective.yml index 602f7a12a6ed..88a1ae68ed80 100644 --- a/.github/workflows/backend-selective.yml +++ b/.github/workflows/backend-selective.yml @@ -179,6 +179,9 @@ jobs: env: MATRIX_INSTANCE_TOTAL: ${{ needs.calculate-shards.outputs.shard-count }} TEST_GROUP_STRATEGY: roundrobin + PYTHONHASHSEED: '0' + XDIST_PER_WORKER_SNUBA: '1' + XDIST_WORKERS: '2' steps: - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7 @@ -195,6 +198,58 @@ jobs: sudo install -m 755 odiff-linux-x64 /usr/local/bin/odiff rm odiff-linux-x64 + - name: Bootstrap per-worker Snuba instances + run: | + set -eo pipefail + SNUBA_IMAGE=$(docker inspect snuba-snuba-1 --format '{{.Config.Image}}') + SNUBA_NETWORK=$(docker inspect snuba-snuba-1 --format '{{range $k, $v := .NetworkSettings.Networks}}{{$k}}{{end}}') + if [ -z "$SNUBA_IMAGE" ] || [ -z "$SNUBA_NETWORK" ]; then + echo "ERROR: Could not inspect snuba-snuba-1 container. Is devservices running?" + exit 1 + fi + + docker stop snuba-snuba-1 || true + + PIDS=() + for i in $(seq 0 $(( ${XDIST_WORKERS} - 1 ))); do + ( + WORKER_DB="default_gw${i}" + WORKER_PORT=$((1230 + i)) + curl -sf 'http://localhost:8123/' --data-binary "CREATE DATABASE IF NOT EXISTS ${WORKER_DB}" + docker run --rm --network "$SNUBA_NETWORK" \ + -e "CLICKHOUSE_DATABASE=${WORKER_DB}" -e "CLICKHOUSE_HOST=clickhouse" \ + -e "CLICKHOUSE_PORT=9000" -e "CLICKHOUSE_HTTP_PORT=8123" \ + -e "DEFAULT_BROKERS=kafka:9093" -e "REDIS_HOST=redis" \ + -e "REDIS_PORT=6379" -e "REDIS_DB=1" -e "SNUBA_SETTINGS=docker" \ + "$SNUBA_IMAGE" bootstrap --force 2>&1 | tail -3 + docker run -d --name "snuba-gw${i}" --network "$SNUBA_NETWORK" \ + -p "${WORKER_PORT}:1218" \ + -e "CLICKHOUSE_DATABASE=${WORKER_DB}" -e "CLICKHOUSE_HOST=clickhouse" \ + -e "CLICKHOUSE_PORT=9000" -e "CLICKHOUSE_HTTP_PORT=8123" \ + -e "DEFAULT_BROKERS=kafka:9093" -e "REDIS_HOST=redis" \ + -e "REDIS_PORT=6379" -e "REDIS_DB=1" -e "SNUBA_SETTINGS=docker" \ + -e "DEBUG=1" "$SNUBA_IMAGE" api + + for attempt in $(seq 1 30); do + if curl -sf "http://127.0.0.1:${WORKER_PORT}/health" > /dev/null 2>&1; then + echo "snuba-gw${i} healthy on port ${WORKER_PORT}" + break + fi + if [ "$attempt" -eq 30 ]; then + echo "ERROR: snuba-gw${i} failed health check after 30 attempts" + docker logs "snuba-gw${i}" 2>&1 | tail -20 || true + exit 1 + fi + sleep 2 + done + ) & + PIDS+=($!) + done + + for pid in "${PIDS[@]}"; do + wait "$pid" || { echo "ERROR: Snuba bootstrap subshell (PID $pid) failed"; exit 1; } + done + - name: Download selected tests artifact if: needs.select-tests.outputs.has-selected-tests == 'true' uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4.3.0 @@ -206,6 +261,7 @@ jobs: env: SELECTED_TESTS_FILE: ${{ needs.select-tests.outputs.has-selected-tests == 'true' && '.artifacts/selected-tests.txt' || '' }} run: | + export PYTEST_ADDOPTS="$PYTEST_ADDOPTS -n ${XDIST_WORKERS} --dist=loadfile" make test-python-ci - name: Inspect failure @@ -214,3 +270,8 @@ jobs: if command -v devservices; then devservices logs fi + + for i in $(seq 0 $(( ${XDIST_WORKERS} - 1 ))); do + echo "--- snuba-gw${i} logs ---" + docker logs "snuba-gw${i}" 2>&1 | tail -30 || true + done diff --git a/.github/workflows/backend.yml b/.github/workflows/backend.yml index b39dd84d6ce0..a37fd5da08c6 100644 --- a/.github/workflows/backend.yml +++ b/.github/workflows/backend.yml @@ -272,10 +272,9 @@ jobs: # Dynamic total from calculate-shards MATRIX_INSTANCE_TOTAL: ${{ needs.calculate-shards.outputs.shard-count }} TEST_GROUP_STRATEGY: roundrobin - # xdist: enabled on master pushes and workflow_dispatch, disabled on PRs - PYTHONHASHSEED: ${{ github.event_name != 'pull_request' && '0' || 'random' }} - XDIST_PER_WORKER_SNUBA: ${{ github.event_name != 'pull_request' && '1' || '' }} - XDIST_WORKERS: ${{ github.event_name != 'pull_request' && '3' || '' }} + PYTHONHASHSEED: '0' + XDIST_PER_WORKER_SNUBA: '1' + XDIST_WORKERS: ${{ github.event_name == 'pull_request' && '2' || '3' }} steps: - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7 diff --git a/src/sentry/api/endpoints/project_rules.py b/src/sentry/api/endpoints/project_rules.py index 3b569a15da33..7b882f5be412 100644 --- a/src/sentry/api/endpoints/project_rules.py +++ b/src/sentry/api/endpoints/project_rules.py @@ -3,7 +3,6 @@ from typing import Any, Literal, NotRequired, TypedDict, cast from django.conf import settings -from django.db import router, transaction from django.db.models.signals import pre_save from django.dispatch import receiver from drf_spectacular.utils import extend_schema @@ -53,9 +52,6 @@ WorkflowInput, WorkflowValidator, ) -from sentry.workflow_engine.endpoints.validators.detector_workflow import ( - BulkWorkflowDetectorsValidator, -) from sentry.workflow_engine.migration_helpers.issue_alert_conditions import ( translate_to_data_condition_data, ) @@ -819,6 +815,11 @@ def format_request_data( } workflow_payload["actionFilters"] = [action_filters] + issue_stream_detector = Detector.get_issue_stream_detector_for_project(project.id) + if issue_stream_detector is None: + raise serializers.ValidationError("Could not find issue stream detector for project") + workflow_payload["detectorIds"] = [issue_stream_detector.id] + return workflow_payload @@ -922,23 +923,7 @@ def post(self, request: Request, project) -> Response: context={"organization": project.organization, "request": request}, ) validator.is_valid(raise_exception=True) - - with transaction.atomic(router.db_for_write(Workflow)): - workflow = validator.create(validator.validated_data) - issue_stream_detector = Detector.get_issue_stream_detector_for_project(project.id) - if issue_stream_detector is None: - raise serializers.ValidationError( - "Could not find issue stream detector for project" - ) - bulk_validator = BulkWorkflowDetectorsValidator( - data={ - "workflow_id": workflow.id, - "detector_ids": [issue_stream_detector.id], - }, - context={"organization": project.organization, "request": request}, - ) - bulk_validator.is_valid(raise_exception=True) - bulk_validator.save() + workflow = validator.create(validator.validated_data) return Response( serialize(workflow, request.user, WorkflowEngineRuleSerializer()), diff --git a/src/sentry/api/endpoints/user_organizations.py b/src/sentry/api/endpoints/user_organizations.py index 988f36e9192d..c931fa6e80c0 100644 --- a/src/sentry/api/endpoints/user_organizations.py +++ b/src/sentry/api/endpoints/user_organizations.py @@ -13,6 +13,8 @@ from sentry.users.services.user import RpcUser +# TODO(cells): Non-routable by Synapse (no org slug in URL). Fix by moving to +# @control_silo_endpoint and querying OrganizationMemberMapping + OrganizationMapping. @cell_silo_endpoint class UserOrganizationsEndpoint(RegionSiloUserEndpoint): publish_status = { diff --git a/src/sentry/api/serializers/models/project.py b/src/sentry/api/serializers/models/project.py index 2d0561e95402..5d3b8c7a1919 100644 --- a/src/sentry/api/serializers/models/project.py +++ b/src/sentry/api/serializers/models/project.py @@ -1174,6 +1174,15 @@ def format_options(self, attrs: Mapping[str, Any]) -> dict[str, Any]: "sentry:preprod_size_status_checks_rules": options.get( "sentry:preprod_size_status_checks_rules" ), + "sentry:preprod_snapshot_status_checks_enabled": options.get( + "sentry:preprod_snapshot_status_checks_enabled", True + ), + "sentry:preprod_snapshot_status_checks_fail_on_added": options.get( + "sentry:preprod_snapshot_status_checks_fail_on_added", False + ), + "sentry:preprod_snapshot_status_checks_fail_on_removed": options.get( + "sentry:preprod_snapshot_status_checks_fail_on_removed", True + ), "quotas:spike-protection-disabled": options.get("quotas:spike-protection-disabled"), "sentry:preprod_size_enabled_query": options.get("sentry:preprod_size_enabled_query"), "sentry:preprod_distribution_enabled_query": options.get( diff --git a/src/sentry/api/validators/project_codeowners.py b/src/sentry/api/validators/project_codeowners.py index cf90dec4248f..c022163de022 100644 --- a/src/sentry/api/validators/project_codeowners.py +++ b/src/sentry/api/validators/project_codeowners.py @@ -2,13 +2,11 @@ from collections import defaultdict from collections.abc import Collection, Mapping, Sequence -from functools import reduce -from operator import or_ from typing import Any -from django.db.models import Subquery -from django.db.models.query_utils import Q +from django.db.models.functions import Lower +from sentry.constants import ObjectStatus from sentry.integrations.models.external_actor import ExternalActor from sentry.integrations.types import ExternalProviders from sentry.issues.ownership.grammar import parse_code_owners @@ -41,22 +39,23 @@ def build_codeowners_associations( filter=dict(emails=emails, organization_id=project.organization_id) ) - # GitHub team and user names are case-insensitive - # We build a query that filters on each name we parsed case-insensitively - queries = [Q(external_name__iexact=xname) for xname in usernames + team_names] - if queries: - query = reduce(or_, queries) - external_actors = ExternalActor.objects.filter( - query, - organization_id=project.organization_id, - provider__in=[ - ExternalProviders.GITHUB.value, - ExternalProviders.GITHUB_ENTERPRISE.value, - ExternalProviders.GITLAB.value, - ], + # GitHub team and user names are case-insensitive. + # Deduplicate and lowercase names, then use a single IN query to filter. + unique_lower_names = {name.lower() for name in usernames + team_names} + if unique_lower_names: + external_actors = list( + ExternalActor.objects.annotate(external_name_lower=Lower("external_name")).filter( + external_name_lower__in=unique_lower_names, + organization_id=project.organization_id, + provider__in=[ + ExternalProviders.GITHUB.value, + ExternalProviders.GITHUB_ENTERPRISE.value, + ExternalProviders.GITLAB.value, + ], + ) ) else: - external_actors = ExternalActor.objects.none() + external_actors = [] # Convert CODEOWNERS into IssueOwner syntax users_dict = {} @@ -70,38 +69,67 @@ def build_codeowners_associations( team_ids_to_external_names: dict[int, list[str]] = defaultdict(list) user_ids_to_external_names: dict[int, list[str]] = defaultdict(list) + # Pre-build lowercase -> original name mappings + lower_to_team_names: dict[str, list[str]] = defaultdict(list) + for name in team_names: + lower_to_team_names[name.lower()].append(name) + lower_to_usernames: dict[str, list[str]] = defaultdict(list) + for name in usernames: + lower_to_usernames[name.lower()].append(name) + for xa in external_actors: + xa_lower = xa.external_name.lower() if xa.team_id is not None: - team_ids_to_external_names[xa.team_id].extend( - filter(lambda team_name: team_name.lower() == xa.external_name.lower(), team_names) - ) + team_ids_to_external_names[xa.team_id].extend(lower_to_team_names.get(xa_lower, [])) if xa.user_id is not None: - user_ids_to_external_names[xa.user_id].extend( - filter(lambda username: username.lower() == xa.external_name.lower(), usernames) - ) + user_ids_to_external_names[xa.user_id].extend(lower_to_usernames.get(xa_lower, [])) + + # Determine which matched users have project access via team membership + user_ids_with_access: set[int] = set() + user_ids = list(user_ids_to_external_names.keys()) + if user_ids and project.status == ObjectStatus.ACTIVE: + om_rows = list( + OrganizationMember.objects.filter( + user_id__in=user_ids, + organization_id=project.organization_id, + ).values_list("id", "user_id") + ) + om_id_to_user_id: dict[int, int] = {om_id: uid for om_id, uid in om_rows if uid is not None} + + omt_rows = OrganizationMemberTeam.objects.filter( + organizationmember_id__in=list(om_id_to_user_id.keys()) + ).values_list("organizationmember_id", "team_id") - for user in user_service.get_many( - filter=dict(user_ids=list(user_ids_to_external_names.keys())) - ): - organization_members_ids = OrganizationMember.objects.filter( - user_id=user.id, organization_id=project.organization_id - ).values_list("id", flat=True) - team_ids = OrganizationMemberTeam.objects.filter( - organizationmember_id__in=Subquery(organization_members_ids) - ).values_list("team_id", flat=True) - projects = Project.objects.get_for_team_ids(Subquery(team_ids)) - - if project in projects: + user_team_ids: dict[int, set[int]] = defaultdict(set) + for om_id, team_id in omt_rows: + user_team_ids[om_id_to_user_id[om_id]].add(team_id) + + all_team_ids = {tid for tids in user_team_ids.values() for tid in tids} + project_team_ids = set( + project.teams.filter(id__in=all_team_ids).values_list("id", flat=True) + ) + user_ids_with_access = { + uid for uid, tids in user_team_ids.items() if tids & project_team_ids + } + + for user in user_service.get_many(filter=dict(user_ids=user_ids)): + if user.id in user_ids_with_access: for external_name in user_ids_to_external_names[user.id]: users_dict[external_name] = user.email else: users_without_access.add(f"{user.get_display_name()}") users_without_access_external_names.update(user_ids_to_external_names[user.id]) - for team in Team.objects.filter(id__in=list(team_ids_to_external_names.keys())): - # make sure the sentry team has access to the project - # tied to the codeowner - if project in team.get_projects(): + # Determine which matched teams have project access + team_ids_list = list(team_ids_to_external_names.keys()) + team_ids_with_access = ( + set(project.teams.filter(id__in=team_ids_list).values_list("id", flat=True)) + if team_ids_list and project.status == ObjectStatus.ACTIVE + else set() + ) + + for team in Team.objects.filter(id__in=team_ids_list): + if team.id in team_ids_with_access: for external_name in team_ids_to_external_names[team.id]: teams_dict[external_name] = f"#{team.slug}" else: diff --git a/src/sentry/core/endpoints/project_details.py b/src/sentry/core/endpoints/project_details.py index a4ecd79c357f..94f73a7923f9 100644 --- a/src/sentry/core/endpoints/project_details.py +++ b/src/sentry/core/endpoints/project_details.py @@ -111,6 +111,9 @@ class ProjectMemberSerializer(serializers.Serializer): required=False, ) preprodSizeStatusChecksRules = serializers.JSONField(required=False) + preprodSnapshotStatusChecksEnabled = serializers.BooleanField(required=False) + preprodSnapshotStatusChecksFailOnAdded = serializers.BooleanField(required=False) + preprodSnapshotStatusChecksFailOnRemoved = serializers.BooleanField(required=False) preprodSizeEnabledByCustomer = serializers.BooleanField(required=False, allow_null=True) preprodDistributionEnabledByCustomer = serializers.BooleanField(required=False, allow_null=True) preprodDistributionPrCommentsEnabledByCustomer = serializers.BooleanField( @@ -160,6 +163,9 @@ class ProjectMemberSerializer(serializers.Serializer): "preprodDistributionEnabledQuery", "preprodSizeEnabledByCustomer", "preprodDistributionEnabledByCustomer", + "preprodSnapshotStatusChecksEnabled", + "preprodSnapshotStatusChecksFailOnAdded", + "preprodSnapshotStatusChecksFailOnRemoved", "preprodDistributionPrCommentsEnabledByCustomer", ] ) @@ -831,6 +837,30 @@ def put(self, request: Request, project) -> Response: changed_proj_settings["sentry:preprod_distribution_enabled_query"] = result[ "preprodDistributionEnabledQuery" ] + if result.get("preprodSnapshotStatusChecksEnabled") is not None: + if project.update_option( + "sentry:preprod_snapshot_status_checks_enabled", + result["preprodSnapshotStatusChecksEnabled"], + ): + changed_proj_settings["sentry:preprod_snapshot_status_checks_enabled"] = result[ + "preprodSnapshotStatusChecksEnabled" + ] + if result.get("preprodSnapshotStatusChecksFailOnAdded") is not None: + if project.update_option( + "sentry:preprod_snapshot_status_checks_fail_on_added", + result["preprodSnapshotStatusChecksFailOnAdded"], + ): + changed_proj_settings["sentry:preprod_snapshot_status_checks_fail_on_added"] = ( + result["preprodSnapshotStatusChecksFailOnAdded"] + ) + if result.get("preprodSnapshotStatusChecksFailOnRemoved") is not None: + if project.update_option( + "sentry:preprod_snapshot_status_checks_fail_on_removed", + result["preprodSnapshotStatusChecksFailOnRemoved"], + ): + changed_proj_settings["sentry:preprod_snapshot_status_checks_fail_on_removed"] = ( + result["preprodSnapshotStatusChecksFailOnRemoved"] + ) if "preprodDistributionPrCommentsEnabledByCustomer" in result: if project.update_option( "sentry:preprod_distribution_pr_comments_enabled_by_customer", diff --git a/src/sentry/dashboards/endpoints/organization_dashboard_generate.py b/src/sentry/dashboards/endpoints/organization_dashboard_generate.py index 9b9b2d11df40..fcd96d724318 100644 --- a/src/sentry/dashboards/endpoints/organization_dashboard_generate.py +++ b/src/sentry/dashboards/endpoints/organization_dashboard_generate.py @@ -18,8 +18,8 @@ from sentry.models.organization import Organization from sentry.ratelimits.config import RateLimitConfig from sentry.seer.explorer.client import SeerExplorerClient -from sentry.seer.explorer.client_utils import has_seer_explorer_access_with_detail from sentry.seer.models import SeerApiError, SeerPermissionError +from sentry.seer.seer_setup import has_seer_access_with_detail from sentry.types.ratelimit import RateLimit, RateLimitCategory logger = logging.getLogger(__name__) @@ -63,7 +63,7 @@ def post(self, request: Request, organization: Organization) -> Response: ): return Response({"detail": "Feature not enabled"}, status=403) - has_access, error = has_seer_explorer_access_with_detail(organization, request.user) + has_access, error = has_seer_access_with_detail(organization, request.user) if not has_access: raise PermissionDenied(error) diff --git a/src/sentry/hybridcloud/services/control_organization_provisioning/model.py b/src/sentry/hybridcloud/services/control_organization_provisioning/model.py index 90806036796a..5ade515f7349 100644 --- a/src/sentry/hybridcloud/services/control_organization_provisioning/model.py +++ b/src/sentry/hybridcloud/services/control_organization_provisioning/model.py @@ -1,7 +1,3 @@ -from typing import Any - -from pydantic import root_validator - from sentry.hybridcloud.rpc import RpcModel @@ -12,14 +8,3 @@ class RpcOrganizationSlugReservation(RpcModel): slug: str cell_name: str reservation_type: int - - @root_validator(pre=True) - @classmethod - def _accept_region_name(cls, values: dict[str, Any]) -> dict[str, Any]: - if "region_name" in values and "cell_name" not in values: - values["cell_name"] = values.pop("region_name") - return values - - @property - def region_name(self) -> str: - return self.cell_name diff --git a/src/sentry/hybridcloud/services/replica/impl.py b/src/sentry/hybridcloud/services/replica/impl.py index 1b2bac5572e5..e5c6272d8c62 100644 --- a/src/sentry/hybridcloud/services/replica/impl.py +++ b/src/sentry/hybridcloud/services/replica/impl.py @@ -296,7 +296,7 @@ def upsert_replicated_org_slug_reservation( slug=slug_reservation.slug, organization_id=slug_reservation.organization_id, user_id=slug_reservation.user_id, - cell_name=slug_reservation.region_name, + cell_name=slug_reservation.cell_name, reservation_type=slug_reservation.reservation_type, organization_slug_reservation_id=slug_reservation.id, ) diff --git a/src/sentry/identity/oauth2.py b/src/sentry/identity/oauth2.py index fa36cc207a09..03f59c1c0a41 100644 --- a/src/sentry/identity/oauth2.py +++ b/src/sentry/identity/oauth2.py @@ -32,6 +32,7 @@ IntegrationPipelineViewEvent, IntegrationPipelineViewType, ) +from sentry.pipeline.base import Pipeline from sentry.pipeline.types import PipelineStepResult from sentry.pipeline.views.base import PipelineView from sentry.shared_integrations.exceptions import ApiError, ApiInvalidRequestError, ApiUnauthorized @@ -282,7 +283,7 @@ def __init__( self.bind_key = bind_key self.extra_authorize_params = extra_authorize_params or {} - def get_step_data(self, pipeline: Any, request: HttpRequest) -> dict[str, str]: + def get_step_data(self, pipeline: Pipeline[Any, Any], request: HttpRequest) -> dict[str, str]: params = urlencode( { "client_id": self.client_id, @@ -301,7 +302,7 @@ def get_serializer_cls(self) -> type: def handle_post( self, validated_data: dict[str, str], - pipeline: Any, + pipeline: Pipeline[Any, Any], request: HttpRequest, ) -> PipelineStepResult: code = validated_data["code"] diff --git a/src/sentry/incidents/endpoints/serializers/workflow_engine_action.py b/src/sentry/incidents/endpoints/serializers/workflow_engine_action.py index 55f5139e6cf2..21383b0dc447 100644 --- a/src/sentry/incidents/endpoints/serializers/workflow_engine_action.py +++ b/src/sentry/incidents/endpoints/serializers/workflow_engine_action.py @@ -29,7 +29,14 @@ def get_attrs( action__in=[item.id for item in item_list] ) } - return {item: {"aarta": aarta_by_action_id.get(item.id)} for item in item_list} + targets_by_action_id = MetricAlertRegistryHandler.get_targets(item_list) + return { + item: { + "aarta": aarta_by_action_id.get(item.id), + "target": targets_by_action_id.get(item.id), + } + for item in item_list + } def serialize( self, obj: Action, attrs: Mapping[str, Any], user: User | RpcUser | AnonymousUser, **kwargs @@ -44,7 +51,7 @@ def serialize( aarta = attrs.get("aarta") priority = obj.data.get("priority") type_value = ActionService.get_value(obj.type) - target = MetricAlertRegistryHandler.target(obj) + target = attrs.get("target") target_type = obj.config.get("target_type") target_identifier = obj.config.get("target_identifier") diff --git a/src/sentry/incidents/endpoints/serializers/workflow_engine_data_condition.py b/src/sentry/incidents/endpoints/serializers/workflow_engine_data_condition.py index e6de3a7444cc..4f4b0f31af78 100644 --- a/src/sentry/incidents/endpoints/serializers/workflow_engine_data_condition.py +++ b/src/sentry/incidents/endpoints/serializers/workflow_engine_data_condition.py @@ -1,5 +1,5 @@ from collections import defaultdict -from collections.abc import Mapping, Sequence +from collections.abc import Mapping, MutableMapping, Sequence from typing import Any from django.contrib.auth.models import AnonymousUser @@ -34,19 +34,19 @@ def get_attrs( item_list: Sequence[DataCondition], user: User | RpcUser | AnonymousUser, **kwargs: Any, - ) -> defaultdict[DataCondition, dict[str, list[str]]]: + ) -> MutableMapping[DataCondition, Any]: # Build the chain: trigger → detector → workflows → workflow DCGs, # keeping per-detector scoping so actions don't bleed across detectors. # trigger.condition_group → detector condition_group_ids = {t.condition_group_id for t in item_list} - cg_to_detector_id: dict[int, int] = { - d.workflow_condition_group_id: d.id + detectors_by_cg: dict[int, Detector] = { + d.workflow_condition_group_id: d for d in Detector.objects.filter(workflow_condition_group__in=condition_group_ids) } # detector → workflow IDs - detector_ids = set(cg_to_detector_id.values()) + detector_ids = {d.id for d in detectors_by_cg.values()} detector_to_workflow_ids: dict[int, set[int]] = defaultdict(set) for det_id, wf_id in DetectorWorkflow.objects.filter(detector__in=detector_ids).values_list( "detector_id", "workflow_id" @@ -105,10 +105,18 @@ def get_attrs( ) ) - result: defaultdict[DataCondition, dict[str, list[str]]] = defaultdict(dict) + # Bulk-fetch alert_rule_id mappings + alert_rule_id_map: dict[int, int | None] = dict( + AlertRuleDetector.objects.filter(detector__in=detector_ids).values_list( + "detector_id", "alert_rule_id" + ) + ) + + result: defaultdict[DataCondition, dict[str, Any]] = defaultdict(dict) for trigger in item_list: - detector_id = cg_to_detector_id.get(trigger.condition_group_id) + detector = detectors_by_cg.get(trigger.condition_group_id) + detector_id = detector.id if detector else None trigger_dcg_ids = detector_to_dcg_ids.get(detector_id, set()) if detector_id else set() # Find DCGs in this detector's workflows that match the trigger's priority level @@ -139,7 +147,17 @@ def get_attrs( WorkflowEngineActionSerializer(), alert_rule_trigger_id=alert_rule_trigger_id, ) + + alert_rule_id = ( + alert_rule_id_map.get(detector_id, get_fake_id_from_object_id(detector_id)) + if detector_id + else None + ) + result[trigger]["actions"] = serialized_actions + result[trigger]["detector"] = detector + result[trigger]["alert_rule_trigger_id"] = alert_rule_trigger_id + result[trigger]["alert_rule_id"] = alert_rule_id return result @@ -151,22 +169,9 @@ def serialize( **kwargs: Any, ) -> dict[str, Any]: # XXX: we are assuming that the obj/DataCondition is a detector trigger - detector = Detector.objects.get(workflow_condition_group=obj.condition_group) - try: - alert_rule_trigger_id = DataConditionAlertRuleTrigger.objects.values_list( - "alert_rule_trigger_id", flat=True - ).get(data_condition=obj) - except DataConditionAlertRuleTrigger.DoesNotExist: - # this data condition does not have an analog in the old system, - # but we need to return *something* - alert_rule_trigger_id = get_fake_id_from_object_id(obj.id) - try: - alert_rule_id = AlertRuleDetector.objects.values_list("alert_rule_id", flat=True).get( - detector=detector.id - ) - except AlertRuleDetector.DoesNotExist: - # this detector does not have an analog in the old system - alert_rule_id = get_fake_id_from_object_id(detector.id) + detector = attrs["detector"] + alert_rule_trigger_id = attrs["alert_rule_trigger_id"] + alert_rule_id = attrs["alert_rule_id"] if obj.type == Condition.ANOMALY_DETECTION: threshold_type = obj.comparison["threshold_type"] diff --git a/src/sentry/integrations/base.py b/src/sentry/integrations/base.py index 4098320f242e..18eabce9b617 100644 --- a/src/sentry/integrations/base.py +++ b/src/sentry/integrations/base.py @@ -240,11 +240,15 @@ class is just a descriptor for how that object functions, and what behavior the installer's identity to the organization integration """ - is_cell_restricted: bool = False - """ - Returns True if each integration installation can only be connected on one cell of Sentry at a - time. It will raise an error if any organization from another cell attempts to install it. - """ + # TODO(cells): Remove once jira integration is updated and works for multi-cell. + # No integrations should be cell restricted. + @property + def is_cell_restricted(self) -> bool: + """ + Returns True if each integration installation can only be connected on one cell of Sentry at a + time. It will raise an error if any organization from another cell attempts to install it. + """ + return False features: frozenset[IntegrationFeatures] = frozenset() """can be any number of IntegrationFeatures""" diff --git a/src/sentry/integrations/jira/endpoints/descriptor.py b/src/sentry/integrations/jira/endpoints/descriptor.py index 01b384985695..42747bfc5af1 100644 --- a/src/sentry/integrations/jira/endpoints/descriptor.py +++ b/src/sentry/integrations/jira/endpoints/descriptor.py @@ -68,7 +68,7 @@ def get(self, request: Request) -> Response: "content": {"type": "label", "label": {"value": "Linked Issues"}}, "target": { "type": "web_panel", - "url": "/extensions/jira/issue/{issue.key}/", + "url": "/extensions/jira/issue-details/{issue.key}/", }, "name": {"value": "Sentry "}, "key": "sentry-issues-glance", diff --git a/src/sentry/integrations/jira/integration.py b/src/sentry/integrations/jira/integration.py index 4742d9a73858..4df5b34c188d 100644 --- a/src/sentry/integrations/jira/integration.py +++ b/src/sentry/integrations/jira/integration.py @@ -1200,10 +1200,12 @@ class JiraIntegrationProvider(IntegrationProvider): metadata = metadata integration_cls = JiraIntegration - # Jira is cell-restricted because the JiraSentryIssueDetailsView view does not currently - # contain organization-identifying information aside from the ExternalIssue. Multiple cells - # may contain a matching ExternalIssue and we could leak data across the organizations. - is_cell_restricted = True + @property + def is_cell_restricted(self) -> bool: + # TODO(cells): Remove this option and property once multi-cell rollout is complete. + from sentry import options + + return not options.get("integrations.jira.multi-cell-enabled") features = frozenset( [ diff --git a/src/sentry/integrations/jira/views/sentry_issue_details.py b/src/sentry/integrations/jira/views/sentry_issue_details.py index f89d7ecd9163..19177f0c9fcf 100644 --- a/src/sentry/integrations/jira/views/sentry_issue_details.py +++ b/src/sentry/integrations/jira/views/sentry_issue_details.py @@ -102,6 +102,8 @@ class JiraSentryIssueDetailsView(JiraSentryUIBaseView): """ Handles requests (from the Sentry integration in Jira) for HTML to display when you click on "Sentry -> Linked Issues" in the RH sidebar of an issue in the Jira UI. + + TODO(cells): Remove once all installs have migrated to JiraSentryIssueDetailsControlView. """ html_file = "sentry/integrations/jira-issue.html" diff --git a/src/sentry/integrations/middleware/hybrid_cloud/parser.py b/src/sentry/integrations/middleware/hybrid_cloud/parser.py index 581c52a2e34d..76520dea5e03 100644 --- a/src/sentry/integrations/middleware/hybrid_cloud/parser.py +++ b/src/sentry/integrations/middleware/hybrid_cloud/parser.py @@ -165,16 +165,14 @@ def get_responses_from_cell_silos(self, cells: list[Cell]) -> dict[str, CellResu def get_response_from_webhookpayload( self, - cells: list[Cell] | None = None, # TODO(cells): make required once getsentry is updated + cells: list[Cell], identifier: int | str | None = None, integration_id: int | None = None, - regions: list[Cell] | None = None, # TODO(cells): remove once getsentry is updated ) -> HttpResponseBase: """ Used to create webhookpayloads for provided cells to handle the webhooks asynchronously. Responds to the webhook provider with a 202 Accepted status. """ - cells = cells or regions or [] if len(cells) < 1: return HttpResponse(status=status.HTTP_202_ACCEPTED) diff --git a/src/sentry/middleware/integrations/parsers/jira.py b/src/sentry/middleware/integrations/parsers/jira.py index be00aea96821..a8e501d22221 100644 --- a/src/sentry/middleware/integrations/parsers/jira.py +++ b/src/sentry/middleware/integrations/parsers/jira.py @@ -67,16 +67,15 @@ def get_response(self) -> HttpResponseBase: if len(cells) == 0: return self.get_default_missing_integration_response() - if len(cells) > 1: - # This shouldn't happen because we block multi cell install at the install time. - raise ValueError("Jira integration is installed in multiple cells") - if self.view_class in self.immediate_response_cell_classes: - try: - return self.get_response_from_cell_silo(cell=cells[0]) - except ApiError as err: - sentry_sdk.capture_exception(err) - return self.get_response_from_control_silo() + if len(cells) == 1: + try: + return self.get_response_from_cell_silo(cell=cells[0]) + except ApiError as err: + sentry_sdk.capture_exception(err) + return self.get_response_from_control_silo() + # TODO(cells): Remove once all installs have migrated to JiraSentryIssueDetailsControlView. + return JiraSentryIssueDetailsControlView.as_view()(self.request, **self.match.kwargs) if self.view_class in self.outbox_response_cell_classes: return self.get_response_from_webhookpayload( diff --git a/src/sentry/models/group.py b/src/sentry/models/group.py index 5c4684f92b0f..8171735a3542 100644 --- a/src/sentry/models/group.py +++ b/src/sentry/models/group.py @@ -685,6 +685,7 @@ class Group(Model): priority_locked_at = models.DateTimeField(null=True) seer_fixability_score = models.FloatField(null=True) seer_autofix_last_triggered = models.DateTimeField(null=True) + # This actually represents the last timestamp when the explorer agent completes a step seer_explorer_autofix_last_triggered = models.DateTimeField(null=True) objects: ClassVar[GroupManager] = GroupManager(cache_fields=("id",)) diff --git a/src/sentry/models/options/project_option.py b/src/sentry/models/options/project_option.py index fd395c6fc7bf..8ff587fb2e42 100644 --- a/src/sentry/models/options/project_option.py +++ b/src/sentry/models/options/project_option.py @@ -74,6 +74,9 @@ "sentry:preprod_distribution_enabled_query", "sentry:preprod_size_enabled_by_customer", "sentry:preprod_distribution_enabled_by_customer", + "sentry:preprod_snapshot_status_checks_enabled", + "sentry:preprod_snapshot_status_checks_fail_on_added", + "sentry:preprod_snapshot_status_checks_fail_on_removed", "sentry:preprod_distribution_pr_comments_enabled_by_customer", "quotas:spike-protection-disabled", "feedback:branding", diff --git a/src/sentry/notifications/notification_action/group_type_notification_registry/handlers/metric_alert_registry_handler.py b/src/sentry/notifications/notification_action/group_type_notification_registry/handlers/metric_alert_registry_handler.py index 39d33c9b0807..c28385285a68 100644 --- a/src/sentry/notifications/notification_action/group_type_notification_registry/handlers/metric_alert_registry_handler.py +++ b/src/sentry/notifications/notification_action/group_type_notification_registry/handlers/metric_alert_registry_handler.py @@ -1,4 +1,5 @@ import logging +from collections.abc import Sequence from typing import override from sentry.incidents.grouptype import MetricIssue @@ -41,28 +42,66 @@ def handle_workflow_action(invocation: ActionInvocation) -> None: @staticmethod def target(action: Action) -> OrganizationMember | Team | str | None: - target_identifier = action.config.get("target_identifier") - if target_identifier is None: - return None + return MetricAlertRegistryHandler.get_targets([action]).get(action.id) - target_type = action.config.get("target_type") - if target_type == ActionTarget.USER.value: - dcga = DataConditionGroupAction.objects.get(action=action) - try: - return OrganizationMember.objects.get( - user_id=int(target_identifier), - organization=dcga.condition_group.organization, - ) - except OrganizationMember.DoesNotExist: - # user is no longer a member of the organization - pass - elif target_type == ActionTarget.TEAM.value: - try: - return Team.objects.get(id=int(target_identifier)) - except Team.DoesNotExist: - pass - elif target_type == ActionTarget.SPECIFIC.value: - # TODO: This is only for email. We should have a way of validating that it's - # ok to contact this email. - return target_identifier - return None + @staticmethod + def get_targets( + actions: Sequence[Action], + ) -> dict[int, OrganizationMember | Team | str | None]: + """ + Batch-load targets for multiple actions to avoid N+1 queries. + Returns a dict mapping action.id to its resolved target. + """ + result: dict[int, OrganizationMember | Team | str | None] = {} + + user_actions: list[Action] = [] + team_ids: list[int] = [] + team_action_ids: dict[int, list[int]] = {} + + for action in actions: + target_identifier = action.config.get("target_identifier") + if target_identifier is None: + result[action.id] = None + continue + + target_type = action.config.get("target_type") + if target_type == ActionTarget.USER.value: + user_actions.append(action) + elif target_type == ActionTarget.TEAM.value: + tid = int(target_identifier) + team_ids.append(tid) + team_action_ids.setdefault(tid, []).append(action.id) + elif target_type == ActionTarget.SPECIFIC.value: + result[action.id] = target_identifier + else: + result[action.id] = None + + if user_actions: + dcgas = DataConditionGroupAction.objects.filter( + action__in=[a.id for a in user_actions] + ).select_related("condition_group") + org_by_action_id = { + dcga.action_id: dcga.condition_group.organization_id for dcga in dcgas + } + + org_members = OrganizationMember.objects.filter( + user_id__in=[int(a.config["target_identifier"]) for a in user_actions], + organization_id__in=set(org_by_action_id.values()), + ) + member_by_key = {(om.user_id, om.organization_id): om for om in org_members} + + for action in user_actions: + org_id = org_by_action_id.get(action.id) + if org_id is not None: + key = (int(action.config["target_identifier"]), org_id) + result[action.id] = member_by_key.get(key) + else: + result[action.id] = None + + if team_ids: + teams = {t.id: t for t in Team.objects.filter(id__in=team_ids)} + for tid, action_ids in team_action_ids.items(): + for action_id in action_ids: + result[action_id] = teams.get(tid) + + return result diff --git a/src/sentry/notifications/notification_action/utils.py b/src/sentry/notifications/notification_action/utils.py index 59354158218d..6b3e4396e13f 100644 --- a/src/sentry/notifications/notification_action/utils.py +++ b/src/sentry/notifications/notification_action/utils.py @@ -114,20 +114,18 @@ def execute_via_metric_alert_handler(invocation: ActionInvocation) -> None: def issue_notification_data_factory(invocation: ActionInvocation) -> IssueNotificationData: from sentry.notifications.notification_action.types import BaseIssueAlertHandler - from sentry.workflow_engine.typings.notification_action import SlackDataBlob action = invocation.action detector = invocation.detector event_data = invocation.event_data - blob = SlackDataBlob(**action.data) - tags_set = {t.strip() for t in blob.tags.split(",") if t.strip()} if blob.tags else set() - rule_instance = BaseIssueAlertHandler.create_rule_instance_from_action( action=action, detector=detector, event_data=event_data, ) + rule_instance.data["tags"] = action.data.get("tags", "") + rule_instance.data["notes"] = action.data.get("notes", "") rule = SerializableRuleProxy.from_rule(rule_instance) event_id = getattr(event_data.event, "event_id", None) if event_data.event else None @@ -135,8 +133,6 @@ def issue_notification_data_factory(invocation: ActionInvocation) -> IssueNotifi return IssueNotificationData( event_id=event_id, group_id=event_data.group.id, - tags=tags_set, - notes=blob.notes, notification_uuid=invocation.notification_uuid, rule=rule, ) diff --git a/src/sentry/notifications/platform/slack/renderers/issue.py b/src/sentry/notifications/platform/slack/renderers/issue.py index ab738e093774..46182c5cc77d 100644 --- a/src/sentry/notifications/platform/slack/renderers/issue.py +++ b/src/sentry/notifications/platform/slack/renderers/issue.py @@ -1,5 +1,6 @@ from __future__ import annotations +from sentry import eventstore from sentry.models.group import Group from sentry.notifications.platform.renderer import NotificationRenderer from sentry.notifications.platform.slack.provider import SlackRenderable @@ -26,14 +27,16 @@ def render[DataT: NotificationData]( group = Group.objects.get_from_cache(id=data.group_id) event = None if data.event_id: - event = group.get_latest_event() + event = eventstore.backend.get_event_by_id(group.project.id, data.event_id) + tags = data.rule.data.get("tags", None) or None blocks_dict = SlackIssuesMessageBuilder( group=group, event=event, - tags=data.tags or None, + tags=set(tag.strip() for tag in tags.split(",")) if tags else None, rules=[data.rule.to_rule()] if data.rule else None, - notes=data.notes or None, + notes=data.rule.data.get("notes", None) or None, + link_to_event=True, ).build(notification_uuid=data.notification_uuid) return SlackRenderable( diff --git a/src/sentry/notifications/platform/templates/issue.py b/src/sentry/notifications/platform/templates/issue.py index 6ecf6491a4ee..128943657da0 100644 --- a/src/sentry/notifications/platform/templates/issue.py +++ b/src/sentry/notifications/platform/templates/issue.py @@ -2,7 +2,7 @@ from typing import Any -from pydantic import BaseModel, ConfigDict, Field +from pydantic import BaseModel, ConfigDict from sentry.models.rule import Rule from sentry.notifications.platform.registry import template_registry @@ -61,9 +61,7 @@ class IssueNotificationData(NotificationData): group_id: int event_id: str | None = None - rule: SerializableRuleProxy | None = None - tags: set[str] = Field(default_factory=set) - notes: str = "" + rule: SerializableRuleProxy notification_uuid: str = "" @@ -73,11 +71,16 @@ class IssueNotificationTemplate(NotificationTemplate[IssueNotificationData]): example_data = IssueNotificationData( group_id=1, event_id="abc123", - tags={"environment", "level"}, - notes="example note", notification_uuid="test-uuid", rule=SerializableRuleProxy( - id=1, project_id=2, environment_id=3, label="Example Rule", data={} + id=1, + project_id=2, + label="Example Rule", + data={ + "actions": [{"workflow_id": 3}], + "tags": "environment,level", + "notes": "example note", + }, ), ) hide_from_debugger = True diff --git a/src/sentry/options/defaults.py b/src/sentry/options/defaults.py index a010f970b05e..45fe2de8d841 100644 --- a/src/sentry/options/defaults.py +++ b/src/sentry/options/defaults.py @@ -4192,3 +4192,10 @@ type=Bool, flags=FLAG_MODIFIABLE_BOOL | FLAG_AUTOMATOR_MODIFIABLE, ) + +register( + "integrations.jira.multi-cell-enabled", + default=False, + type=Bool, + flags=FLAG_MODIFIABLE_BOOL | FLAG_AUTOMATOR_MODIFIABLE, +) diff --git a/src/sentry/preprod/artifact_search.py b/src/sentry/preprod/artifact_search.py index 5d197ae85627..15e7c5562bac 100644 --- a/src/sentry/preprod/artifact_search.py +++ b/src/sentry/preprod/artifact_search.py @@ -315,3 +315,49 @@ def apply_filters( q = ~q queryset = queryset.filter(q) return queryset + + +def get_sequential_base_artifact( + artifact: PreprodArtifact, + query: str, + organization: Organization, +) -> PreprodArtifact | None: + """ + Find the most recent prior artifact matching the given query and structural + identity fields, with completed size metrics. + + Used by sequential (n-1) monitors to resolve the base artifact for diff + comparisons. Unlike the git-based lookup (get_base_artifact_for_commit), + this orders by date_added rather than commit ancestry. + + Args: + artifact: The current (head) artifact to find a base for. + query: The detector's query string (e.g. "app_name:MyApp"). May be empty. + organization: The organization for scoping query filters. + + Returns: + The most recent prior PreprodArtifact matching the criteria, or None. + """ + queryset = PreprodArtifact.objects.get_queryset() + + if query and query.strip(): + search_filters = parse_search_query( + query, config=search_config, get_field_type=get_field_type + ) + queryset = apply_filters(queryset, search_filters, organization) + + queryset = queryset.filter( + project_id=artifact.project_id, + app_id=artifact.app_id, + artifact_type=artifact.artifact_type, + build_configuration=artifact.build_configuration, + ) + + queryset = queryset.filter( + preprodartifactsizemetrics__state=PreprodArtifactSizeMetrics.SizeAnalysisState.COMPLETED, + preprodartifactsizemetrics__metrics_artifact_type=PreprodArtifactSizeMetrics.MetricsArtifactType.MAIN_ARTIFACT, + ) + + queryset = queryset.exclude(pk=artifact.pk) + + return queryset.order_by("-date_added").first() diff --git a/src/sentry/preprod/size_analysis/tasks.py b/src/sentry/preprod/size_analysis/tasks.py index 29b847128348..d5b53478eacf 100644 --- a/src/sentry/preprod/size_analysis/tasks.py +++ b/src/sentry/preprod/size_analysis/tasks.py @@ -8,7 +8,9 @@ from django.utils import timezone from sentry import features +from sentry.exceptions import InvalidSearchQuery from sentry.models.files.file import File +from sentry.preprod.artifact_search import get_sequential_base_artifact from sentry.preprod.models import ( PreprodArtifact, PreprodArtifactSizeComparison, @@ -21,7 +23,7 @@ SizeAnalysisMetadata, SizeAnalysisValue, ) -from sentry.preprod.size_analysis.models import ComparisonResults, SizeAnalysisResults +from sentry.preprod.size_analysis.models import SizeAnalysisResults from sentry.preprod.size_analysis.utils import build_size_metrics_map, can_compare_size_metrics from sentry.preprod.size_analysis.webhooks import send_size_analysis_webhook from sentry.preprod.vcs.status_checks.size.tasks import create_preprod_status_check_task @@ -96,6 +98,8 @@ def compare_preprod_artifact_size_analysis( "caller": "compare_build_config_mismatch", } ) + # Still evaluate sequential monitors even on build config mismatch + maybe_emit_issues_from_diff_size_results(artifact, org_id) return base_size_metrics_qs = PreprodArtifactSizeMetrics.objects.filter( @@ -279,6 +283,9 @@ def compare_preprod_artifact_size_analysis( finally: send_size_analysis_webhook(artifact=artifact, organization_id=org_id) + # Evaluate sequential monitors unconditionally — independent of git metadata + maybe_emit_issues_from_diff_size_results(artifact, org_id) + @instrumented_task( name="sentry.preprod.tasks.manual_size_analysis_comparison", @@ -543,50 +550,39 @@ def _run_size_analysis_comparison( comparison.state = PreprodArtifactSizeComparison.State.SUCCESS comparison.save() - maybe_emit_issues( - comparison_results=comparison_results, - head_metric=head_size_metric, - base_metric=base_size_metric, - ) - logger.info( "preprod.size_analysis.compare.success", extra={"comparison_id": comparison.id}, ) -def maybe_emit_issues( - comparison_results: ComparisonResults, - head_metric: PreprodArtifactSizeMetrics, - base_metric: PreprodArtifactSizeMetrics, +def maybe_emit_issues_from_diff_size_results( + artifact: PreprodArtifact, + org_id: int, ) -> None: try: - _maybe_emit_issues( - comparison_results=comparison_results, head_metric=head_metric, base_metric=base_metric - ) + _maybe_emit_issues_from_diff_size_results(artifact=artifact, org_id=org_id) except Exception: - logger.exception("Error emitting issues") + logger.exception("Error emitting issues from diff size results") def _get_platform(artifact: PreprodArtifact) -> str: return artifact.platform or "unknown" -def _maybe_emit_issues( - comparison_results: ComparisonResults, - head_metric: PreprodArtifactSizeMetrics, - base_metric: PreprodArtifactSizeMetrics, +def _maybe_emit_issues_from_diff_size_results( + artifact: PreprodArtifact, + org_id: int, ) -> None: - project = head_metric.preprod_artifact.project + project = artifact.project project_id = project.id - organization_id = project.organization.id if not features.has("organizations:preprod-issues", project.organization): logger.info( - "preprod.size_analysis.compare.issues.disabled", + "preprod.size_analysis.diff_results.issues.disabled", extra={ "project_id": project_id, - "organization_id": organization_id, + "organization_id": org_id, }, ) return @@ -600,62 +596,129 @@ def _maybe_emit_issues( ) if not detectors: logger.info( - "preprod.size_analysis.no_detectors", + "preprod.size_analysis.diff_results.no_detectors", extra={"project_id": project_id}, ) return - # Only process diff-based detectors from the comparison path detectors = [d for d in detectors if d.config.get("threshold_type") in DIFF_THRESHOLD_TYPES] if not detectors: logger.info( - "preprod.size_analysis.no_diff_detectors", + "preprod.size_analysis.diff_results.no_diff_detectors", extra={"project_id": project_id}, ) return - diff = comparison_results.size_metric_diff_item - head_artifact = head_metric.preprod_artifact - base_artifact = base_metric.preprod_artifact + head_metrics = list( + PreprodArtifactSizeMetrics.objects.filter( + preprod_artifact=artifact, + metrics_artifact_type=PreprodArtifactSizeMetrics.MetricsArtifactType.MAIN_ARTIFACT, + state=PreprodArtifactSizeMetrics.SizeAnalysisState.COMPLETED, + ).select_related("preprod_artifact", "preprod_artifact__mobile_app_info") + ) + if not head_metrics: + logger.info( + "preprod.size_analysis.diff_results.no_head_metrics", + extra={"artifact_id": artifact.id}, + ) + return - metadata: SizeAnalysisMetadata = { - "platform": _get_platform(head_artifact), - "head_metric_id": head_metric.id, - "base_metric_id": base_metric.id, - "head_artifact_id": head_artifact.id, - "base_artifact_id": base_artifact.id, - "head_artifact": head_artifact, - "base_artifact": base_artifact, - } + head_metric = head_metrics[0] + organization = project.organization - size_data: SizeAnalysisValue = { - "head_install_size_bytes": diff.head_install_size, - "head_download_size_bytes": diff.head_download_size, - "base_install_size_bytes": diff.base_install_size, - "base_download_size_bytes": diff.base_download_size, - "metadata": metadata, - } + # Batch-resolve sequential bases by unique query string + query_to_base: dict[str, PreprodArtifact | None] = {} + for detector in detectors: + query = detector.config.get("query", "") + normalized_query = query.strip() if query else "" + if normalized_query not in query_to_base: + try: + base = get_sequential_base_artifact(artifact, normalized_query, organization) + except InvalidSearchQuery: + logger.exception( + "preprod.size_analysis.diff_results.invalid_query", + extra={"detector_id": detector.id, "query": normalized_query}, + ) + base = None + query_to_base[normalized_query] = base + + # Run comparisons for unique sequential bases (persists file-level diffs) + base_artifact_to_metric: dict[int, PreprodArtifactSizeMetrics] = {} + seen_base_ids: set[int] = set() + for base_artifact in query_to_base.values(): + if base_artifact is None or base_artifact.id in seen_base_ids: + continue + seen_base_ids.add(base_artifact.id) + + base_metrics = list( + PreprodArtifactSizeMetrics.objects.filter( + preprod_artifact=base_artifact, + metrics_artifact_type=PreprodArtifactSizeMetrics.MetricsArtifactType.MAIN_ARTIFACT, + state=PreprodArtifactSizeMetrics.SizeAnalysisState.COMPLETED, + ).select_related("preprod_artifact", "preprod_artifact__mobile_app_info") + ) + if not base_metrics: + continue - data_packet: SizeAnalysisDataPacket = DataPacket( - source_id=f"preprod-size-analysis:{project_id}", - packet=size_data, - ) + base_metric = base_metrics[0] + base_artifact_to_metric[base_artifact.id] = base_metric - logger.info( - "preprod.size_analysis.process_detectors.starting", - extra={ - "project_id": project_id, - "detector_count": len(detectors), - }, - ) - results = process_detectors(data_packet, detectors) - logger.info( - "preprod.size_analysis.process_detectors.completed", - extra={ - "project_id": project_id, - "detector_count": len(results), - }, - ) + with transaction.atomic(router.db_for_write(PreprodArtifactSizeComparison)): + PreprodArtifactSizeComparison.objects.get_or_create( + head_size_analysis=head_metric, + base_size_analysis=base_metric, + organization_id=org_id, + defaults={"state": PreprodArtifactSizeComparison.State.PENDING}, + ) + _run_size_analysis_comparison(org_id, head_metric, base_metric) + + for detector in detectors: + query = detector.config.get("query", "") + normalized_query = query.strip() if query else "" + base_artifact = query_to_base.get(normalized_query) + if base_artifact is None: + logger.info( + "preprod.size_analysis.diff_results.no_base", + extra={"detector_id": detector.id, "artifact_id": artifact.id}, + ) + continue + + detector_base_metric = base_artifact_to_metric.get(base_artifact.id) + if detector_base_metric is None: + continue + + metadata: SizeAnalysisMetadata = { + "platform": _get_platform(artifact), + "head_metric_id": head_metric.id, + "base_metric_id": detector_base_metric.id, + "head_artifact_id": artifact.id, + "base_artifact_id": base_artifact.id, + "head_artifact": artifact, + "base_artifact": base_artifact, + } + + size_data: SizeAnalysisValue = { + "head_install_size_bytes": head_metric.max_install_size or 0, + "head_download_size_bytes": head_metric.max_download_size or 0, + "base_install_size_bytes": detector_base_metric.max_install_size or 0, + "base_download_size_bytes": detector_base_metric.max_download_size or 0, + "metadata": metadata, + } + + data_packet: SizeAnalysisDataPacket = DataPacket( + source_id=f"preprod-size-analysis:{project_id}", + packet=size_data, + ) + + logger.info( + "preprod.size_analysis.diff_results.evaluating", + extra={ + "detector_id": detector.id, + "artifact_id": artifact.id, + "base_artifact_id": base_artifact.id, + }, + ) + process_detectors(data_packet, [detector]) def maybe_emit_issues_from_absolute_size_results( diff --git a/src/sentry/preprod/snapshots/tasks.py b/src/sentry/preprod/snapshots/tasks.py index 32107e22af75..5ce11f0f8b4a 100644 --- a/src/sentry/preprod/snapshots/tasks.py +++ b/src/sentry/preprod/snapshots/tasks.py @@ -1,6 +1,7 @@ from __future__ import annotations import logging +import threading from typing import NamedTuple import orjson @@ -27,6 +28,7 @@ from sentry.tasks.base import instrumented_task from sentry.taskworker.namespaces import preprod_tasks from sentry.utils import metrics +from sentry.utils.concurrent import ContextPropagatingThreadPoolExecutor logger = logging.getLogger(__name__) @@ -353,15 +355,30 @@ def compare_snapshots( batch_names: list[str] = [] batch_hashes: list[tuple[str, str]] = [] + unique_hashes: set[str] = set() for candidate in batch: + unique_hashes.add(candidate.head_hash) + unique_hashes.add(candidate.base_hash) + + fetch_cache: dict[str, bytes] = {} + failed_hashes: set[str] = set() + cache_lock = threading.Lock() + + def _fetch_hash(h: str) -> None: try: - head_data = session.get( - f"{image_key_prefix}/{candidate.head_hash}" - ).payload.read() - base_data = session.get( - f"{image_key_prefix}/{candidate.base_hash}" - ).payload.read() + data = session.get(f"{image_key_prefix}/{h}").payload.read() + with cache_lock: + fetch_cache[h] = data except Exception: + with cache_lock: + failed_hashes.add(h) + + # Fetch unique hashes in parallel; session.get() is thread-safe + with ContextPropagatingThreadPoolExecutor(max_workers=8) as executor: + list(executor.map(_fetch_hash, unique_hashes)) + + for candidate in batch: + if candidate.head_hash in failed_hashes or candidate.base_hash in failed_hashes: logger.warning( "compare_snapshots: failed to fetch images for %s", candidate.name, @@ -379,6 +396,8 @@ def compare_snapshots( "reason": "image_fetch_failed", } continue + head_data = fetch_cache[candidate.head_hash] + base_data = fetch_cache[candidate.base_hash] total_fetched_bytes += len(head_data) + len(base_data) total_fetched_count += 2 diff_pairs.append((base_data, head_data)) @@ -386,8 +405,9 @@ def compare_snapshots( batch_hashes.append((candidate.head_hash, candidate.base_hash)) logger.info( - "compare_snapshots: running batch of %d pairs", + "compare_snapshots: running batch of %d pairs (%d unique hashes fetched)", len(diff_pairs), + len(fetch_cache), extra={"head_artifact_id": head_artifact_id, "names": batch_names}, ) diff_results = compare_images_batch(diff_pairs, server=server) diff --git a/src/sentry/preprod/vcs/status_checks/snapshots/tasks.py b/src/sentry/preprod/vcs/status_checks/snapshots/tasks.py index 98865bf08f4f..14e9276b85a4 100644 --- a/src/sentry/preprod/vcs/status_checks/snapshots/tasks.py +++ b/src/sentry/preprod/vcs/status_checks/snapshots/tasks.py @@ -35,6 +35,10 @@ # Action identifier for the "Approve" button on snapshot GitHub check runs. APPROVE_SNAPSHOT_ACTION_IDENTIFIER = "approve_snapshots" +ENABLED_OPTION_KEY = "sentry:preprod_snapshot_status_checks_enabled" +FAIL_ON_ADDED_OPTION_KEY = "sentry:preprod_snapshot_status_checks_fail_on_added" +FAIL_ON_REMOVED_OPTION_KEY = "sentry:preprod_snapshot_status_checks_fail_on_removed" + @instrumented_task( name="sentry.preprod.tasks.create_preprod_snapshot_status_check", @@ -89,7 +93,19 @@ def create_preprod_snapshot_status_check_task( ) return - # TODO(EME-911) Wireup enabled options once gating/billing comes into play + status_checks_enabled = preprod_artifact.project.get_option(ENABLED_OPTION_KEY, default=True) + if not status_checks_enabled: + logger.info( + "preprod.snapshot_status_checks.create.disabled", + extra={ + "artifact_id": preprod_artifact.id, + "project_id": preprod_artifact.project.id, + }, + ) + return + + fail_on_added = preprod_artifact.project.get_option(FAIL_ON_ADDED_OPTION_KEY, default=False) + fail_on_removed = preprod_artifact.project.get_option(FAIL_ON_REMOVED_OPTION_KEY, default=True) all_artifacts = list(preprod_artifact.get_sibling_artifacts_for_commit()) @@ -182,8 +198,19 @@ def create_preprod_snapshot_status_check_task( snapshot_metrics_map, ) else: + changes_map = _build_changes_map( + all_artifacts, + snapshot_metrics_map, + comparisons_map, + fail_on_added=fail_on_added, + fail_on_removed=fail_on_removed, + ) status = _compute_snapshot_status( - all_artifacts, snapshot_metrics_map, comparisons_map, approvals_map + all_artifacts, + snapshot_metrics_map, + comparisons_map, + approvals_map, + changes_map, ) title, subtitle, summary = format_snapshot_status_check_messages( all_artifacts, @@ -191,8 +218,9 @@ def create_preprod_snapshot_status_check_task( comparisons_map, status, base_artifact_map, + changes_map, ) - if _has_snapshot_changes(all_artifacts, snapshot_metrics_map, comparisons_map): + if any(changes_map.values()): approve_action_identifier = APPROVE_SNAPSHOT_ACTION_IDENTIFIER completed_at: datetime | None = None @@ -266,12 +294,27 @@ def create_preprod_snapshot_status_check_task( ) -def _has_snapshot_changes( +def _comparison_has_changes( + comparison: PreprodSnapshotComparison, + fail_on_added: bool = False, + fail_on_removed: bool = True, +) -> bool: + return ( + comparison.images_changed > 0 + or comparison.images_renamed > 0 + or (fail_on_added and comparison.images_added > 0) + or (fail_on_removed and comparison.images_removed > 0) + ) + + +def _build_changes_map( artifacts: list[PreprodArtifact], snapshot_metrics_map: dict[int, PreprodSnapshotMetrics], comparisons_map: dict[int, PreprodSnapshotComparison], -) -> bool: - """Check if any artifact has snapshot changes (added/removed/changed).""" + fail_on_added: bool = False, + fail_on_removed: bool = True, +) -> dict[int, bool]: + changes_map: dict[int, bool] = {} for artifact in artifacts: metrics = snapshot_metrics_map.get(artifact.id) if not metrics: @@ -279,15 +322,10 @@ def _has_snapshot_changes( comparison = comparisons_map.get(metrics.id) if not comparison or comparison.state != PreprodSnapshotComparison.State.SUCCESS: continue - - if ( - comparison.images_changed > 0 - or comparison.images_added > 0 - or comparison.images_removed > 0 - or comparison.images_renamed > 0 - ): - return True - return False + changes_map[artifact.id] = _comparison_has_changes( + comparison, fail_on_added, fail_on_removed + ) + return changes_map def _compute_snapshot_status( @@ -295,13 +333,8 @@ def _compute_snapshot_status( snapshot_metrics_map: dict[int, PreprodSnapshotMetrics], comparisons_map: dict[int, PreprodSnapshotComparison], approvals_map: dict[int, PreprodComparisonApproval], + changes_map: dict[int, bool], ) -> StatusCheckStatus: - """Compute the overall snapshot status check status. - - - IN_PROGRESS if any comparison is pending/processing - - FAILURE if any comparison failed, or if any has changes and not approved - - SUCCESS if all comparisons succeeded with no changes, or all approved - """ has_in_progress = False for artifact in artifacts: @@ -320,12 +353,7 @@ def _compute_snapshot_status( case PreprodSnapshotComparison.State.FAILED: return StatusCheckStatus.FAILURE case PreprodSnapshotComparison.State.SUCCESS: - if ( - comparison.images_changed > 0 - or comparison.images_added > 0 - or comparison.images_removed > 0 - or comparison.images_renamed > 0 - ) and artifact.id not in approvals_map: + if changes_map.get(artifact.id, False) and artifact.id not in approvals_map: return StatusCheckStatus.FAILURE if has_in_progress: diff --git a/src/sentry/preprod/vcs/status_checks/snapshots/templates.py b/src/sentry/preprod/vcs/status_checks/snapshots/templates.py index 0b74ee9ee2ed..2555a7209935 100644 --- a/src/sentry/preprod/vcs/status_checks/snapshots/templates.py +++ b/src/sentry/preprod/vcs/status_checks/snapshots/templates.py @@ -18,6 +18,7 @@ def format_snapshot_status_check_messages( comparisons_map: dict[int, PreprodSnapshotComparison], overall_status: StatusCheckStatus, base_artifact_map: dict[int, PreprodArtifact], + changes_map: dict[int, bool], ) -> tuple[str, str, str]: if not artifacts: raise ValueError("Cannot format messages for empty artifact list") @@ -108,6 +109,7 @@ def format_snapshot_status_check_messages( snapshot_metrics_map, comparisons_map, base_artifact_map, + changes_map, ) return str(title), str(subtitle), str(summary) @@ -219,6 +221,7 @@ def _format_snapshot_summary( snapshot_metrics_map: dict[int, PreprodSnapshotMetrics], comparisons_map: dict[int, PreprodSnapshotComparison], base_artifact_map: dict[int, PreprodArtifact], + changes_map: dict[int, bool], ) -> str: table_rows = [] @@ -264,7 +267,7 @@ def _format_snapshot_summary( modified = comparison.images_changed renamed = comparison.images_renamed unchanged = comparison.images_unchanged - has_changes = modified > 0 or added > 0 or removed > 0 or renamed > 0 + has_changes = changes_map.get(artifact.id, False) status = "⏳ Needs approval" if has_changes else "✅ Unchanged" def _section_cell(count: int, section: str) -> str: diff --git a/src/sentry/receivers/outbox/control.py b/src/sentry/receivers/outbox/control.py index 5264a502ab98..de3c63ed7398 100644 --- a/src/sentry/receivers/outbox/control.py +++ b/src/sentry/receivers/outbox/control.py @@ -25,7 +25,7 @@ from sentry.sentry_apps.models.sentry_app import SentryApp from sentry.sentry_apps.models.sentry_app_installation import SentryAppInstallation from sentry.sentry_apps.services.hook.service import hook_service -from sentry.sentry_apps.tasks.sentry_apps import clear_region_cache +from sentry.sentry_apps.tasks.sentry_apps import clear_cell_cache from sentry.users.models.identity import Identity from sentry.workflow_engine.service.action.service import action_service @@ -57,8 +57,7 @@ def process_sentry_app_updates(object_identifier: int, cell_name: str, **kwds: A # Spawn a task to clear caches, as there can be 1000+ installations # for a sentry app. - # TODO(cells): switch to clear_cell_cache once deployed on all pods - clear_region_cache.delay(sentry_app_id=sentry_app.id, region_name=cell_name) + clear_cell_cache.delay(sentry_app_id=sentry_app.id, cell_name=cell_name) @receiver(process_control_outbox, sender=OutboxCategory.SENTRY_APP_DELETE) diff --git a/src/sentry/runner/commands/cleanup.py b/src/sentry/runner/commands/cleanup.py index 9d839d1ac602..103630fbcecf 100644 --- a/src/sentry/runner/commands/cleanup.py +++ b/src/sentry/runner/commands/cleanup.py @@ -368,12 +368,13 @@ def is_filtered(model: type[BaseModel]) -> bool: return model.__name__.lower() not in model_list deletes = models_which_use_deletions_code_path() + bulk_query_deletes = generate_bulk_query_deletes() _run_specialized_cleanups(is_filtered, days, models_attempted) # Handle project/organization specific logic project_id, organization_id = _handle_project_organization_cleanup( - project, organization, days, deletes + project, organization, days, deletes, bulk_query_deletes ) if organization_id is not None: transaction.set_tag("organization_id", organization_id) @@ -381,6 +382,7 @@ def is_filtered(model: type[BaseModel]) -> bool: transaction.set_tag("project_id", project_id) run_bulk_query_deletes( + bulk_query_deletes, is_filtered, days, project, @@ -511,6 +513,7 @@ def _handle_project_organization_cleanup( organization: str | None, days: int, deletes: list[tuple[type[BaseModel], str, str]], + bulk_query_deletes: list[tuple[type[BaseModel], str, str | None]], ) -> tuple[int | None, int | None]: """Handle project/organization specific cleanup logic.""" project_id = None @@ -519,6 +522,7 @@ def _handle_project_organization_cleanup( if SiloMode.get_current_mode() != SiloMode.CONTROL: if project: remove_cross_project_models(deletes) + remove_cross_project_bulk_query_models(bulk_query_deletes) project_id = get_project_id_or_fail(project) elif organization: organization_id = get_organization_id_or_fail(organization) @@ -732,11 +736,21 @@ def remove_old_nodestore_values(days: int) -> None: click.echo("NodeStore backend does not support cleanup operation", err=True) +def remove_cross_project_bulk_query_models( + bulk_query_deletes: list[tuple[type[BaseModel], str, str | None]], +) -> list[tuple[type[BaseModel], str, str | None]]: + from sentry.workflow_engine.models.workflow_fire_history import WorkflowFireHistory + + bulk_query_deletes.remove((WorkflowFireHistory, "date_added", None)) + return bulk_query_deletes + + def generate_bulk_query_deletes() -> list[tuple[type[BaseModel], str, str | None]]: from django.apps import apps from sentry.models.groupemailthread import GroupEmailThread from sentry.models.userreport import UserReport + from sentry.workflow_engine.models.workflow_fire_history import WorkflowFireHistory # Deletions that use `BulkDeleteQuery` (and don't need to worry about child relations) # (model, datetime_field, order_by) @@ -749,12 +763,14 @@ def generate_bulk_query_deletes() -> list[tuple[type[BaseModel], str, str | None BULK_QUERY_DELETES = [ (UserReport, "date_added", None), (GroupEmailThread, "date", None), + (WorkflowFireHistory, "date_added", None), ] + additional_bulk_query_deletes return BULK_QUERY_DELETES def run_bulk_query_deletes( + bulk_query_deletes: list[tuple[type[BaseModel], str, str | None]], is_filtered: Callable[[type[BaseModel]], bool], days: int, project: str | None, @@ -770,7 +786,6 @@ def run_bulk_query_deletes( raise CleanupExecutionAborted() debug_output("Running bulk query deletes in bulk_query_deletes") - bulk_query_deletes = generate_bulk_query_deletes() for model_tp, dtfield, order_by in bulk_query_deletes: chunk_size = 10000 diff --git a/src/sentry/runner/commands/createorg.py b/src/sentry/runner/commands/createorg.py new file mode 100644 index 000000000000..67605918c7f5 --- /dev/null +++ b/src/sentry/runner/commands/createorg.py @@ -0,0 +1,58 @@ +from __future__ import annotations + +import click + +from sentry.runner.decorators import configuration + + +@click.command() +@click.option("--name", required=True, help="Organization name.") +@click.option( + "--slug", + default=None, + help="URL-friendly slug. Derived from name if omitted.", +) +@click.option("--owner-email", required=True, help="Email of the organization owner.") +@click.option( + "--no-default-team", + default=False, + is_flag=True, + help="Skip creating a default team.", +) +@configuration +def createorg( + name: str, + slug: str | None, + owner_email: str, + no_default_team: bool, +) -> None: + "Create a new organization." + + from sentry.services.organization.model import ( + OrganizationOptions, + OrganizationProvisioningOptions, + PostProvisionOptions, + ) + from sentry.services.organization.provisioning import ( + OrganizationProvisioningException, + organization_provisioning_service, + ) + + provision_args = OrganizationProvisioningOptions( + provision_options=OrganizationOptions( + name=name, + slug=slug or name, + owning_email=owner_email, + create_default_team=not no_default_team, + ), + post_provision_options=PostProvisionOptions(), + ) + + try: + rpc_org = organization_provisioning_service.provision_organization_in_cell( + provisioning_options=provision_args, + ) + except OrganizationProvisioningException as e: + raise click.ClickException(str(e)) + + click.echo(f"Organization created: {rpc_org.name} (slug: {rpc_org.slug}, id: {rpc_org.id})") diff --git a/src/sentry/runner/commands/createproject.py b/src/sentry/runner/commands/createproject.py new file mode 100644 index 000000000000..7cd0fea24260 --- /dev/null +++ b/src/sentry/runner/commands/createproject.py @@ -0,0 +1,90 @@ +from __future__ import annotations + +from typing import TYPE_CHECKING + +import click + +if TYPE_CHECKING: + from sentry.models.organization import Organization +from sentry.runner.decorators import configuration + + +def _resolve_organization(org_value: str) -> Organization: + from sentry.models.organization import Organization + + try: + if org_value.isdigit(): + return Organization.objects.get(id=int(org_value)) + return Organization.objects.get(slug=org_value) + except Organization.DoesNotExist: + raise click.ClickException(f"Organization not found: {org_value}") + + +@click.command() +@click.option("--name", required=True, help="The name of the project.") +@click.option( + "--platform", + help="The platform for the project (e.g., 'python', 'javascript-react').", +) +@click.option("--organization", "org", required=True, help="Organization ID or slug.") +@click.option("--team", default=None, help="Team slug to grant access to the project.") +@configuration +def createproject( + name: str, + platform: str, + org: str, + team: str | None, +) -> None: + """Create a new project.""" + from django.contrib.auth.models import AnonymousUser + from django.db import router, transaction + + from sentry.api.helpers.default_symbol_sources import set_default_symbol_sources + from sentry.core.endpoints.team_projects import apply_default_project_settings + from sentry.models.project import Project + from sentry.models.projectkey import ProjectKey + from sentry.models.team import Team, TeamStatus + from sentry.signals import project_created + + organization = _resolve_organization(org) + + team_instance = None + if team: + try: + team_instance = Team.objects.get( + organization=organization, + slug=team, + status=TeamStatus.ACTIVE, + ) + except Team.DoesNotExist: + raise click.ClickException( + f"Team not found: '{team}' in organization '{organization.slug}'" + ) + + with transaction.atomic(router.db_for_write(Project)): + project = Project.objects.create( + name=name, + organization=organization, + platform=platform, + ) + + if team_instance: + project.add_team(team_instance) + + set_default_symbol_sources(project) + apply_default_project_settings(organization, project) + + project_created.send_robust( + project=project, + default_rules=True, + user=AnonymousUser(), + sender=createproject, + ) + + key = ProjectKey.get_default(project) + dsn = key.dsn_public if key else "(no DSN available)" + + click.echo("Created project:") + click.echo(f" ID: {project.id}") + click.echo(f" Slug: {project.slug}") + click.echo(f" DSN: {dsn}") diff --git a/src/sentry/runner/main.py b/src/sentry/runner/main.py index 821ba6350a47..425905244bb7 100644 --- a/src/sentry/runner/main.py +++ b/src/sentry/runner/main.py @@ -45,6 +45,8 @@ def cli(config: str) -> None: "sentry.runner.commands.configoptions.configoptions", "sentry.runner.commands.createflag.createflag", "sentry.runner.commands.createflag.createissueflag", + "sentry.runner.commands.createorg.createorg", + "sentry.runner.commands.createproject.createproject", "sentry.runner.commands.createuser.createuser", "sentry.runner.commands.devserver.devserver", "sentry.runner.commands.django.django", diff --git a/src/sentry/scripts/spans/add-buffer.lua b/src/sentry/scripts/spans/add-buffer.lua index 849a4ef84dd7..fb5bfa944d66 100644 --- a/src/sentry/scripts/spans/add-buffer.lua +++ b/src/sentry/scripts/spans/add-buffer.lua @@ -14,7 +14,7 @@ is received. This implies that it has to operate according to these steps: 1. Identify the highest level known span for a subsegment. -2. Merge subsegments when a common parent is found. +2. Update the member-keys index and counters when a common parent is found. 3. Update the redirect set to reflect the current state of the tree. @@ -26,14 +26,11 @@ ARGS: - parent_span_id -- str -- The parent span id of the root of the subsegment. - has_root_span -- "true" or "false" -- Whether the subsegment contains the root of the segment. - set_timeout -- int -- max_segment_bytes -- int -- The maximum number of bytes the segment can contain. - byte_count -- int -- The total number of bytes in the subsegment. -- write_distributed_payloads -- "true" or "false" -- When true, maintain member-keys tracking sets for distributed payload keys. -- write_merged_payloads -- "true" or "false" -- When false, skip set merges and set keys expire cmds. - *span_id -- str[] -- The span ids in the subsegment. RETURNS: -- set_key -- str -- The Redis key of the segment this subsegment was merged into. +- set_key -- str -- The key of the segment, used to look up member-keys index and identify the segment in the queue. - has_root_span -- bool -- Whether this segment contains a root span. - latency_ms -- number -- Milliseconds elapsed during script execution. - latency_table -- table -- Per-step latency measurements. @@ -47,11 +44,8 @@ local num_spans = ARGV[1] local parent_span_id = ARGV[2] local has_root_span = ARGV[3] == "true" local set_timeout = tonumber(ARGV[4]) -local max_segment_bytes = tonumber(ARGV[5]) -local byte_count = tonumber(ARGV[6]) -local write_distributed_payloads = ARGV[7] == "true" -local write_merged_payloads = ARGV[8] == "true" -local NUM_ARGS = 8 +local byte_count = tonumber(ARGV[5]) +local NUM_ARGS = 5 local function get_time_ms() local time = redis.call("TIME") @@ -82,11 +76,7 @@ local latency_table = {} local metrics_table = {} table.insert(metrics_table, {"redirect_table_size", redis.call("hlen", main_redirect_key)}) table.insert(metrics_table, {"redirect_depth", redirect_depth}) -local redirect_end_time_ms = get_time_ms() -table.insert(latency_table, {"redirect_step_latency_ms", redirect_end_time_ms - start_time_ms}) - local set_key = string.format("span-buf:s:{%s}:%s", project_and_trace, set_span_id) -local parent_key = string.format("span-buf:s:{%s}:%s", project_and_trace, parent_span_id) -- Reset the set expiry as we saw a new subsegment for this set local has_root_span_key = string.format("span-buf:hrs:%s", set_key) @@ -96,201 +86,99 @@ if has_root_span then end local hset_args = {} -local sunionstore_args = {} - --- Merge the subsegment into the segment we are assembling. --- Merging the spans (`sunionstore_args`) is needed to compose the payloads in --- the same segment for them to be flushed later. --- Updating the redirect set instead is needed when we receive higher level spans --- for a tree we are assembling as the segment root each span points at in the --- redirect set changes when a new root is found. -if write_merged_payloads and set_span_id ~= parent_span_id and redis.call("scard", parent_key) > 0 then - table.insert(sunionstore_args, parent_key) -end for i = NUM_ARGS + 1, NUM_ARGS + num_spans do local span_id = ARGV[i] - local is_root_span = span_id == parent_span_id table.insert(hset_args, span_id) table.insert(hset_args, set_span_id) - - if not is_root_span and write_merged_payloads then - local span_key = string.format("span-buf:s:{%s}:%s", project_and_trace, span_id) - table.insert(sunionstore_args, span_key) - end end redis.call("hset", main_redirect_key, unpack(hset_args)) redis.call("expire", main_redirect_key, set_timeout) -local sunionstore_args_end_time_ms = get_time_ms() -table.insert(latency_table, {"sunionstore_args_step_latency_ms", sunionstore_args_end_time_ms - redirect_end_time_ms}) +local redirect_end_time_ms = get_time_ms() +table.insert(latency_table, {"redirect_step_latency_ms", redirect_end_time_ms - start_time_ms}) --- Merge spans into the parent span set. --- Used outside the if statement -local arg_cleanup_end_time_ms = sunionstore_args_end_time_ms -- Maintain member-keys (span-buf:mk) tracking sets so the flusher --- knows which distributed keys to fetch. This runs in both write-only and --- full distributed mode. -if write_distributed_payloads then - local member_keys_key = string.format("span-buf:mk:{%s}:%s", project_and_trace, set_span_id) - redis.call("sadd", member_keys_key, parent_span_id) - - -- Merge child tracking sets from span_ids that were previously segment roots. - for i = NUM_ARGS + 1, NUM_ARGS + num_spans do - local span_id = ARGV[i] - if span_id ~= parent_span_id then - local child_mk_key = string.format("span-buf:mk:{%s}:%s", project_and_trace, span_id) - local child_members = redis.call("smembers", child_mk_key) - if #child_members > 0 then - redis.call("sadd", member_keys_key, unpack(child_members)) - redis.call("del", child_mk_key) - end - end - end +-- knows which payload keys to fetch. +local member_keys_key = string.format("span-buf:mk:{%s}:%s", project_and_trace, set_span_id) +redis.call("sadd", member_keys_key, parent_span_id) - -- Merge parent's tracking set if parent_span_id redirected to a different root. - if set_span_id ~= parent_span_id then - local parent_mk_key = string.format("span-buf:mk:{%s}:%s", project_and_trace, parent_span_id) - local parent_members = redis.call("smembers", parent_mk_key) - if #parent_members > 0 then - redis.call("sadd", member_keys_key, unpack(parent_members)) - redis.call("del", parent_mk_key) +-- Merge child tracking sets from span_ids that were previously segment roots. +for i = NUM_ARGS + 1, NUM_ARGS + num_spans do + local span_id = ARGV[i] + if span_id ~= parent_span_id then + local child_mk_key = string.format("span-buf:mk:{%s}:%s", project_and_trace, span_id) + local child_members = redis.call("smembers", child_mk_key) + if #child_members > 0 then + redis.call("sadd", member_keys_key, unpack(child_members)) + redis.call("del", child_mk_key) end end - - redis.call("expire", member_keys_key, set_timeout) - arg_cleanup_end_time_ms = get_time_ms() - table.insert(latency_table, {"distributed_tracking_step_latency_ms", arg_cleanup_end_time_ms - sunionstore_args_end_time_ms}) end --- When write_merged_payloads is false, merged set merges are skipped but we --- still need to merge ic/ibc counters from child keys into the segment root. -if not write_merged_payloads then - local ingested_count_key = string.format("span-buf:ic:%s", set_key) - local ingested_byte_count_key = string.format("span-buf:ibc:%s", set_key) - for i = NUM_ARGS + 1, NUM_ARGS + num_spans do - local span_id = ARGV[i] - if span_id ~= parent_span_id then - local child_merged = string.format("span-buf:s:{%s}:%s", project_and_trace, span_id) - local child_ic_key = string.format("span-buf:ic:%s", child_merged) - local child_ibc_key = string.format("span-buf:ibc:%s", child_merged) - local child_count = redis.call("get", child_ic_key) - local child_byte_count = redis.call("get", child_ibc_key) - if child_count then - redis.call("incrby", ingested_count_key, child_count) - redis.call("del", child_ic_key) - end - if child_byte_count then - redis.call("incrby", ingested_byte_count_key, child_byte_count) - redis.call("del", child_ibc_key) - end - end +-- Merge parent's tracking set if parent_span_id redirected to a different root. +if set_span_id ~= parent_span_id then + local parent_mk_key = string.format("span-buf:mk:{%s}:%s", project_and_trace, parent_span_id) + local parent_members = redis.call("smembers", parent_mk_key) + if #parent_members > 0 then + redis.call("sadd", member_keys_key, unpack(parent_members)) + redis.call("del", parent_mk_key) end - if set_span_id ~= parent_span_id then - local parent_merged = string.format("span-buf:s:{%s}:%s", project_and_trace, parent_span_id) - local parent_ic_key = string.format("span-buf:ic:%s", parent_merged) - local parent_ibc_key = string.format("span-buf:ibc:%s", parent_merged) - local parent_count = redis.call("get", parent_ic_key) - local parent_byte_count = redis.call("get", parent_ibc_key) - if parent_count then - redis.call("incrby", ingested_count_key, parent_count) - redis.call("del", parent_ic_key) +end + +redis.call("expire", member_keys_key, set_timeout) +local merge_payload_keys_end_time_ms = get_time_ms() +table.insert(latency_table, {"merge_payload_keys_step_latency_ms", merge_payload_keys_end_time_ms - redirect_end_time_ms}) + +-- Merge ic/ibc counters from child keys into the segment root. +local ingested_count_key = string.format("span-buf:ic:%s", set_key) +local ingested_byte_count_key = string.format("span-buf:ibc:%s", set_key) +for i = NUM_ARGS + 1, NUM_ARGS + num_spans do + local span_id = ARGV[i] + if span_id ~= parent_span_id then + local child_merged = string.format("span-buf:s:{%s}:%s", project_and_trace, span_id) + local child_ic_key = string.format("span-buf:ic:%s", child_merged) + local child_ibc_key = string.format("span-buf:ibc:%s", child_merged) + local child_count = redis.call("get", child_ic_key) + local child_byte_count = redis.call("get", child_ibc_key) + if child_count then + redis.call("incrby", ingested_count_key, child_count) + redis.call("del", child_ic_key) end - if parent_byte_count then - redis.call("incrby", ingested_byte_count_key, parent_byte_count) - redis.call("del", parent_ibc_key) + if child_byte_count then + redis.call("incrby", ingested_byte_count_key, child_byte_count) + redis.call("del", child_ibc_key) end end - arg_cleanup_end_time_ms = get_time_ms() - table.insert(latency_table, {"distributed_ibc_merge_step_latency_ms", arg_cleanup_end_time_ms - sunionstore_args_end_time_ms}) - -elseif #sunionstore_args > 0 then - local ingested_byte_count_key = string.format("span-buf:ibc:%s", set_key) - local dest_bytes = tonumber(redis.call("get", ingested_byte_count_key) or 0) - - local already_oversized = dest_bytes > max_segment_bytes - table.insert(metrics_table, {"parent_span_set_already_oversized", already_oversized and 1 or 0}) - - local start_output_size = redis.call("scard", set_key) - local scard_end_time_ms = get_time_ms() - table.insert(latency_table, {"scard_step_latency_ms", scard_end_time_ms - sunionstore_args_end_time_ms}) - - local output_size - if already_oversized then - -- Dest already exceeds max_segment_bytes, skip merge entirely. - output_size = start_output_size - else - -- Read each source set and SADD its members into dest. - -- We use SMEMBERS+SADD instead of SUNIONSTORE because SUNIONSTORE - -- re-serialises the entire destination set on every call, making it - -- O(destination + sources). SMEMBERS+SADD is O(sources) only, which - -- is significantly cheaper when the destination set is large. - local all_members = {} - for i = 1, #sunionstore_args do - local members = redis.call("smembers", sunionstore_args[i]) - for j = 1, #members do - all_members[#all_members + 1] = members[j] - end - end - table.insert(metrics_table, {"zero_copy_dest_source_members", #all_members}) - -- Batch SADD in chunks to avoid exceeding Lua's unpack() stack limit. - local BATCH = 7000 - for i = 1, #all_members, BATCH do - local last = math.min(i + BATCH - 1, #all_members) - redis.call("sadd", set_key, unpack(all_members, i, last)) - end - output_size = redis.call("scard", set_key) +end +if set_span_id ~= parent_span_id then + local parent_merged = string.format("span-buf:s:{%s}:%s", project_and_trace, parent_span_id) + local parent_ic_key = string.format("span-buf:ic:%s", parent_merged) + local parent_ibc_key = string.format("span-buf:ibc:%s", parent_merged) + local parent_count = redis.call("get", parent_ic_key) + local parent_byte_count = redis.call("get", parent_ibc_key) + if parent_count then + redis.call("incrby", ingested_count_key, parent_count) + redis.call("del", parent_ic_key) end - local sunionstore_end_time_ms = get_time_ms() - table.insert(latency_table, {"sunionstore_step_latency_ms", sunionstore_end_time_ms - scard_end_time_ms}) - - redis.call("unlink", unpack(sunionstore_args)) - local unlink_end_time_ms = get_time_ms() - table.insert(latency_table, {"unlink_step_latency_ms", unlink_end_time_ms - sunionstore_end_time_ms}) - - table.insert(metrics_table, {"parent_span_set_before_size", start_output_size}) - table.insert(metrics_table, {"parent_span_set_after_size", output_size}) - table.insert(metrics_table, {"elements_added", output_size - start_output_size}) - - -- Merge ingested count keys for merged spans - local ingested_count_key = string.format("span-buf:ic:%s", set_key) - for i = 1, #sunionstore_args do - local merged_key = sunionstore_args[i] - local merged_ic_key = string.format("span-buf:ic:%s", merged_key) - local merged_ibc_key = string.format("span-buf:ibc:%s", merged_key) - local merged_count = redis.call("get", merged_ic_key) - local merged_byte_count = redis.call("get", merged_ibc_key) - if merged_count then - redis.call("incrby", ingested_count_key, merged_count) - end - if merged_byte_count then - redis.call("incrby", ingested_byte_count_key, merged_byte_count) - end - redis.call("del", merged_ic_key) - redis.call("del", merged_ibc_key) + if parent_byte_count then + redis.call("incrby", ingested_byte_count_key, parent_byte_count) + redis.call("del", parent_ibc_key) end - - arg_cleanup_end_time_ms = get_time_ms() - table.insert(latency_table, {"arg_cleanup_step_latency_ms", arg_cleanup_end_time_ms - unlink_end_time_ms}) end - +local counter_merge_end_time_ms = get_time_ms() +table.insert(latency_table, {"counter_merge_step_latency_ms", counter_merge_end_time_ms - merge_payload_keys_end_time_ms}) -- Track total number of spans ingested for this segment -local ingested_count_key = string.format("span-buf:ic:%s", set_key) -local ingested_byte_count_key = string.format("span-buf:ibc:%s", set_key) redis.call("incrby", ingested_count_key, num_spans) redis.call("incrby", ingested_byte_count_key, byte_count) redis.call("expire", ingested_count_key, set_timeout) redis.call("expire", ingested_byte_count_key, set_timeout) -if write_merged_payloads then - redis.call("expire", set_key, set_timeout) -end - local ingested_count_end_time_ms = get_time_ms() -local ingested_count_step_latency_ms = ingested_count_end_time_ms - arg_cleanup_end_time_ms +local ingested_count_step_latency_ms = ingested_count_end_time_ms - counter_merge_end_time_ms table.insert(latency_table, {"ingested_count_step_latency_ms", ingested_count_step_latency_ms}) -- Capture end time and calculate latency in milliseconds diff --git a/src/sentry/scripts/spans/done-flush-segment-data.lua b/src/sentry/scripts/spans/done-flush-segment-data.lua index b4df7a1a6b9a..9a5e3ffe29a0 100644 --- a/src/sentry/scripts/spans/done-flush-segment-data.lua +++ b/src/sentry/scripts/spans/done-flush-segment-data.lua @@ -1,4 +1,4 @@ --- Conditionally delete segment data only if the ingested count hasn't changed. +-- Conditionally delete segment metadata only if the ingested count hasn't changed. -- This is atomic with add-buffer.lua on the same {project_id:trace_id} slot, -- preventing data loss when process_spans adds new spans between flush and cleanup. -- @@ -17,7 +17,6 @@ if ic and tonumber(ic) == expected_ic then redis.call("DEL", "span-buf:hrs:" .. segment_key) redis.call("DEL", ic_key) redis.call("DEL", "span-buf:ibc:" .. segment_key) - redis.call("UNLINK", segment_key) return 1 end diff --git a/src/sentry/seer/autofix/autofix_agent.py b/src/sentry/seer/autofix/autofix_agent.py index fba4ae85ed74..16bca7feffb6 100644 --- a/src/sentry/seer/autofix/autofix_agent.py +++ b/src/sentry/seer/autofix/autofix_agent.py @@ -5,7 +5,6 @@ from enum import StrEnum from typing import TYPE_CHECKING, Literal -from django.utils import timezone from pydantic import BaseModel from sentry.seer.autofix.artifact_schemas import ( @@ -248,8 +247,6 @@ def trigger_autofix_explorer( artifact_schema=artifact_schema, ) - group.update(seer_explorer_autofix_last_triggered=timezone.now()) - payload = { "run_id": run_id, "group_id": group.id, @@ -510,6 +507,7 @@ def trigger_push_changes( run_id: int, referrer: AutofixReferrer, state: SeerRunState | None = None, + repo_name: str | None = None, ): client = get_autofix_explorer_client(group) @@ -523,7 +521,14 @@ def trigger_push_changes( if group_id != group.id: raise SeerPermissionError("Unknown run id for group") - client.push_changes(run_id, blocking=False) + client.push_changes( + run_id, + repo_name=repo_name, + pr_description_suffix=( + f"Fixes {group.qualified_short_id}" if group.qualified_short_id else None + ), + blocking=False, + ) metrics.incr( "autofix.explorer.trigger", diff --git a/src/sentry/seer/autofix/on_completion_hook.py b/src/sentry/seer/autofix/on_completion_hook.py index e82ae9412bf2..cd682cc37627 100644 --- a/src/sentry/seer/autofix/on_completion_hook.py +++ b/src/sentry/seer/autofix/on_completion_hook.py @@ -3,6 +3,8 @@ import logging from typing import TYPE_CHECKING +from django.utils import timezone + from sentry import features from sentry.models.group import Group from sentry.models.organization import Organization @@ -76,13 +78,20 @@ def execute(cls, organization: Organization, run_id: int) -> None: ) return + group_id = state.metadata.get("group_id") if state.metadata else None + if group_id is None: + return + + group = Group.objects.get(id=group_id, project__organization_id=organization.id) + group.update(seer_explorer_autofix_last_triggered=timezone.now()) + # Send webhook for the completed step - cls._send_step_webhook(organization, run_id, state) + cls._send_step_webhook(organization, run_id, state, group) - cls._maybe_trigger_supergroups_embedding(organization, run_id, state) + cls._maybe_trigger_supergroups_embedding(organization, run_id, state, group) # Continue the automated pipeline if stopping_point hasn't been reached - cls._maybe_continue_pipeline(organization, run_id, state) + cls._maybe_continue_pipeline(organization, run_id, state, group) @classmethod def find_latest_artifact_for_step(cls, state: SeerRunState, key: str) -> Artifact | None: @@ -95,7 +104,13 @@ def find_latest_artifact_for_step(cls, state: SeerRunState, key: str) -> Artifac return None @classmethod - def _send_step_webhook(cls, organization, run_id, state: SeerRunState): + def _send_step_webhook( + cls, + organization: Organization, + run_id: int, + state: SeerRunState, + group: Group, + ): """ Send webhook for the completed step. @@ -103,11 +118,10 @@ def _send_step_webhook(cls, organization, run_id, state: SeerRunState): """ current_step = cls._get_current_step(state) - webhook_payload = {"run_id": run_id} - - group_id = state.metadata.get("group_id") if state.metadata else None - if group_id is not None: - webhook_payload["group_id"] = group_id + webhook_payload = { + "run_id": run_id, + "group_id": group.id, + } # Iterate through blocks in reverse order (most recent first) # to find which step just completed @@ -213,25 +227,13 @@ def _maybe_trigger_supergroups_embedding( organization: Organization, run_id: int, state: SeerRunState, + group: Group, ) -> None: """Trigger supergroups embedding if feature flag is enabled.""" current_step = cls._get_current_step(state) if current_step != AutofixStep.ROOT_CAUSE: return - group_id = state.metadata.get("group_id") if state.metadata else None - if group_id is None: - return - - try: - group = Group.objects.get(id=group_id) - except Group.DoesNotExist: - logger.warning( - "autofix.supergroup_embedding.group_not_found", - extra={"group_id": group_id}, - ) - return - if not features.has("projects:supergroup-embeddings-explorer", group.project): return @@ -242,7 +244,7 @@ def _maybe_trigger_supergroups_embedding( try: trigger_supergroups_embedding( organization_id=organization.id, - group_id=group_id, + group_id=group.id, project_id=group.project_id, artifact_data=root_cause_artifact.data, ) @@ -252,7 +254,7 @@ def _maybe_trigger_supergroups_embedding( extra={ "run_id": run_id, "organization_id": organization.id, - "group_id": group_id, + "group_id": group.id, }, ) @@ -289,6 +291,7 @@ def _maybe_continue_pipeline( organization: Organization, run_id: int, state: SeerRunState, + group: Group, ) -> None: """ Continue to the next step if stopping_point hasn't been reached. @@ -307,28 +310,6 @@ def _maybe_continue_pipeline( return stopping_point = AutofixStoppingPoint(metadata["stopping_point"]) - group_id = metadata.get("group_id") - - if not group_id: - logger.warning( - "autofix.on_completion_hook.no_group_id_in_metadata", - extra={"run_id": run_id, "organization_id": organization.id}, - ) - return - - # Get the group - try: - group = Group.objects.get(id=group_id, project__organization=organization) - except Group.DoesNotExist: - logger.warning( - "autofix.on_completion_hook.group_not_found", - extra={ - "run_id": run_id, - "organization_id": organization.id, - "group_id": group_id, - }, - ) - return if current_step is None: logger.warning( diff --git a/src/sentry/seer/autofix/trigger.py b/src/sentry/seer/autofix/trigger.py index e0687c7e8fd4..b52e3ca9478a 100644 --- a/src/sentry/seer/autofix/trigger.py +++ b/src/sentry/seer/autofix/trigger.py @@ -11,6 +11,13 @@ ) from sentry.utils.cache import cache +# This timeout should be longer than what we expect the seer agents to take. +# This is because we do not want to trigger another run while the previous +# run is still running. +# +# NOTE: The timeout on the seer job is 5 minutes. +SEER_AUTOMATION_TIMEOUT_SECONDS = 30 * 60 + if TYPE_CHECKING: from sentry.models.group import Group from sentry.utils.locking.manager import LockManager @@ -171,7 +178,7 @@ def get_seat_based_seer_automation_skip_reason( # Atomically set cache to prevent duplicate dispatches (returns False if key exists) automation_dispatch_cache_key = f"seer-automation-dispatched:{group.id}" - if not cache.add(automation_dispatch_cache_key, True, timeout=300): + if not cache.add(automation_dispatch_cache_key, True, timeout=SEER_AUTOMATION_TIMEOUT_SECONDS): return "automation_already_dispatched" # Another process already dispatched automation # Check if project has connected repositories - requirement for new pricing diff --git a/src/sentry/seer/endpoints/group_ai_autofix.py b/src/sentry/seer/endpoints/group_ai_autofix.py index f4d32cca2525..d2ccb4f613c8 100644 --- a/src/sentry/seer/endpoints/group_ai_autofix.py +++ b/src/sentry/seer/endpoints/group_ai_autofix.py @@ -125,6 +125,10 @@ class ExplorerAutofixRequestSerializer(CamelSnakeSerializer): help_text="Optional user context to append to the step prompt.", allow_blank=True, ) + repo_name = serializers.CharField( + required=False, + help_text="Optional repository name for which to create the pull request. Do not pass a repository name to create pull requests in all relevant repositories.", + ) def validate(self, data: dict[str, Any]) -> dict[str, Any]: stopping_point = data.get("stopping_point", None) @@ -263,11 +267,13 @@ def _post_explorer(self, request: Request, group: Group) -> Response: return Response( {"detail": "run_id is required for open_pr"}, status=status.HTTP_400_BAD_REQUEST ) + repo_name = data.get("repo_name") try: trigger_push_changes( group, run_id, referrer=AutofixReferrer.GROUP_AUTOFIX_ENDPOINT, + repo_name=repo_name, ) except SeerPermissionError: return Response(status=status.HTTP_404_NOT_FOUND) diff --git a/src/sentry/seer/endpoints/organization_seer_explorer_chat.py b/src/sentry/seer/endpoints/organization_seer_explorer_chat.py index f07d39ed0a7b..3969a125af1f 100644 --- a/src/sentry/seer/endpoints/organization_seer_explorer_chat.py +++ b/src/sentry/seer/endpoints/organization_seer_explorer_chat.py @@ -17,6 +17,7 @@ from sentry.seer.explorer.client import SeerExplorerClient from sentry.seer.explorer.client_utils import has_seer_explorer_access_with_detail from sentry.seer.models import SeerPermissionError +from sentry.seer.seer_setup import has_seer_access_with_detail from sentry.types.ratelimit import RateLimit, RateLimitCategory logger = logging.getLogger(__name__) @@ -83,7 +84,13 @@ def get( Get the current state of a Seer Explorer session. """ has_access, error = has_seer_explorer_access_with_detail(organization, request.user) - if not has_access: + + has_seer_access, _ = has_seer_access_with_detail(organization, request.user) + has_dashboards_ai_generate_access = has_seer_access and features.has( + "organizations:dashboards-ai-generate", organization, actor=request.user + ) + + if not has_access and not has_dashboards_ai_generate_access: raise PermissionDenied(error) if not run_id: @@ -115,7 +122,17 @@ def post( - run_id: The run ID. """ has_access, error = has_seer_explorer_access_with_detail(organization, request.user) - if not has_access: + + has_seer_access, _ = has_seer_access_with_detail(organization, request.user) + has_dashboards_ai_generate_access = has_seer_access and features.has( + "organizations:dashboards-ai-generate", organization, actor=request.user + ) + # Orgs with dashboards AI generate access can continue existing dashboard generate runs, but cannot start new runs from this endpoint. + can_continue_dashboards_generate_run = ( + has_dashboards_ai_generate_access and run_id is not None + ) + + if not has_access and not can_continue_dashboards_generate_run: raise PermissionDenied(error) serializer = SeerExplorerChatSerializer(data=request.data) diff --git a/src/sentry/seer/endpoints/search_agent_start.py b/src/sentry/seer/endpoints/search_agent_start.py index 05cf1303c89e..4d221cef2fe7 100644 --- a/src/sentry/seer/endpoints/search_agent_start.py +++ b/src/sentry/seer/endpoints/search_agent_start.py @@ -42,7 +42,7 @@ class SearchAgentStartSerializer(serializers.Serializer): strategy = serializers.CharField( required=False, default="Traces", - help_text="Search strategy to use (Traces, Issues, Logs, Errors).", + help_text="Search strategy to use (Traces, Issues, Logs, Errors, Metrics).", ) options = serializers.DictField( required=False, diff --git a/src/sentry/seer/explorer/client.py b/src/sentry/seer/explorer/client.py index 9e2d1baf14f0..5d8867ad06ea 100644 --- a/src/sentry/seer/explorer/client.py +++ b/src/sentry/seer/explorer/client.py @@ -527,7 +527,8 @@ def push_changes( self, run_id: int, repo_name: str | None = None, - blocking=True, + blocking: bool = True, + pr_description_suffix: str | None = None, poll_interval: float = 2.0, poll_timeout: float = 120.0, ) -> SeerRunState | None: @@ -554,6 +555,8 @@ def push_changes( payload: dict[str, Any] = {"type": "create_pr"} if repo_name: payload["repo_name"] = repo_name + if pr_description_suffix: + payload["pr_description_suffix"] = pr_description_suffix if self.on_completion_hook: payload["on_completion_hook"] = extract_hook_definition(self.on_completion_hook).dict() update_body = ExplorerUpdateRequest( diff --git a/src/sentry/sentry_apps/tasks/sentry_apps.py b/src/sentry/sentry_apps/tasks/sentry_apps.py index 8677fe9e15a5..3800cb619db7 100644 --- a/src/sentry/sentry_apps/tasks/sentry_apps.py +++ b/src/sentry/sentry_apps/tasks/sentry_apps.py @@ -493,18 +493,6 @@ def installation_webhook(installation_id: int, user_id: int, *args: Any, **kwarg ).run() -# TODO(cells): remove once in-flight tasks with old name have drained -@instrumented_task( - name="sentry.sentry_apps.tasks.sentry_apps.clear_region_cache", - namespace=sentryapp_control_tasks, - retry=Retry(times=3, delay=60 * 5), - processing_deadline_duration=30, - silo_mode=SiloMode.CONTROL, -) -def clear_region_cache(sentry_app_id: int, region_name: str) -> None: - clear_cell_cache(sentry_app_id=sentry_app_id, cell_name=region_name) - - @instrumented_task( name="sentry.sentry_apps.tasks.sentry_apps.clear_cell_cache", namespace=sentryapp_control_tasks, diff --git a/src/sentry/spans/README.md b/src/sentry/spans/README.md index 764bfe24eef0..e8d0b271a785 100644 --- a/src/sentry/spans/README.md +++ b/src/sentry/spans/README.md @@ -215,39 +215,49 @@ erDiagram are assembled without knowledge of the whole Segment tree, so the common parent between two subsegments may have not been received at the time of assembling them. +**Payload keys** + +``` +span-buf:s:{PROJECT_ID:TRACE_ID:SPAN_ID}:SPAN_ID +``` + +Each subsegment gets its own payload key (`{project_id:trace_id:span_id}:span_id`). +This distributes payloads across Redis cluster nodes instead of concentrating +all spans of a trace on a single node. + +**Member-keys index** + +``` +span-buf:mk:{PROJECT_AND_TRACE}:ROOT_SPAN_ID +``` + +This is an unsorted set that tracks which payload keys belong to a +segment. The flusher reads this index to discover all payload keys, then scans +each one to load the span data. + ## Redis data model The redis code that is used to assemble segments is in [`add-buffer.lua`](../scripts/spans/add-buffer.lua). Redis keeps the content of a segment and its structure in a few keys described -later. The script receives a set of spans that represent a `Subsegment` from -the python code and: +later. + +Span payloads are stored in **payload keys**. The Python code writes payloads +to these keys before invoking the Lua script. The Lua script receives only +the span IDs of a `Subsegment` and: 1. Assigns them to the right segment by traversing the tree to the root or what we assume the root of the tree is at the moment the script is invoked. As we receive spans out of order we may receive the leaves before the root of the tree. We traverse the tree each time as high as we can -2. Assigns the appropriate timeouts to the segments to inform the flusher logic - of when a segment should be flushed. -3. Merges the spans payload into the same segment sorted set and drops spans that - are too old. - -**Spans payload** - -``` -span-buf:s:{PROJECT_AND_TRACE}:{PARENT_SPAN_ID} -``` - -This is an unsorted set that contains the spans payload corresponding to a segment. - -- The python code stores the subsegments, while the LUA code merges subsegments - along the way. -- The parent_span_id, which is part of the key, represent the root span of the - segment. -- While we are still assembling the tree we may have multiple keys like this - for subsegments. For each of them, the parent_span_id is the parent of the - root of the subsegment. -- The parent span id represents the parent span of the root of the subsegment -- Each entry is the payload of a span. +2. Updates the redirect table so future spans can find the segment root. +3. Merges **member-keys indexes** into the current segment root. This happens in two passes: + - **Child spans**: For each span ID in the subsegment (except the parent), + if it was previously a segment root with its own member-keys index or + counters, those are merged into the current root and the old keys are + deleted. + - **Parent span**: If the parent span ID redirects to a different root + (i.e. it is not the segment root itself), its member-keys are also merged + into the new root. **Redirect set** @@ -292,8 +302,7 @@ The score of each element is the timestamp the segment will expire at. span-buf:q:{SHARD} or span-buf:q:{SLICE_ID}-{SHARD} ``` -- The elements are the redis keys of the segments payload after being merged by - the LUA scripts. +- The elements are segment keys, used to look up metadata and the member-keys index. - This is used as a queue by the Flusher to find the oldest segment to be flushed. ## Flushing @@ -305,46 +314,12 @@ by at most one consumer, no two consumers can flush from the same queue. Flushing happens in two steps: 1. `flush_segments`: reads segment keys from the queue via `ZRANGEBYSCORE` - (segments past their flush deadline), loads their span payloads via `SSCAN`, - and produces them to the `buffered-segments` Kafka topic. -2. `done_flush_segments`: cleans up Redis keys (`UNLINK` the segment data, - `DELETE` metadata keys, `HDEL` redirect entries, `ZREM` the queue entry). - -### Rebalancing - -The consumer routes each span to a queue based on its trace ID and the -consumer's currently assigned partitions: - -```python -shard = self.assigned_shards[ - int(trace_id, 16) % len(self.assigned_shards) -] -``` - -This means the queue a segment lands in depends on both the trace ID and the -current set of assigned partitions. On a Kafka rebalance (triggered by -consumer deployment, scaling, or crashes), `assigned_shards` changes, possibly -in length or in values. Then, the same trace ID may route to a different queue. - -As a consequence, during every rebalance: - -1. In-flight segments may have their key present in two different queues - (the old queue from before the rebalance and the new queue after). -2. Whichever flusher processes the segment first gets the data and deletes the - Redis key via `done_flush_segments`. -3. `done_flush_segments` only removes the queue entry from its own queue - (`ZREM` on `flushed_segment.queue_key`). It does not know about the stale - entry in the other queue. -4. When the other flusher later picks up the stale entry, `SSCAN` returns - nothing (the key was already deleted), resulting in an empty segment - (`spans.buffer.empty_segments` metric). - -This is expected on every deployment. The typical consequence (and is a confirmed bug) -is that a segment gets broken up: spans that arrived before the rebalance are in one -queue, and spans that arrived after are in another. Each queue's flusher produces an -incomplete segment independently — one with the earlier spans, the others with -the later spans. In the worst case, if both flushers race and read the same segment -before either deletes it, the result is a duplicated segment. + (segments past their flush deadline), looks up the member-keys index to + discover payload keys, loads span payloads from each via `SSCAN`, and + produces them to the `buffered-segments` Kafka topic. +2. `done_flush_segments`: cleans up Redis keys (`DELETE` metadata, + `UNLINK` payload keys, `DELETE` member-keys index, `HDEL` redirect + entries, `ZREM` the queue entry). # GCP Log Analyzer Tool diff --git a/src/sentry/spans/buffer.py b/src/sentry/spans/buffer.py index 5cf0c2367fb8..a00d7517a62b 100644 --- a/src/sentry/spans/buffer.py +++ b/src/sentry/spans/buffer.py @@ -32,16 +32,21 @@ Now how does that look like in Redis? For each incoming span, we: -1. Try to figure out what the name of the respective span buffer is (`set_key` in `add-buffer.lua`) - a. We look up any "redirects" from the span buffer's parent_span_id (hashmap at "span-buf:ssr:{project_id:trace_id}") to another key. - b. Otherwise we use "span-buf:s:{project_id:trace_id}:span_id" -2. Rename any span buffers keyed under the span's own span ID to `set_key`, merging their contents. -3. Add the ingested span's payload to the set under `set_key`. -4. To a "global queue", we write the set's key, sorted by timeout. +1. Store the span payload in a payload key: + "span-buf:s:{project_id:trace_id:span_id}:span_id". Each subsegment + gets its own key, distributed across Redis cluster nodes. +2. The Lua script (add-buffer.lua) receives the span IDs and: + a. Follows redirects from parent_span_id (hashmap at + "span-buf:ssr:{project_id:trace_id}") to find the segment root. + b. Updates the redirect table so future spans can find the segment root. + c. Merges member-keys indexes and counters (ingested count, byte count) + from span IDs that were previously separate segment roots into the + current segment root. +3. To a "global queue", we write the segment key, sorted by timeout. Eventually, flushing cronjob looks at that global queue, and removes all timed -out keys from it. Then fetches the sets associated with those keys, and deletes -the sets. +out keys from it. Then fetches the payload keys for each segment via the +member-keys index, loads their data, and cleans up. This happens in two steps: Get the to-be-flushed segments in `flush_segments`, then the consumer produces them, then they are deleted from Redis @@ -55,12 +60,14 @@ Glossary for types of keys: - * span-buf:s:* -- the actual set keys, containing span payloads. Each key contains all data for a segment. The most memory-intensive kind of key. + * span-buf:s:{project_id:trace_id:span_id}:span_id -- payload keys containing span payloads, distributed across cluster nodes. + * span-buf:mk:{project_id:trace_id}:root_span_id -- member-keys index, tracks which payload keys belong to a segment. * span-buf:q:* -- the priority queue, used to determine which segments are ready to be flushed. - * span-buf:hrs:* -- simple bool key to flag a segment as "has root span" (HRS) - * span-buf:ssr:* -- redirect mappings so that each incoming span ID can be mapped to the right span-buf:s: set. - * span-buf:ic:* -- ingested count, tracks total number of spans originally ingested for a segment (used to calculate dropped spans for outcome tracking) - * span-buf:ibc:* -- ingested byte count, tracks total bytes originally ingested for a segment + * span-buf:ssr:{project_id:trace_id} -- redirect mappings so that each incoming span ID can be mapped to the right segment. + * span-buf:hrs: -- flags a segment as "has root span" (HRS). + * span-buf:ic: -- ingested count, total spans originally ingested for a segment. + * span-buf:ibc: -- ingested byte count, total bytes originally ingested for a segment. + -- an internal identifier, see `spans.segment_key` module. """ from __future__ import annotations @@ -204,11 +211,7 @@ def process_spans(self, spans: Sequence[Span], now: int): redis_ttl = options.get("spans.buffer.redis-ttl") timeout = options.get("spans.buffer.timeout") root_timeout = options.get("spans.buffer.root-timeout") - max_segment_bytes = options.get("spans.buffer.max-segment-bytes") max_spans_per_evalsha = options.get("spans.buffer.max-spans-per-evalsha") - write_distributed_payloads = options.get("spans.buffer.write-distributed-payloads") - write_merged_payloads = options.get("spans.buffer.write-merged-payloads") - result_meta = [] is_root_span_count = 0 @@ -236,14 +239,9 @@ def process_spans(self, spans: Sequence[Span], now: int): with self.client.pipeline(transaction=False) as p: for (project_and_trace, parent_span_id), subsegment in batch: set_members = self._prepare_payloads(subsegment) - if write_distributed_payloads: - dist_key = self._get_payload_key(project_and_trace, parent_span_id) - p.sadd(dist_key, *set_members) - p.expire(dist_key, redis_ttl) - - if write_merged_payloads: - set_key = self._get_span_key(project_and_trace, parent_span_id) - p.sadd(set_key, *set_members) + payload_key = self._get_payload_key(project_and_trace, parent_span_id) + p.sadd(payload_key, *set_members) + p.expire(payload_key, redis_ttl) p.execute() @@ -282,10 +280,7 @@ def process_spans(self, spans: Sequence[Span], now: int): parent_span_id, is_segment_span, redis_ttl, - max_segment_bytes, byte_count, - "true" if write_distributed_payloads else "false", - "true" if write_merged_payloads else "false", *span_ids, ) @@ -685,8 +680,6 @@ def _load_segment_data( page_size = options.get("spans.buffer.segment-page-size") max_segment_bytes = options.get("spans.buffer.max-segment-bytes") - read_distributed_payloads = options.get("spans.buffer.read-distributed-payloads") - write_distributed_payloads = options.get("spans.buffer.write-distributed-payloads") payloads: dict[SegmentKey, list[bytes]] = {key: [] for key in segment_keys} payload_keys_map: dict[SegmentKey, list[PayloadKey]] = {key: [] for key in segment_keys} @@ -694,37 +687,28 @@ def _load_segment_data( self._last_decompress_latency_ms = 0 decompress_latency_ms = 0.0 - # Maps each scan key back to the segment it belongs to. For merged - # keys these are the same; for distributed keys many map to one segment. + # Maps each payload key back to the segment it belongs to. + # Multiple distributed payload keys map to one segment. scan_key_to_segment: dict[SegmentKey | PayloadKey, SegmentKey] = {} - - # When read_distributed_payloads is off, scan merged segment keys directly. - # When on, skip them — all data lives in distributed keys. cursors: dict[bytes, int] = {} - if not read_distributed_payloads: - for key in segment_keys: - scan_key_to_segment[key] = key - cursors[key] = 0 - - if write_distributed_payloads: - with self.client.pipeline(transaction=False) as p: - for key in segment_keys: - p.smembers(self._get_payload_key_index(key)) - mk_results = p.execute() - for key, payload_key_span_ids in zip(segment_keys, mk_results): - project_id, trace_id, _ = parse_segment_key(key) - project_and_trace = f"{project_id.decode('ascii')}:{trace_id.decode('ascii')}" - segment_payload_keys: list[PayloadKey] = [] - for payload_key_span_id in payload_key_span_ids: - payload_key = self._get_payload_key( - project_and_trace, payload_key_span_id.decode("ascii") - ) - segment_payload_keys.append(payload_key) - if read_distributed_payloads: - scan_key_to_segment[payload_key] = key - cursors[payload_key] = 0 - payload_keys_map[key] = segment_payload_keys + with self.client.pipeline(transaction=False) as p: + for key in segment_keys: + p.smembers(self._get_payload_key_index(key)) + mk_results = p.execute() + + for key, payload_key_span_ids in zip(segment_keys, mk_results): + project_id, trace_id, _ = parse_segment_key(key) + project_and_trace = f"{project_id.decode('ascii')}:{trace_id.decode('ascii')}" + segment_payload_keys: list[PayloadKey] = [] + for payload_key_span_id in payload_key_span_ids: + payload_key = self._get_payload_key( + project_and_trace, payload_key_span_id.decode("ascii") + ) + segment_payload_keys.append(payload_key) + scan_key_to_segment[payload_key] = key + cursors[payload_key] = 0 + payload_keys_map[key] = segment_payload_keys dropped_segments: set[SegmentKey] = set() diff --git a/src/sentry/spans/segment_key.py b/src/sentry/spans/segment_key.py index eae58585049e..3322ff9fb0e5 100644 --- a/src/sentry/spans/segment_key.py +++ b/src/sentry/spans/segment_key.py @@ -1,6 +1,6 @@ -# SegmentKey is an internal identifier used by the redis buffer that is also -# directly used as raw redis key. the format is -# "span-buf:s:{project_id:trace_id}:span_id", and the type is bytes because our +# SegmentKey is an internal identifier used by the redis buffer to identify a segment +# in a queue or find the segment's spans payloads. +# The format is "span-buf:s:{project_id:trace_id}:span_id", and the type is bytes because our # redis client is bytes. # # The segment ID in the Kafka protocol is only the span ID. diff --git a/src/sentry/testutils/cell.py b/src/sentry/testutils/cell.py index 40a8cc8e17d9..7594b2b4edb0 100644 --- a/src/sentry/testutils/cell.py +++ b/src/sentry/testutils/cell.py @@ -100,7 +100,3 @@ def override_cells(cells: Sequence[Cell], local_cell: Cell | None = None) -> Gen """ with get_test_env_directory().swap_state(cells, local_cell=local_cell): yield - - -# TODO(cells): Remove alias once no longer used in getsentry -override_regions = override_cells diff --git a/src/sentry/testutils/region.py b/src/sentry/testutils/region.py deleted file mode 100644 index 47b22f30ec7a..000000000000 --- a/src/sentry/testutils/region.py +++ /dev/null @@ -1,2 +0,0 @@ -# TODO(cells): temporary shim for getsentry, remove once migrated -from sentry.testutils.cell import * # noqa: F401, F403 diff --git a/src/sentry/testutils/silo.py b/src/sentry/testutils/silo.py index 7a93e28c3e66..98a1be33456b 100644 --- a/src/sentry/testutils/silo.py +++ b/src/sentry/testutils/silo.py @@ -101,10 +101,6 @@ def create_test_cells(*names: str, single_tenants: Iterable[str] = ()) -> tuple[ ) -# TODO(cells): Remove alias once no longer used in getsentry -create_test_regions = create_test_cells - - def _model_silo_limit(t: type[Model]) -> ModelSiloLimit: from sentry.db.models.base import ModelSiloLimit @@ -173,8 +169,6 @@ def __call__[ self, *, cells: Sequence[Cell] = (), - # TODO(cells): Remove alias once no longer used in getsentry - regions: Sequence[Cell] | None = None, include_monolith_run: bool = False, ) -> Callable[[T], T]: ... @@ -183,16 +177,13 @@ def __call__( decorated_obj: Any = None, *, cells: Sequence[Cell] = (), - # TODO(cells): Remove alias once no longer used in getsentry - regions: Sequence[Cell] | None = None, include_monolith_run: bool = False, ) -> Any: silo_modes = self.silo_modes if include_monolith_run: silo_modes |= frozenset([SiloMode.MONOLITH]) - resolved_cells = tuple(cells or regions or ()) - mod = _SiloModeTestModification(silo_modes=silo_modes, cells=resolved_cells) + mod = _SiloModeTestModification(silo_modes=silo_modes, cells=tuple(cells)) return mod.apply if decorated_obj is None else mod.apply(decorated_obj) diff --git a/src/sentry/users/api/endpoints/user_regions.py b/src/sentry/users/api/endpoints/user_regions.py index 1cd3e49edb78..69e4fb70a210 100644 --- a/src/sentry/users/api/endpoints/user_regions.py +++ b/src/sentry/users/api/endpoints/user_regions.py @@ -40,6 +40,8 @@ def has_object_permission( return False +# TODO(cells): Deprecate once organization listing is moved to control and the frontend +# no longer needs locality URLs for client-side fan-out. @control_silo_endpoint class UserRegionsEndpoint(UserEndpoint): owner = ApiOwner.HYBRID_CLOUD diff --git a/src/sentry/users/models/authenticator.py b/src/sentry/users/models/authenticator.py index f3bb694f6f37..dc61849fbe28 100644 --- a/src/sentry/users/models/authenticator.py +++ b/src/sentry/users/models/authenticator.py @@ -184,9 +184,9 @@ class Meta: unique_together = (("user", "type"),) def outboxes_for_update(self, shard_identifier: int | None = None) -> list[ControlOutboxBase]: - regions = find_cells_for_user(self.user_id) + cells = find_cells_for_user(self.user_id) return OutboxCategory.USER_UPDATE.as_control_outboxes( - cell_names=regions, + cell_names=cells, shard_identifier=self.user_id, object_identifier=self.user_id, ) diff --git a/src/sentry/users/models/identity.py b/src/sentry/users/models/identity.py index de285fba13e2..81e13d7d7fb4 100644 --- a/src/sentry/users/models/identity.py +++ b/src/sentry/users/models/identity.py @@ -115,7 +115,7 @@ def link_identity( SlackIntegrationIdentityLinked( provider=IntegrationProviderSlug.SLACK.value, # Note that prior to circa March 2023 this was user.actor_id. It changed - # when actor ids were no longer stable between regions for the same user + # when actor ids were no longer stable between cells for the same user actor_id=user.id, actor_type="user", ) @@ -218,15 +218,15 @@ class Meta: def delete(self, *args: Any, **kwargs: Any) -> tuple[int, dict[str, int]]: with outbox_context(transaction.atomic(router.db_for_write(Identity))): - # Fan out to all regions to ensure HybridCloudForeignKey cascade works even without org memberships - region_names = find_all_cell_names() - for region_name in region_names: + # Fan out to all cells to ensure HybridCloudForeignKey cascade works even without org memberships + cell_names = find_all_cell_names() + for cell_name in cell_names: ControlOutbox( shard_scope=OutboxScope.USER_SCOPE, shard_identifier=self.user_id, object_identifier=self.id, category=OutboxCategory.IDENTITY_UPDATE, - cell_name=region_name, + cell_name=cell_name, ).save() return super().delete(*args, **kwargs) diff --git a/src/sentry/users/models/user.py b/src/sentry/users/models/user.py index b532e647eb7d..e80cd4689eb4 100644 --- a/src/sentry/users/models/user.py +++ b/src/sentry/users/models/user.py @@ -367,16 +367,16 @@ def outboxes_for_update(self, is_user_delete: bool = False) -> list[ControlOutbo def outboxes_for_user_update( identifier: int, is_user_delete: bool = False ) -> list[ControlOutboxBase]: - # User deletions must fan out to all regions to ensure cascade behavior + # User deletions must fan out to all cells to ensure cascade behavior # of anything with a HybridCloudForeignKey, even if the user is no longer - # a member of any organizations in that region. + # a member of any organizations in that cell. if is_user_delete: - user_regions = set(find_all_cell_names()) + user_cells = set(find_all_cell_names()) else: - user_regions = find_cells_for_user(identifier) + user_cells = find_cells_for_user(identifier) return OutboxCategory.USER_UPDATE.as_control_outboxes( - cell_names=user_regions, + cell_names=user_cells, object_identifier=identifier, shard_identifier=identifier, ) diff --git a/src/sentry/users/models/user_avatar.py b/src/sentry/users/models/user_avatar.py index fbc0c797440c..41232cf04584 100644 --- a/src/sentry/users/models/user_avatar.py +++ b/src/sentry/users/models/user_avatar.py @@ -55,9 +55,9 @@ class Meta: url_path = "avatar" def outboxes_for_update(self, shard_identifier: int | None = None) -> list[ControlOutboxBase]: - regions = find_cells_for_user(self.user_id) + cells = find_cells_for_user(self.user_id) return OutboxCategory.USER_UPDATE.as_control_outboxes( - cell_names=regions, + cell_names=cells, shard_identifier=self.user_id, object_identifier=self.user_id, ) diff --git a/src/sentry/users/models/useremail.py b/src/sentry/users/models/useremail.py index 7b8d37892e68..ec375d5c426e 100644 --- a/src/sentry/users/models/useremail.py +++ b/src/sentry/users/models/useremail.py @@ -78,11 +78,11 @@ class Meta: __repr__ = sane_repr("user_id", "email") def outboxes_for_update(self, shard_identifier: int | None = None) -> list[ControlOutboxBase]: - regions = find_cells_for_user(self.user_id) + cells = find_cells_for_user(self.user_id) return [ outbox for outbox in OutboxCategory.USER_UPDATE.as_control_outboxes( - cell_names=regions, + cell_names=cells, shard_identifier=self.user_id, object_identifier=self.user_id, ) diff --git a/src/sentry/users/models/userpermission.py b/src/sentry/users/models/userpermission.py index f59fe4d4d07d..11fd8ca234af 100644 --- a/src/sentry/users/models/userpermission.py +++ b/src/sentry/users/models/userpermission.py @@ -43,11 +43,11 @@ def for_user(cls, user_id: int) -> frozenset[str]: return frozenset(cls.objects.filter(user=user_id).values_list("permission", flat=True)) def outboxes_for_update(self, shard_identifier: int | None = None) -> list[ControlOutboxBase]: - regions = find_cells_for_user(self.user_id) + cells = find_cells_for_user(self.user_id) return [ outbox for outbox in OutboxCategory.USER_UPDATE.as_control_outboxes( - cell_names=regions, + cell_names=cells, shard_identifier=self.user_id, object_identifier=self.user_id, ) diff --git a/src/sentry/users/models/userrole.py b/src/sentry/users/models/userrole.py index b7a59de4c87a..90b05c97b54c 100644 --- a/src/sentry/users/models/userrole.py +++ b/src/sentry/users/models/userrole.py @@ -44,12 +44,12 @@ class Meta: __repr__ = sane_repr("name", "permissions") def outboxes_for_update(self, shard_identifier: int | None = None) -> list[ControlOutboxBase]: - regions = list(find_all_cell_names()) + cells = list(find_all_cell_names()) return [ outbox for user_id in self.users.values_list("id", flat=True) for outbox in OutboxCategory.USER_UPDATE.as_control_outboxes( - cell_names=regions, + cell_names=cells, shard_identifier=user_id, object_identifier=user_id, ) @@ -67,9 +67,9 @@ class UserRoleUser(ControlOutboxProducingModel): role = FlexibleForeignKey("sentry.UserRole") def outboxes_for_update(self, shard_identifier: int | None = None) -> list[ControlOutboxBase]: - regions = list(find_all_cell_names()) + cells = list(find_all_cell_names()) return OutboxCategory.USER_UPDATE.as_control_outboxes( - cell_names=regions, + cell_names=cells, shard_identifier=self.user_id, object_identifier=self.user_id, ) diff --git a/src/sentry/users/services/user/impl.py b/src/sentry/users/services/user/impl.py index 4071bb2a391b..c0c60eb9f3aa 100644 --- a/src/sentry/users/services/user/impl.py +++ b/src/sentry/users/services/user/impl.py @@ -151,16 +151,16 @@ def get_organizations( org_query = org_query.filter(status=OrganizationStatus.ACTIVE) return [serialize_organization_mapping(o) for o in org_query] - def get_member_region_names(self, *, user_id: int) -> list[str]: + def get_member_cell_names(self, *, user_id: int) -> list[str]: org_ids = OrganizationMemberMapping.objects.filter(user_id=user_id).values_list( "organization_id", flat=True ) - region_query = ( + cell_query = ( OrganizationMapping.objects.filter(organization_id__in=org_ids) .values_list("cell_name", flat=True) .distinct() ) - return list(region_query) + return list(cell_query) def flush_nonce(self, *, user_id: int) -> None: user = User.objects.filter(id=user_id).first() diff --git a/src/sentry/users/services/user/service.py b/src/sentry/users/services/user/service.py index c9aea08294cc..244c6db1c9f6 100644 --- a/src/sentry/users/services/user/service.py +++ b/src/sentry/users/services/user/service.py @@ -112,8 +112,8 @@ def get_many_by_id(self, *, ids: list[int]) -> list[RpcUser]: """ Get many users by id. - Will use region local cache to minimize network overhead. - Cache keys in regions will be expired as users are updated via outbox receivers. + Will use cell local cache to minimize network overhead. + Cache keys in cells will be expired as users are updated via outbox receivers. :param ids: A list of user ids to fetch """ @@ -152,7 +152,7 @@ def get_organizations( """ Get summary data for all organizations of which the user is a member. - The organizations may span multiple regions. + The organizations may span multiple cells. :param user_id: The user to find organizations from. :param only_visible: Whether or not to only fetch visible organizations @@ -160,11 +160,11 @@ def get_organizations( @rpc_method @abstractmethod - def get_member_region_names(self, *, user_id: int) -> list[str]: + def get_member_cell_names(self, *, user_id: int) -> list[str]: """ - Get a list of region names where the user is a member of at least one org. + Get a list of cell names where the user is a member of at least one org. - :param user_id: The user to fetch region names for. + :param user_id: The user to fetch cell names for. """ @rpc_method diff --git a/src/sentry/users/web/accounts.py b/src/sentry/users/web/accounts.py index 1fbfc47ed980..f1bfd5849ea5 100644 --- a/src/sentry/users/web/accounts.py +++ b/src/sentry/users/web/accounts.py @@ -237,7 +237,7 @@ def recover_confirm( if mode == "relocate": # Relocation form requires users to accept TOS and privacy policy with an org # associated. We only need the first membership, since all of user's orgs will be in - # the same region. + # the same cell. membership = OrganizationMemberMapping.objects.filter(user=user).first() assert membership is not None mapping = OrganizationMapping.objects.get( diff --git a/src/sentry/web/client_config.py b/src/sentry/web/client_config.py index 0bb5e9eb05f7..21b90e36c6a1 100644 --- a/src/sentry/web/client_config.py +++ b/src/sentry/web/client_config.py @@ -343,7 +343,7 @@ def _member_locality_names(self) -> frozenset[str]: if not self.user or not self.user.id: return frozenset() - cell_names = user_service.get_member_region_names(user_id=self.user.id) + cell_names = user_service.get_member_cell_names(user_id=self.user.id) directory = get_global_directory() return frozenset( loc.name diff --git a/src/sentry/web/frontend/oauth_authorize.py b/src/sentry/web/frontend/oauth_authorize.py index 7d0ecf0fcc2f..337fee20b44e 100644 --- a/src/sentry/web/frontend/oauth_authorize.py +++ b/src/sentry/web/frontend/oauth_authorize.py @@ -339,12 +339,17 @@ def _logged_out_post( response = super().post(request, application=application, **kwargs) # once they login, bind their user ID if request.user.is_authenticated: + # Save OAuth payload before session regeneration + oa2_payload = request.session.get("oa2") + # Regenerate session to prevent session fixation attacks request.session.cycle_key() - # Update OAuth payload with authenticated user ID for validation in post() - request.session["oa2"]["uid"] = request.user.id - request.session.modified = True + # Restore OAuth payload after session regeneration and update user ID + if oa2_payload is not None: + oa2_payload["uid"] = request.user.id + request.session["oa2"] = oa2_payload + request.session.modified = True return response def post(self, request: HttpRequest, **kwargs) -> HttpResponseBase: diff --git a/src/sentry/workflow_engine/endpoints/organization_workflow_details.py b/src/sentry/workflow_engine/endpoints/organization_workflow_details.py index 176a6cbbe7c8..7d5d444074e0 100644 --- a/src/sentry/workflow_engine/endpoints/organization_workflow_details.py +++ b/src/sentry/workflow_engine/endpoints/organization_workflow_details.py @@ -1,6 +1,4 @@ -from django.db import router, transaction from drf_spectacular.utils import extend_schema -from rest_framework.exceptions import ValidationError from rest_framework.request import Request from rest_framework.response import Response @@ -27,9 +25,6 @@ ) from sentry.workflow_engine.endpoints.serializers.workflow_serializer import WorkflowSerializer from sentry.workflow_engine.endpoints.validators.base.workflow import WorkflowValidator -from sentry.workflow_engine.endpoints.validators.detector_workflow import ( - BulkWorkflowDetectorsValidator, -) from sentry.workflow_engine.models import Workflow @@ -103,34 +98,15 @@ def put(self, request: Request, organization: Organization, workflow: Workflow) ) validator.is_valid(raise_exception=True) + validator.update(workflow, validator.validated_data) - with transaction.atomic(router.db_for_write(Workflow)): - validator.update(workflow, validator.validated_data) - - detector_ids = request.data.get("detectorIds") - if detector_ids is not None: - bulk_validator = BulkWorkflowDetectorsValidator( - data={ - "workflow_id": workflow.id, - "detector_ids": detector_ids, - }, - context={ - "organization": organization, - "request": request, - }, - ) - if not bulk_validator.is_valid(): - raise ValidationError({"detectorIds": bulk_validator.errors}) - - bulk_validator.save() - - create_audit_entry( - request=request, - organization=organization, - target_object=workflow.id, - event=audit_log.get_event_id("WORKFLOW_EDIT"), - data=workflow.get_audit_log_data(), - ) + create_audit_entry( + request=request, + organization=organization, + target_object=workflow.id, + event=audit_log.get_event_id("WORKFLOW_EDIT"), + data=workflow.get_audit_log_data(), + ) workflow.refresh_from_db() diff --git a/src/sentry/workflow_engine/endpoints/organization_workflow_index.py b/src/sentry/workflow_engine/endpoints/organization_workflow_index.py index 12a49c633e1a..c8de5592ac33 100644 --- a/src/sentry/workflow_engine/endpoints/organization_workflow_index.py +++ b/src/sentry/workflow_engine/endpoints/organization_workflow_index.py @@ -59,9 +59,6 @@ from sentry.workflow_engine.endpoints.utils.ids import to_valid_int_id, to_valid_int_id_list from sentry.workflow_engine.endpoints.utils.sortby import SortByParam from sentry.workflow_engine.endpoints.validators.base.workflow import WorkflowValidator -from sentry.workflow_engine.endpoints.validators.detector_workflow import ( - BulkWorkflowDetectorsValidator, -) from sentry.workflow_engine.endpoints.validators.detector_workflow_mutation import ( DetectorWorkflowMutationValidator, ) @@ -316,24 +313,8 @@ def post(self, request: Request, organization: Organization) -> Response: data=request.data, context={"organization": organization, "request": request}, ) - validator.is_valid(raise_exception=True) - - with transaction.atomic(router.db_for_write(Workflow)): - workflow = validator.create(validator.validated_data) - - detector_ids = request.data.get("detectorIds", []) - if detector_ids: - bulk_validator = BulkWorkflowDetectorsValidator( - data={ - "workflow_id": workflow.id, - "detector_ids": detector_ids, - }, - context={"organization": organization, "request": request}, - ) - bulk_validator.is_valid(raise_exception=True) - bulk_validator.save() - + workflow = validator.create(validator.validated_data) return Response(serialize(workflow, request.user), status=status.HTTP_201_CREATED) @extend_schema( diff --git a/src/sentry/workflow_engine/endpoints/validators/base/workflow.py b/src/sentry/workflow_engine/endpoints/validators/base/workflow.py index 3bfbdaa3c907..c359f2b4f666 100644 --- a/src/sentry/workflow_engine/endpoints/validators/base/workflow.py +++ b/src/sentry/workflow_engine/endpoints/validators/base/workflow.py @@ -22,6 +22,7 @@ DataConditionGroupInput, ) from sentry.workflow_engine.endpoints.validators.utils import ( + connect_workflows_to_detectors, log_alerting_quota_hit, remove_items_by_api_input, update_owner, @@ -53,6 +54,7 @@ class WorkflowInput(TypedDict): triggers: NotRequired[DataConditionGroupInput] actionFilters: NotRequired[list[ActionFilterInput]] owner: NotRequired[str | int | None] + detectorIds: NotRequired[list[int]] class WorkflowValidator(CamelSnakeSerializer[Any]): @@ -61,6 +63,11 @@ class WorkflowValidator(CamelSnakeSerializer[Any]): enabled = serializers.BooleanField( required=False, default=True, help_text="Whether the alert is enabled or disabled" ) + detector_ids = serializers.ListField( + child=serializers.IntegerField(), + required=False, + help_text="The IDs of the monitors to connect this alert to. Use 'Fetch an Organization's Monitors' to find the IDs.", + ) config = serializers.JSONField( required=False, help_text=WORKFLOW_CONFIG_HELP_TEXT, @@ -257,6 +264,13 @@ def update_action_filters(self, action_filters: ListInputData) -> list[DataCondi return filters def update(self, instance: Workflow, validated_data: InputData) -> Workflow: + organization = self.context["organization"] + request = self.context["request"] + + detector_ids = None + if "detector_ids" in validated_data: + detector_ids = validated_data.pop("detector_ids") + with transaction.atomic(router.db_for_write(Workflow)): # Update the Workflow.when_condition_group triggers = validated_data.pop("triggers", None) @@ -276,6 +290,12 @@ def update(self, instance: Workflow, validated_data: InputData) -> Workflow: # Update the workflow instance.update(**validated_data) + + # Update detector connections + connect_workflows_to_detectors( + request, organization, instance.id, detector_ids, update=True + ) + instance.save() return instance @@ -306,6 +326,8 @@ def _validate_workflow_limits(self) -> None: def create(self, validated_value: InputData) -> Workflow: condition_group_validator = BaseDataConditionGroupValidator(context=self.context) action_validator = BaseActionValidator(context=self.context) + organization = self.context["organization"] + request = self.context["request"] self._validate_workflow_limits() @@ -329,13 +351,16 @@ def create(self, validated_value: InputData) -> Workflow: name=validated_value["name"], enabled=validated_value["enabled"], config=validated_value["config"], - organization_id=self.context["organization"].id, + organization_id=organization.id, environment_id=environment.id if environment else None, when_condition_group=when_condition_group, - created_by_id=self.context["request"].user.id, + created_by_id=request.user.id, owner_user_id=owner_user_id, owner_team_id=owner_team_id, ) + # connect detectors + detector_ids = validated_value.get("detector_ids") + connect_workflows_to_detectors(request, organization, workflow.id, detector_ids) # TODO -- can we bulk create: actions, dcga's and the workflow dcg? # Create actions and action filters, then associate them to the workflow @@ -357,8 +382,8 @@ def create(self, validated_value: InputData) -> Workflow: ) create_audit_entry( - request=self.context["request"], - organization=self.context["organization"], + request=request, + organization=organization, target_object=workflow.id, event=audit_log.get_event_id("WORKFLOW_ADD"), data=workflow.get_audit_log_data(), diff --git a/src/sentry/workflow_engine/endpoints/validators/detector_workflow.py b/src/sentry/workflow_engine/endpoints/validators/detector_workflow.py index df5f98f54429..d7b07598ed2d 100644 --- a/src/sentry/workflow_engine/endpoints/validators/detector_workflow.py +++ b/src/sentry/workflow_engine/endpoints/validators/detector_workflow.py @@ -52,46 +52,3 @@ def create(self, validated_data: dict[str, Any]) -> list[DetectorWorkflow]: ) return list(DetectorWorkflow.objects.filter(detector_id=validated_data["detector_id"])) - - -class BulkWorkflowDetectorsValidator(CamelSnakeSerializer[Any]): - """ - Connect/disconnect multiple detectors to a single workflow all at once. - """ - - workflow_id = serializers.IntegerField(required=True) - detector_ids = serializers.ListField(child=serializers.IntegerField(), required=True) - - def create(self, validated_data: dict[str, Any]) -> list[DetectorWorkflow]: - validate_workflows_exist([validated_data["workflow_id"]], self.context["organization"]) - validate_detectors_exist_and_have_permissions( - validated_data["detector_ids"], self.context["organization"], self.context["request"] - ) - - existing_detector_workflows = list( - DetectorWorkflow.objects.filter( - workflow_id=validated_data["workflow_id"], - ) - ) - new_detector_ids = set(validated_data["detector_ids"]) - { - dw.detector_id for dw in existing_detector_workflows - } - - detector_workflows_to_add: list[dict[Literal["detector_id", "workflow_id"], int]] = [ - {"detector_id": detector_id, "workflow_id": validated_data["workflow_id"]} - for detector_id in new_detector_ids - ] - detector_workflows_to_remove = [ - dw - for dw in existing_detector_workflows - if dw.detector_id not in validated_data["detector_ids"] - ] - - perform_bulk_detector_workflow_operations( - detector_workflows_to_add, - detector_workflows_to_remove, - self.context["request"], - self.context["organization"], - ) - - return list(DetectorWorkflow.objects.filter(workflow_id=validated_data["workflow_id"])) diff --git a/src/sentry/workflow_engine/endpoints/validators/utils.py b/src/sentry/workflow_engine/endpoints/validators/utils.py index 60a4fa1d32b4..7ed8d70de84e 100644 --- a/src/sentry/workflow_engine/endpoints/validators/utils.py +++ b/src/sentry/workflow_engine/endpoints/validators/utils.py @@ -154,6 +154,53 @@ def validate_workflows_exist( return workflows +def connect_workflows_to_detectors( + request: Request, + organization: Organization, + workflow_id: int, + detector_ids: list[int] | None, + update: bool = False, +) -> None: + if detector_ids is not None: + validate_detectors_exist_and_have_permissions(detector_ids, organization, request) + + def get_detector_workflows_to_add( + workflow_id: int, detector_ids: set[int] + ) -> list[dict[Literal["detector_id", "workflow_id"], int]]: + detector_workflows_to_add: list[dict[Literal["detector_id", "workflow_id"], int]] = [ + {"detector_id": detector_id, "workflow_id": workflow_id} + for detector_id in detector_ids + ] + return detector_workflows_to_add + + if update: + existing_detector_workflows = list( + DetectorWorkflow.objects.filter( + workflow_id=workflow_id, + ) + ) + new_detector_ids = set(detector_ids) - { + dw.detector_id for dw in existing_detector_workflows + } + + detector_workflows_to_add = get_detector_workflows_to_add(workflow_id, new_detector_ids) + detector_workflows_to_remove = [ + dw for dw in existing_detector_workflows if dw.detector_id not in detector_ids + ] + else: + detector_workflows_to_add = get_detector_workflows_to_add( + workflow_id, set(detector_ids) + ) + detector_workflows_to_remove = [] + + perform_bulk_detector_workflow_operations( + detector_workflows_to_add=detector_workflows_to_add, + detector_workflows_to_remove=detector_workflows_to_remove, + request=request, + organization=organization, + ) + + def perform_bulk_detector_workflow_operations( detector_workflows_to_add: list[dict[Literal["detector_id", "workflow_id"], int]], detector_workflows_to_remove: Sequence[DetectorWorkflow], diff --git a/static/app/components/events/eventBreadcrumbsSection.spec.tsx b/static/app/components/events/eventBreadcrumbsSection.spec.tsx deleted file mode 100644 index a440443d24ca..000000000000 --- a/static/app/components/events/eventBreadcrumbsSection.spec.tsx +++ /dev/null @@ -1,51 +0,0 @@ -import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary'; - -import { - MOCK_BREADCRUMBS, - MOCK_DATA_SECTION_PROPS, -} from 'sentry/components/events/breadcrumbs/testUtils'; -import {EventBreadcrumbsSection} from 'sentry/components/events/eventBreadcrumbsSection'; - -// Needed to mock useVirtualizer lists. -jest.spyOn(window.Element.prototype, 'getBoundingClientRect').mockImplementation(() => ({ - x: 0, - y: 0, - width: 500, - height: 400, - left: 0, - top: 0, - right: 500, - bottom: 400, - toJSON: jest.fn(), -})); - -describe('EventBreadcrumbsSection', () => { - it('renders breadcrumbs from the event', async () => { - render(); - - expect(screen.getByText('Breadcrumbs')).toBeInTheDocument(); - // Verify breadcrumb categories from mock data are rendered - expect(await screen.findByText(MOCK_BREADCRUMBS[0].category)).toBeInTheDocument(); - expect(screen.getByText(MOCK_BREADCRUMBS[1].category)).toBeInTheDocument(); - }); - - it('filters breadcrumbs by type', async () => { - render(); - - // Multiple breadcrumbs visible initially - expect(await screen.findByText(MOCK_BREADCRUMBS[0].category)).toBeInTheDocument(); - expect(screen.getByText(MOCK_BREADCRUMBS[2].category)).toBeInTheDocument(); - - // Open filter dropdown and select Navigation type - const filterButton = screen.getByRole('button', {name: 'Filter Breadcrumbs'}); - await userEvent.click(filterButton); - await userEvent.click(screen.getByRole('option', {name: 'Navigation'})); - - // Close dropdown - await userEvent.keyboard('{Escape}'); - - // Only navigation breadcrumb should be visible (index 2 has type NAVIGATION) - expect(screen.queryByText(MOCK_BREADCRUMBS[0].category)).not.toBeInTheDocument(); - expect(screen.getByText(MOCK_BREADCRUMBS[2].category)).toBeInTheDocument(); - }); -}); diff --git a/static/app/components/events/eventBreadcrumbsSection.tsx b/static/app/components/events/eventBreadcrumbsSection.tsx deleted file mode 100644 index f701de7f9c29..000000000000 --- a/static/app/components/events/eventBreadcrumbsSection.tsx +++ /dev/null @@ -1,192 +0,0 @@ -import {useMemo, useState} from 'react'; -import {useTheme} from '@emotion/react'; -import styled from '@emotion/styled'; - -import {CompactSelect} from '@sentry/scraps/compactSelect'; -import {InputGroup} from '@sentry/scraps/input'; -import {OverlayTrigger} from '@sentry/scraps/overlayTrigger'; - -import {BreadcrumbsTimeline} from 'sentry/components/events/breadcrumbs/breadcrumbsTimeline'; -import { - BREADCRUMB_TIME_DISPLAY_LOCALSTORAGE_KEY, - BreadcrumbTimeDisplay, - getEnhancedBreadcrumbs, - useBreadcrumbFilters, -} from 'sentry/components/events/breadcrumbs/utils'; -import { - applyBreadcrumbSearch, - BREADCRUMB_SORT_LOCALSTORAGE_KEY, - BREADCRUMB_SORT_OPTIONS, - BreadcrumbSort, -} from 'sentry/components/events/interfaces/breadcrumbs'; -import {IconFilter, IconSearch, IconSort} from 'sentry/icons'; -import {t} from 'sentry/locale'; -import type {Event} from 'sentry/types/event'; -import {useLocalStorageState} from 'sentry/utils/useLocalStorageState'; -import {InterimSection} from 'sentry/views/issueDetails/streamline/interimSection'; - -const MAX_BREADCRUMBS_HEIGHT = 400; - -interface EventBreadcrumbsSectionProps { - event: Event; -} - -export function EventBreadcrumbsSection({event}: EventBreadcrumbsSectionProps) { - const theme = useTheme(); - const [container, setContainer] = useState(null); - const [search, setSearch] = useState(''); - const [filters, setFilters] = useState([]); - const [timeDisplay] = useLocalStorageState( - BREADCRUMB_TIME_DISPLAY_LOCALSTORAGE_KEY, - BreadcrumbTimeDisplay.ABSOLUTE - ); - const [sort, setSort] = useLocalStorageState( - BREADCRUMB_SORT_LOCALSTORAGE_KEY, - BreadcrumbSort.NEWEST - ); - - const enhancedCrumbs = useMemo( - () => getEnhancedBreadcrumbs(event, theme), - [event, theme] - ); - - const {filterOptions, applyFilters} = useBreadcrumbFilters(enhancedCrumbs); - - const displayCrumbs = useMemo(() => { - const sortedCrumbs = - sort === BreadcrumbSort.OLDEST ? enhancedCrumbs : [...enhancedCrumbs].reverse(); - const filteredCrumbs = applyFilters(sortedCrumbs, filters); - const searchedCrumbs = applyBreadcrumbSearch(filteredCrumbs, search); - return searchedCrumbs; - }, [enhancedCrumbs, sort, filters, search, applyFilters]); - - const startTimeString = useMemo( - () => - timeDisplay === BreadcrumbTimeDisplay.RELATIVE - ? displayCrumbs?.at(0)?.breadcrumb?.timestamp - : undefined, - [displayCrumbs, timeDisplay] - ); - - if (enhancedCrumbs.length === 0) { - return null; - } - - const actions = ( - - - - setSearch(e.target.value)} - placeholder={t('Search')} - aria-label={t('Search Breadcrumbs')} - /> - - - - - - setFilters(options.map(({value}) => value))} - options={filterOptions} - maxMenuHeight={400} - trigger={props => ( - } - aria-label={t('Filter Breadcrumbs')} - title={t('Filter')} - {...props} - > - {filters.length > 0 ? filters.length : ''} - - )} - /> - ( - } - aria-label={t('Sort Breadcrumbs')} - title={t('Sort')} - {...props} - /> - )} - onChange={selectedOption => setSort(selectedOption.value)} - value={sort} - options={BREADCRUMB_SORT_OPTIONS} - /> - - ); - - return ( - - - - {displayCrumbs.length === 0 ? ( - - {t('No breadcrumbs match your search or filters.')} - - ) : ( - - )} - - - - ); -} - -const BreadcrumbsContainer = styled('div')` - container-type: inline-size; -`; - -const ActionsWrapper = styled('div')` - display: flex; - gap: ${p => p.theme.space.md}; - align-items: center; - - @container (max-width: 400px) { - display: none; - } -`; - -const SearchWrapper = styled('div')` - @container (max-width: 500px) { - display: none; - } -`; - -const SearchInput = styled(InputGroup.Input)` - width: 180px; -`; - -const ScrollContainer = styled('div')` - max-height: ${MAX_BREADCRUMBS_HEIGHT}px; - overflow-y: auto; -`; - -const EmptyMessage = styled('div')` - color: ${p => p.theme.tokens.content.secondary}; - padding: ${p => p.theme.space.xl}; - text-align: center; -`; diff --git a/static/app/components/events/eventEntries.spec.tsx b/static/app/components/events/eventEntries.spec.tsx deleted file mode 100644 index b551b9882682..000000000000 --- a/static/app/components/events/eventEntries.spec.tsx +++ /dev/null @@ -1,68 +0,0 @@ -import {EventFixture} from 'sentry-fixture/event'; -import {EntryDebugMetaFixture, EventEntryFixture} from 'sentry-fixture/eventEntry'; -import {LocationFixture} from 'sentry-fixture/locationFixture'; -import {OrganizationFixture} from 'sentry-fixture/organization'; -import {ProjectFixture} from 'sentry-fixture/project'; - -import {render, screen, waitFor} from 'sentry-test/reactTestingLibrary'; - -import {EventEntries} from 'sentry/components/events/eventEntries'; - -describe('EventEntries', () => { - const defaultProps = { - organization: OrganizationFixture(), - project: ProjectFixture(), - event: EventFixture(), - location: LocationFixture(), - }; - - beforeEach(() => { - const project = ProjectFixture({platform: 'javascript'}); - - MockApiClient.addMockResponse({ - url: '/projects/org-slug/project-slug/events/1/grouping-info/', - }); - MockApiClient.addMockResponse({ - url: '/projects/org-slug/project-slug/events/1/committers/', - body: [], - }); - MockApiClient.addMockResponse({ - url: '/organizations/org-slug/projects/', - body: [project], - }); - MockApiClient.addMockResponse({ - url: '/projects/org-slug/project-slug/events/1/actionable-items/', - }); - }); - - it('renders the replay section in the correct place', async () => { - MockApiClient.addMockResponse({ - url: '/organizations/org-slug/prompts-activity/', - body: {data: {dismissed_ts: null}}, - }); - render( - , - {organization: OrganizationFixture({features: ['session-replay']})} - ); - - await waitFor(() => { - expect(screen.getAllByRole('button', {name: /Section/i})).toHaveLength(4); - }); - - const sections = screen.getAllByRole('button', {name: /Section/i}); - - // Replay should be after message but before images loaded - expect(sections[0]).toHaveTextContent(/message/i); - expect(sections[1]).toHaveTextContent(/replay/i); - expect(sections[2]).toHaveTextContent(/images loaded/i); - expect(sections[3]).toHaveTextContent(/event grouping/i); - }); -}); diff --git a/static/app/components/events/eventEntries.tsx b/static/app/components/events/eventEntries.tsx deleted file mode 100644 index cc07b49eec4f..000000000000 --- a/static/app/components/events/eventEntries.tsx +++ /dev/null @@ -1,229 +0,0 @@ -import {Fragment} from 'react'; -import styled from '@emotion/styled'; - -import {CommitRow} from 'sentry/components/commitRow'; -import {EventEvidence} from 'sentry/components/events/eventEvidence'; -import {EventHydrationDiff} from 'sentry/components/events/eventHydrationDiff'; -import {EventReplay} from 'sentry/components/events/eventReplay'; -import {EventGroupingInfoSection} from 'sentry/components/events/groupingInfo/groupingInfoSection'; -import {ActionableItems} from 'sentry/components/events/interfaces/crashContent/exception/actionableItems'; -import {actionableItemsEnabled} from 'sentry/components/events/interfaces/crashContent/exception/useActionableItems'; -import {t} from 'sentry/locale'; -import type {Entry, Event} from 'sentry/types/event'; -import {EntryType} from 'sentry/types/event'; -import type {Group} from 'sentry/types/group'; -import type {Organization, SharedViewOrganization} from 'sentry/types/organization'; -import type {Project} from 'sentry/types/project'; -import {isNotSharedOrganization} from 'sentry/types/utils'; -import {isEmptyObject} from 'sentry/utils/object/isEmptyObject'; - -import {EventContexts} from './contexts'; -import {EventDevice} from './device'; -import {EventAttachments} from './eventAttachments'; -import {EventDataSection} from './eventDataSection'; -import {EventEntry} from './eventEntry'; -import {EventExtraData} from './eventExtraData'; -import {EventSdk} from './eventSdk'; -import {EventTagsAndScreenshot} from './eventTagsAndScreenshot'; -import {EventViewHierarchy} from './eventViewHierarchy'; -import {EventPackageData} from './packageData'; -import {EventRRWebIntegration} from './rrwebIntegration'; -import {DataSection} from './styles'; -import {SuspectCommits} from './suspectCommits'; -import {EventUserFeedback} from './userFeedback'; - -type Props = { - /** - * The organization can be the shared view on a public issue view. - */ - organization: Organization | SharedViewOrganization; - project: Project; - className?: string; - event?: Event; - group?: Group; - isShare?: boolean; - showTagSummary?: boolean; -}; - -function EventEntries({ - organization, - project, - event, - group, - className, - isShare = false, - showTagSummary = true, -}: Props) { - const orgSlug = organization.slug; - const projectSlug = project.slug; - const orgFeatures = organization?.features ?? []; - - if (!event) { - return ( - -

{t('Latest Event Not Available')}

- - ); - } - - const hasContext = !isEmptyObject(event.user ?? {}) || !isEmptyObject(event.contexts); - const hasActionableItems = actionableItemsEnabled({ - eventId: event.id, - organization, - projectSlug, - }); - - return ( -
- {!isShare && hasActionableItems && ( - - )} - {!isShare && isNotSharedOrganization(organization) && ( - - )} - {event.userReport && group && ( - - - - )} - {showTagSummary && ( - - )} - - - {hasContext && } - - - {!isShare && } - {!isShare && } - - - {!isShare && event.groupID && ( - - )} - {!isShare && ( - - )} -
- ); -} - -// The ordering for event entries is owned by the interface serializers on the backend. -// Because replays are not an interface, we need to manually insert the replay section -// into the array of entries. The long-term solution here is to move the ordering -// logic to this component, similar to how GroupEventDetailsContent works. -function partitionEntriesForReplay(entries: Entry[]) { - let replayIndex = 0; - - for (const [i, entry] of entries.entries()) { - if ( - [ - // The following entry types should be placed before the replay - // This is similar to the ordering in GroupEventDetailsContent - EntryType.MESSAGE, - EntryType.STACKTRACE, - EntryType.EXCEPTION, - EntryType.THREADS, - EntryType.SPANS, - ].includes(entry.type) - ) { - replayIndex = i + 1; - } - } - - return [entries.slice(0, replayIndex), entries.slice(replayIndex)]; -} - -function Entries({ - definedEvent, - projectSlug, - isShare, - group, - organization, - hideBeforeReplayEntries = false, - hideBreadCrumbs = false, -}: { - definedEvent: Event; - projectSlug: string; - hideBeforeReplayEntries?: boolean; - hideBreadCrumbs?: boolean; - isShare?: boolean; -} & Pick) { - if (!Array.isArray(definedEvent.entries)) { - return null; - } - - const [beforeReplayEntries, afterReplayEntries] = partitionEntriesForReplay( - definedEvent.entries - ); - - const eventEntryProps = { - projectSlug, - group, - organization, - event: definedEvent, - isShare, - }; - - return ( - - {!hideBeforeReplayEntries && - beforeReplayEntries!.map((entry, entryIdx) => ( - - ))} - {!isShare && } - {!isShare && } - {afterReplayEntries!.map((entry, entryIdx) => { - if (hideBreadCrumbs && entry.type === EntryType.BREADCRUMBS) { - return null; - } - - return ; - })} - - ); -} - -const LatestEventNotAvailable = styled('div')` - padding: ${p => p.theme.space.xl} ${p => p.theme.space['3xl']}; -`; - -const BorderlessEventEntries = styled(EventEntries)` - & ${DataSection} { - margin-left: 0 !important; - margin-right: 0 !important; - padding: ${p => p.theme.space['2xl']} 0 0 0; - } - & ${DataSection}:first-child { - padding-top: 0; - border-top: 0; - } -`; - -export {EventEntries, BorderlessEventEntries}; diff --git a/static/app/components/events/eventEntry.tsx b/static/app/components/events/eventEntry.tsx deleted file mode 100644 index 02d12ca926a9..000000000000 --- a/static/app/components/events/eventEntry.tsx +++ /dev/null @@ -1,154 +0,0 @@ -import {ErrorBoundary} from 'sentry/components/errorBoundary'; -import {EventBreadcrumbsSection} from 'sentry/components/events/eventBreadcrumbsSection'; -import {t} from 'sentry/locale'; -import type {Entry, Event, EventTransaction} from 'sentry/types/event'; -import {EntryType} from 'sentry/types/event'; -import type {Group} from 'sentry/types/group'; -import type {Organization, SharedViewOrganization} from 'sentry/types/organization'; -import type {Project} from 'sentry/types/project'; -import {getConfigForIssueType} from 'sentry/utils/issueTypeConfig'; -import {isJavascriptPlatform} from 'sentry/utils/platform'; -import type {SectionKey} from 'sentry/views/issueDetails/streamline/context'; -import {InterimSection} from 'sentry/views/issueDetails/streamline/interimSection'; - -import {Csp} from './interfaces/csp'; -import {DebugMeta} from './interfaces/debugMeta'; -import {Exception} from './interfaces/exception'; -import {Generic} from './interfaces/generic'; -import {Message} from './interfaces/message'; -import {SpanEvidenceSection} from './interfaces/performance/spanEvidence'; -import {Request} from './interfaces/request'; -import {StackTrace} from './interfaces/stackTrace'; -import {Template} from './interfaces/template'; -import {Threads} from './interfaces/threads'; - -type Props = { - entry: Entry; - event: Event; - organization: SharedViewOrganization | Organization; - projectSlug: Project['slug']; - group?: Group; - isShare?: boolean; - sectionKey?: SectionKey; -}; - -function EventEntryContent({ - entry, - projectSlug, - event, - organization, - group, - isShare, -}: Props) { - const groupingCurrentLevel = group?.metadata?.current_level; - const issueTypeConfig = group ? getConfigForIssueType(group, group.project) : null; - - switch (entry.type) { - case EntryType.EXCEPTION: - return ( - - ); - - case EntryType.MESSAGE: - return ; - - case EntryType.REQUEST: - return ; - - case EntryType.STACKTRACE: - return ( - - ); - - case EntryType.TEMPLATE: - return