Skip to content
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

feat: [log retention improvements pt. 1] introduce file layout interface #2534

Merged
merged 39 commits into from
Aug 16, 2024

Conversation

tedim52
Copy link
Contributor

@tedim52 tedim52 commented Aug 14, 2024

Description

Log Retention Improvements:

Users have had issues with logs from long running enclaves taking up tons of storage. We have a retention mechanism that automatically rotates logs after some time but currently it a) is only able to rotate logs weekly b) can not be configured.

These improvements will allow retention to be as granular as hourly and will allow users to configure the retention period (eg. 1hr, 2hr, 1day, 1week). To do this, a few changes need to happen. Most notably the way logs are stored and retrieved needs to change to support rotating log files hourly. Implementing this requires changes across a few components(LogsAggregator, LogsDatabaseClient, LogFileManager, cli, so I'll be making them incrementally.

  • PR 1: Introduce LogFileLayout and PerWeekLogFileLayout and adjust LogFileManager and LogsDatabaseClient to use this for retrieving log file paths
  • PR 2: Make retention configurable via a CLI flag
  • PR 3: Implement PerHourFileLayout and swap storage format from PerWeekFileLayout to PerHourFileLayout
  • PR 4: Make LogsAggregator use LogFileLayout for determining storage format and set it to use PerHourFileLayout

This first PR sets the stage for this change by introducing a new interface called LogFileLayout and migrating the existing storage format to use it. Right now, knowledge about the log storage format is spread across several entities (LogsAggregator - to store, LogFileManager - to rotate logs, StreamLogsStrategy - to read logs). This interface consolidates knowledge of how logs are stored into one module that can be used by any entity doing operations that require retrieving log files from the filesystem. This will make the transition to a different storage format safer (only requires implementing + testing PerHourFileLayout module) and makes it easier to swap out the storage format in the future (eg. if even more granular retention is need) without breaking other entities.

Is this change user facing?

NO

References

#2443
#2190

@tedim52 tedim52 force-pushed the tedi/granularetention branch from 9ca8d22 to 11449c1 Compare August 14, 2024 22:40
@tedim52 tedim52 requested a review from h4ck3rk3y August 15, 2024 16:03
@tedim52 tedim52 added this pull request to the merge queue Aug 16, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 16, 2024
@tedim52 tedim52 added this pull request to the merge queue Aug 16, 2024
Merged via the queue into main with commit a494278 Aug 16, 2024
71 checks passed
@tedim52 tedim52 deleted the tedi/granularetention branch August 16, 2024 13:45
github-merge-queue bot pushed a commit that referenced this pull request Aug 16, 2024
🤖 I have created a release *beep* *boop*
---


##
[1.1.0](1.0.0...1.1.0)
(2024-08-16)


### Features

* [log retention improvements pt. 1] introduce file layout interface
([#2534](#2534))
([a494278](a494278))
* make kurtosis service logs faster
([#2525](#2525))
([d6b246a](d6b246a))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: kurtosisbot <[email protected]>
tedim52 added a commit that referenced this pull request Aug 30, 2024
… retention period (#2536)

## Description
Log Retention Improvements:

This PR allows users to configure log retention period with
`--log-retention-period` flag on engine starts/restarts. Default is set
to 1 week.

- [x] PR 1: Introduce `LogFileLayout` and `PerWeekLogFileLayout` and
adjust `LogFileManager` and `LogsDatabaseClient` to use this for
retrieving log file paths
- [x] PR 2: Make retention configurable via a CLI flag
- [ ] PR 3: Implement `PerHourFileLayout` and swap storage format from
`PerWeekFileLayout` to `PerHourFileLayout`
- [ ] PR 4: Make `LogsAggregator` use `LogFileLayout` for determining
storage format and set it to use `PerHourFileLayout`
------------------------------

## Is this change user facing?
NO

## References 
#2443
#2190
#2534
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants