aws-greengrass / aws-greengrass/aws-greengrass-logging-java
greengrass removes logs older than 11 hours by default
- Dominant language
- Java
- Stars
- 4
- Forks
- 2
- PR merge metrics
- No merged PRs in 30d
Description
**Describe the bug**
With the default configuration of `totalLogsSizeKB` being `10240` and `fileSizeKB` being `1024` according to [greengrass nucleus component documentation](https://docs.aws.amazon.com/greengrass/v2/developerguide/greengrass-nucleus-component.html) `maxHistory` is then set to 10 in the PersistenceConfig.
https://github.com/aws-greengrass/aws-greengrass-logging-java/blob/3058bc52fe12dc3ea4cd8c9f738952a4e957d200/src/main/java/com/aws/greengrass/logging/impl/config/PersistenceConfig.java#L351-L352
Then, since `setCleanLogsByLastModifiedDate(true)` is set, which I assume uses similar code to what has been proposed by @tiancishen in https://github.com/qos-ch/logback/pull/763/ (though I did not find the source for the logback-core 1.3.14-GG-SNAPSHOT release which is unfortunate) files are then removed based on their modified timestamp and rolling periodicity, which is hours in greengrass case.
Cleanup removes files which are older than the cleanupCutoff, calculated as
```java
Instant cleanupCutoff = rc.getEndOfNextNthPeriod(now, getPeriodOffsetForDeletionTarget());
```
where `getPeriodOffsetForDeletionTarget` depends solely on `maxHistory` which is `10240/1024` = `10` in the default case
```java
protected int getPeriodOffsetForDeletionTarget() {
return -maxHistory - 1;
}
```
**To Reproduce**
Create dummy log files with modified timestamps:
```
root@test:/greengrass/v2/logs# touch -d "13 hours ago" greengrass_2025_01_01_13_0.log
root@test:/greengrass/v2/logs# touch -d "12 hours ago" greengrass_2025_01_01_12_0.log
root@test:/greengrass/v2/logs# touch -d "11 hours ago" greengrass_2025_01_01_11_0.log
root@test:/greengrass/v2/logs# touch -d "10 hours ago" greengrass_2025_01_01_10_0.log
```
```
root@test:/greengrass/v2/logs# ls -la
total 560
drwx------ 2 root root 20480 Nov 7 10:48 .
drwxr-xr-x 12 root root 4096 Nov 7 10:46 ..
-rw-r--r-- 1 root root 0 Nov 15 2024 aws.greengrass.Nucleus.log
-rw-r--r-- 1 root root 0 Nov 7 00:48 greengrass_2025_01_01_10_0.log
-rw-r--r-- 1 root root 0 Nov 6 23:48 greengrass_2025_01_01_11_0.log
-rw-r--r-- 1 root root 0 Nov 6 22:48 greengrass_2025_01_01_12_0.log
-rw-r--r-- 1 root root 0 Nov 6 21:48 greengrass_2025_01_01_13_0.log
-rw-r--r-- 1 root root 65478 Nov 7 10:46 greengrass.log
...
```
restart greengrass service to trigger log rotation:
```
root@test:/greengrass/v2/logs# sudo systemctl restart greengrass.service
```
observe that the files older than 11 hours are gone:
```
root@test:/greengrass/v2/logs# ls -la
total 576
drwx------ 2 root root 20480 Nov 7 10:50 .
drwxr-xr-x 12 root root 4096 Nov 7 10:50 ..
-rw-r--r-- 1 root root 0 Nov 15 2024 aws.greengrass.Nucleus.log
-rw-r--r-- 1 root root 0 Nov 7 00:48 greengrass_2025_01_01_10_0.log
-rw-r--r-- 1 root root 0 Nov 6 23:48 greengrass_2025_01_01_11_0.log
-rw-r--r-- 1 root root 78719 Nov 7 10:50 greengrass.log
...
```
**Expected behavior**
Log rotation and removal should follow contract laid out in the documentation, i.e. the total size of logs should not exceed `totalLogsSizeKB` and each file should not exceed `fileSizeKB`.
I would expect all 4 files to remain after the restart in the example above as they do not exceed the default total size of 10MB and even asuming they were 1MB each, they would still fit within the limit of 10 files of 1MB each.
**Actual behavior**
Files are removed based on their last modified timestamp and rolling periodicity, according to maxHistory formula that assumes fixed size log files.
**Additional context**
Files should be removed based on their size without unexpected limit on the file age. If greengrass component only logs occasionally, log files should not be removed just because they are old if the total size is still under the limit. If a component needs to be able to limit log age, a separate configuration option should be provided for that.
In case size-based cleanup is not feasible, at least the documentation should be updated to reflect the actual behavior.
Contributor guide
Research direction
Start with src/main/java/com/aws/greengrass/logging/impl/config/PersistenceConfig.java at the referenced maxHistory calculation, then reproduce cleanup by restarting greengrass.service with the timestamped files described in the issue. Done means log removal follows the documented totalLogsSizeKB and fileSizeKB limits, or the component documentation explicitly describes the age-based behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- observability-sre
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100