feat(inputs.directory_monitor): Allow to preserve timestamps when moving file#18921
feat(inputs.directory_monitor): Allow to preserve timestamps when moving file#18921highlyavailable wants to merge 3 commits into
Conversation
When a processed file is moved to the finished or error directory, the plugin copies it to a new file, so the moved file gets the current time as its modification time and the original file history is lost. This adds an optional preserve_timestamps setting. When enabled, the source file's access and modification times are captured before the original is removed and reapplied to the moved file with os.Chtimes. The option defaults to false, so existing behavior is unchanged.
|
Thanks so much for the pull request! |
|
!signed-cla |
srebhan
left a comment
There was a problem hiding this comment.
Nice. Thanks @highlyavailable for your contribution! Just one comment from my side...
Co-authored-by: Sven Rebhan <36194019+srebhan@users.noreply.github.com>
|
Does the current state of the code actually sets the created date to the original file create date? According to the documentation, it would only set the access and modification time. |
|
@highlyavailable can you please run |
|
AND please restore the PR description template, especially the AI checkboxes. Those items are there for a reason. :-) |
|
no, it only sets the access and modification time. The call is here: the sample.conf comment says "access and modification" so the docs match the behavior. do you thhink this should be changed? |
|
@srebhan I addressed both of your comments! |
|
Download PR build artifacts for linux_amd64.tar.gz, darwin_arm64.tar.gz, and windows_amd64.zip. 📦 Click here to get additional PR build artifactsArtifact URLs |
|
So this PR does not solve the mentioned Feature Request, were it states:
|
to go back on my last response, the "updated" part of the issue's wording is modification time, which this PR actually does preserve. so the only remaining issue is not perserving creation time. Go's stlib doesn't have a setter (that I know of) to set that creation/birth time. os.Chtimes covers access and mod times, but I believe birth time would need per-platform syscalls (macOS setattrlist, Windows SetFileTime, not sure about Linux but I could look into it) I feel this is a reasonable (partial) fix given the intent of the issue, but I should have called out that it doesn't satisfy the full reqs in the feture request. I could either update the docs to make clear only access+mod times are covered, or update the PR attempting to include per-platform creation-time support as a follow-up. any thoughts either way? also, just wanted to say thank you @Hipska for the thorough review, its my first time contributing to this project and I appreciate it. |
|
Would the creation time be preserved when using |
yes, on the same FS os.Rename (which wraps rename(2), source) just relinks the directory entry, so access+modification+ creation times would all stay intact. And as we discussed, cross-FS it fails with EXDEV and we would need to fall back to copy + Chtimes, which only restores access and modification time |
|
@Hipska we cannot use @highlyavailable let us get this in as-is first. Please adapt your PR description and change "resolves" to "partially resolves"... In a second PR, implement platform dependent ways of restoring the creation/birth time. On Windows that's pretty easy (use |
|
@srebhan I just updated the description, thank you both for working on this with me |
Summary
When a processed file is moved to the finished or error directory, the plugin copies it to a new file, so the moved file gets the current time as its modification time and the original file history is lost. This is a problem for the auditing and tracking use cases described in #16885.
This adds an optional
preserve_timestampssetting. When enabled, the source file's access and modification times are captured before the original is removed, then reapplied to the moved file withos.Chtimes. It defaults tofalse, so existing behavior is unchanged. Added a test covering both the enabled and disabled paths.Checklist
Related issues
partially resolves #16885