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

Harvester Cleanup and FileEvent #2090

Merged
merged 1 commit into from
Jul 26, 2016
Merged

Conversation

ruflin
Copy link
Contributor

@ruflin ruflin commented Jul 25, 2016

  • Rename FileEvent to Event
  • Remove double information between FileEvent and State (Source, Fileinfo)
  • Remove readLine function as not needed anymore

@ruflin ruflin added in progress Pull request is currently in progress. Filebeat Filebeat labels Jul 25, 2016
@ruflin ruflin force-pushed the cleanup-reader-start branch 2 times, most recently from f393b1e to 190db27 Compare July 25, 2016 15:46
* Rename FileEvent to Event
* Remove double information between FileEvent and State (Source, Fileinfo)
* Remove readLine function as not needed anymore
@@ -63,8 +64,7 @@ func (h *Harvester) Harvest() {
default:
}

// Partial lines return error and are only read on completion
ts, text, bytesRead, jsonFields, err := readLine(r)
message, err := r.Next()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@urso The behaviour here should be identical to before, but hopefully better readable. An event is still created if Byte == 0 but this is filtered out as a state update only by the publisher. I assume this happens when an event only consists of a newline. So the state will be updated no event will be sent to the output. Alternatively we could also check inside shouldExportLine for bytes == 0.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

normally new lines characters are added to byte count, so to have correct offset.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for using struct over 4 variables

@ruflin ruflin force-pushed the cleanup-reader-start branch from 190db27 to ac4a155 Compare July 25, 2016 15:51
@ruflin ruflin changed the title Cleanup reader start Harvester Cleanup and FileEvent Jul 25, 2016
@ruflin ruflin added review and removed in progress Pull request is currently in progress. labels Jul 25, 2016
@urso
Copy link

urso commented Jul 25, 2016

LGTM. waiting for travis.

@tsg tsg merged commit 1ce32d2 into elastic:master Jul 26, 2016
@ruflin ruflin mentioned this pull request Jul 26, 2016
ruflin added a commit to ruflin/beats that referenced this pull request Jul 26, 2016
Empty lines should not be sent as an event. To make sure this does not happen a test was added.

This was probably broken in elastic#2090
tsg pushed a commit that referenced this pull request Jul 26, 2016
Empty lines should not be sent as an event. To make sure this does not happen a test was added.

This was probably broken in #2090
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants