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

cmd/scollector: RabbitMQ #1202

Merged
merged 1 commit into from
Aug 14, 2015
Merged

Conversation

d10v
Copy link
Contributor

@d10v d10v commented Jul 25, 2015

Fairly comprehensive collector for RabbitMQ 3.x Probably needs careful review of metric/tags design.

@d10v d10v force-pushed the collector-rabbitmq branch from ea6fbb8 to 4287c36 Compare July 26, 2015 20:07
@@ -238,6 +238,11 @@ or just one.
[[HTTPUnit]]
TOML = "/some/other.toml"

RabbitMQ (array of table, keys are URL): RabbitMQ hosts to poll.
Copy link
Contributor

Choose a reason for hiding this comment

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

perhaps document that it will check localhost, even with no config given.

@d10v
Copy link
Contributor Author

d10v commented Jul 27, 2015

One of the problems I've encountered is "consumer_utilisation" field. When no one consumes from queue it's: "consumer_utilisation": "", , when there's one it's "consumer_utilisation": 0.9842390041698995, .

@captncraig
Copy link
Contributor

That would be an acceptable use of an interface{} field. Most others look like they could be mapped into structs. Have you seen this tool: http://mholt.github.io/json-to-go/?

@d10v
Copy link
Contributor Author

d10v commented Jul 27, 2015

Yeah, seen that, will do some research on how does RMQ output stats in different versions.

@kylebrandt
Copy link
Member

@dimamedvedev Also the gojson as a local utility is quite nice https://github.com/ChimeraCoder/gojson , you just do curl ... | gojson once you have it installed

@d10v d10v force-pushed the collector-rabbitmq branch 2 times, most recently from 10de452 to c65c98d Compare August 1, 2015 18:51
@d10v
Copy link
Contributor Author

d10v commented Aug 1, 2015

  • Updated PR to use structs
  • Left off a couple of metrics - can't find a way enable them in RabbitMQ
  • Some metrics could be converted to tags

Add(&md, p+"message_bytes_ready", q.MessageBytesReady, ts, metadata.Gauge, metadata.Bytes, DescRmqMessageBytesReady)
Add(&md, p+"message_bytes_unack", q.MessageBytesUnacknowledged, ts, metadata.Gauge, metadata.Bytes, DescRmqMessageBytesUnacknowledged)
Add(&md, p+"messages_queue_depth", q.Messages, ts, metadata.Gauge, metadata.Message, DescRmqMessages)
Add(&md, p+"messages_persistent", q.MessagesPersistent, ts, metadata.Gauge, metadata.Message, DescRmqMessagesPersistent)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These could be tagged, not 100% sure.

@d10v d10v force-pushed the collector-rabbitmq branch 2 times, most recently from 1f6af72 to 91de289 Compare August 8, 2015 20:34
return nil, err
}
var o rmqOverview
if err := json.NewDecoder(bytes.NewReader(content[:])).Decode(&o); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

You don't even need to read the body prior to this. Just use json.NewDecoder(res.Body)

@d10v
Copy link
Contributor Author

d10v commented Aug 13, 2015

@captncraig updated PR, will squash when ready

metadata.Rate, metadata.Message, DescRmqBackingQueueStatusAvgAckIngressRate)
Add(&md, p+"avg_rate", bqs.AvgAckEgressRate, ts.Copy().Merge(opentsdb.TagSet{"direction": "out"}),
metadata.Rate, metadata.Message, DescRmqBackingQueueStatusAvgEgressRate)
Add(&md, p+"avg_rate", bqs.AvgAckEgressRate, ts.Copy().Merge(opentsdb.TagSet{"direction": "in"}),
Copy link
Contributor

Choose a reason for hiding this comment

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

You are adding the same two data points twice, with different tagsets (once with direction and method, once with only direction). This scheme seems like it may cause confusing data conditions. Why Not have just the first two Adds and the last 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.

Oh, my bad, easy to get lost in so many metrics. Used wrong datapoints. Thank you for being thorough.

@d10v d10v force-pushed the collector-rabbitmq branch from ab3f1f0 to 98bf1d1 Compare August 14, 2015 12:57
@d10v d10v force-pushed the collector-rabbitmq branch from 98bf1d1 to f74cdf1 Compare August 14, 2015 14:44
captncraig pushed a commit that referenced this pull request Aug 14, 2015
@captncraig captncraig merged commit 9723be4 into bosun-monitor:master Aug 14, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants