From 33211fe7d04e7a83be1523dd1dc4a85b7ac72b50 Mon Sep 17 00:00:00 2001 From: "Armen Zambrano G." <44410+armenzg@users.noreply.github.com> Date: Wed, 15 Oct 2025 16:48:29 -0400 Subject: [PATCH 1/3] ref(cleanup): Minor changes Changes included: * Start transaction for each process * Add metric to track process errors --- src/sentry/runner/commands/cleanup.py | 51 ++++++++++++++------------- 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/src/sentry/runner/commands/cleanup.py b/src/sentry/runner/commands/cleanup.py index 4d71680566340e..60fa3bef857a8b 100644 --- a/src/sentry/runner/commands/cleanup.py +++ b/src/sentry/runner/commands/cleanup.py @@ -78,8 +78,9 @@ def multiprocess_worker(task_queue: _WorkQueue) -> None: configure() from sentry import deletions, models, similarity + from sentry.utils import metrics - skip_models = [ + skip_child_relations_models = [ # Handled by other parts of cleanup models.EventAttachment, models.UserReport, @@ -98,22 +99,26 @@ def multiprocess_worker(task_queue: _WorkQueue) -> None: return model_name, chunk = j - model = import_string(model_name) - try: - task = deletions.get( - model=model, - query={"id__in": chunk}, - skip_models=skip_models, - transaction_id=uuid4().hex, - ) - while True: - if not task.chunk(apply_filter=True): - break - except Exception: - logger.exception("Error in multiprocess_worker.") - finally: - task_queue.task_done() + with sentry_sdk.start_transaction(op="cleanup", name="multiprocess_worker") as transaction: + transaction.set_tag("model", model_name) + model = import_string(model_name) + try: + task = deletions.get( + model=model, + query={"id__in": chunk}, + skip_models=skip_child_relations_models, + transaction_id=uuid4().hex, + ) + + while True: + if not task.chunk(apply_filter=True): + break + except Exception: + metrics.incr("cleanup.error", instance=model_name) + logger.exception("Error in multiprocess_worker.") + finally: + task_queue.task_done() @click.command() @@ -137,7 +142,8 @@ def multiprocess_worker(task_queue: _WorkQueue) -> None: "-t", default=False, is_flag=True, - help="Send the duration of this command to internal metrics.", + hidden=True, + help="(deprecated) Send the duration of this command to internal metrics.", ) @log_options() def cleanup( @@ -148,7 +154,7 @@ def cleanup( silent: bool, model: tuple[str, ...], router: str | None, - timed: bool, + _: bool, ) -> None: """Delete a portion of trailing data based on creation date. @@ -166,7 +172,6 @@ def cleanup( concurrency=concurrency, silent=silent, router=router, - timed=timed, ) @@ -178,7 +183,6 @@ def _cleanup( concurrency: int, silent: bool, router: str | None, - timed: bool, ) -> None: _validate_and_setup_environment(concurrency, silent) # Make sure we fork off multiprocessing pool @@ -190,7 +194,6 @@ def _cleanup( # main process tracks the overall cleanup operation performance. with sentry_sdk.start_transaction(op="cleanup", name="cleanup") as transaction: transaction.set_tag("router", router) - transaction.set_tag("model", model) try: from sentry.runner import configure @@ -198,9 +201,7 @@ def _cleanup( from sentry.utils import metrics - start_time = None - if timed: - start_time = time.time() + start_time = time.time() # list of models which this query is restricted to model_list = {m.lower() for m in model} @@ -286,7 +287,7 @@ def is_filtered(model: type[Model]) -> bool: # Shut down our pool _stop_pool(pool, task_queue) - if timed and start_time: + if start_time: duration = int(time.time() - start_time) metrics.timing("cleanup.duration", duration, instance=router, sample_rate=1.0) click.echo("Clean up took %s second(s)." % duration) From 71265b6e9dbdad7bc793e0ca9de3a24854c7b936 Mon Sep 17 00:00:00 2001 From: "Armen Zambrano G." <44410+armenzg@users.noreply.github.com> Date: Wed, 15 Oct 2025 17:27:24 -0400 Subject: [PATCH 2/3] Address feedback --- src/sentry/runner/commands/cleanup.py | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/sentry/runner/commands/cleanup.py b/src/sentry/runner/commands/cleanup.py index 60fa3bef857a8b..858ba356954760 100644 --- a/src/sentry/runner/commands/cleanup.py +++ b/src/sentry/runner/commands/cleanup.py @@ -100,10 +100,12 @@ def multiprocess_worker(task_queue: _WorkQueue) -> None: model_name, chunk = j - with sentry_sdk.start_transaction(op="cleanup", name="multiprocess_worker") as transaction: - transaction.set_tag("model", model_name) - model = import_string(model_name) - try: + try: + with sentry_sdk.start_transaction( + op="cleanup", name="multiprocess_worker" + ) as transaction: + transaction.set_tag("model", model_name) + model = import_string(model_name) task = deletions.get( model=model, query={"id__in": chunk}, @@ -114,11 +116,11 @@ def multiprocess_worker(task_queue: _WorkQueue) -> None: while True: if not task.chunk(apply_filter=True): break - except Exception: - metrics.incr("cleanup.error", instance=model_name) - logger.exception("Error in multiprocess_worker.") - finally: - task_queue.task_done() + except Exception: + metrics.incr("cleanup.error", instance=model_name) + logger.exception("Error in multiprocess_worker.") + finally: + task_queue.task_done() @click.command() @@ -193,7 +195,6 @@ def _cleanup( # transaction context issues in child processes. This ensures only the # main process tracks the overall cleanup operation performance. with sentry_sdk.start_transaction(op="cleanup", name="cleanup") as transaction: - transaction.set_tag("router", router) try: from sentry.runner import configure From 95d599650f505926ed8a88ae075b7c6f4556e473 Mon Sep 17 00:00:00 2001 From: "Armen Zambrano G." <44410+armenzg@users.noreply.github.com> Date: Fri, 17 Oct 2025 08:25:07 -0400 Subject: [PATCH 3/3] Address feedback --- src/sentry/runner/commands/cleanup.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sentry/runner/commands/cleanup.py b/src/sentry/runner/commands/cleanup.py index 858ba356954760..c620817784ea6e 100644 --- a/src/sentry/runner/commands/cleanup.py +++ b/src/sentry/runner/commands/cleanup.py @@ -117,7 +117,7 @@ def multiprocess_worker(task_queue: _WorkQueue) -> None: if not task.chunk(apply_filter=True): break except Exception: - metrics.incr("cleanup.error", instance=model_name) + metrics.incr("cleanup.error", instance=model_name, sample_rate=1.0) logger.exception("Error in multiprocess_worker.") finally: task_queue.task_done() @@ -156,7 +156,7 @@ def cleanup( silent: bool, model: tuple[str, ...], router: str | None, - _: bool, + timed: bool, ) -> None: """Delete a portion of trailing data based on creation date.