From fbe69559743e59334c572d742e5126d266b5a068 Mon Sep 17 00:00:00 2001 From: zhangzhifei16 Date: Tue, 13 May 2025 16:12:36 +0800 Subject: [PATCH] chore(kubelet): migrate metrics to contextual logging. --- hack/golangci-hints.yaml | 1 + hack/golangci.yaml | 1 + hack/logcheck.conf | 1 + pkg/kubelet/metrics/collectors/cri_metrics.go | 21 ++++++++++++------- pkg/kubelet/metrics/collectors/log_metrics.go | 8 +++++-- .../metrics/collectors/resource_metrics.go | 7 +++++-- .../collectors/resource_metrics_test.go | 2 +- .../metrics/collectors/volume_stats.go | 4 +++- .../metrics/collectors/volume_stats_test.go | 4 ++-- 9 files changed, 33 insertions(+), 16 deletions(-) diff --git a/hack/golangci-hints.yaml b/hack/golangci-hints.yaml index 8306b7be5a160..ab70c414913a2 100644 --- a/hack/golangci-hints.yaml +++ b/hack/golangci-hints.yaml @@ -193,6 +193,7 @@ linters: contextual k8s.io/kubernetes/pkg/kubelet/clustertrustbundle/.* contextual k8s.io/kubernetes/pkg/kubelet/token/.* contextual k8s.io/kubernetes/pkg/kubelet/cadvisor/.* + contextual k8s.io/kubernetes/pkg/kubelet/metrics/collectors/.* contextual k8s.io/kubernetes/pkg/kubelet/oom/.* contextual k8s.io/kubernetes/pkg/kubelet/status/.* contextual k8s.io/kubernetes/pkg/kubelet/sysctl/.* diff --git a/hack/golangci.yaml b/hack/golangci.yaml index bc9c4bf9e724f..b0e4d8c9d0de5 100644 --- a/hack/golangci.yaml +++ b/hack/golangci.yaml @@ -207,6 +207,7 @@ linters: contextual k8s.io/kubernetes/pkg/kubelet/clustertrustbundle/.* contextual k8s.io/kubernetes/pkg/kubelet/token/.* contextual k8s.io/kubernetes/pkg/kubelet/cadvisor/.* + contextual k8s.io/kubernetes/pkg/kubelet/metrics/collectors/.* contextual k8s.io/kubernetes/pkg/kubelet/oom/.* contextual k8s.io/kubernetes/pkg/kubelet/status/.* contextual k8s.io/kubernetes/pkg/kubelet/sysctl/.* diff --git a/hack/logcheck.conf b/hack/logcheck.conf index dfa502327b46c..fe35321bc6ae5 100644 --- a/hack/logcheck.conf +++ b/hack/logcheck.conf @@ -54,6 +54,7 @@ contextual k8s.io/kubernetes/pkg/kubelet/pleg/.* contextual k8s.io/kubernetes/pkg/kubelet/clustertrustbundle/.* contextual k8s.io/kubernetes/pkg/kubelet/token/.* contextual k8s.io/kubernetes/pkg/kubelet/cadvisor/.* +contextual k8s.io/kubernetes/pkg/kubelet/metrics/collectors/.* contextual k8s.io/kubernetes/pkg/kubelet/oom/.* contextual k8s.io/kubernetes/pkg/kubelet/status/.* contextual k8s.io/kubernetes/pkg/kubelet/sysctl/.* diff --git a/pkg/kubelet/metrics/collectors/cri_metrics.go b/pkg/kubelet/metrics/collectors/cri_metrics.go index 8736eb9e3197c..9ae50db409acc 100644 --- a/pkg/kubelet/metrics/collectors/cri_metrics.go +++ b/pkg/kubelet/metrics/collectors/cri_metrics.go @@ -41,7 +41,8 @@ var _ metrics.StableCollector = &criMetricsCollector{} func NewCRIMetricsCollector(ctx context.Context, listPodSandboxMetricsFn func(context.Context) ([]*runtimeapi.PodSandboxMetrics, error), listMetricDescriptorsFn func(context.Context) ([]*runtimeapi.MetricDescriptor, error)) metrics.StableCollector { descs, err := listMetricDescriptorsFn(ctx) if err != nil { - klog.ErrorS(err, "Error reading MetricDescriptors") + logger := klog.FromContext(ctx) + logger.Error(err, "Error reading MetricDescriptors") return &criMetricsCollector{ listPodSandboxMetricsFn: listPodSandboxMetricsFn, } @@ -68,22 +69,26 @@ func (c *criMetricsCollector) DescribeWithStability(ch chan<- *metrics.Desc) { // Collect implements the metrics.CollectWithStability interface. // TODO(haircommander): would it be better if these were processed async? func (c *criMetricsCollector) CollectWithStability(ch chan<- metrics.Metric) { - podMetrics, err := c.listPodSandboxMetricsFn(context.Background()) + // Use context.TODO() because we currently do not have a proper context to pass in. + // Replace this with an appropriate context when refactoring this function to accept a context parameter. + ctx := context.TODO() + logger := klog.FromContext(ctx) + podMetrics, err := c.listPodSandboxMetricsFn(ctx) if err != nil { - klog.ErrorS(err, "Failed to get pod metrics") + logger.Error(err, "Failed to get pod metrics") return } for _, podMetric := range podMetrics { for _, metric := range podMetric.GetMetrics() { - promMetric, err := c.criMetricToProm(metric) + promMetric, err := c.criMetricToProm(logger, metric) if err == nil { ch <- promMetric } } for _, ctrMetric := range podMetric.GetContainerMetrics() { for _, metric := range ctrMetric.GetMetrics() { - promMetric, err := c.criMetricToProm(metric) + promMetric, err := c.criMetricToProm(logger, metric) if err == nil { ch <- promMetric } @@ -98,11 +103,11 @@ func criDescToProm(m *runtimeapi.MetricDescriptor) *metrics.Desc { return metrics.NewDesc(m.Name, m.Help, m.LabelKeys, nil, metrics.INTERNAL, "") } -func (c *criMetricsCollector) criMetricToProm(m *runtimeapi.Metric) (metrics.Metric, error) { +func (c *criMetricsCollector) criMetricToProm(logger klog.Logger, m *runtimeapi.Metric) (metrics.Metric, error) { desc, ok := c.descriptors[m.Name] if !ok { err := fmt.Errorf("error converting CRI Metric to prometheus format") - klog.V(5).ErrorS(err, "Descriptor not present in pre-populated list of descriptors", "name", m.Name) + logger.V(5).Error(err, "Descriptor not present in pre-populated list of descriptors", "name", m.Name) return nil, err } @@ -110,7 +115,7 @@ func (c *criMetricsCollector) criMetricToProm(m *runtimeapi.Metric) (metrics.Met pm, err := metrics.NewConstMetric(desc, typ, float64(m.GetValue().Value), m.LabelValues...) if err != nil { - klog.ErrorS(err, "Error getting CRI prometheus metric", "descriptor", desc.String()) + logger.Error(err, "Error getting CRI prometheus metric", "descriptor", desc.String()) return nil, err } // If Timestamp is 0, then the runtime did not cache the result. diff --git a/pkg/kubelet/metrics/collectors/log_metrics.go b/pkg/kubelet/metrics/collectors/log_metrics.go index 4b2237fbbadd0..1443557a1f66f 100644 --- a/pkg/kubelet/metrics/collectors/log_metrics.go +++ b/pkg/kubelet/metrics/collectors/log_metrics.go @@ -63,9 +63,13 @@ func (c *logMetricsCollector) DescribeWithStability(ch chan<- *metrics.Desc) { // CollectWithStability implements the metrics.StableCollector interface. func (c *logMetricsCollector) CollectWithStability(ch chan<- metrics.Metric) { - podStats, err := c.podStats(context.Background()) + // Use context.TODO() because we currently do not have a proper context to pass in. + // Replace this with an appropriate context when refactoring this function to accept a context parameter. + ctx := context.TODO() + logger := klog.FromContext(ctx) + podStats, err := c.podStats(ctx) if err != nil { - klog.ErrorS(err, "Failed to get pod stats") + logger.Error(err, "Failed to get pod stats") return } diff --git a/pkg/kubelet/metrics/collectors/resource_metrics.go b/pkg/kubelet/metrics/collectors/resource_metrics.go index c1b1cdffc2ab0..8b28b15eff4b9 100644 --- a/pkg/kubelet/metrics/collectors/resource_metrics.go +++ b/pkg/kubelet/metrics/collectors/resource_metrics.go @@ -149,7 +149,9 @@ func (rc *resourceMetricsCollector) DescribeWithStability(ch chan<- *metrics.Des // leak metric collectors for containers or pods that no longer exist. Instead, implement // custom collector in a way that only collects metrics for active containers. func (rc *resourceMetricsCollector) CollectWithStability(ch chan<- metrics.Metric) { - ctx := context.Background() + // Use context.TODO() because we currently do not have a proper context to pass in. + // Replace this with an appropriate context when refactoring this function to accept a context parameter. + ctx := context.TODO() var errorCount float64 defer func() { ch <- metrics.NewLazyConstMetric(resourceScrapeResultDesc, metrics.GaugeValue, errorCount) @@ -157,8 +159,9 @@ func (rc *resourceMetricsCollector) CollectWithStability(ch chan<- metrics.Metri }() statsSummary, err := rc.provider.GetCPUAndMemoryStats(ctx) if err != nil { + logger := klog.FromContext(ctx) errorCount = 1 - klog.ErrorS(err, "Error getting summary for resourceMetric prometheus endpoint") + logger.Error(err, "Error getting summary for resourceMetric prometheus endpoint") return } diff --git a/pkg/kubelet/metrics/collectors/resource_metrics_test.go b/pkg/kubelet/metrics/collectors/resource_metrics_test.go index 29ab3f3f3093e..56d194fa015ef 100644 --- a/pkg/kubelet/metrics/collectors/resource_metrics_test.go +++ b/pkg/kubelet/metrics/collectors/resource_metrics_test.go @@ -404,7 +404,7 @@ func TestCollectResourceMetrics(t *testing.T) { for _, test := range tests { tc := test t.Run(tc.name, func(t *testing.T) { - ctx := context.Background() + ctx := context.TODO() provider := summaryprovidertest.NewMockSummaryProvider(t) provider.EXPECT().GetCPUAndMemoryStats(ctx).Return(tc.summary, tc.summaryErr).Maybe() collector := NewResourceMetricsCollector(provider) diff --git a/pkg/kubelet/metrics/collectors/volume_stats.go b/pkg/kubelet/metrics/collectors/volume_stats.go index b565bf1d376f5..b4b4145567f43 100644 --- a/pkg/kubelet/metrics/collectors/volume_stats.go +++ b/pkg/kubelet/metrics/collectors/volume_stats.go @@ -98,7 +98,9 @@ func (collector *volumeStatsCollector) DescribeWithStability(ch chan<- *metrics. // CollectWithStability implements the metrics.StableCollector interface. func (collector *volumeStatsCollector) CollectWithStability(ch chan<- metrics.Metric) { - ctx := context.Background() + // Use context.TODO() because we currently do not have a proper context to pass in. + // Replace this with an appropriate context when refactoring this function to accept a context parameter. + ctx := context.TODO() podStats, err := collector.statsProvider.ListPodStats(ctx) if err != nil { return diff --git a/pkg/kubelet/metrics/collectors/volume_stats_test.go b/pkg/kubelet/metrics/collectors/volume_stats_test.go index d5d2e824e8a4a..7fb0d714f3d6f 100644 --- a/pkg/kubelet/metrics/collectors/volume_stats_test.go +++ b/pkg/kubelet/metrics/collectors/volume_stats_test.go @@ -32,7 +32,7 @@ func newUint64Pointer(i uint64) *uint64 { } func TestVolumeStatsCollector(t *testing.T) { - ctx := context.Background() + ctx := context.TODO() // Fixed metadata on type and help text. We prepend this to every expected // output so we only have to modify a single place when doing adjustments. const metadata = ` @@ -151,7 +151,7 @@ func TestVolumeStatsCollector(t *testing.T) { } func TestVolumeStatsCollectorWithNullVolumeStatus(t *testing.T) { - ctx := context.Background() + ctx := context.TODO() // Fixed metadata on type and help text. We prepend this to every expected // output so we only have to modify a single place when doing adjustments. const metadata = `