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

[marathon] Count running/pending deployments #2788

Merged
merged 7 commits into from
Oct 25, 2016
Merged

Conversation

bradhe
Copy link
Contributor

@bradhe bradhe commented Aug 25, 2016

What does this PR do?

Adds a new metric for to Marathon checks to count the number of deployments that are pending.

Motivation

Basically, we'd like to track when that number stays high (e.g. > 1) so that we can alert on stuck deployments.

Testing Guidelines

An overview on testing
is available in our contribution guidelines.

Added some tests, there was no real coverage for this guy.

Additional Notes

I added a new helper method for loading a JSON fixture. I'm happy to pull that out, though, as it feels like it's extending this a little much.

tags = ["url:{0}".format(url)]
)

return r.json()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Converted spaces to tabs here for consistency.

@@ -56,6 +56,10 @@ def check(self, instance):
if attr in app:
self.gauge('marathon.' + attr, app[attr], tags=tags)

response = self.get_json(urljoin(url, "v2/deployments"), timeout, auth)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think we can get this behind a config flag? I imagine this will slow down the check a bit, being not a Marathon expert was wondering if this is something any Marathon user would trade for the new metric or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm cool with tossing this behind a config flag. I'll push up a commit for that momentarily.

@bradhe
Copy link
Contributor Author

bradhe commented Aug 26, 2016

@masci I've pushed some commits that address your feedback.

@bradhe
Copy link
Contributor Author

bradhe commented Sep 1, 2016

It'd be really great to get this merged so that we can start monitoring this! I'd like to avoid having to start sending my own metrics via an external script :(

@masci
Copy link
Contributor

masci commented Sep 2, 2016

@bradhe I'll push this up on our queue but at the time you submitted the PR the agent was already in feature freeze for 5.9, let me see what we can do.
Thanks for your patience!

@bradhe
Copy link
Contributor Author

bradhe commented Sep 2, 2016

Thanks @masci, no problem-o.

@masci masci modified the milestones: 5.10.0, Triage Sep 6, 2016
@masci masci merged commit 4a1fca0 into DataDog:master Oct 25, 2016
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.

2 participants