-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Fix docker json message.Bytes when partial docker logs are joined #8177
Conversation
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? |
Thank you so much for opening this @wflyer! 🌮 🎉 I left a minor comment jenkins, test this please |
Also, could you please add a changelog entry to |
63d5b62
to
bcc6ae8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Waiting for green
jenkins, test this please |
@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 :) |
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! |
…astic#8177) (cherry picked from commit aefc948)
…astic#8177) (cherry picked from commit aefc948)
@exekias You made my day 😄 Thank you for your message! |
…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)
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 expectedmessage.Bytes
values. Hard-coding each byte size looks messy, but I didn't make a better approach on this. (feedback welcomed)Fixes #8175