Skip to content

[Monitoring] LocalExporter should catch synchronous exceptions #36606

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

Merged

Conversation

pickypg
Copy link
Member

@pickypg pickypg commented Dec 13, 2018

In the unlikely scenario that LocalExporter::resolveBulk throws an exception, then we should mark the listener as having failed.

As discussed with @martijnvg, this is a follow-up to #35765, which already ensured that this call was async, but did not catch the [unlikely] exception.

A future PR should cleanup the LocalExporter to make it a bit cleaner to test (more unit tests and less integration tests taking 20s+) as well remove the dependence on receiving cluster state changes to initialize the exporter.

Closes #35797

In the unlikely scenario that `LocalExporter::resolveBulk` throws
an exception, then we should mark the `listener` as having failed.
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features

Copy link
Member

@martijnvg martijnvg left a comment

Choose a reason for hiding this comment

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

LGTM - Thanks for addressing this!

@pickypg pickypg merged commit 42d76a7 into elastic:master Dec 13, 2018
@pickypg pickypg deleted the localexporter/catch-exception-and-mark-failure branch December 13, 2018 21:13
pickypg added a commit that referenced this pull request Dec 13, 2018
In the unlikely scenario that `LocalExporter::resolveBulk` throws
an exception, then we should mark the `listener` as having failed.
@pickypg
Copy link
Member Author

pickypg commented Dec 13, 2018

6.x/6.6: 64f8f86

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Monitoring] Make LocalExporter::openBulk Async
4 participants