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.

Sign up for GitHub

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

Jump to bottom

Fix docker json message.Bytes when partial docker logs are joined #8177

Merged
merged 1 commit into from
Aug 31, 2018
Merged

Fix docker json message.Bytes when partial docker logs are joined #8177

merged 1 commit into from
Aug 31, 2018

Conversation

wflyer
Copy link
Contributor

@wflyer wflyer commented Aug 31, 2018

In the current implementation of partial docker log join, message.Bytes of the concatenated log is not updated, so the harvester gets incorrect offset of the docker log file.

This change updates message.Bytes to count all log bytes that are joined.

Also, updated docker_json_test test code to check expected message.Bytes values. Hard-coding each byte size looks messy, but I didn't make a better approach on this. (feedback welcomed)

Fixes #8175

@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@exekias exekias added bug review Filebeat Filebeat needs_backport PR is waiting to be backported to other branches. labels Aug 31, 2018
@exekias
Copy link
Contributor

exekias commented Aug 31, 2018

Thank you so much for opening this @wflyer! 🌮 🎉 I left a minor comment

jenkins, test this please

@exekias
Copy link
Contributor

exekias commented Aug 31, 2018

Also, could you please add a changelog entry to CHANGELOG.asciidoc?

exekias
exekias approved these changes Aug 31, 2018
View reviewed changes
Copy link
Contributor

@exekias exekias left a comment

Choose a reason for hiding this comment

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

Waiting for green

@exekias
Copy link
Contributor

exekias commented Aug 31, 2018

jenkins, test this please

@wflyer
Copy link
Contributor Author

wflyer commented Aug 31, 2018

@exekias I appreciate your comment! I've updated my commit. Since it is my first PR on a major project, please let me know if there is anything I've missed :)

@exekias
Copy link
Contributor

exekias commented Aug 31, 2018

I would say this is the perfect example of a great contribution, you opened a detailed issue (#8175) and then even provided the fix. Both the PR and the issue have good descriptions, and the fix includes tests 🎉 💪

Keep them coming!

@exekias exekias merged commit aefc948 into elastic:master Aug 31, 2018
exekias pushed a commit to exekias/beats that referenced this pull request Aug 31, 2018
@wflyer
Fix docker json message.Bytes when partial docker logs are joined (el…
bd541db 
…astic#8177)

(cherry picked from commit aefc948)
@exekias exekias mentioned this pull request Aug 31, 2018
Merged
@exekias exekias added v6.4.1 and removed needs_backport PR is waiting to be backported to other branches. labels Aug 31, 2018
exekias pushed a commit to exekias/beats that referenced this pull request Aug 31, 2018
@wflyer
Fix docker json message.Bytes when partial docker logs are joined (el…
53ba96b 
…astic#8177)

(cherry picked from commit aefc948)
@exekias exekias mentioned this pull request Aug 31, 2018
Merged
@exekias exekias added the v6.5.0 label Aug 31, 2018
@wflyer
Copy link
Contributor Author

wflyer commented Aug 31, 2018

@exekias You made my day 😄 Thank you for your message!

@wflyer wflyer deleted the fix-docker-log-offset branch August 31, 2018 13:27
exekias added a commit that referenced this pull request Sep 3, 2018
@exekias
Cherry-pick #8177 to 6.4: Fix docker json message.Bytes when partial …
4592ded 
…docker logs are joined (#8178)

* Fix docker json message.Bytes when partial docker logs are joined (#8177)

(cherry picked from commit aefc948)
exekias added a commit that referenced this pull request Sep 3, 2018
@exekias
Cherry-pick #8177 to 6.x: Fix docker json message.Bytes when partial …
3154262 
…docker logs are joined (#8179)

* Fix docker json message.Bytes when partial docker logs are joined (#8177)

(cherry picked from commit aefc948)
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
@exekias
Cherry-pick elastic#8177 to 6.4: Fix docker json message.Bytes when p…
839dcdc 
…artial docker logs are joined (elastic#8178)

* Fix docker json message.Bytes when partial docker logs are joined (elastic#8177)

(cherry picked from commit e4c69cd)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@exekias exekias exekias approved these changes

Assignees
No one assigned
Labels
bug Filebeat Filebeat review v6.4.1 v6.5.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@wflyer @elasticmachine @exekias