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 etcd support with self and store metrics. #1235

Merged
merged 11 commits into from
Dec 26, 2014
Merged

Add etcd support with self and store metrics. #1235

merged 11 commits into from
Dec 26, 2014

Conversation

gphat
Copy link
Contributor

@gphat gphat commented Dec 7, 2014

What this patch does

This patch adds a check for etcd. It fetches metrics from etcd's /v2/metrics/self and /v2/metrics/store. I ripped most of the code form the existing Mesos check.

Motivation

We use etcd in production and would love to have Datadog visibility into it's work. In addition to the various gauges and rates it also turns the 'leader: true` value in to a 1 or 0 and creates a metric. This way we can watch for leadership changes and split brains.

Testing

I'm running this check on a staging machine and seeing metrics!

screenshot 2014-12-07 11 18 04

@LeoCavaille
Copy link
Member

Hi @gphat, thanks a lot for your contribution!
We'll review this and let's aim at the 5.2 release for this new check! 😄

@gphat
Copy link
Contributor Author

gphat commented Dec 7, 2014

Excellent. Thanks @LeoCavaille!

default_timeout = self.init_config.get('default_timeout', 5)
timeout = float(instance.get('timeout', default_timeout))

tags = instance_tags
Copy link
Member

Choose a reason for hiding this comment

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

This new variable assignment could probably be avoided.

@gphat
Copy link
Contributor Author

gphat commented Dec 11, 2014

I haven't tested this yet. I'll do that tomorrow! Tried to address each of your comments. Thanks!

@gphat
Copy link
Contributor Author

gphat commented Dec 11, 2014

Okey doke! I believe this has everything we've discussed so far addressed. I have yet to test it, but I'll do that tomorrow. Today was all meetings. :P

@gphat
Copy link
Contributor Author

gphat commented Dec 19, 2014

Ok, this is working and tested locally. I'm getting metrics and everything looks good! Any more feedback @LeoCavaille?

# stdlib
import time
from hashlib import md5
import urllib2
Copy link
Member

Choose a reason for hiding this comment

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

time, urllib2 and md5 do not seem to be used

@LeoCavaille
Copy link
Member

@gphat looks really good. Thanks a lot for the hard work. I made a last round of comments, mainly inspired by running locally pylint and pep8 on the check. After that it's a go 👍

@gphat
Copy link
Contributor Author

gphat commented Dec 23, 2014

Thank you for taking the time to come back and help me improve this each time. Like I said, I totally jacked this from the Mesos check. :)

I've made the improvements but I will verify things work tomorrow, since I might've left in runtime problems by editing this on my home computer and not verifying. 😄

@gphat
Copy link
Contributor Author

gphat commented Dec 23, 2014

Okay @LeoCavaille! Things work locally and your great feedback has been rolled in. I have only one question which is if we want to somehow escape the URL tag?

screenshot 2014-12-23 08 42 25

@LeoCavaille
Copy link
Member

@gphat the tag can be an arbitrary string, so it shouldn't cause any trouble. Do you think of something more sensible to tag instances with? One last thing (maybe because of my comment..) but your service checks should report like

service_check_tags = ["url:{}".format(url)]
self.service_check('first', ..., tags=service_check_tags)
self.service_check('second', ..., tags=service_check_tags)

the expected named parameter is tags

@gphat
Copy link
Contributor Author

gphat commented Dec 24, 2014

Derp. Fixed!

Nah, the URL is fine. I just wasn't sure about escaping of tags and how concerned you were about that.

@LeoCavaille
Copy link
Member

🎅

LeoCavaille added a commit that referenced this pull request Dec 26, 2014
Add etcd support with self and store metrics.
@LeoCavaille LeoCavaille merged commit 944f73c into DataDog:master Dec 26, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants