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

Fixing multiEventFetch error reporting when no events are returned #4153

Merged
merged 1 commit into from
May 1, 2017

Conversation

vjsamuel
Copy link
Contributor

@vjsamuel vjsamuel commented May 1, 2017

When a metricbeat module implements func (m *MetricSet) Fetch() ([]common.MapStr, error) and returns an error, it is not added to the output event because of:

events, err := fetcher.Fetch()
for _, event := range events {
	reporter.ErrorWith(err, event)
}

when events is of size 0 this is never executed and this prevents updating of module metrics like:

  "metricbeat": {
    "prometheus": {
      "collector": {
        "events": 0,
        "failures": 0,
        "success": 0
      }
    }
  },

which can be very misleading.

This PR fixes the same by checking size of the event list and passing a nil event and the error object.

@vjsamuel vjsamuel force-pushed the fix_err_multievents_fetcher branch from 49a22e5 to 2d25ea6 Compare May 1, 2017 04:55
@elasticmachine
Copy link
Collaborator

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run.

1 similar comment
@elasticmachine
Copy link
Collaborator

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run.

@ruflin
Copy link
Member

ruflin commented May 1, 2017

jenkins, test it

Copy link
Member

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

Looks like a regression we introduced when adding the new interfaces. Thanks for fixing.

@ruflin ruflin merged commit d3d1407 into elastic:master May 1, 2017
@vjsamuel vjsamuel deleted the fix_err_multievents_fetcher branch May 2, 2017 15:11
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.

4 participants