-
Notifications
You must be signed in to change notification settings - Fork 69
fix(logs): automated log retention and API page_size cap #1621
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,79 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Copyright 2025 Red Hat, Inc. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # you may not use this file except in compliance with the License. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # You may obtain a copy of the License at | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Unless required by applicable law or agreed to in writing, software | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # See the License for the specific language governing permissions and | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # limitations under the License. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from datetime import datetime | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from django.db.models import Q | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from django.utils import timezone | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from aap_eda.core import models | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def delete_logs_older_than(cutoff: datetime) -> int: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Delete all RulebookProcessLog records older than the cutoff. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Returns the number of records deleted. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cutoff_ts = int(cutoff.timestamp()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| deleted, _ = models.RulebookProcessLog.objects.filter( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| log_timestamp__lt=cutoff_ts, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ).delete() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return deleted | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def create_audit_trail( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cutoff: datetime, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| activation_ids: list[int] | None = None, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| activation_names: list[str] | None = None, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) -> int: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Create audit trail log entries recording a purge operation. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| If activation_ids or activation_names are provided, audit entries | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| are scoped to those activations plus orphaned records. Otherwise, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| audit entries are created for all RulebookProcess instances. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Returns the number of audit entries created. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ids = activation_ids or [] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| names = activation_names or [] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if not ids and not names: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| instances = models.RulebookProcess.objects.all() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| instances = models.RulebookProcess.objects.filter( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Q(activation__id__in=ids) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| | Q(activation__name__in=names) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| | Q(activation__isnull=True), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| dt = timezone.now().strftime("%Y-%m-%d %H:%M:%S") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cutoff_ts = int(cutoff.timestamp()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| audit_logs = [ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| models.RulebookProcessLog( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| log=( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| f"All log records older than " | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| f"{cutoff.strftime('%Y-%m-%d')} " | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| f"were purged at {dt}." | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| activation_instance_id=instance.id, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| log_timestamp=cutoff_ts, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for instance in instances | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if audit_logs: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| models.RulebookProcessLog.objects.bulk_create(audit_logs) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+63
to
+77
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win Timestamp audit entries at creation time. Line 71 assigns the retention cutoff to the audit row. Because the next hourly cutoff is newer, that audit entry is deleted on the next cleanup run. Store Proposed fix- cutoff_ts = int(cutoff.timestamp())
+ audit_ts = int(timezone.now().timestamp())
...
- log_timestamp=cutoff_ts,
+ log_timestamp=audit_ts,As per path instructions, focus on major issues impacting performance, readability, maintainability and security. 📝 Committable suggestion
Suggested change
🤖 Prompt for AI AgentsSource: Path instructions |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return len(audit_logs) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
Index and batch the hourly retention deletion.
RulebookProcessLog.log_timestamphas no index in the supplied model, so this hourly filter scans the growing log table. The single unboundedDELETEcan also create a large transaction and prolonged locks during initial cleanup. Add an indexed migration and delete in bounded batches.As per path instructions, focus on major issues impacting performance, readability, maintainability and security.
🤖 Prompt for AI Agents
Source: Path instructions