Skip to content

chore(kubelet): migrate metrics to contextual logging #130157

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+33 −16
Open

chore(kubelet): migrate metrics to contextual logging #130157

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions hack/golangci-hints.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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/.*
Expand Down
1 change: 1 addition & 0 deletions hack/golangci.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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/.*
Expand Down
1 change: 1 addition & 0 deletions hack/logcheck.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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/.*
Expand Down
21 changes: 13 additions & 8 deletions pkg/kubelet/metrics/collectors/cri_metrics.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand All @@ -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()
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a comment explaining when this should be removed.

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
}
Expand All @@ -98,19 +103,19 @@ 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
}

typ := criTypeToProm[m.MetricType]

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.
Expand Down
8 changes: 6 additions & 2 deletions pkg/kubelet/metrics/collectors/log_metrics.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
7 changes: 5 additions & 2 deletions pkg/kubelet/metrics/collectors/resource_metrics.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,16 +149,19 @@ 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)
ch <- metrics.NewLazyConstMetric(resourceScrapeErrorResultDesc, metrics.GaugeValue, errorCount)
}()
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
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/kubelet/metrics/collectors/resource_metrics_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 3 additions & 1 deletion pkg/kubelet/metrics/collectors/volume_stats.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions pkg/kubelet/metrics/collectors/volume_stats_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = `
Expand Down Expand Up @@ -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 = `
Expand Down