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

Skipping unparsable lines in docker input #12268

Merged
merged 1 commit into from May 27, 2019

Conversation

@marqc
Copy link
Contributor

marqc commented May 24, 2019

fix proposal for problem referenced in https://discuss.elastic.co/t/input-docker-stuck-when-bad-offset-is-saved-in-registry-file-bug/181828 where docker input is unparsable (either input itself is corrupted, or offset in registry is corrupted)

@marqc marqc requested a review from elastic/beats as a code owner May 24, 2019
@elasticmachine

This comment has been minimized.

Copy link
Collaborator

elasticmachine commented May 24, 2019

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?

@kvch

This comment has been minimized.

Copy link
Contributor

kvch commented May 27, 2019

jenkins test this

@@ -276,6 +276,10 @@ func (h *Harvester) Run() error {
logp.Info("End of file reached: %s. Closing because close_eof is enabled.", h.state.Source)
case ErrInactive:
logp.Info("File is inactive: %s. Closing because close_inactive of %v reached.", h.state.Source, h.config.CloseInactive)
case reader.ErrLineUnparsable:
logp.Err("Skipping unparsable line in file: %v", h.state.Source)

This comment has been minimized.

Copy link
@kvch

kvch May 27, 2019

Contributor

Nit: I think this log message should be Info, as we have already emitted an error message from DockerJSON with the error message. To me this is rather a notification telling the user that a line was skipped due to an error before.

This comment has been minimized.

Copy link
@marqc

marqc May 27, 2019

Author Contributor

I changed it to info

This comment has been minimized.

Copy link
@kvch

kvch May 27, 2019

Contributor

I think you have changed the wrong line for the case default.

This comment has been minimized.

Copy link
@marqc

marqc May 27, 2019

Author Contributor

this time, I changed what i was supposed to change ;)

@kvch
kvch approved these changes May 27, 2019
@marqc marqc force-pushed the marqc:docker_input_skip_unparsable_lines branch from cbaafbf to eda4a25 May 27, 2019
libbeat/reader/reader.go Outdated Show resolved Hide resolved
@marqc marqc force-pushed the marqc:docker_input_skip_unparsable_lines branch 2 times, most recently from 7789894 to 5c3ab65 May 27, 2019
@exekias

This comment has been minimized.

Copy link
Member

exekias commented May 27, 2019

Thank you for opening, this is defensive code that should help when this problems arises. Could you add an entry in CHANGELOG.next.asciidoc?

@exekias

This comment has been minimized.

Copy link
Member

exekias commented May 27, 2019

would love to see some unit tests in docker_json_test.go

@marqc marqc force-pushed the marqc:docker_input_skip_unparsable_lines branch from 5c3ab65 to 281f5b9 May 27, 2019
@marqc

This comment has been minimized.

Copy link
Contributor Author

marqc commented May 27, 2019

@exekias changelog entry added, unit tests for docker_json changed to cover error message verification

@exekias

This comment has been minimized.

Copy link
Member

exekias commented May 27, 2019

jenkins, test this please

@exekias exekias merged commit 3451d06 into elastic:master May 27, 2019
4 checks passed
4 checks passed
CLA All commits in pull request signed
Details
Hound No violations found. Woof!
beats-ci Build finished.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@exekias exekias added v7.2.0 and removed needs_backport labels Jun 19, 2019
exekias added a commit to exekias/beats that referenced this pull request Jun 19, 2019
(cherry picked from commit 3451d06)
@adawalli

This comment has been minimized.

Copy link

adawalli commented Jun 20, 2019

Will this be backported to 6.x? Really hitting this with a bunch of our containers

exekias added a commit that referenced this pull request Jul 8, 2019
…12608)

* Skipping unparsable lines in docker input (#12268)

(cherry picked from commit 3451d06)
exekias added a commit to exekias/beats that referenced this pull request Jul 8, 2019
(cherry picked from commit 3451d06)
@exekias exekias added the v6.8.2 label Jul 8, 2019
@exekias

This comment has been minimized.

Copy link
Member

exekias commented Jul 8, 2019

I've opened a backport to 6.8 branch: #12813

exekias added a commit that referenced this pull request Jul 11, 2019
…12813)

* Skipping unparsable lines in docker input (#12268)

(cherry picked from commit 3451d06)
@detro

This comment has been minimized.

Copy link

detro commented Jul 16, 2019

Hello.
I have a question about this fix: why is it tagged with 7.2.0 but it seems like it will be out only in 7.2.1? Was this just a hickup or am I misunderstanding the meaning of the tag?

Regardless, thanks for this: looking forward to update Filebeat and get rid of this issue. We have had long running containers that logged nothing for the last 4 days because of the issue that this code fixes.

@detro

This comment has been minimized.

Copy link

detro commented Jul 22, 2019

Hello. Any update about the release of the 7.2.1 Docker image?
Alternatively, can I ask for a pointer (a doc?) so I can build and package a Docker image with Filebeat 7.2.1?

Thanks in advance :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.