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

Add subscriptions metricset of nats module #10159

Merged
merged 4 commits into from
Jan 21, 2019
Merged

Add subscriptions metricset of nats module #10159

merged 4 commits into from
Jan 21, 2019

Conversation

MichaelKatsoulis
Copy link
Contributor

@MichaelKatsoulis MichaelKatsoulis commented Jan 18, 2019

This PR adds subscriptions metricset to nats module as part of #10071.

subscriptions metricset retrieves metrics from subsz monitoring URI of nats.

Co-Authored-By: Christos Markou [email protected]
Co-Authored-By: Stamatis Katsaounis [email protected]

@MichaelKatsoulis MichaelKatsoulis requested review from a team as code owners January 18, 2019 10:14
@elasticmachine
Copy link
Collaborator

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?

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.

Overall code LGTM. To make CI happy you need to run make fmt. I think some headers are missing.

- Add `nats` module with `stats` metricset. {pull}9825[9825]
- Add `connections` metricset to the `nats` module. {pull}10095[10095]
- Add `routes` metricset to the `nats` module. {pull}10095[10095]
- Add `nats` module. {issue}10071[10071]
Copy link
Member

Choose a reason for hiding this comment

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

You can remove it here, it's only needed in the next changelog.

@ruflin
Copy link
Member

ruflin commented Jan 18, 2019

jenkins, test this

@ruflin
Copy link
Member

ruflin commented Jan 18, 2019

This CI failure looks related:

command [go test -tags integration -cover -coverprofile /tmp/gotestcover-379447219 github.com/elastic/beats/metricbeat/module/nats/subscriptions]: exit status 1
--- FAIL: TestData (1.01s)
	modules.go:119: MetricSet does not implement EventFetcher
--- FAIL: TestFetch (1.02s)
	modules.go:119: MetricSet does not implement EventFetcher
FAIL
coverage: 61.3% of statements

@MichaelKatsoulis
Copy link
Contributor Author

cc: @skatsaounis , @ChrsMark

MichaelKatsoulis and others added 4 commits January 21, 2019 10:25
Signed-off-by: Michael Katsoulis <[email protected]>

Co-Authored-By: Christos Markou <[email protected]>
Co-Authored-By: Stamatis Katsaounis <[email protected]>
Signed-off-by: Michael Katsoulis <[email protected]>

Co-Authored-By: Christos Markou <[email protected]>
Co-Authored-By: Stamatis Katsaounis <[email protected]>
Signed-off-by: Michael Katsoulis <[email protected]>

Co-Authored-By: Christos Markou <[email protected]>
Co-Authored-By: Stamatis Katsaounis <[email protected]>
@ruflin
Copy link
Member

ruflin commented Jan 21, 2019

jenkins, test this

@ruflin
Copy link
Member

ruflin commented Jan 21, 2019

I merged the changelog conflict, let's see if CI goes green now.

@ruflin ruflin added module review Metricbeat Metricbeat Team:Integrations Label for the Integrations team labels Jan 21, 2019
@ruflin ruflin merged commit 94e58e9 into elastic:master Jan 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Metricbeat Metricbeat module review Team:Integrations Label for the Integrations team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants