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 namespace tag to kubernetes.pods.running #2931

Merged
merged 2 commits into from Nov 2, 2016

Conversation

hkaj
Copy link
Member

@hkaj hkaj commented Oct 19, 2016

Note: Please remember to review the Datadog Contribution Guidelines
if you have not yet done so.

What does this PR do?

Add the kube_namespace tag to kubernetes.pods.running.

Motivation

User request, it makes aggregation easier and more useful.

Testing Guidelines

Relevant tests have been update.

Additional Notes

We should update the kube_replication_controller tag name (must be done carefully as it will break things for users relying on it). It used to be accurate but now pods can be created by daemon sets, jobs, deployments, and replica sets too.

@@ -326,22 +326,25 @@ def _update_pods_metrics(self, instance, pods):
"ReplicaSet",
]

controllers_map = defaultdict(int)
controllers_map = defaultdict(lambda: {'count': 0, 'namespace': None})
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a weird datastructure you use.

You should keep a defaultdict(int) (or a collections.Counter) where the keys would be a tuple (namespace, controller)

Copy link
Contributor

@remh remh left a comment

Choose a reason for hiding this comment

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

Looks good beside the small comment about the data structure being used

@hkaj hkaj force-pushed the haissam/k8s-add-namespace-tag branch from b004906 to 8ee6968 Compare November 2, 2016 10:37
@remh
Copy link
Contributor

remh commented Nov 2, 2016

👍

@remh remh merged commit 60a708b into master Nov 2, 2016
@olivielpeau olivielpeau deleted the haissam/k8s-add-namespace-tag branch November 2, 2016 22:05
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.

None yet

3 participants