Merge pull request #89412 from coderanger/fix-kubelet-method-metrics

Apply the same style of fix as #87913 but for HTTP methods too.

pkg/kubelet/server/BUILD

@@ -38,6 +38,7 @@ go_library(
         "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/proxy:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/runtime:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/authentication/authenticator:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/authentication/user:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/authorization/authorizer:go_default_library",

pkg/kubelet/server/server.go

@@ -48,6 +48,7 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/proxy"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apiserver/pkg/authentication/authenticator"
 	"k8s.io/apiserver/pkg/authorization/authorizer"
 	"k8s.io/apiserver/pkg/server/healthz"
@@ -90,7 +91,8 @@ type Server struct {
 	auth                       AuthInterface
 	host                       HostInterface
 	restfulCont                containerInterface
-	metricsBuckets             map[string]bool
+	metricsBuckets             sets.String
+	metricsMethodBuckets       sets.String
 	resourceAnalyzer           stats.ResourceAnalyzer
 	redirectContainerStreaming bool
 }
@@ -227,7 +229,8 @@ func NewServer(
 		resourceAnalyzer:           resourceAnalyzer,
 		auth:                       auth,
 		restfulCont:                &filteringContainer{Container: restful.NewContainer()},
-		metricsBuckets:             make(map[string]bool),
+		metricsBuckets:             sets.NewString(),
+		metricsMethodBuckets:       sets.NewString("OPTIONS", "GET", "HEAD", "POST", "PUT", "DELETE", "TRACE", "CONNECT"),
 		redirectContainerStreaming: redirectContainerStreaming,
 	}
 	if auth != nil {
@@ -286,16 +289,24 @@ func (s *Server) InstallAuthFilter() {
 // addMetricsBucketMatcher adds a regexp matcher and the relevant bucket to use when
 // it matches. Please be aware this is not thread safe and should not be used dynamically
 func (s *Server) addMetricsBucketMatcher(bucket string) {
-	s.metricsBuckets[bucket] = true
+	s.metricsBuckets.Insert(bucket)
 }
 
 // getMetricBucket find the appropriate metrics reporting bucket for the given path
 func (s *Server) getMetricBucket(path string) string {
 	root := getURLRootPath(path)
-	if s.metricsBuckets[root] == true {
+	if s.metricsBuckets.Has(root) {
 		return root
 	}
-	return "Invalid path"
+	return "other"
+}
+
+// getMetricMethodBucket checks for unknown or invalid HTTP verbs
+func (s *Server) getMetricMethodBucket(method string) string {
+	if s.metricsMethodBuckets.Has(method) {
+		return method
+	}
+	return "other"
 }
 
 // InstallDefaultHandlers registers the default set of supported HTTP request
@@ -909,7 +920,7 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 		serverType = "readwrite"
 	}
 
-	method, path := req.Method, s.getMetricBucket(req.URL.Path)
+	method, path := s.getMetricMethodBucket(req.Method), s.getMetricBucket(req.URL.Path)
 
 	longRunning := strconv.FormatBool(isLongRunningRequest(path))
 

pkg/kubelet/server/server_test.go

@@ -1510,8 +1510,8 @@ func TestMetricBuckets(t *testing.T) {
 		"stats summary sub":               {url: "/stats/summary", bucket: "stats"},
 		"stats containerName with uid":    {url: "/stats/namespace/podName/uid/containerName", bucket: "stats"},
 		"stats containerName":             {url: "/stats/podName/containerName", bucket: "stats"},
-		"invalid path":                    {url: "/junk", bucket: "Invalid path"},
+		"invalid path":                    {url: "/junk", bucket: "other"},
-		"invalid path starting with good": {url: "/healthzjunk", bucket: "Invalid path"},
+		"invalid path starting with good": {url: "/healthzjunk", bucket: "other"},
 	}
 	fw := newServerTest()
 	defer fw.testHTTPServer.Close()
@@ -1523,6 +1523,26 @@ func TestMetricBuckets(t *testing.T) {
 	}
 }
 
+func TestMetricMethodBuckets(t *testing.T) {
+	tests := map[string]struct {
+		method string
+		bucket string
+	}{
+		"normal GET":     {method: "GET", bucket: "GET"},
+		"normal POST":    {method: "POST", bucket: "POST"},
+		"invalid method": {method: "WEIRD", bucket: "other"},
+	}
+
+	fw := newServerTest()
+	defer fw.testHTTPServer.Close()
+
+	for _, test := range tests {
+		method := test.method
+		bucket := test.bucket
+		require.Equal(t, fw.serverUnderTest.getMetricMethodBucket(method), bucket)
+	}
+}
+
 func TestDebuggingDisabledHandlers(t *testing.T) {
 	fw := newServerTestWithDebug(false, false, nil)
 	defer fw.testHTTPServer.Close()