Skip to content

Commit b8099db

Browse files
authored
fix(seer): attribute remaining grouping cleanup paths (#128247)
## Summary - carry organization and project attribution through per-project retention cleanup workers - restore the user actor already recorded by project and organization deletion entry points - preserve the explicit Taskworker scheduler system context for actorless scheduled deletions - establish explicit system context for manual `sentry deletions run` executions - reuse the scheduled-deletion context boundary so manual runs restore recorded users and target scope - derive organization and project from the scheduled-deletion target - enrich Seer hash-deletion dispatch with the resolved organization and project - never create a system fallback at the deletion or Seer call site The Taskworker scheduler and manual deletion CLI establish system context at their entrypoints. Scheduled deletions with a recorded actor override that context with the user; actorless deletions retain the entrypoint context and add their organization/project attribution. ## Testing - focused CLI, scheduled-deletion, and cleanup context suite: 70 passed - repository formatting and lint hooks - pre-push mypy checks
1 parent 21ffa6d commit b8099db

10 files changed

Lines changed: 351 additions & 82 deletions

File tree

‎src/sentry/deletions/tasks/scheduled.py‎

Lines changed: 66 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
import logging
2+
from contextlib import AbstractContextManager, nullcontext
3+
from dataclasses import replace
24
from datetime import timedelta
35
from typing import Any
46

@@ -16,18 +18,60 @@
1618
ScheduledDeletion,
1719
)
1820
from sentry.exceptions import DeleteAborted
21+
from sentry.models.organization import Organization
22+
from sentry.models.project import Project
1923
from sentry.signals import pending_delete
2024
from sentry.silo.base import SiloMode
2125
from sentry.tasks.base import instrumented_task
2226
from sentry.taskworker.namespaces import deletion_control_tasks, deletion_tasks
2327
from sentry.utils.env import in_test_environment
28+
from sentry.viewer_context import (
29+
ActorType,
30+
ViewerContext,
31+
get_viewer_context,
32+
viewer_context_scope,
33+
)
2434

2535
logger = logging.getLogger("sentry.deletions.api")
2636

2737

2838
MAX_RETRIES = 5
2939

3040

41+
def scheduled_deletion_viewer_context(
42+
deletion: BaseScheduledDeletion,
43+
instance: Any,
44+
*,
45+
origin_context: ViewerContext | None = None,
46+
) -> AbstractContextManager[None]:
47+
"""Apply the recorded actor and target scope while executing a scheduled deletion."""
48+
if not isinstance(instance, (Organization, Project)):
49+
return nullcontext()
50+
51+
viewer_context = (
52+
ViewerContext(user_id=deletion.actor_id, actor_type=ActorType.USER)
53+
if deletion.actor_id is not None
54+
else origin_context or get_viewer_context()
55+
)
56+
if viewer_context is None:
57+
return nullcontext()
58+
59+
if isinstance(instance, Project):
60+
viewer_context = replace(
61+
viewer_context,
62+
organization_id=instance.organization_id,
63+
project_id=instance.id,
64+
)
65+
else:
66+
viewer_context = replace(
67+
viewer_context,
68+
organization_id=instance.id,
69+
project_id=None,
70+
)
71+
72+
return viewer_context_scope(viewer_context)
73+
74+
3175
@instrumented_task(
3276
name="sentry.deletions.tasks.reattempt_deletions_control",
3377
namespace=deletion_control_tasks,
@@ -201,24 +245,25 @@ def _run_deletion(
201245
actor = deletion.get_actor()
202246
pending_delete.send(sender=type(instance), instance=instance, actor=actor)
203247

204-
try:
205-
has_more = task.chunk()
206-
if has_more:
207-
process_task.delay(deletion_id=deletion_id, first_pass=False)
208-
else:
209-
deletion.delete()
210-
except Exception as err:
211-
sentry_sdk.set_context(
212-
"deletion",
213-
{
214-
"id": deletion.id,
215-
"model": deletion.model_name,
216-
"object_id": deletion.object_id,
217-
},
218-
)
219-
sentry_sdk.set_attribute("deletion.id", deletion.id)
220-
sentry_sdk.set_attribute("deletion.model", deletion.model_name)
221-
sentry_sdk.set_attribute("deletion.object_id", deletion.object_id)
222-
sentry_sdk.capture_exception(err)
223-
if in_test_environment():
224-
raise
248+
with scheduled_deletion_viewer_context(deletion, instance):
249+
try:
250+
has_more = task.chunk()
251+
if has_more:
252+
process_task.delay(deletion_id=deletion_id, first_pass=False)
253+
else:
254+
deletion.delete()
255+
except Exception as err:
256+
sentry_sdk.set_context(
257+
"deletion",
258+
{
259+
"id": deletion.id,
260+
"model": deletion.model_name,
261+
"object_id": deletion.object_id,
262+
},
263+
)
264+
sentry_sdk.set_attribute("deletion.id", deletion.id)
265+
sentry_sdk.set_attribute("deletion.model", deletion.model_name)
266+
sentry_sdk.set_attribute("deletion.object_id", deletion.object_id)
267+
sentry_sdk.capture_exception(err)
268+
if in_test_environment():
269+
raise

‎src/sentry/runner/commands/cleanup.py‎

Lines changed: 59 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import os
66
import time
77
from collections.abc import Callable, Mapping, Sequence
8+
from contextlib import nullcontext
89
from datetime import timedelta
910
from multiprocessing import JoinableQueue as Queue
1011
from multiprocessing import Process
@@ -22,6 +23,7 @@
2223

2324
from sentry.runner.decorators import log_options
2425
from sentry.silo.base import SiloLimit, SiloMode
26+
from sentry.viewer_context import ActorType, ViewerContext, viewer_context_scope
2527

2628
logger = logging.getLogger(__name__)
2729

@@ -76,6 +78,7 @@ def get_organization(value: str) -> int | None:
7678
"tuple[str, tuple[int, ...]]"
7779
" | tuple[str, tuple[int, ...], int | None]"
7880
" | tuple[str, tuple[int, ...], int | None, dict[str, Any]]"
81+
" | tuple[str, tuple[int, ...], int | None, dict[str, Any], int]"
7982
)
8083
_WorkQueue: TypeAlias = "Queue[Literal['91650ec271ae4b3e8a67cdc909d80f8c'] | _WorkItem]"
8184

@@ -118,16 +121,23 @@ def multiprocess_worker(task_queue: _WorkQueue) -> None:
118121
task_queue.task_done()
119122
return
120123

121-
# Handle all formats: (model_name, chunk), (model_name, chunk, project_id) and (model_name, chunk, project_id, deferred_filter)
124+
# Handle all formats: (model_name, chunk), (model_name, chunk, project_id),
125+
# (model_name, chunk, project_id, deferred_filter), and the project-scoped
126+
# form with organization_id appended.
122127
if len(j) == 2:
123128
model_name, chunk = j
124129
project_id = None
125130
deferred_filter: dict[str, Any] = {}
131+
organization_id = None
126132
elif len(j) == 3:
127133
model_name, chunk, project_id = j
128134
deferred_filter = {}
129-
else:
135+
organization_id = None
136+
elif len(j) == 4:
130137
model_name, chunk, project_id, deferred_filter = j
138+
organization_id = None
139+
else:
140+
model_name, chunk, project_id, deferred_filter, organization_id = j
131141

132142
if options.get("cleanup.abort_execution"):
133143
logger.warning("Cleanup worker aborting due to cleanup.abort_execution flag")
@@ -154,7 +164,9 @@ def multiprocess_worker(task_queue: _WorkQueue) -> None:
154164
propagation_context.custom_sampling_context = prev_sampling_context
155165

156166
with span:
157-
task_execution(model_name, chunk, project_id, deferred_filter)
167+
task_execution(
168+
model_name, chunk, project_id, deferred_filter, organization_id=organization_id
169+
)
158170
except Exception:
159171
metrics.incr(
160172
"cleanup.error",
@@ -188,6 +200,7 @@ def task_execution(
188200
chunk: tuple[int, ...],
189201
project_id: int | None,
190202
deferred_filter: Mapping[str, Any] | None = None,
203+
organization_id: int | None = None,
191204
) -> None:
192205
"""
193206
Execute deletion for a chunk of objects.
@@ -224,25 +237,38 @@ def task_execution(
224237
if deferred_filter:
225238
query.update(deferred_filter)
226239

227-
task = deletions.get(
228-
model=model,
229-
query=query,
230-
skip_models=skip_child_relations_models,
231-
transaction_id=uuid4().hex,
240+
context = (
241+
viewer_context_scope(
242+
ViewerContext(
243+
organization_id=organization_id,
244+
project_id=project_id,
245+
actor_type=ActorType.SYSTEM,
246+
)
247+
)
248+
if organization_id is not None and project_id is not None
249+
else nullcontext()
232250
)
233251

234-
# Track deletion iterations (task.chunk() may need multiple calls to fully delete)
235-
iteration = 0
236-
while True:
237-
iteration += 1
238-
has_more = task.chunk(apply_filter=True)
239-
240-
debug_output(
241-
f"[ITERATION {iteration}] {model_name} chunk (project_id={project_id} has_more={has_more})"
252+
with context:
253+
task = deletions.get(
254+
model=model,
255+
query=query,
256+
skip_models=skip_child_relations_models,
257+
transaction_id=uuid4().hex,
242258
)
243259

244-
if not has_more:
245-
break
260+
# Track deletion iterations (task.chunk() may need multiple calls to fully delete)
261+
iteration = 0
262+
while True:
263+
iteration += 1
264+
has_more = task.chunk(apply_filter=True)
265+
266+
debug_output(
267+
f"[ITERATION {iteration}] {model_name} chunk (project_id={project_id} has_more={has_more})"
268+
)
269+
270+
if not has_more:
271+
break
246272

247273
# Increment metric once per chunk, after all iterations complete
248274
metrics.incr("cleanup.chunk_processed", tags={"model": model_name}, amount=chunk_size)
@@ -903,6 +929,7 @@ def _schedule_bulk_delete_chunks(
903929
q: BulkDeleteQuery, # Imported locally in functions that use it
904930
model_tp: type[BaseModel],
905931
project_id: int | None,
932+
organization_id: int | None = None,
906933
context_str: str = "",
907934
) -> tuple[int, int]:
908935
"""
@@ -939,7 +966,10 @@ def _schedule_bulk_delete_chunks(
939966
},
940967
)
941968
_last_progress_log[imp] = time.monotonic()
942-
task_queue.put((imp, chunk, project_id, q.deferred_filter))
969+
if organization_id is None:
970+
task_queue.put((imp, chunk, project_id, q.deferred_filter))
971+
else:
972+
task_queue.put((imp, chunk, project_id, q.deferred_filter, organization_id))
943973
chunk_count += 1
944974
total_objects += len(chunk)
945975

@@ -1027,9 +1057,9 @@ def run_bulk_deletes_by_project(
10271057
if project_deletion_query is not None and len(to_delete_by_project):
10281058
debug_output("Running bulk deletes in DELETES_BY_PROJECT")
10291059

1030-
for project_id_for_deletion in RangeQuerySetWrapper(
1031-
project_deletion_query.values_list("id", flat=True),
1032-
result_value_getter=lambda item: item,
1060+
for project_id_for_deletion, organization_id_for_deletion in RangeQuerySetWrapper(
1061+
project_deletion_query.values_list("id", "organization_id"),
1062+
result_value_getter=lambda item: item[0],
10331063
):
10341064
for model_tp, dtfield, order_by in to_delete_by_project:
10351065
models_attempted.add(model_tp.__name__.lower())
@@ -1046,7 +1076,13 @@ def run_bulk_deletes_by_project(
10461076
order_by=order_by,
10471077
)
10481078

1049-
_schedule_bulk_delete_chunks(task_queue, q, model_tp, project_id_for_deletion)
1079+
_schedule_bulk_delete_chunks(
1080+
task_queue,
1081+
q,
1082+
model_tp,
1083+
project_id_for_deletion,
1084+
organization_id_for_deletion,
1085+
)
10501086
except Exception:
10511087
capture_exception(
10521088
tags={"model": model_tp.__name__, "project_id": project_id_for_deletion}

‎src/sentry/runner/commands/deletions.py‎

Lines changed: 33 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import click
66

77
from sentry.runner.decorators import configuration
8+
from sentry.viewer_context import ActorType, ViewerContext
89

910
if TYPE_CHECKING:
1011
from sentry.deletions.models.scheduleddeletion import BaseScheduledDeletion
@@ -146,6 +147,7 @@ def _run_one(*, deletion: BaseScheduledDeletion, verbose: bool = False) -> None:
146147
from django.core.exceptions import ObjectDoesNotExist
147148

148149
from sentry import deletions as deletions_module
150+
from sentry.deletions.tasks.scheduled import scheduled_deletion_viewer_context
149151
from sentry.signals import pending_delete
150152

151153
click.echo(f"Running deletion {deletion.id} ({deletion.model_name} #{deletion.object_id})...")
@@ -156,32 +158,37 @@ def _run_one(*, deletion: BaseScheduledDeletion, verbose: bool = False) -> None:
156158
deletion.delete()
157159
return
158160

159-
task = deletions_module.get(
160-
model=deletion.get_model(),
161-
query={"id": deletion.object_id},
162-
transaction_id=deletion.guid,
163-
actor_id=deletion.actor_id,
164-
)
165-
166-
if not task.should_proceed(instance):
167-
click.echo(" Deletion aborted (should_proceed returned False).")
168-
deletion.delete()
169-
return
161+
with scheduled_deletion_viewer_context(
162+
deletion,
163+
instance,
164+
origin_context=ViewerContext(actor_type=ActorType.SYSTEM),
165+
):
166+
task = deletions_module.get(
167+
model=deletion.get_model(),
168+
query={"id": deletion.object_id},
169+
transaction_id=deletion.guid,
170+
actor_id=deletion.actor_id,
171+
)
170172

171-
actor = deletion.get_actor()
172-
pending_delete.send(sender=type(instance), instance=instance, actor=actor)
173+
if not task.should_proceed(instance):
174+
click.echo(" Deletion aborted (should_proceed returned False).")
175+
deletion.delete()
176+
return
173177

174-
try:
175-
has_more = True
176-
while has_more:
177-
has_more = task.chunk()
178-
deletion.delete()
179-
click.echo(" Done.")
180-
except Exception as e:
181-
if verbose:
182-
import traceback
178+
actor = deletion.get_actor()
179+
pending_delete.send(sender=type(instance), instance=instance, actor=actor)
183180

184-
click.echo(f" Failed:\n{traceback.format_exc()}", err=True)
185-
else:
186-
click.echo(f" Failed: {e}", err=True)
187-
click.echo(" Deletion record preserved for retry. Re-run to continue.")
181+
try:
182+
has_more = True
183+
while has_more:
184+
has_more = task.chunk()
185+
deletion.delete()
186+
click.echo(" Done.")
187+
except Exception as e:
188+
if verbose:
189+
import traceback
190+
191+
click.echo(f" Failed:\n{traceback.format_exc()}", err=True)
192+
else:
193+
click.echo(f" Failed: {e}", err=True)
194+
click.echo(" Deletion record preserved for retry. Re-run to continue.")

0 commit comments

Comments
 (0)