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] add kubernetes.pods.running metric #2277

Merged
merged 1 commit into from
Mar 3, 2016

Conversation

masci
Copy link
Contributor

@masci masci commented Feb 22, 2016

Using the kubelet api, query the number of pods and group them by the Replication Controller that spawned them.
Report the number of pods spawned by each RC, tagging with kube_replication_controller and node_name thus allowing sums.

@@ -5,6 +5,8 @@
import numbers
from fnmatch import fnmatch
import re
import json
Copy link
Member

Choose a reason for hiding this comment

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

we usually work with simplejson instead:
import simplejson as json

@hkaj
Copy link
Member

hkaj commented Feb 25, 2016

Great job @masci 👏
@remh you might want to have a quick look too.

@masci masci changed the title Add kubernetes.pods.running metric [kubernetes] add kubernetes.pods.running metric Feb 26, 2016
@masci masci force-pushed the massi/pods_running_metric branch from bc90562 to 6406350 Compare February 26, 2016 13:10
@remh
Copy link

remh commented Feb 26, 2016

Great first PR @masci
Looks like you are querying the same endpoint as the one used to collect the labels https://github.com/DataDog/dd-agent/blob/massi/pods_running_metric/utils/kubeutil.py#L42

Could we refactor that so that we don't query it twice for the same data ?

# at the moment kubelet api reports data only for the current node,
# we expect exactly one item in the set.
for node_name in set(pods):
_tags.append('node_name:{0}'.format(node_name))
Copy link

Choose a reason for hiding this comment

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

There is no need to tag by node_name. As you mentioned, the kubelet will just return the data for the current node, and we already have the host name passed automatically so you don't have to add this tag.

@olivielpeau olivielpeau added this to the 5.8.0 milestone Feb 29, 2016
…hosts

added tests and fixtures for pods.running metric

fixed typo

cleaned up code

python2.6 compat fix

added checks in test_historate

fixed reviewed items

allow caller to avoid strings escaping while reading fixture files

refactoring: call kubelet api only once for labels and running pods

remove node_name tag, we already have host tag carrying same info
@hkaj
Copy link
Member

hkaj commented Mar 3, 2016

nice optimization, you can squash your commits and 🚢 it. It would also be cool to rebase it with master to make sure the tests are passing.

@masci masci force-pushed the massi/pods_running_metric branch from 600a389 to c37b244 Compare March 3, 2016 13:56
@masci masci merged commit c37b244 into master Mar 3, 2016
@masci masci deleted the massi/pods_running_metric branch March 3, 2016 14:40
@masci masci restored the massi/pods_running_metric branch March 8, 2016 17:01
@remh remh deleted the massi/pods_running_metric branch April 18, 2016 17:38
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