Detect new files under known paths in filestream input #31268

kvch · 2022-04-12T13:38:29Z

What does this PR do?

This PR fixes the FileWatcher of filestream input. Now a file is considered new even if the scanner has already found it in the previous iteration and the underlying file is different.

In the PR the file comparator function is passed as a parameter to make unit testing easier.

Why is it important?

The problem is if an input file is renamed and a new file shows up, Filebeat did not register is as a new file. The new file was either considered updated. Or if the new file was smaller than the previous file, the file was deemed truncated and the complete contents of the previous file was reread from the beginning.

The issue was reported initially on Discuss: https://discuss.elastic.co/t/filebeat-filestream-input-rereading-rotated-log-files/300038

Checklist

My code follows the style guidelines of this project
I have commented my code, particularly in hard-to-understand areas
~~- [ ] I have made corresponding changes to the documentation~~
~~- [ ] I have made corresponding change to the default configuration files~~
I have added tests that prove my fix is effective or that my feature works
I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

elasticmachine · 2022-04-12T13:38:42Z

Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane)

mergify · 2022-04-12T13:43:37Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b fix-filebeat-filestream-new-file-under-existing-path upstream/fix-filebeat-filestream-new-file-under-existing-path
git merge upstream/main
git push upstream fix-filebeat-filestream-new-file-under-existing-path

…ream-new-file-under-existing-path

elasticmachine · 2022-04-12T13:49:25Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2022-04-12T17:57:49.089+0000
Duration: 67 min 42 sec

Test stats 🧪

Test	Results
Failed	0
Passed	6200
Skipped	728
Total	6928

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.
/package : Generate the packages and run the E2E tests.
/beats-tester : Run the installation tests with beats-tester.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

belimawr

Looks good, just some small details in the tests.

filebeat/input/filestream/fswatch_test.go

Co-authored-by: Tiago Queiroz <[email protected]>

belimawr

🎉

kvch · 2022-04-12T18:02:13Z

The test is not passing on Windows for some reason. So I moved the test to non-windows tests for now. I am comfortable with merging this because we are not breaking anything for Windows users. We just still do not support something that was broken in the first place. The test failure needs more investigation on Windows.

## What does this PR do? This PR fixes the `FileWatcher` of `filestream` input. Now a file is considered new even if the scanner has already found it in the previous iteration and the underlying file is different. In the PR the file comparator function is passed as a parameter to make unit testing easier. ## Why is it important? The problem is if an input file is renamed and a new file shows up, Filebeat did not register is as a new file. The new file was either considered updated. Or if the new file was smaller than the previous file, the file was deemed truncated and the complete contents of the previous file was reread from the beginning. (cherry picked from commit 54997ac)

This PR fixes the `FileWatcher` of `filestream` input. Now a file is considered new even if the scanner has already found it in the previous iteration and the underlying file is different. In the PR the file comparator function is passed as a parameter to make unit testing easier. The problem is if an input file is renamed and a new file shows up, Filebeat did not register is as a new file. The new file was either considered updated. Or if the new file was smaller than the previous file, the file was deemed truncated and the complete contents of the previous file was reread from the beginning. (cherry picked from commit 54997ac)

## What does this PR do? This PR fixes the `FileWatcher` of `filestream` input. Now a file is considered new even if the scanner has already found it in the previous iteration and the underlying file is different. In the PR the file comparator function is passed as a parameter to make unit testing easier. ## Why is it important? The problem is if an input file is renamed and a new file shows up, Filebeat did not register is as a new file. The new file was either considered updated. Or if the new file was smaller than the previous file, the file was deemed truncated and the complete contents of the previous file was reread from the beginning. (cherry picked from commit 54997ac) Co-authored-by: Noémi Ványi <[email protected]>

This PR fixes the `FileWatcher` of `filestream` input. Now a file is considered new even if the scanner has already found it in the previous iteration and the underlying file is different. In the PR the file comparator function is passed as a parameter to make unit testing easier. The problem is if an input file is renamed and a new file shows up, Filebeat did not register is as a new file. The new file was either considered updated. Or if the new file was smaller than the previous file, the file was deemed truncated and the complete contents of the previous file was reread from the beginning. (cherry picked from commit 54997ac)

This PR fixes the `FileWatcher` of `filestream` input. Now a file is considered new even if the scanner has already found it in the previous iteration and the underlying file is different. In the PR the file comparator function is passed as a parameter to make unit testing easier. The problem is if an input file is renamed and a new file shows up, Filebeat did not register is as a new file. The new file was either considered updated. Or if the new file was smaller than the previous file, the file was deemed truncated and the complete contents of the previous file was reread from the beginning. (cherry picked from commit 54997ac) Co-authored-by: Noémi Ványi <[email protected]>

…er-tar-gz * upstream/main: (139 commits) [Automation] Update elastic stack version to 8.3.0-c655cda8 for testing (elastic#31322) Define a queue metrics reporter interface (elastic#31289) [Oracle Module] Change tablespace metricset collection period (elastic#31259) libbeat/reader/syslog: relax timestamp parsing to allow leading zero (elastic#31254) [Automation] Update elastic stack version to 8.3.0-55ba6f37 for testing (elastic#31311) [libbeat] Remove unused fields and functions in the memory queue (elastic#31302) [libbeat] Cleaning up some unneeded helper types (elastic#31290) Readme for kibana module (elastic#31276) [Automation] Update elastic stack version to 8.3.0-4be61f32 for testing (elastic#31296) x-pack/winlogbeat/module/routing/ingest: fix typo for channel name (elastic#31291) Small pipeline cleanup removing some unused data fields (elastic#31288) removing info log (elastic#30971) Simplify TLS config deserialization (elastic#31168) Detect new files under known paths in filestream input (elastic#31268) Add support for port mapping in docker hints (elastic#31243) Update qa-labels.yml (elastic#31260) libbeat: log debug for `proxy_url` and fixed docs (elastic#31130) [heartbeat][docs] Add note about ensuring correct index settings for uptime (elastic#31146) [Automation] Update elastic stack version to 8.3.0-2c8f9574 for testing (elastic#31256) [Filebeat] fix m365_defender pipeline bug (elastic#31227) ...

## What does this PR do? This PR fixes the `FileWatcher` of `filestream` input. Now a file is considered new even if the scanner has already found it in the previous iteration and the underlying file is different. In the PR the file comparator function is passed as a parameter to make unit testing easier. ## Why is it important? The problem is if an input file is renamed and a new file shows up, Filebeat did not register is as a new file. The new file was either considered updated. Or if the new file was smaller than the previous file, the file was deemed truncated and the complete contents of the previous file was reread from the beginning.

fix

41807b6

kvch requested a review from a team as a code owner April 12, 2022 13:38

botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Apr 12, 2022

kvch added the Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team label Apr 12, 2022

botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Apr 12, 2022

kvch added the bug label Apr 12, 2022

mergify bot assigned kvch Apr 12, 2022

kvch added backport-v8.1.0 Automated backport with mergify backport-v8.2.0 Automated backport with mergify backport-7.17 Automated backport to the 7.17 branch with mergify labels Apr 12, 2022

add changelog entry

6a1ef70

kvch requested a review from belimawr April 12, 2022 13:43

kvch added 3 commits April 12, 2022 15:43

Merge remote-tracking branch 'upstream/main' into fix-filebeat-filest…

60fefdc

…ream-new-file-under-existing-path

fix changelog

6e687e5

really fix changelog

e1e33ec

kvch added 2 commits April 12, 2022 15:55

fix issues reported by linter

9fcef6f

add missing function

4fa1460

belimawr requested changes Apr 12, 2022

View reviewed changes

filebeat/input/filestream/fswatch_test.go Outdated Show resolved Hide resolved

filebeat/input/filestream/fswatch_test.go Outdated Show resolved Hide resolved

filebeat/input/filestream/fswatch_test.go Outdated Show resolved Hide resolved

kvch and others added 3 commits April 12, 2022 17:21

Update filebeat/input/filestream/fswatch_test.go

6cfba93

Co-authored-by: Tiago Queiroz <[email protected]>

address review notes && add extra assertion

d297c4f

speed up test

6ed2a88

belimawr approved these changes Apr 12, 2022

View reviewed changes

move test to non windwos, we are not making the problem worse

d4903ab

kvch merged commit 54997ac into elastic:main Apr 13, 2022

mergify bot mentioned this pull request Apr 13, 2022

[7.17](backport #31268) Detect new files under known paths in filestream input #31275

Merged

mergify bot mentioned this pull request Apr 13, 2022

[8.1](backport #31268) Detect new files under known paths in filestream input #31277

Merged

mergify bot mentioned this pull request Apr 13, 2022

[8.2](backport #31268) Detect new files under known paths in filestream input #31278

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Detect new files under known paths in filestream input #31268

Detect new files under known paths in filestream input #31268

kvch commented Apr 12, 2022 •

edited

Loading

elasticmachine commented Apr 12, 2022

mergify bot commented Apr 12, 2022

elasticmachine commented Apr 12, 2022 •

edited by jenkins-beats-ci bot

Loading

Build stats

Test stats 🧪

belimawr left a comment

belimawr left a comment

kvch commented Apr 12, 2022

Detect new files under known paths in filestream input #31268

Detect new files under known paths in filestream input #31268

Conversation

kvch commented Apr 12, 2022 • edited Loading

What does this PR do?

Why is it important?

Checklist

elasticmachine commented Apr 12, 2022

mergify bot commented Apr 12, 2022

elasticmachine commented Apr 12, 2022 • edited by jenkins-beats-ci bot Loading

💚 Build Succeeded

Build stats

Test stats 🧪

💚 Flaky test report

🤖 GitHub comments

belimawr left a comment

Choose a reason for hiding this comment

belimawr left a comment

Choose a reason for hiding this comment

kvch commented Apr 12, 2022

kvch commented Apr 12, 2022 •

edited

Loading

elasticmachine commented Apr 12, 2022 •

edited by jenkins-beats-ci bot

Loading