Merge pull request #1009 from XiaoXiaoSN/master

fix: Handle error while parsing node metrics
kubernetes-sigs · Apr 27, 2022 · 4436807 · 4436807
2 parents d892dcd + 88edec4
commit 4436807
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 16 deletions.
diff --git a/pkg/scraper/client/resource/client.go b/pkg/scraper/client/resource/client.go
@@ -116,7 +116,10 @@ func (kc *kubeletClient) getMetrics(ctx context.Context, url, nodeName string) (
 		return nil, fmt.Errorf("failed to read response body - %v", err)
 	}
 	b = buf.Bytes()
-	ms := decodeBatch(b, requestTime, nodeName)
+	ms, err := decodeBatch(b, requestTime, nodeName)
 	kc.buffers.Put(b)
+	if err != nil {
+		return nil, err
+	}
 	return ms, nil
 }
diff --git a/pkg/scraper/client/resource/decode.go b/pkg/scraper/client/resource/decode.go
@@ -16,6 +16,7 @@ package resource
 
 import (
 	"bytes"
+	"fmt"
 	"io"
 	"time"
 
@@ -34,7 +35,7 @@ var (
 	containerStartTimeMetricName = []byte("container_start_time_seconds")
 )
 
-func decodeBatch(b []byte, defaultTime time.Time, nodeName string) *storage.MetricsBatch {
+func decodeBatch(b []byte, defaultTime time.Time, nodeName string) (*storage.MetricsBatch, error) {
 	res := &storage.MetricsBatch{
 		Nodes: make(map[string]storage.MetricsPoint),
 		Pods:  make(map[apitypes.NamespacedName]storage.PodMetricsPoint),
@@ -51,6 +52,8 @@ func decodeBatch(b []byte, defaultTime time.Time, nodeName string) *storage.Metr
 		if et, err = parser.Next(); err != nil {
 			if err == io.EOF {
 				break
+			} else {
+				return nil, fmt.Errorf("failed parsing metrics: %w", err)
 			}
 		}
 		if et != textparse.EntrySeries {
@@ -100,7 +103,7 @@ func decodeBatch(b []byte, defaultTime time.Time, nodeName string) *storage.Metr
 			}
 		}
 	}
-	return res
+	return res, nil
 }
 
 func timeseriesMatchesName(ts, name []byte) bool {

diff --git a/pkg/scraper/client/resource/decode_test.go b/pkg/scraper/client/resource/decode_test.go
@@ -24,11 +24,17 @@ import (
 )
 
 func TestDecode(t *testing.T) {
+	emptyMetrics := storage.MetricsBatch{
+		Nodes: map[string]storage.MetricsPoint{},
+		Pods:  map[apitypes.NamespacedName]storage.PodMetricsPoint{},
+	}
+
 	tcs := []struct {
 		name          string
 		input         string
 		defaultTime   time.Time
 		expectMetrics *storage.MetricsBatch
+		wantError     bool
 	}{
 		{
 			name: "Normal",
@@ -139,30 +145,30 @@ container_start_time_seconds{container="coredns",namespace="kube-system",pod="co
 			input: `
 container_memory_working_set_bytes{container="coredns",namespace="kube-system",pod="coredns-558bd4d5db-4dpjz"} 1.253376e+07 1633253812125
 `,
-			expectMetrics: nil,
+			expectMetrics: &emptyMetrics,
 		},
 		{
 			name: "Empty container CPU drops container metrics",
 			input: `
 container_cpu_usage_seconds_total{container="coredns",namespace="kube-system",pod="coredns-558bd4d5db-4dpjz"} 0 1633253812125
 container_memory_working_set_bytes{container="coredns",namespace="kube-system",pod="coredns-558bd4d5db-4dpjz"} 1.253376e+07 1633253812125
 `,
-			expectMetrics: nil,
+			expectMetrics: &emptyMetrics,
 		},
 		{
 			name: "No container Memory drops container metrics",
 			input: `
 container_cpu_usage_seconds_total{container="coredns",namespace="kube-system",pod="coredns-558bd4d5db-4dpjz"} 4.710169 1633253812125
 `,
-			expectMetrics: nil,
+			expectMetrics: &emptyMetrics,
 		},
 		{
 			name: "Empty container Memory drops container metrics",
 			input: `
 container_cpu_usage_seconds_total{container="coredns",namespace="kube-system",pod="coredns-558bd4d5db-4dpjz"} 4.710169 1633253812125
 container_memory_working_set_bytes{container="coredns",namespace="kube-system",pod="coredns-558bd4d5db-4dpjz"} 0 1633253812125
 `,
-			expectMetrics: nil,
+			expectMetrics: &emptyMetrics,
 		},
 		{
 			name: "Single node",
@@ -186,41 +192,49 @@ node_memory_working_set_bytes 1.616273408e+09 1633253809720
 			input: `
 node_memory_working_set_bytes 1.616273408e+09 1633253809720
 `,
-			expectMetrics: nil,
+			expectMetrics: &emptyMetrics,
 		},
 		{
 			name: "Empty node CPU drops metric",
 			input: `
 node_cpu_usage_seconds_total 0 1633253809720
 node_memory_working_set_bytes 1.616273408e+09 1633253809720
 `,
-			expectMetrics: nil,
+			expectMetrics: &emptyMetrics,
 		},
 		{
 			name: "No node Memory drops metrics",
 			input: `
 node_cpu_usage_seconds_total 357.35491 1633253809720
 `,
-			expectMetrics: nil,
+			expectMetrics: &emptyMetrics,
 		},
 		{
 			name: "Empty node Memory drops metric",
 			input: `
 node_cpu_usage_seconds_total 357.35491 1633253809720
 node_memory_working_set_bytes 0 1633253809720
+`,
+			expectMetrics: &emptyMetrics,
+		},
+		{
+			name: "Containing an incorrect timestamp",
+			input: `
+# HELP container_start_time_seconds [ALPHA] Start time of the container since unix epoch in seconds
+# TYPE container_start_time_seconds gauge
+container_start_time_seconds{container="metrics-server",namespace="kubernetes-dashboard",pod="kubernetes-dashboard-metrics-server-77db45cdf4-fppzx"} -6.7953645788713455e+09 -62135596800000
+container_start_time_seconds{container="metrics-server",namespace="kubernetes-dashboard",pod="kubernetes-dashboard-metrics-server-77db45cdf4-tpx4v"} 1.6509742024191372e+09 1650974202419
 `,
 			expectMetrics: nil,
+			wantError:     true,
 		},
 	}
 	for _, tc := range tcs {
 		t.Run(tc.name, func(t *testing.T) {
-			if tc.expectMetrics == nil {
-				tc.expectMetrics = &storage.MetricsBatch{
-					Nodes: map[string]storage.MetricsPoint{},
-					Pods:  map[apitypes.NamespacedName]storage.PodMetricsPoint{},
-				}
+			ms, err := decodeBatch([]byte(tc.input), tc.defaultTime, "node1")
+			if (err != nil) != tc.wantError {
+				t.Fatalf("Unexpected error: %v", err)
 			}
-			ms := decodeBatch([]byte(tc.input), tc.defaultTime, "node1")
 			if diff := cmp.Diff(tc.expectMetrics, ms); diff != "" {
 				t.Errorf(`Metrics diff: %s`, diff)
 			}