From bdc1c259c81f8cd56199e537ad1066e4f935e9dc Mon Sep 17 00:00:00 2001 From: Keval Bhogayata <90185475+bhogayatakb@users.noreply.github.com> Date: Thu, 27 Jun 2024 18:25:45 +0530 Subject: [PATCH] Fix initiated (#81) Co-authored-by: Hardik Choksi --- .../internal/kubelet/accumulator.go | 1 - .../internal/kubelet/accumulator_test.go | 6 ++-- .../internal/kubelet/cpu.go | 11 ++++-- .../internal/kubelet/metadata.go | 4 +-- .../internal/kubelet/metadata_provider.go | 13 +++++++ .../kubelet/metadata_provider_test.go | 4 +++ .../internal/kubelet/metadata_test.go | 34 +++++++++---------- .../internal/kubelet/metrics_test.go | 7 +++- .../internal/kubelet/rest_client.go | 5 +++ .../internal/kubelet/volume_test.go | 22 ++++++------ receiver/kubeletstatsreceiver/scraper_test.go | 4 +++ .../kubeletstatsreceiver/testdata/nodes.json | 0 12 files changed, 74 insertions(+), 37 deletions(-) create mode 100644 receiver/kubeletstatsreceiver/testdata/nodes.json diff --git a/receiver/kubeletstatsreceiver/internal/kubelet/accumulator.go b/receiver/kubeletstatsreceiver/internal/kubelet/accumulator.go index 2ee7589c2e12..81bbfb3ca2b1 100644 --- a/receiver/kubeletstatsreceiver/internal/kubelet/accumulator.go +++ b/receiver/kubeletstatsreceiver/internal/kubelet/accumulator.go @@ -172,7 +172,6 @@ func (a *metricDataAccumulator) containerStats(sPod stats.PodStats, s stats.Cont resourceKey := sPod.PodRef.UID + s.Name currentTime := pcommon.NewTimestampFromTime(a.time) - resourceKey := sPod.PodRef.UID + s.Name addUptimeMetric(a.mbs.ContainerMetricsBuilder, metadata.ContainerUptimeMetrics.Uptime, s.StartTime, currentTime) addCPUMetrics(a.mbs.ContainerMetricsBuilder, metadata.ContainerCPUMetrics, s.CPU, currentTime, a.metadata.containerResources[resourceKey], a.metadata.nodeCapacity.CPUCapacity) addMemoryMetrics(a.mbs.ContainerMetricsBuilder, metadata.ContainerMemoryMetrics, s.Memory, currentTime, a.metadata.containerResources[resourceKey], a.metadata.nodeCapacity.MemoryCapacity) diff --git a/receiver/kubeletstatsreceiver/internal/kubelet/accumulator_test.go b/receiver/kubeletstatsreceiver/internal/kubelet/accumulator_test.go index 6b9ca8b48627..87068b178389 100644 --- a/receiver/kubeletstatsreceiver/internal/kubelet/accumulator_test.go +++ b/receiver/kubeletstatsreceiver/internal/kubelet/accumulator_test.go @@ -275,7 +275,7 @@ func TestGetServiceName(t *testing.T) { acc := metricDataAccumulator{ metadata: NewMetadata([]MetadataLabel{MetadataLabelContainerID}, &v1.PodList{ Items: pods, - }, nil, nil), + }, &v1.NodeList{Items: []v1.Node{}}, NodeLimits{}, nil), } // Create a Service with the same labels as the Pod @@ -324,7 +324,7 @@ func TestGetServiceAccountName(t *testing.T) { acc := metricDataAccumulator{ metadata: NewMetadata([]MetadataLabel{MetadataLabelContainerID}, &v1.PodList{ Items: pods, - }, nil, nil), + }, &v1.NodeList{Items: []v1.Node{}}, NodeLimits{}, nil), } // Call the getServiceName method @@ -358,7 +358,7 @@ func TestGetJobInfo(t *testing.T) { acc := metricDataAccumulator{ metadata: NewMetadata([]MetadataLabel{MetadataLabelContainerID}, &v1.PodList{ Items: pods, - }, nil, nil), + }, &v1.NodeList{Items: []v1.Node{}}, NodeLimits{}, nil), } // Create a Job with the same labels as the Pod diff --git a/receiver/kubeletstatsreceiver/internal/kubelet/cpu.go b/receiver/kubeletstatsreceiver/internal/kubelet/cpu.go index f3d2360a1efe..b458152e81c9 100644 --- a/receiver/kubeletstatsreceiver/internal/kubelet/cpu.go +++ b/receiver/kubeletstatsreceiver/internal/kubelet/cpu.go @@ -25,15 +25,20 @@ func addCPUMetrics( usageCores := float64(*s.UsageNanoCores) / 1_000_000_000 cpuMetrics.Usage(mb, currentTime, usageCores) addCPUUtilizationMetrics(mb, cpuMetrics, usageCores, currentTime, r, nodeCPULimit) - addCPUUsageMetric(mb, cpuMetrics, s, currentTime, r) + addCPUUsageMetric(mb, cpuMetrics, s, currentTime, r, nodeCPULimit) } addCPUTimeMetric(mb, cpuMetrics.Time, s, currentTime) } -func addCPUUsageMetric(mb *metadata.MetricsBuilder, cpuMetrics metadata.CPUMetrics, s *stats.CPUStats, currentTime pcommon.Timestamp, r resources) { +func addCPUUsageMetric(mb *metadata.MetricsBuilder, cpuMetrics metadata.CPUMetrics, s *stats.CPUStats, currentTime pcommon.Timestamp, r resources, nodeCPULimit float64) { + if s == nil { + return + } + value := float64(*s.UsageNanoCores) / 1_000_000_000 cpuMetrics.Utilization(mb, currentTime, value) - if nodeCPULimit > 0 { + if nodeCPULimit > 0 && s.UsageNanoCores != nil { + usageCores := float64(*s.UsageNanoCores) / 1_000_000_000 cpuMetrics.NodeUtilization(mb, currentTime, usageCores/nodeCPULimit) } if r.cpuLimit > 0 { diff --git a/receiver/kubeletstatsreceiver/internal/kubelet/metadata.go b/receiver/kubeletstatsreceiver/internal/kubelet/metadata.go index 815935354c17..49f304a61c3a 100644 --- a/receiver/kubeletstatsreceiver/internal/kubelet/metadata.go +++ b/receiver/kubeletstatsreceiver/internal/kubelet/metadata.go @@ -56,7 +56,7 @@ type Metadata struct { Labels map[MetadataLabel]bool PodsMetadata *v1.PodList NodesMetadata *v1.NodeList - DetailedPVCResourceSetter func(rb *metadata.ResourceBuilder, volCacheID, volumeClaim, namespace string) error + DetailedPVCResourceSetter func(rb *metadata.ResourceBuilder, volCacheID, volumeClaim, namespace string) ([]metadata.ResourceMetricsOption, error) podResources map[string]resources containerResources map[string]resources nodeCapacity NodeCapacity @@ -90,7 +90,7 @@ func getContainerResources(r *v1.ResourceRequirements) resources { } } -func NewMetadata(labels []MetadataLabel, podsMetadata *v1.PodList, nodeCap NodeCapacity, +func NewMetadata(labels []MetadataLabel, podsMetadata *v1.PodList, nodesMetadata *v1.NodeList, nodeCap NodeCapacity, detailedPVCResourceSetter func(rb *metadata.ResourceBuilder, volCacheID, volumeClaim, namespace string) error, ) Metadata { m := Metadata{ diff --git a/receiver/kubeletstatsreceiver/internal/kubelet/metadata_provider.go b/receiver/kubeletstatsreceiver/internal/kubelet/metadata_provider.go index 98aa35715cb9..3c2dabadb89d 100644 --- a/receiver/kubeletstatsreceiver/internal/kubelet/metadata_provider.go +++ b/receiver/kubeletstatsreceiver/internal/kubelet/metadata_provider.go @@ -32,3 +32,16 @@ func (p *MetadataProvider) Pods() (*v1.PodList, error) { } return &out, nil } + +func (p *MetadataProvider) Nodes() (*v1.NodeList, error) { + nodes, err := p.rc.Nodes() + if err != nil { + return nil, err + } + var out v1.NodeList + err = json.Unmarshal(nodes, &out) + if err != nil { + return nil, err + } + return &out, nil +} diff --git a/receiver/kubeletstatsreceiver/internal/kubelet/metadata_provider_test.go b/receiver/kubeletstatsreceiver/internal/kubelet/metadata_provider_test.go index 303c49c18ec4..410aaf95a24b 100644 --- a/receiver/kubeletstatsreceiver/internal/kubelet/metadata_provider_test.go +++ b/receiver/kubeletstatsreceiver/internal/kubelet/metadata_provider_test.go @@ -32,6 +32,10 @@ func (f testRestClient) Pods() ([]byte, error) { return os.ReadFile("../../testdata/pods.json") } +func (f testRestClient) Nodes() ([]byte, error) { + return []byte{}, nil +} + func TestPods(t *testing.T) { tests := []struct { name string diff --git a/receiver/kubeletstatsreceiver/internal/kubelet/metadata_test.go b/receiver/kubeletstatsreceiver/internal/kubelet/metadata_test.go index fa35e320c2d0..268f19ef98e1 100644 --- a/receiver/kubeletstatsreceiver/internal/kubelet/metadata_test.go +++ b/receiver/kubeletstatsreceiver/internal/kubelet/metadata_test.go @@ -70,7 +70,7 @@ func TestSetExtraLabels(t *testing.T) { }{ { name: "no_labels", - metadata: NewMetadata([]MetadataLabel{}, nil, NodeCapacity{}, nil), + metadata: NewMetadata([]MetadataLabel{}, nil, nil, NodeCapacity{}, nil), args: []string{"uid", "container.id", "container"}, want: map[string]any{}, }, @@ -98,7 +98,7 @@ func TestSetExtraLabels(t *testing.T) { }, }, }, - }, NodeCapacity{}, nil), + }, &v1.NodeList{Items: []v1.Node{}}, NodeCapacity{}, nil), args: []string{"uid-1234", "container.id", "container1"}, want: map[string]any{ string(MetadataLabelContainerID): "test-container", @@ -128,7 +128,7 @@ func TestSetExtraLabels(t *testing.T) { }, }, }, - }, NodeCapacity{}, nil), + }, &v1.NodeList{Items: []v1.Node{}}, NodeCapacity{}, nil), args: []string{"uid-1234", "container.id", "init-container1"}, want: map[string]any{ string(MetadataLabelContainerID): "test-init-container", @@ -136,7 +136,7 @@ func TestSetExtraLabels(t *testing.T) { }, { name: "set_container_id_no_metadata", - metadata: NewMetadata([]MetadataLabel{MetadataLabelContainerID}, nil, NodeCapacity{}, nil), + metadata: NewMetadata([]MetadataLabel{MetadataLabelContainerID}, nil, nil, NodeCapacity{}, nil), args: []string{"uid-1234", "container.id", "container1"}, wantError: "pods metadata were not fetched", }, @@ -158,7 +158,7 @@ func TestSetExtraLabels(t *testing.T) { }, }, }, - }, NodeCapacity{}, nil), + }, &v1.NodeList{Items: []v1.Node{}}, NodeCapacity{}, nil), args: []string{"uid-1234", "container.id", "container1"}, wantError: "pod \"uid-1234\" with container \"container1\" not found in the fetched metadata", }, @@ -180,13 +180,13 @@ func TestSetExtraLabels(t *testing.T) { }, }, }, - }, NodeCapacity{}, nil), + }, &v1.NodeList{Items: []v1.Node{}}, NodeCapacity{}, nil), args: []string{"uid-1234", "container.id", "container1"}, wantError: "pod \"uid-1234\" with container \"container1\" has an empty containerID", }, { name: "set_volume_type_no_metadata", - metadata: NewMetadata([]MetadataLabel{MetadataLabelVolumeType}, nil, NodeCapacity{}, nil), + metadata: NewMetadata([]MetadataLabel{MetadataLabelVolumeType}, nil, nil, NodeCapacity{}, nil), args: []string{"uid-1234", "k8s.volume.type", "volume0"}, wantError: "pods metadata were not fetched", }, @@ -208,7 +208,7 @@ func TestSetExtraLabels(t *testing.T) { }, }, }, - }, NodeCapacity{}, nil), + }, &v1.NodeList{Items: []v1.Node{}}, NodeCapacity{}, nil), args: []string{"uid-1234", "k8s.volume.type", "volume1"}, wantError: "pod \"uid-1234\" with volume \"volume1\" not found in the fetched metadata", }, @@ -376,8 +376,8 @@ func TestSetExtraLabelsForVolumeTypes(t *testing.T) { }, }, }, - }, NodeCapacity{}, func(*metadata.ResourceBuilder, string, string, string) error { - return nil + }, &v1.NodeList{Items: []v1.Node{}}, NodeCapacity{}, func(*metadata.ResourceBuilder, string, string, string) error { + return return []metadata.ResourceMetricsOption{}, nil }) rb := metadata.NewResourceBuilder(metadata.DefaultResourceAttributesConfig()) err := md.setExtraResources(rb, stats.PodReference{UID: tt.args[0]}, MetadataLabel(tt.args[1]), volName) @@ -406,7 +406,7 @@ func TestCpuAndMemoryGetters(t *testing.T) { }{ { name: "no metadata", - metadata: NewMetadata([]MetadataLabel{}, nil, NodeCapacity{}, nil), + metadata: NewMetadata([]MetadataLabel{}, nil, nil, NodeCapacity{}, nil), }, { name: "pod happy path", @@ -448,7 +448,7 @@ func TestCpuAndMemoryGetters(t *testing.T) { }, }, }, - }, NodeCapacity{}, nil), + }, &v1.NodeList{Items: []v1.Node{}}, NodeCapacity{}, nil), podUID: "uid-1234", containerName: "container-2", wantPodCPULimit: 2.1, @@ -500,7 +500,7 @@ func TestCpuAndMemoryGetters(t *testing.T) { }, }, }, - }, NodeCapacity{}, nil), + }, &v1.NodeList{Items: []v1.Node{}}, NodeCapacity{}, nil), podUID: "uid-12345", }, { @@ -543,7 +543,7 @@ func TestCpuAndMemoryGetters(t *testing.T) { }, }, }, - }, NodeCapacity{}, nil), + }, &v1.NodeList{Items: []v1.Node{}}, NodeCapacity{}, nil), podUID: "uid-1234", containerName: "container-3", wantPodCPULimit: 0.7, @@ -583,7 +583,7 @@ func TestCpuAndMemoryGetters(t *testing.T) { }, }, }, - }, NodeCapacity{}, nil), + }, &v1.NodeList{Items: []v1.Node{}}, NodeCapacity{}, nil), podUID: "uid-1234", containerName: "container-2", wantPodCPURequest: 2, @@ -623,7 +623,7 @@ func TestCpuAndMemoryGetters(t *testing.T) { }, }, }, - }, NodeCapacity{}, nil), + }, &v1.NodeList{Items: []v1.Node{}}, NodeCapacity{}, nil), podUID: "uid-1234", containerName: "container-2", wantPodCPULimit: 2, @@ -661,7 +661,7 @@ func TestCpuAndMemoryGetters(t *testing.T) { }, }, }, - }, NodeCapacity{}, nil), + }, &v1.NodeList{Items: []v1.Node{}}, NodeCapacity{}, nil), podUID: "uid-1234", containerName: "container-1", wantContainerCPULimit: 1, diff --git a/receiver/kubeletstatsreceiver/internal/kubelet/metrics_test.go b/receiver/kubeletstatsreceiver/internal/kubelet/metrics_test.go index 94fef59f0946..558ddd829a1f 100644 --- a/receiver/kubeletstatsreceiver/internal/kubelet/metrics_test.go +++ b/receiver/kubeletstatsreceiver/internal/kubelet/metrics_test.go @@ -26,13 +26,18 @@ func (f fakeRestClient) Pods() ([]byte, error) { return os.ReadFile("../../testdata/pods.json") } +func (f fakeRestClient) Nodes() ([]byte, error) { + return os.ReadFile("../../testdata/nodes.json") +} + func TestMetricAccumulator(t *testing.T) { rc := &fakeRestClient{} statsProvider := NewStatsProvider(rc) summary, _ := statsProvider.StatsSummary() metadataProvider := NewMetadataProvider(rc) podsMetadata, _ := metadataProvider.Pods() - k8sMetadata := NewMetadata([]MetadataLabel{MetadataLabelContainerID}, podsMetadata, NodeCapacity{}, nil) + nodesMetadata, _ := metadataProvider.Nodes() + k8sMetadata := NewMetadata([]MetadataLabel{MetadataLabelContainerID}, podsMetadata, nodesMetadata, NodeCapacity{}, nil) mbs := &metadata.MetricsBuilders{ NodeMetricsBuilder: metadata.NewMetricsBuilder(metadata.DefaultMetricsBuilderConfig(), receivertest.NewNopSettings()), PodMetricsBuilder: metadata.NewMetricsBuilder(metadata.DefaultMetricsBuilderConfig(), receivertest.NewNopSettings()), diff --git a/receiver/kubeletstatsreceiver/internal/kubelet/rest_client.go b/receiver/kubeletstatsreceiver/internal/kubelet/rest_client.go index a4afa0028de5..a02f0102d36a 100644 --- a/receiver/kubeletstatsreceiver/internal/kubelet/rest_client.go +++ b/receiver/kubeletstatsreceiver/internal/kubelet/rest_client.go @@ -11,6 +11,7 @@ import ( type RestClient interface { StatsSummary() ([]byte, error) Pods() ([]byte, error) + Nodes() ([]byte, error) } // HTTPRestClient is a thin wrapper around a kubelet client, encapsulating endpoints @@ -32,3 +33,7 @@ func (c *HTTPRestClient) StatsSummary() ([]byte, error) { func (c *HTTPRestClient) Pods() ([]byte, error) { return c.client.Get("/pods") } + +func (c *HTTPRestClient) Nodes() ([]byte, error) { + return c.client.Get("/nodes") +} diff --git a/receiver/kubeletstatsreceiver/internal/kubelet/volume_test.go b/receiver/kubeletstatsreceiver/internal/kubelet/volume_test.go index a0c98f6699d4..782d6e19a283 100644 --- a/receiver/kubeletstatsreceiver/internal/kubelet/volume_test.go +++ b/receiver/kubeletstatsreceiver/internal/kubelet/volume_test.go @@ -28,7 +28,7 @@ func TestDetailedPVCLabels(t *testing.T) { volumeName string volumeSource v1.VolumeSource pod pod - detailedPVCLabelsSetterOverride func(rb *metadata.ResourceBuilder, volCacheID, volumeClaim, namespace string) error + detailedPVCLabelsSetterOverride func(rb *metadata.ResourceBuilder, volCacheID, volumeClaim, namespace string) ([]metadata.ResourceMetricsOption, error) want map[string]any }{ { @@ -40,7 +40,7 @@ func TestDetailedPVCLabels(t *testing.T) { }, }, pod: pod{uid: "uid-1234", name: "pod-name", namespace: "pod-namespace"}, - detailedPVCLabelsSetterOverride: func(rb *metadata.ResourceBuilder, _, _, _ string) error { + detailedPVCLabelsSetterOverride: func(rb *metadata.ResourceBuilder, _, _, _ string) ([]metadata.ResourceMetricsOption, error) { SetPersistentVolumeLabels(rb, v1.PersistentVolumeSource{ AWSElasticBlockStore: &v1.AWSElasticBlockStoreVolumeSource{ VolumeID: "volume_id", @@ -48,7 +48,7 @@ func TestDetailedPVCLabels(t *testing.T) { Partition: 10, }, }) - return nil + return []metadata.ResourceMetricsOption{}, nil }, want: map[string]any{ "k8s.volume.name": "volume0", @@ -71,7 +71,7 @@ func TestDetailedPVCLabels(t *testing.T) { }, }, pod: pod{uid: "uid-1234", name: "pod-name", namespace: "pod-namespace"}, - detailedPVCLabelsSetterOverride: func(rb *metadata.ResourceBuilder, _, _, _ string) error { + detailedPVCLabelsSetterOverride: func(rb *metadata.ResourceBuilder, _, _, _ string) ([]metadata.ResourceMetricsOption, error) { SetPersistentVolumeLabels(rb, v1.PersistentVolumeSource{ GCEPersistentDisk: &v1.GCEPersistentDiskVolumeSource{ PDName: "pd_name", @@ -79,7 +79,7 @@ func TestDetailedPVCLabels(t *testing.T) { Partition: 10, }, }) - return nil + return []metadata.ResourceMetricsOption{}, nil }, want: map[string]any{ "k8s.volume.name": "volume0", @@ -102,14 +102,14 @@ func TestDetailedPVCLabels(t *testing.T) { }, }, pod: pod{uid: "uid-1234", name: "pod-name", namespace: "pod-namespace"}, - detailedPVCLabelsSetterOverride: func(rb *metadata.ResourceBuilder, _, _, _ string) error { + detailedPVCLabelsSetterOverride: func(rb *metadata.ResourceBuilder, _, _, _ string) ([]metadata.ResourceMetricsOption, error) { SetPersistentVolumeLabels(rb, v1.PersistentVolumeSource{ Glusterfs: &v1.GlusterfsPersistentVolumeSource{ EndpointsName: "endpoints_name", Path: "path", }, }) - return nil + return []metadata.ResourceMetricsOption{}, nil }, want: map[string]any{ "k8s.volume.name": "volume0", @@ -131,13 +131,13 @@ func TestDetailedPVCLabels(t *testing.T) { }, }, pod: pod{uid: "uid-1234", name: "pod-name", namespace: "pod-namespace"}, - detailedPVCLabelsSetterOverride: func(rb *metadata.ResourceBuilder, _, _, _ string) error { + detailedPVCLabelsSetterOverride: func(rb *metadata.ResourceBuilder, _, _, _ string) ([]metadata.ResourceMetricsOption, error) { SetPersistentVolumeLabels(rb, v1.PersistentVolumeSource{ Local: &v1.LocalVolumeSource{ Path: "path", }, }) - return nil + return []metadata.ResourceMetricsOption{}, nil }, want: map[string]any{ "k8s.volume.name": "volume0", @@ -177,7 +177,9 @@ func TestDetailedPVCLabels(t *testing.T) { }, }, }, - }, NodeCapacity{}, nil) + }, &v1.NodeList{ + Items: []v1.Node{}, + }, NodeCapacity{}, nil) metadata.DetailedPVCResourceSetter = tt.detailedPVCLabelsSetterOverride res, err := getVolumeResourceOptions(rb, podStats, stats.VolumeStats{Name: tt.volumeName}, metadata) diff --git a/receiver/kubeletstatsreceiver/scraper_test.go b/receiver/kubeletstatsreceiver/scraper_test.go index 8905ea9c015c..b4badb2d04de 100644 --- a/receiver/kubeletstatsreceiver/scraper_test.go +++ b/receiver/kubeletstatsreceiver/scraper_test.go @@ -838,3 +838,7 @@ func (f *fakeRestClient) Pods() ([]byte, error) { } return os.ReadFile("testdata/pods.json") } + +func (f *fakeRestClient) Nodes() ([]byte, error) { + return []byte{}, nil +} diff --git a/receiver/kubeletstatsreceiver/testdata/nodes.json b/receiver/kubeletstatsreceiver/testdata/nodes.json new file mode 100644 index 000000000000..e69de29bb2d1