-
Notifications
You must be signed in to change notification settings - Fork 14
updated emit_cadvisor_metrics #103
Conversation
| namespace = grep_using_regex(metric, /namespace="([^"]*)"/).to_s | ||
| metric_labels = { 'pod_name' => pod_name, 'image' => image_name, 'namespace' => namespace, 'value' => metric_val, 'node' => @node_name } | ||
| if metric =~ /^((?!container_name="POD").)*$/ | ||
| if container_name=="POD" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regex /^((?!container_name="POD").)*$/ wan't working as expected. So made this change.
| end | ||
|
|
||
| # Make sure regex has only one capturing group | ||
| def grep_using_regex(metric, regex) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved field parsing to this function to improve readability
| container_name = container_name.split('"')[1] | ||
| container_label = { 'container_name' => container_name } | ||
| metric_labels.merge(container_label) | ||
| metric_labels.merge!(container_label) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
merge method will return a new object. So changed it to merge! which will update the object itself
| setup do | ||
| stub_k8s_requests | ||
|
|
||
| return unless @@hash_map_test.empty? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test setup was called on every testcase execution and took too much time executing unit test
rockb1017
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Fixing #87
In kubernetes 1.17+, pod_name and container_name metric labels were renamed to pod and container in cAdvisor api response