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

Basic RabbitMQ queues metricset #4788

Merged
merged 12 commits into from
Sep 27, 2017
Merged

Basic RabbitMQ queues metricset #4788

merged 12 commits into from
Sep 27, 2017

Conversation

kvalev
Copy link
Contributor

@kvalev kvalev commented Jul 30, 2017

Extend the RabbitMQ metricbeat module with a metricset providing basic queue information. The data is retrieved from the RabbitMQ management API.

Related to #3887

@elasticmachine
Copy link
Collaborator

Can one of the admins verify this patch?

@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically on build-eu-00. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@exekias
Copy link
Contributor

exekias commented Aug 1, 2017

jenkins test it

Copy link
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.

Awesome contribution, thank you!!

I've left a comment, normally we generate example data.json thanks to system tests, I think it would make sense to add some for this metricset? Check https://github.com/elastic/beats/tree/master/metricbeat/tests/system

Also in the past I've seen

"rabbitmq":{
"queues":{
"example": "queues"
}
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like some fields are missing from this example data.json, can you generate a complete one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@kvalev
Copy link
Contributor Author

kvalev commented Aug 18, 2017

Hello, thanks for the feedback. I have an additional question about the naming conventions - currently the metricset is named 'queues' (as it collects information about queues). This results in the following keys - rabbitmq.queues.name, which looks slightly awkward. Is 'queue' is this case a better name for the metricset?

@exekias
Copy link
Contributor

exekias commented Aug 20, 2017

I would say, queue looks better for that case

Total number of persistent messages in the queue (will always be 0 for transient queues).
- name: memory.bytes
type: long
description: >
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add here format: bytes? Like this Kibana will now it's a bytes value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@exekias
Copy link
Contributor

exekias commented Aug 22, 2017

ok to test

@exekias
Copy link
Contributor

exekias commented Aug 22, 2017

LGTM, ready to go, could you add a new entry to CHANGELOG.asciidoc?

@kvalev
Copy link
Contributor Author

kvalev commented Aug 22, 2017

I have updated the changelog and rebased to the latest master.

@exekias
Copy link
Contributor

exekias commented Aug 25, 2017

jenkins, test it please

@exekias
Copy link
Contributor

exekias commented Aug 25, 2017

Sorry @kvalev, it took me a while to come back to this, could you please check the errors in https://beats-ci.elastic.co/job/elastic+beats+pull-request+multijob-intake/1504/console? Basically you need to run make update to update docs with the new added fields

@exekias
Copy link
Contributor

exekias commented Sep 27, 2017

jenkins retest this please

@exekias
Copy link
Contributor

exekias commented Sep 27, 2017

jenkins retest this please

@exekias exekias merged commit 71bfaaa into elastic:master Sep 27, 2017
@kvalev kvalev deleted the metricbeat_rabbitmq branch September 28, 2017 05:04
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