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

[kubernetes_state] - Add support for StatefulSet metrics #561

Merged
merged 6 commits into from
Jul 18, 2017

Conversation

bksteiny
Copy link
Contributor

What does this PR do?

This adds support for StatefulSet metrics, which are now exposed in kube-state-metrics (kubernetes/kube-state-metrics#151). The changes in this PR are heavily based off of #492

Motivation

As mentioned previously, these metrics are now exposed via kube-state-metrics. A kube-state-metrics release for this functionality hasn't been cut yet, but if you build the project yourself, you're able to take advantage of them.

Testing Guidelines

An overview on testing
is available in our contribution guidelines.

Before change

    kubernetes_state
    -------------------------
      - instance #0 [OK]
      - Collected 7667 metrics, 0 events & 368 service checks

After change

    kubernetes_state
    -------------------------
      - instance #0 [OK]
      - Collected 7676 metrics, 0 events & 434 service checks

Versioning

  • Bumped the version check in manifest.json
  • Updated CHANGELOG.md

Additional Notes

Anything else we should know when reviewing?

I'm a little confused about the manifest version and CHANGELOG.md version changes in #492, so if what I did isn't correct, please let me know and I'll update it

@bits-bot
Copy link
Collaborator

@bksteiny, thanks for your PR! By analyzing the history of the files in this pull request, we identified @xvello, @hkaj and @gmmeyer to be potential reviewers.

@xvello xvello self-assigned this Jul 12, 2017
@bksteiny
Copy link
Contributor Author

Hey @xvello- It looks like the build failure on this occurs with the Prometheus protobuf.bin file, but I'm a little lost on how/if that can be updated to include statefulset metrics. If you have any info you can share about it, I can take a look.

@xvello xvello added this to the 5.16 milestone Jul 13, 2017
@xvello
Copy link
Contributor

xvello commented Jul 13, 2017

Hi @bksteiny,

Thanks for your contribution! As we are currently finishing up the 5.15 release, I will have a deeper look at your PR next week, although it looks good to me on a first scan.
As the code for extracting all metrics listed in the metrics_mapper array is common, we will not update the test payload to include these two new metrics. You should just revert your changes to test_kubernetes_state.py and the test should go back to green.

We do have a todo card for improving the testing of the PrometheusCheck classes, by programmatically generating payloads instead of relying on captures that are hard to update when adding new metrics.

Copy link
Contributor

@xvello xvello left a comment

Choose a reason for hiding this comment

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

Great contribution, even the metadata is updated. That's a merge!

Thanks

@xvello xvello merged commit 943370a into DataDog:master Jul 18, 2017
@bksteiny bksteiny deleted the statefulset-metrics branch July 18, 2017 12:32
@bksteiny bksteiny restored the statefulset-metrics branch July 24, 2017 13:41
@truthbk truthbk modified the milestones: 5.16, 5.17 Jul 25, 2017
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.

5 participants