Add tests for truncated and symlinked files #24425
Conversation
botelastic
bot
added
the
needs_team
kvch
changed the title
|
Pinging @elastic/agent (Team:Agent) |
botelastic
bot
removed
the
needs_team
💔 Build Failed
Expand to view the summary
Build stats
Test stats 🧪
Trends 🧪Steps errors
Expand to view the steps failures
|
| Test | Results |
|---|---|
| Failed | 0 |
| Passed | 13513 |
| Skipped | 2271 |
| Total | 15784 |
kvch
force-pushed
the
test-filebeat-filestream-symlinking
branch
from
7d4b85d
to
bc1c3a2
Compare
kvch
force-pushed
the
test-filebeat-filestream-symlinking
branch
from
bc1c3a2
to
371b1ba
Compare
|
jenkins run tests |
|
|
||
| // rotate symlink | ||
| env.mustRemoveFile(symlinkName) | ||
| env.mustSymlink(secondTestlogName, symlinkName) |
There was a problem hiding this comment.
Interesting. I would not expect the symlink to be removed when rotating it. Just change the location it points to. What would happen in that case? What would happen if the new log file is larger then the old one when we update the link?
There was a problem hiding this comment.
I have just ported blindly the test from the original Python one. There should not be any change in behaviour of Filebeat if the symlink is renamed.
If the log file is bigger than the previous, Filebeat cannot detect truncation regardless of what we do with the file symlink, rename, remove and create again. That is why we need the special copytruncate prospector.
There was a problem hiding this comment.
I see. So we do track the inode of the symlink file, and not the inode of the target file.
I have just ported blindly the test from the original Python one. There should not be any change in behaviour of Filebeat if the symlink is renamed.
I still would expect a slight difference. If we remove the symlink file before, then the symlink is a new file with a new inode. If we just overwrite the symlink with a new file the inode is not changed and filebeat would/should see it as a truncation event. Also the observable result (collect from offset 0) should be the same, the internal handling of these 2 cases differ, right? So it would be nice to have additonal tests for symlinks, but I would say these cases can be tested as unit tests in the prospector or file watcher.
I guess we need to rethink symlink support to some extent. Detecting changes on a symlink should not only depend on the file size. Have we also considered the case of a symlink becoming an actual file (or the other way around)?
There was a problem hiding this comment.
Is it OK if I add a test for the scenario you had described in a follow up PR? You are raising valid points. However, we should decide what we want to tackle in the default prospector and what we want to cover in the copytruncate prospector. I would rather give us a bit more time for that and address your concerns in a new PR. WDYT?
| env.mustAppendLinesToFile(testlogName, line) | ||
|
|
||
| env.waitUntilEventCount(2) | ||
| env.requireOffsetInRegistry(testlogName, 2*len(line)) |
There was a problem hiding this comment.
What would happen if we have close...removed: true? Will remove act on the symlink state, or on the resolved file?
What happens on close...renamed?
There was a problem hiding this comment.
Filebeat cares about the original file, not the symlink. The setting has no impact on the behaviour of the reader if the symlink is deleted. I am not sure why the original test contains it.
| env.requireOffsetInRegistry(testlogName, len(line)) | ||
|
|
||
| // remove symlink | ||
| env.mustRemoveFile(symlinkName) |
There was a problem hiding this comment.
What happens if I don't remove/update the symlink, but the file the symlink points to? Do we even detect that case?
There was a problem hiding this comment.
Yes, the reader opens the original file and tracks everything the same way as done in ordinary files.
|
ARM tests fail because we are hitting docker registry limits. Are we missing some images in our caches? |
* upstream/master: packer cache support for the 7.x and 7.latestMinor branches (elastic#25091) Remove EventFetcher and EventsFetcher interface (elastic#25093) Update go-structform to 0.0.8 (elastic#25051) Update copy_fields.asciidoc (elastic#25053) [elastic-agent] ensure container is backwards compatible (elastic#25092) Add --fleet-server-service-token. Rename --fleet-server to --fleet-server-es. (elastic#25083) Add cgroup.cpuacct percentages (elastic#25057) Add tests for truncated and symlinked files in filestream input (elastic#24425) Fix panic when Hearbeat monitor initialization fails twice (elastic#25073) [Filebeat][httpjson] Change append transform to initiate new fields as a slice (elastic#25074) Osquerybeat: Result values type translation (elastic#25012) Update Osquerybeat spec to get it downloading from the correct artifactory path (elastic#25076) Fix changelog (elastic#25079) Strip Azure EventHub connection string in debug logs (elastic#25066) Change googlecloud to gcp in field names (elastic#25038) Bump stack version to 7.12.0 for testing (elastic#24957) packer-cache: cache the existing docker images on ARM and some more (elastic#25068) Disable logstash TestFetch flaky test (elastic#25044)
…tic#24425) (cherry picked from commit a9279cd)
kvch
added
v7.13.0
and removed
needs_backport
What does this PR do?
Add support for better support truncated files with symlinks and adds migrates related tests from
test_harvester.py.Why is it important?
Increase coverage.
Checklist
- [ ] I have made corresponding changes to the documentation- [ ] I have made corresponding change to the default configuration files- [ ] I have added an entry inCHANGELOG.next.asciidocorCHANGELOG-developer.next.asciidoc.