Add log retention period

[igitur]

Fixes #274

@bart6114 Your comments/feedback on this would be greatly appreciated.

Add Log Retention Period Feature (AI generated content below)

Summary

This PR adds a new optional log_retention_period parameter to job configurations in cheek, allowing automatic cleanup of old log entries from the SQLite database. The feature uses the github.com/pawelszydlo/humanize library to parse human-readable duration strings.

Changes

1. New Configuration Parameter (pkg/job.go)

• Added LogRetentionPeriod string field to JobSpec struct with YAML tag log_retention_period,omitempty
• Added logRetentionDuration time.Duration field to store parsed duration (not serialized to YAML)

2. Humanize Library Integration (pkg/schedule.go)

• Added github.com/pawelszydlo/humanize dependency
• Implemented cached Humanizer instance using sync.Once for performance
• Supports units: seconds, minutes, hours, days, weeks, months, years
• Examples: "7 days", "3 months", "1 hour and 30 minutes", "2.5 hours"

3. Log Retention Implementation (pkg/job.go)

• Added rotateLogs() method that deletes log entries older than the specified duration
• Called automatically in finalize() method after each job execution
• Logs number of rows deleted when retention occurs
• Handles edge cases: no duration set, no DB connection

4. Database Optimization (pkg/db.go)

• Added index idx_log_job_triggered_at on (job, triggered_at) columns for efficient deletion queries

5. Comprehensive Testing (pkg/job_test.go)

• Unit tests for duration parsing with various formats
• Integration tests for log retention functionality
• Edge case tests for zero duration, negative duration, missing DB
• Backward compatibility tests ensuring old formats fail

6. Updated Example (testdata/readme_example.yaml)

• Added log_retention_period: 7 days to the coffee job as an example

Usage Examples

jobs:
  my_job:
    command: echo "Hello"
    cron: "* * * * *"
    log_retention_period: 7 days  # Rotate logs older than 1 week
    retries: 2

  another_job:
    command: ./backup.sh
    cron: "0 2 * * *"
    log_retention_period: 3 months  # Rotate logs older than 3 months

Supported Duration Formats

• "24 hours"
• "7 days"
• "2 weeks"
• "3 months"
• "1 hour and 30 minutes"
• "90 minutes"
• "2.5 hours" (decimal values)
• "0 seconds" (fails validation - must be positive)

Validation Rules

1. Duration must be parseable by humanize library
2. Parsed duration must be positive (> 0)

Performance Considerations

• Cached Humanizer instance avoids repeated initialization
• Database index ensures efficient deletion queries
• Retention runs after job execution, not affecting job timing
• Minimal overhead when no retention period is specified

Testing Coverage

• ✅ Duration parsing with valid/invalid formats
• ✅ Log retention integration tests
• ✅ Edge cases (no duration, no DB, zero duration)
• ✅ All existing tests pass

Future Considerations

• Could add global schedule-level log retention configuration
• Could support cron-style periodic retention instead of per-execution
• Could add metrics for retention statistics

This feature helps manage database growth by automatically cleaning up old log entries while maintaining flexibility through human-readable duration specifications.

[igitur]

@bart6114 If I inspect the cheek sqlite file, I see that there's already a sqlite_autoindex_log_1 index, so I'm unsure whether this additional index adds any value.

[igitur]

@bart6114 Ready for review.

[bart6114]

Thanks for the PR @igitur!

A few pieces of feedback from my pov. Could you have a look? 🙏

1. Rename cleanupOldLogs → rotateLogs

To align with the "log rotation" terminology used elsewhere.

2. Negative duration handling

This test caught my attention:

{
    name:        "negative duration becomes positive",
    durationStr: "-1 hour",
    shouldError: false, // humanize converts negative to positive
}

Silent conversion of -1 hour to a positive value is unexpected. Better to return an error than allow magic behavior.

3. Add global-level support

It's not unlikely that users will want a single retention period for all jobs. Please add support for setting log_retention_period at the schedule level (next to on_success, on_error, etc.), with possibility for setting config at jobspec level.

4. Documentation

• Please document the supported duration formats (what strings are valid for log_retention_period)
• Add this feature to the configuration docs: https://cheek.barts.space/docs/configuration/
• Perhaps add a dedicated "Log Rotation" docs page, similar to how there's already pages for events etc.

[igitur]

Hi, thanks for the feedback.

1. Rename cleanupOldLogs → rotateLogs

I originally called the new property log_rotation_period, but then settled on log_retention_period as you suggested in #274. The the verb rotate changed to cleanup. I don't see references to rotation anymore. Where do you see it? Given this, do you still prefer rotateLogs over cleanupOldLogs ?

2. Negative duration handling

This test caught my attention:

{
    name:        "negative duration becomes positive",
    durationStr: "-1 hour",
    shouldError: false, // humanize converts negative to positive
}

Agreed. I thought I did explicitly test that these fail. I'll double-check.

3. Add global-level support

It's not unlikely that users will want a single retention period for all jobs. Please add support for setting log_retention_period at the schedule level (next to on_success, on_error, etc.), with possibility for setting config at jobspec level.

Sure thing.

4. Documentation

• Please document the supported duration formats (what strings are valid for log_retention_period)
• Add this feature to the configuration docs: https://cheek.barts.space/docs/configuration/
• Perhaps add a dedicated "Log Rotation" docs page, similar to how there's already pages for events etc.

Gotcha.

[igitur]

I'd like to fix support for negative durations upstream. See pawelszydlo/humanize#1 . Then we can avoid checking for a - prefix in cheek itself, and just check that the parsed duration is positive.

[bart6114]

I originally called the new property log_rotation_period, but then settled on log_retention_period as you suggested in #274. The the verb rotate changed to cleanup. I don't see references to rotation anymore. Where do you see it? Given this, do you still prefer rotateLogs over cleanupOldLogs ?

Fair point. Slight preference for rotate, but perhaps cleanupOldLogs is easier to understand when browsing throuhg the code. Up to you

I'd like to fix support for negative durations upstream.

Did you check out https://github.com/dustin/go-humanize? Not sure if they're related, but this one seems much better maintained.