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

Minerkasch/yarn #2147

Merged
merged 1 commit into from
Jan 5, 2016
Merged

Minerkasch/yarn #2147

merged 1 commit into from
Jan 5, 2016

Conversation

wjsl
Copy link

@wjsl wjsl commented Dec 15, 2015

PR contains a dd-agent check for YARN clusters.


# Cluster metrics for YARN
YARN_CLUSTER_METRICS = {
'appsSubmitted': ('yarn.metrics.appsSubmitted', GAUGE),
Copy link

Choose a reason for hiding this comment

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

The usual datadog metrics name don't use camelCase usually but camel_case.

@remh
Copy link

remh commented Dec 15, 2015

Thanks that's a pretty nice first version! Nicely written and easy to read!

Can you ping me when the comments made are fixed please ?

Thanks again

@irabinovitch
Copy link
Contributor

Retrigered the ZK and ES. I dont think they failed a result of this PR.


# Get properties from conf file
rm_address = instance.get('resourcemanager_uri', DEFAULT_RM_URI)
self._timeout = int(instance.get('timeout', DEFAULT_TIMEOUT))
Copy link

Choose a reason for hiding this comment

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

You shouldn't store instance level setting at the check level.

@remh
Copy link

remh commented Dec 22, 2015

Thanks a lot @wjsl
This looks almost ready ?
Can you make sure to respect the contributing guidelines please ?

Especially regarding the commit messages. Can you squash your commits into one and also write a more descriptive PR message ?

Thanks again!

@remh remh added this to the 5.7.0 milestone Dec 22, 2015
@zachradtka
Copy link
Contributor

Squashed and updated the commit message. Anything else?

@wjsl
Copy link
Author

wjsl commented Jan 5, 2016

I looked at the TravisCI failure and they seem to be related to ZK, which we shouldn't be touching. Anything on our end we can do to get this PR closed?

@remh remh self-assigned this Jan 5, 2016
@remh
Copy link

remh commented Jan 5, 2016

Thanks @wjsl @zachradtka added one last comment.
Regarding the failing ZK test, can you rebase your branch with master ? That should fix the issue. We'll be able to merge after.

@zachradtka zachradtka force-pushed the minerkasch/yarn branch 2 times, most recently from 11412c3 to d6f7fe3 Compare January 5, 2016 17:59
remh pushed a commit that referenced this pull request Jan 5, 2016
@remh remh merged commit de52ac3 into DataDog:master Jan 5, 2016
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.

4 participants