Merge pull request #105648 from kkkkun/kkkkun/fix-metric

GET should be transformed to watch in kube-Apiserver

staging/src/k8s.io/apiserver/pkg/endpoints/metrics/metrics.go

@@ -380,7 +380,7 @@ func RecordRequestAbort(req *http.Request, requestInfo *request.RequestInfo) {
 	}
 
 	scope := CleanScope(requestInfo)
-	reportedVerb := cleanVerb(CanonicalVerb(strings.ToUpper(req.Method), scope), "", req)
+	reportedVerb := cleanVerb(CanonicalVerb(strings.ToUpper(req.Method), scope), getVerbIfWatch(req), req)
 	resource := requestInfo.Resource
 	subresource := requestInfo.Subresource
 	group := requestInfo.APIGroup
@@ -403,7 +403,7 @@ func RecordRequestTermination(req *http.Request, requestInfo *request.RequestInf
 	// InstrumentRouteFunc which is registered in installer.go with predefined
 	// list of verbs (different than those translated to RequestInfo).
 	// However, we need to tweak it e.g. to differentiate GET from LIST.
-	reportedVerb := cleanVerb(CanonicalVerb(strings.ToUpper(req.Method), scope), "", req)
+	reportedVerb := cleanVerb(CanonicalVerb(strings.ToUpper(req.Method), scope), getVerbIfWatch(req), req)
 
 	if requestInfo.IsResourceRequest {
 		requestTerminationsTotal.WithContext(req.Context()).WithLabelValues(reportedVerb, requestInfo.APIGroup, requestInfo.APIVersion, requestInfo.Resource, requestInfo.Subresource, scope, component, codeToString(code)).Inc()
@@ -425,7 +425,7 @@ func RecordLongRunning(req *http.Request, requestInfo *request.RequestInfo, comp
 	// InstrumentRouteFunc which is registered in installer.go with predefined
 	// list of verbs (different than those translated to RequestInfo).
 	// However, we need to tweak it e.g. to differentiate GET from LIST.
-	reportedVerb := cleanVerb(CanonicalVerb(strings.ToUpper(req.Method), scope), "", req)
+	reportedVerb := cleanVerb(CanonicalVerb(strings.ToUpper(req.Method), scope), getVerbIfWatch(req), req)
 
 	if requestInfo.IsResourceRequest {
 		e = longRunningRequestsGauge.WithContext(req.Context()).WithLabelValues(reportedVerb, requestInfo.APIGroup, requestInfo.APIVersion, requestInfo.Resource, requestInfo.Subresource, scope, component)
@@ -544,13 +544,8 @@ func CanonicalVerb(verb string, scope string) string {
 // LIST and APPLY from PATCH.
 func CleanVerb(verb string, request *http.Request) string {
 	reportedVerb := verb
-	if verb == "LIST" {
-		// see apimachinery/pkg/runtime/conversion.go Convert_Slice_string_To_bool
-		if values := request.URL.Query()["watch"]; len(values) > 0 {
-			if value := strings.ToLower(values[0]); value != "0" && value != "false" {
-				reportedVerb = "WATCH"
-			}
-		}
+	if verb == "LIST" && checkIfWatch(request) {
+		reportedVerb = "WATCH"
 	}
 	// normalize the legacy WATCHLIST to WATCH to ensure users aren't surprised by metrics
 	if verb == "WATCHLIST" {
@@ -564,20 +559,43 @@ func CleanVerb(verb string, request *http.Request) string {
 
 // cleanVerb additionally ensures that unknown verbs don't clog up the metrics.
 func cleanVerb(verb, suggestedVerb string, request *http.Request) string {
-	reportedVerb := CleanVerb(verb, request)
 	// CanonicalVerb (being an input for this function) doesn't handle correctly the
 	// deprecated path pattern for watch of:
 	//   GET /api/{version}/watch/{resource}
 	// We correct it manually based on the pass verb from the installer.
+	var reportedVerb string
 	if suggestedVerb == "WATCH" || suggestedVerb == "WATCHLIST" {
 		reportedVerb = "WATCH"
+	} else {
+		reportedVerb = CleanVerb(verb, request)
 	}
 	if validRequestMethods.Has(reportedVerb) {
 		return reportedVerb
 	}
 	return OtherRequestMethod
 }
 
+//getVerbIfWatch additionally ensures that GET or List would be transformed to WATCH
+func getVerbIfWatch(req *http.Request) string {
+	if strings.ToUpper(req.Method) == "GET" || strings.ToUpper(req.Method) == "LIST" {
+		if checkIfWatch(req) {
+			return "WATCH"
+		}
+	}
+	return ""
+}
+
+//checkIfWatch check request is watch
+func checkIfWatch(req *http.Request) bool {
+	// see apimachinery/pkg/runtime/conversion.go Convert_Slice_string_To_bool
+	if values := req.URL.Query()["watch"]; len(values) > 0 {
+		if value := strings.ToLower(values[0]); value != "0" && value != "false" {
+			return true
+		}
+	}
+	return false
+}
+
 func cleanDryRun(u *url.URL) string {
 	// avoid allocating when we don't see dryRun in the query
 	if !strings.Contains(u.RawQuery, "dryRun") {

staging/src/k8s.io/apiserver/pkg/endpoints/metrics/metrics_test.go

@@ -65,6 +65,16 @@ func TestCleanVerb(t *testing.T) {
 			},
 			expectedVerb: "LIST",
 		},
+		{
+			desc:        "GET isn't be transformed to WATCH if we have the right query param on the request",
+			initialVerb: "GET",
+			request: &http.Request{
+				URL: &url.URL{
+					RawQuery: "watch=true",
+				},
+			},
+			expectedVerb: "GET",
+		},
 		{
 			desc:          "LIST is transformed to WATCH for the old pattern watch",
 			initialVerb:   "LIST",