NH-57036: Fix metrics relation to Nodes on Fargate #381

pstranak-sw · 2023-09-20T08:03:42Z

Don't use 'kubernetes_io_hostname' on Fargate Nodes
Use 'service.instance.id' from Resource attributes instead of DataPoint attributes
Remove nonexistent metrics from mocked data

etichy · 2023-09-21T07:02:13Z

deploy/helm/metrics-collector-config.yaml

+          - set(attributes["k8s.node.name"], attributes["node"]) where IsMatch(metric.name, "^(container_.*)|(kube_node_.*)|(kube_pod_info)|(kube_pod_container_resource_requests)|(kube_pod_container_resource_limits)|(kube_pod_init_container_resource_requests)|(kube_pod_init_container_resource_limits)$") == true and attributes["k8s.node.name"] == nil
+          # "kubernetes_io_hostname", unlike "service.instance.id", provides a nice Node name in environments like local Docker, but for Fargate, its value is different from the other attributes
+          - set(attributes["k8s.node.name"], attributes["kubernetes_io_hostname"]) where IsMatch(metric.name, "^(container_.*)|(kube_node_.*)|(kube_pod_info)|(kube_pod_container_resource_requests)|(kube_pod_container_resource_limits)|(kube_pod_init_container_resource_requests)|(kube_pod_init_container_resource_limits)$") == true and attributes["eks_amazonaws_com_compute_type"] != "fargate" and attributes["k8s.node.name"] == nil
+          # use "service.instance.id" for Node name when the above attributes are not available


Hmm it's weirs aws-otel-collector is taking node name directly from kubernetes_io_hostname for fargate

https://github.com/aws-observability/aws-otel-collector/blob/main/deployment-template/eks/otel-fargate-container-insights.yaml#L103C24-L103C46

After discussion with Peter I do understand this needs.

etichy · 2023-09-21T08:50:52Z

deploy/helm/metrics-collector-config.yaml

+          - set(attributes["k8s.node.name"], attributes["node"]) where IsMatch(metric.name, "^(container_.*)|(kube_node_.*)|(kube_pod_info)|(kube_pod_container_resource_requests)|(kube_pod_container_resource_limits)|(kube_pod_init_container_resource_requests)|(kube_pod_init_container_resource_limits)$") == true and attributes["k8s.node.name"] == nil
+          # "kubernetes_io_hostname", unlike "service.instance.id", provides a nice Node name in environments like local Docker, but for Fargate, its value is different from the other attributes
+          - set(attributes["k8s.node.name"], attributes["kubernetes_io_hostname"]) where IsMatch(metric.name, "^(container_.*)|(kube_node_.*)|(kube_pod_info)|(kube_pod_container_resource_requests)|(kube_pod_container_resource_limits)|(kube_pod_init_container_resource_requests)|(kube_pod_init_container_resource_limits)$") == true and attributes["eks_amazonaws_com_compute_type"] != "fargate" and attributes["k8s.node.name"] == nil
+          # use "service.instance.id" for Node name when the above attributes are not available


After discussion with Peter I do understand this needs.

* Don't use 'kubernetes_io_hostname' on Fargate Nodes * Use 'service.instance.id' from Resource attributes instead of DataPoint attributes * Remove nonexistent metrics from mocked data

pstranak-sw · 2023-10-04T11:30:46Z

build/docker/wiremockFiles/redirectKubeStateMetricsResponse.txt

@@ -199,11 +199,6 @@ kube_pod_container_resource_limits{container="test-container",endpoint="http",in
 kube_pod_container_resource_limits{container="test-container",endpoint="http",instance="test-node",job="test-job",namespace="test-namespace",node="test-node",pod="test-pod",resource="memory",service="test-service",uid="bafeef2c-1292-4a5e-a92c-d709480b04b6",unit="byte",prometheus="prometheus-system/kube-prometheus-kube-prome-prometheus",prometheus_replica="prometheus-kube-prometheus-kube-prome-prometheus-0"} 3.221225472e+09 1675856675021
 kube_pod_container_resource_limits{container="test-container",endpoint="tcp-model",instance="test-node",job="test-job",namespace="test-namespace",node="test-node",pod="test-pod",resource="memory",service="test-service",uid="bafeef2c-1292-4a5e-a92c-d709480b04b6",unit="byte",prometheus="prometheus-system/kube-prometheus-kube-prome-prometheus",prometheus_replica="prometheus-kube-prometheus-kube-prome-prometheus-0"} 3.221225472e+09 1675856675021
 kube_pod_container_resource_limits{container="test-container",endpoint="tcp-model",instance="test-node",job="test-job",namespace="test-namespace",node="test-node",pod="test-pod",resource="cpu",service="test-service",uid="bafeef2c-1292-4a5e-a92c-d709480b04b6",unit="core",prometheus="prometheus-system/kube-prometheus-kube-prome-prometheus",prometheus_replica="prometheus-kube-prometheus-kube-prome-prometheus-0"} 0.1 1675856675021
-# TYPE kube_pod_container_resource_limits_cpu_cores untyped


These metrics existed only in kube-state-metrics v2.0.0-alpha and were removed in v2.0.0-alpha.2. I'm not even sure how they appeared in the mocked data.

…s are more stable

…rgateNodeNames

… reliable now

gantrior · 2023-10-06T15:48:04Z

tests/integration/test_utils.py

@@ -198,17 +211,20 @@ def merge_resources(existing_resource, new_resource):
            existing_scopes.append(new_scope)

 def custom_json_merge(result, new_json):
-    new_resources = {resource_sorting_key(resource): resource for resource in new_json["resourceMetrics"]}


@pstranak-sw This line (and few similar above) was main source of instability. Basically what it did was that it create Map and in case there was multiple resources in the same json with the same key, it did not merge them, but override them completely. So instead of creating Map I now create array of Tuples so not overriding anything

pstranak-sw requested review from a team September 20, 2023 08:03

DavidKana assigned etichy Sep 20, 2023

etichy reviewed Sep 21, 2023

View reviewed changes

etichy previously approved these changes Sep 21, 2023

View reviewed changes

pstranak-sw dismissed etichy’s stale review via 211b535 September 22, 2023 08:31

pstranak-sw force-pushed the feature/NH-57036-fargateNodeNames branch from bc1ba77 to 3ffaffc Compare September 24, 2023 17:52

NH-57036: Fix metrics relation to Nodes on Fargate

42278c2

* Don't use 'kubernetes_io_hostname' on Fargate Nodes * Use 'service.instance.id' from Resource attributes instead of DataPoint attributes * Remove nonexistent metrics from mocked data

pstranak-sw force-pushed the feature/NH-57036-fargateNodeNames branch from 3ffaffc to 42278c2 Compare October 4, 2023 07:35

pstranak-sw commented Oct 4, 2023

View reviewed changes

pstranak-sw and others added 5 commits October 4, 2023 17:24

NH-57036: Avoid using the 'transform' processor when possible

f8ab64b

NH-57036: Using only last 5 jsons out of all so that integration test…

8662b3b

…s are more stable

Merge remote-tracking branch 'origin/master' into feature/NH-57036-fa…

c5d4458

…rgateNodeNames

NH-57036: Updating expected output

fb787d6

NH-57036: I reworked the merge script of integration tests to be more…

cf8222d

… reliable now

gantrior reviewed Oct 6, 2023

View reviewed changes

NH-57036: Minor adjustment

03e66d5

gantrior force-pushed the feature/NH-57036-fargateNodeNames branch from 3f5ed9c to 03e66d5 Compare October 6, 2023 15:51

NH-57036: Further stabilization of integration tests

7cf531a

gantrior approved these changes Oct 9, 2023

View reviewed changes

gantrior merged commit 3616f74 into master Oct 9, 2023

gantrior deleted the feature/NH-57036-fargateNodeNames branch October 9, 2023 10:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NH-57036: Fix metrics relation to Nodes on Fargate #381

NH-57036: Fix metrics relation to Nodes on Fargate #381

pstranak-sw commented Sep 20, 2023 •

edited

Loading

etichy Sep 21, 2023

etichy Sep 21, 2023

etichy Sep 21, 2023

pstranak-sw Oct 4, 2023

gantrior Oct 6, 2023 •

edited

Loading

NH-57036: Fix metrics relation to Nodes on Fargate #381

NH-57036: Fix metrics relation to Nodes on Fargate #381

Conversation

pstranak-sw commented Sep 20, 2023 • edited Loading

etichy Sep 21, 2023

Choose a reason for hiding this comment

etichy Sep 21, 2023

Choose a reason for hiding this comment

etichy Sep 21, 2023

Choose a reason for hiding this comment

pstranak-sw Oct 4, 2023

Choose a reason for hiding this comment

gantrior Oct 6, 2023 • edited Loading

Choose a reason for hiding this comment

pstranak-sw commented Sep 20, 2023 •

edited

Loading

gantrior Oct 6, 2023 •

edited

Loading