Skip to content

Commit

Permalink
Allow request and application logs to have separate retention policies (
Browse files Browse the repository at this point in the history
  • Loading branch information
djperrefort authored Sep 16, 2024
1 parent 94fb52c commit cec5164
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 26 deletions.
15 changes: 8 additions & 7 deletions docs/install/settings.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,14 @@ Improperly configured settings can introduce dangerous vulnerabilities and may d
Keystone uses various static files and user content to facilitate operation.
By default, these files are stored in subdirectories of the installed application directory (`<app>`).

| Setting Name | Default Value | Description |
|---------------------------|--------------------------|---------------------------------------------------------------------------------------------------|
| `CONFIG_TIMEZONE` | `UTC` | The application timezone. |
| `CONFIG_STATIC_DIR` | `<app>/static_files` | Where to store internal static files required by the application. |
| `CONFIG_UPLOAD_DIR` | `<app>/upload_files` | Where to store file data uploaded by users. |
| `CONFIG_LOG_RETENTION` | 30 days | How long to store log records in seconds. Set to 0 to keep all records. |
| `CONFIG_LOG_LEVEL` | `WARNING` | Only record logs above this level (`CRITICAL`, `ERROR`, `WARNING`, `INFO`, `DEBUG`, or `NOTSET`). |
| Setting Name | Default Value | Description |
|----------------------------|----------------------|-------------------------------------------------------------------------------------------------------------|
| `CONFIG_TIMEZONE` | `UTC` | The timezone to use when rendering date/time values. |
| `CONFIG_STATIC_DIR` | `<app>/static_files` | Where to store internal static files required by the application. |
| `CONFIG_UPLOAD_DIR` | `<app>/upload_files` | Where to store file data uploaded by users. |
| `CONFIG_LOG_LEVEL` | `WARNING` | Only record application logs above this level (accepts `CRITICAL`, `ERROR`, `WARNING`, `INFO`, or `DEBUG`). |
| `CONFIG_LOG_RETENTION` | `2592000` (30 days) | How long to store application logs in seconds. Set to 0 to keep all records. |
| `CONFIG_REQUEST_RETENTION` | `2592000` (30 days) | How long to store request logs in seconds. Set to 0 to keep all records. |

## API Throttling

Expand Down
15 changes: 8 additions & 7 deletions keystone_api/apps/logging/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,13 @@


@shared_task()
def rotate_log_files() -> None:
"""Delete old log files."""
def clear_log_files() -> None:
"""Delete request and application logs according to retention policies set in application settings."""

if settings.LOG_RECORD_ROTATION == 0:
return
if settings.CONFIG_LOG_RETENTION > 0:
max_app_log_age = timezone.now() - timedelta(seconds=settings.CONFIG_LOG_RETENTION)
AppLog.objects.filter(time__lt=max_app_log_age).delete()

max_record_age = timezone.now() - timedelta(seconds=settings.LOG_RECORD_ROTATION)
AppLog.objects.filter(time__lt=max_record_age).delete()
RequestLog.objects.filter(time__lt=max_record_age).delete()
if settings.CONFIG_REQUEST_RETENTION > 0:
max_request_log_age = timezone.now() - timedelta(seconds=settings.CONFIG_REQUEST_RETENTION)
RequestLog.objects.filter(time__lt=max_request_log_age).delete()
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from django.utils.timezone import now

from apps.logging.models import AppLog, RequestLog
from apps.logging.tasks import rotate_log_files
from apps.logging.tasks import clear_log_files


class LogFileDeletion(TestCase):
Expand Down Expand Up @@ -37,9 +37,10 @@ def create_dummy_records(self, timestamp: datetime) -> None:
time=timestamp
)

@override_settings(LOG_RECORD_ROTATION=4)
@override_settings(CONFIG_LOG_RETENTION=4)
@override_settings(CONFIG_REQUEST_RETENTION=4)
@patch('django.utils.timezone.now')
def test_log_files_rotated(self, mock_now: Mock) -> None:
def test_log_files_deleted(self, mock_now: Mock) -> None:
"""Test expired log files are deleted."""

# Mock the current time
Expand All @@ -61,18 +62,19 @@ def test_log_files_rotated(self, mock_now: Mock) -> None:
self.assertEqual(2, RequestLog.objects.count())

# Run rotation
rotate_log_files()
clear_log_files()

# Assert only the newer records remain
self.assertEqual(1, AppLog.objects.count())
self.assertEqual(1, RequestLog.objects.count())

@override_settings(LOG_RECORD_ROTATION=0)
def test_rotation_disabled(self) -> None:
"""Test log files are not deleted when rotation is disabled."""
@override_settings(CONFIG_LOG_RETENTION=0)
@override_settings(CONFIG_REQUEST_RETENTION=0)
def test_deletion_disabled(self) -> None:
"""Test log files are not deleted when log clearing is disabled."""

self.create_dummy_records(now())

rotate_log_files()
clear_log_files()
self.assertEqual(1, AppLog.objects.count())
self.assertEqual(1, RequestLog.objects.count())
4 changes: 2 additions & 2 deletions keystone_api/apps/scheduler/celery.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
'schedule': crontab(minute='0'),
'description': 'This task synchronizes user data against LDAP. This task does nothing if LDAP is disabled.'
},
'apps.logging.tasks.rotate_log_files': {
'task': 'apps.logging.tasks.rotate_log_files',
'apps.logging.tasks.clear_log_files': {
'task': 'apps.logging.tasks.clear_log_files',
'schedule': crontab(hour='0', minute='0'),
'description': 'This task deletes old log entries according to application settings.'
},
Expand Down
6 changes: 4 additions & 2 deletions keystone_api/main/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@
]

TEMPLATES = [
{ # The default backend rquired by Django builtins (e.g., the admin)
{ # The default backend required by Django builtins (e.g., the admin)
'BACKEND': 'django.template.backends.django.DjangoTemplates',
'APP_DIRS': True,
'OPTIONS': {
Expand Down Expand Up @@ -282,7 +282,9 @@

# Logging

LOG_RECORD_ROTATION = env.int('CONFIG_LOG_RETENTION', timedelta(days=30).total_seconds())
CONFIG_LOG_RETENTION = env.int('CONFIG_LOG_RETENTION', timedelta(days=30).total_seconds())
CONFIG_REQUEST_RETENTION = env.int('CONFIG_REQUEST_RETENTION', timedelta(days=30).total_seconds())

LOGGING = {
"version": 1,
"disable_existing_loggers": False,
Expand Down

0 comments on commit cec5164

Please sign in to comment.