feat(activations): log retention Phase 2 — timestamp filters, DEBUG toggle, purge API, safety valve - #1650
Open
B-Whitt wants to merge 4 commits into
Open
Conversation
Adds log_timestamp__gt and log_timestamp__lt query parameters to the activation instance logs endpoint, enabling timestamp-based filtering for polling new logs and fetching historical data. Resolves: AAP-84682 Assisted by: Claude Opus 4.6
…DEBUG lines When store_debug_logs is false (the default), DEBUG-level log lines are still sent to container stdout but excluded from the database. This drastically reduces DB log volume (~225x fewer rows) for activations running at DEBUG level while preserving observability through container logs. Resolves: AAP-84681 Assisted by: Claude Opus 4.6
Adds POST /activations/{id}/clear-logs/ for per-activation log purge
and POST /logs/purge/ for global purge (superuser only). Both support
an optional before_date parameter for partial purges. Deletion is
batched (10K rows per batch) to avoid long-running queries and lock
contention on large tables.
Resolves: AAP-84683
Assisted by: Claude Opus 4.6
Adds EDA_MAX_LOG_LINES_PER_INSTANCE (default 500K, 0 = unlimited) that trims the oldest log rows when a per-instance cap is exceeded. The COUNT check runs every 1000 lines to amortize the query cost. This prevents any single activation from consuming unbounded DB storage even when the DEBUG toggle is enabled. Resolves: AAP-84680 Assisted by: Claude Opus 4.6
📝 WalkthroughWalkthroughActivation logging now supports configurable DEBUG persistence, periodic retention trimming, activation-scoped and global purge endpoints, and timestamp range filtering for activation-instance logs. ChangesActivation logging and log management
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant logs_route
participant LogPurgeViewSet
participant LogPurgeRequestSerializer
participant delete_all_logs
participant RulebookProcessLog
Client->>logs_route: Submit purge request
logs_route->>LogPurgeViewSet: Dispatch purge action
LogPurgeViewSet->>LogPurgeRequestSerializer: Validate before_date
LogPurgeViewSet->>delete_all_logs: Delete matching logs
delete_all_logs->>RulebookProcessLog: Batch-delete records
RulebookProcessLog-->>LogPurgeViewSet: Return deleted count
LogPurgeViewSet-->>Client: Return deleted count
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/aap_eda/core/utils/delete_log_util.py`:
- Around line 82-90: Capture the highest matching record ID from the queryset
before entering the batch loop, then constrain each batch query and deletion in
the purge flow to IDs at or below that captured boundary. Update the loop around
queryset.values_list and RulebookProcessLog.objects.filter so newly written
matching logs are excluded while preserving existing batch deletion and logging
behavior.
In `@src/aap_eda/services/activation/db_log_handler.py`:
- Around line 100-105: Update DBLogger._enforce_max_log_lines so retention-check
progress persists across logger instances created by
ActivationManager.update_logs(), rather than relying on the instance-local
line_count. Store durable per-activation-instance state or derive the check
interval from persisted log data, while preserving the existing max_lines guard
and periodic trimming behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5fa7ce1d-9e87-47c6-8ef8-1dd3e8bae01c
📒 Files selected for processing (19)
src/aap_eda/api/filters/activation.pysrc/aap_eda/api/serializers/__init__.pysrc/aap_eda/api/serializers/activation.pysrc/aap_eda/api/urls.pysrc/aap_eda/api/views/__init__.pysrc/aap_eda/api/views/activation.pysrc/aap_eda/core/migrations/0074_activation_store_debug_logs.pysrc/aap_eda/core/models/activation.pysrc/aap_eda/core/utils/delete_log_util.pysrc/aap_eda/services/activation/activation_manager.pysrc/aap_eda/services/activation/db_log_handler.pysrc/aap_eda/services/activation/tee_system_logger.pysrc/aap_eda/settings/defaults.pytests/integration/api/test_activation.pytests/integration/api/test_activation_instance.pytests/integration/api/test_log_purge.pytests/integration/conftest.pytests/integration/services/activation/test_db_log_handler.pytests/integration/services/activation/test_tee_system_logger.py
Comment on lines
+82
to
+90
| while True: | ||
| batch_ids = list(queryset.values_list("id", flat=True)[:BATCH_SIZE]) | ||
| if not batch_ids: | ||
| break | ||
| deleted, _ = models.RulebookProcessLog.objects.filter( | ||
| id__in=batch_ids, | ||
| ).delete() | ||
| total_deleted += deleted | ||
| logger.info("Purged %d log records (batch)", deleted) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Bound the purge to a stable record set.
Line 83 reevaluates an unbounded queryset for every batch. Active log writers can add matching rows faster than this loop deletes them. A global purge without before_date can then run until the request times out.
Capture the highest matching ID before the loop. Delete only records at or below that ID.
Proposed fix
def _batched_delete(queryset) -> int:
"""Delete queryset in batches to avoid long-running queries."""
total_deleted = 0
+ upper_id = queryset.order_by("-id").values_list("id", flat=True).first()
+ if upper_id is None:
+ return total_deleted
+ queryset = queryset.filter(id__lte=upper_id)
+
while True:
batch_ids = list(queryset.values_list("id", flat=True)[:BATCH_SIZE])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| while True: | |
| batch_ids = list(queryset.values_list("id", flat=True)[:BATCH_SIZE]) | |
| if not batch_ids: | |
| break | |
| deleted, _ = models.RulebookProcessLog.objects.filter( | |
| id__in=batch_ids, | |
| ).delete() | |
| total_deleted += deleted | |
| logger.info("Purged %d log records (batch)", deleted) | |
| upper_id = queryset.order_by("-id").values_list("id", flat=True).first() | |
| if upper_id is None: | |
| return total_deleted | |
| queryset = queryset.filter(id__lte=upper_id) | |
| while True: | |
| batch_ids = list(queryset.values_list("id", flat=True)[:BATCH_SIZE]) | |
| if not batch_ids: | |
| break | |
| deleted, _ = models.RulebookProcessLog.objects.filter( | |
| id__in=batch_ids, | |
| ).delete() | |
| total_deleted += deleted | |
| logger.info("Purged %d log records (batch)", deleted) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/aap_eda/core/utils/delete_log_util.py` around lines 82 - 90, Capture the
highest matching record ID from the queryset before entering the batch loop,
then constrain each batch query and deletion in the purge flow to IDs at or
below that captured boundary. Update the loop around queryset.values_list and
RulebookProcessLog.objects.filter so newly written matching logs are excluded
while preserving existing batch deletion and logging behavior.
Comment on lines
+100
to
+105
| def _enforce_max_log_lines(self) -> None: | ||
| max_lines = int(settings.EDA_MAX_LOG_LINES_PER_INSTANCE) | ||
| if max_lines <= 0: | ||
| return | ||
| if self.line_count % 1000 != 0: | ||
| return |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
Keep retention check state across logger instances.
line_count starts at zero for every DBLogger. ActivationManager.update_logs() creates a new logger for each poll. If each poll writes fewer than 1,000 lines, this method never counts or trims records. The activation instance can exceed EDA_MAX_LOG_LINES_PER_INSTANCE without bound.
Use durable per-instance check state, or calculate the trim interval from persisted state.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/aap_eda/services/activation/db_log_handler.py` around lines 100 - 105,
Update DBLogger._enforce_max_log_lines so retention-check progress persists
across logger instances created by ActivationManager.update_logs(), rather than
relying on the instance-local line_count. Store durable per-activation-instance
state or derive the check interval from persisted log data, while preserving the
existing max_lines guard and periodic trimming behavior.
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
max_page_sizecap and-idordering.log_timestamp__gt/log_timestamp__ltquery params on the logs endpointstore_debug_logsboolean on Activation (default false); DEBUG lines go to container stdout but not DBPOST /activations/{id}/clear-logs/(per-activation) andPOST /logs/purge/(global, superuser only) with batched deletionEDA_MAX_LOG_LINES_PER_INSTANCE(default 500K) trims oldest rows every 1000 linesChanges
src/aap_eda/api/filters/activation.py— added timestamp filters toActivationInstanceLogFiltersrc/aap_eda/core/models/activation.py— addedstore_debug_logsfieldsrc/aap_eda/core/migrations/0074_activation_store_debug_logs.py— migrationsrc/aap_eda/services/activation/tee_system_logger.py— filter DEBUG lines from DB buffersrc/aap_eda/services/activation/db_log_handler.py— acceptstore_debug_logsparam, enforce line capsrc/aap_eda/services/activation/activation_manager.py— passstore_debug_logsviafunctools.partialsrc/aap_eda/api/serializers/activation.py— exposestore_debug_logsin Create/Update/Read/List/Copy, add purge serializerssrc/aap_eda/api/views/activation.py—clear_logsaction +LogPurgeViewSetsrc/aap_eda/api/urls.py— register/logs/routesrc/aap_eda/core/utils/delete_log_util.py— per-activation purge + batched deletionsrc/aap_eda/settings/defaults.py—EDA_MAX_LOG_LINES_PER_INSTANCEsettingTest Plan
poetry run python -m pytest tests/integration/api/test_activation_instance.py -k "timestamp"— 4 timestamp filter testspoetry run python -m pytest tests/integration/services/activation/test_tee_system_logger.py— 5 tests (3 new for DEBUG toggle)poetry run python -m pytest tests/integration/api/test_activation.py::test_create_activation—store_debug_logsin base assertionpoetry run python -m pytest tests/integration/api/test_log_purge.py— 6 purge API testspoetry run python -m pytest tests/integration/services/activation/test_db_log_handler.py— 3 safety valve testsJira
Resolves: AAP-84682, AAP-84681, AAP-84683, AAP-84680
Parent: AAP-77938
Summary by CodeRabbit
New Features
Bug Fixes