-
Notifications
You must be signed in to change notification settings - Fork 812
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
Moving MongoDB to checks.d - fixes #387 #397
Conversation
return { | ||
'instances': [{ | ||
'url': agentConfig.get('mongodb_server'), | ||
'tags': agentConfig.get('tags', []) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't want to use the agent tags to tag mongo metrics.
Tags must be an empty list in this case.
Looks good beside the few comments. |
|
||
class MongoDb(AgentCheck): | ||
|
||
GAUGES = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I missed that.
Can you change the metric names to their proper names ?
Currently, these metric names are "translated" in the server side to their proper names mongodb.the_metric
Can you do the translation in the check please ?
data['health'] = current['health'] | ||
|
||
data['state'] = replSet['myState'] | ||
event = self.checkLastState(data['state'], self.agentConfig, serverVersion) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pcockwell You'll have to change how this bit works because events are submitted with self.event
. I believe the dict format is slightly different too. It looks like the state
and version
keys may have been transformed on the server-side but it'd be better to just submit the event with the title/text without having to do any transform on the server side.
👍 |
Moving MongoDB to checks.d - fixes #387
msg = "MongoDB: %s just reported as %s" % (hostname, status) | ||
|
||
self.event({ | ||
'timestamp': int(time.mktime(datetime.now().timetuple())), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably a carryover from the old code, but this will give the wrong time, since it's converting local time assuming it's utc time. It should just be int(time.time())
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will change now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made a fix for this on master - literally changed that line to:
'timestamp': int(time.time()),
Build failed though - failures were unrelated to Mongo (Lighttpd and Apache)
No description provided.