[Logfile] Clean up state folder on init #1114
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1114 +/- ##
==========================================
+ Coverage 57.58% 64.58% +7.00%
==========================================
Files 370 373 +3
Lines 17548 19709 +2161
==========================================
+ Hits 10105 12730 +2625
+ Misses 6848 6331 -517
- Partials 595 648 +53 ☔ View full report in Codecov by Sentry. |
plugins/inputs/logfile/logfile.go
Outdated
| @@ -107,7 +107,8 @@ func (t *LogFile) Start(acc telegraf.Accumulator) error { | |||
| return fmt.Errorf("failed to create state file directory %s: %v", t.FileStateFolder, err) | |||
| } | |||
|
|
|||
| // Clean state file regularly | |||
| // Clean state file on init and regularly | |||
| t.cleanupStateFolder() | |||
There was a problem hiding this comment.
The agent reads the state to track which part of the log file it left off reading. Does this execute before or after the state file has been read?
There was a problem hiding this comment.
nit: Move this into the goroutine. I don't think we need to block on the clean up.
plugins/inputs/logfile/logfile.go
Outdated
| @@ -107,7 +107,8 @@ func (t *LogFile) Start(acc telegraf.Accumulator) error { | |||
| return fmt.Errorf("failed to create state file directory %s: %v", t.FileStateFolder, err) | |||
| } | |||
|
|
|||
| // Clean state file regularly | |||
| // Clean state file on init and regularly | |||
| t.cleanupStateFolder() | |||
There was a problem hiding this comment.
nit: Move this into the goroutine. I don't think we need to block on the clean up.
|
This PR was marked stale due to lack of activity. |
| go func() { | ||
| t.cleanupStateFolder() |
There was a problem hiding this comment.
Can you add a unit test or integ test which creates some files monitored by the agent, write 1 MiB of logs to them, verify the state files point to the EOF, then kill the agent, then write more data, then start the agent back up and make sure they report logs events from the 1 MiB point onwards? (i.e. state file is used correectly)
Maybe we already have a test like this, if so great.
My concern is that you are deleting the state files before the agent has a chance to start up and read them. (I think Zhihong has the same concern).
Description of the issue
Currently, customers using the logs plugin have a state folder tracking log files on the host that are being watched. This folder gets cleaned up periodically every hour. If the agent dies or crashes before the 1 hour, the agent could end up creating more state files causing excessive inodes to fill up on the host.
Description of changes
We can clean up the state folder on initialization of the logfile plugin, as well as periodically.
License
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Tests
pass unit tests
Requirements
Before commit the code, please do the following steps.
make fmtandmake fmt-shmake lint