Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Finish porting all tests from Python to Golang from test_harvester.py #24397

Closed

Conversation

kvch
Copy link
Contributor

@kvch kvch commented Mar 5, 2021

What does this PR do?

This PR finishes porting tests from test_harvester.py. It is based on #24250.

Why is it important?

Two problems were discovered while porting tests:

  1. lack of support of trucated files

    The support for stopping reading from truncated files was already implemented. However, filestream input could not start reading it from the beginning.

    A new file system event is added called OpTruncate. When the size of a file has shrinked compared to the last time the scanner has encountered it, an OpTruncate event is emitted. When the prospector gets this event, the HarvesterGroup is restarting the Harvester of the file. Restarting basically means that the new Harvester waits for the previous one to finish and then starts reading the file from the beginning. The wait has a predefined timeout which is not the best solution, but I did not want to introduce waiting indefinitely until the context is cancelled for a Harvester. Although, this might be the right approach...

  2. encoding configuration

    Encoding configuration could not be parsed from the configuration. After fixing this, there was not any new problems because most of the underlying code is already covered by unit tests.

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.

@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Mar 5, 2021
@kvch kvch added the Team:Elastic-Agent Label for the Agent team label Mar 5, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/agent (Team:Agent)

@kvch kvch requested a review from urso March 5, 2021 16:18
@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Mar 5, 2021
@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: Pull request #24397 opened

  • Start Time: 2021-03-05T16:18:13.219+0000

  • Duration: 128 min 21 sec

  • Commit: e18e32f

Test stats 🧪

Test Results
Failed 0
Passed 13074
Skipped 2215
Total 15289

Trends 🧪

Image of Build Times

Image of Tests

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test Results
Failed 0
Passed 13074
Skipped 2215
Total 15289

@urso
Copy link

urso commented Mar 5, 2021

A new file system event is added called OpTruncate. When the size of a file has shrinked compared to the last time the scanner has encountered it, an OpTruncate event is emitted. When the prospector gets this event, the HarvesterGroup is restarting the Harvester of the file.

Nice. Would it be possible to split the PR into 2 PRs to speed up review and ease discussions? The first PR only adds the new tests that run through. And the second PR with the fix + tests for the 'truncate' problem.

@kvch kvch closed this Mar 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Elastic-Agent Label for the Agent team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants