Skip to content

[Metricbeat] Migrate NATS to reporterv2 with error handling#11881

Merged
fearful-symmetry merged 2 commits intoelastic:masterfrom
fearful-symmetry:migration/mb/reporterv2error/nats
Apr 23, 2019
Merged

[Metricbeat] Migrate NATS to reporterv2 with error handling#11881
fearful-symmetry merged 2 commits intoelastic:masterfrom
fearful-symmetry:migration/mb/reporterv2error/nats

Conversation

@fearful-symmetry
Copy link
Copy Markdown
Contributor

@fearful-symmetry fearful-symmetry commented Apr 19, 2019

see #11374

@fearful-symmetry fearful-symmetry added Metricbeat Metricbeat Team:Integrations Label for the Integrations team technical_debt labels Apr 19, 2019
@fearful-symmetry fearful-symmetry requested a review from a team April 19, 2019 18:58
@fearful-symmetry fearful-symmetry self-assigned this Apr 19, 2019
return err
return errors.Wrap(err, "failure applying module schema")
}
r.Event(event)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we add the check here for report.Event? If you want to open a separate PR for that, I guess that's fine too 😄

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think it is needed? this metricset will report a single event on every fetch, so there is no need to check for stop in case of stop

@exekias exekias added the review label Apr 23, 2019
Copy link
Copy Markdown
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.

LGTM, left a minor comment

err = errors.Wrap(err, "failure parsing Nats connections API response")
r.Error(err)
return err
return errors.Wrap(err, "failure parsing Nats connections API response")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I know this was there already, but it should say NATS

return err
return errors.Wrap(err, "failure applying module schema")
}
r.Event(event)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think it is needed? this metricset will report a single event on every fetch, so there is no need to check for stop in case of stop

@fearful-symmetry
Copy link
Copy Markdown
Contributor Author

Alright @exekias I updated that, since it kinda bugged me as well.

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

Labels

Metricbeat Metricbeat review Team:Integrations Label for the Integrations team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants