Collapse all metrics handlers into common code

Remove the MonitorRequest method and replace with a method that takes
request.RequestInfo, which is our default way to talk about API objects.
Preserves existing semantics for calls.

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/proxy.go

@@ -56,20 +56,14 @@ func (r *ProxyHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 	reqStart := time.Now()
 	proxyHandlerTraceID := rand.Int63()
 
-	var verb, apiResource, subresource, scope string
 	var httpCode int
-
+	var requestInfo *request.RequestInfo
 	defer func() {
 		responseLength := 0
 		if rw, ok := w.(*metrics.ResponseWriterDelegator); ok {
 			responseLength = rw.ContentLength()
 		}
-		metrics.Monitor(
-			verb, apiResource, subresource, scope,
-			net.GetHTTPClient(req),
-			w.Header().Get("Content-Type"),
-			httpCode, responseLength, reqStart,
-		)
+		metrics.Record(req, requestInfo, w.Header().Get("Content-Type"), httpCode, responseLength, time.Now().Sub(reqStart))
 	}()
 
 	ctx, ok := r.Mapper.Get(req)
@@ -79,7 +73,7 @@ func (r *ProxyHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 		return
 	}
 
-	requestInfo, ok := request.RequestInfoFrom(ctx)
+	requestInfo, ok = request.RequestInfoFrom(ctx)
 	if !ok {
 		responsewriters.InternalError(w, req, errors.New("Error getting RequestInfo from context"))
 		httpCode = http.StatusInternalServerError
@@ -90,15 +84,7 @@ func (r *ProxyHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 		httpCode = http.StatusNotFound
 		return
 	}
-	verb = requestInfo.Verb
-	namespace, resource, subresource, parts := requestInfo.Namespace, requestInfo.Resource, requestInfo.Subresource, requestInfo.Parts
-	scope = "cluster"
-	if namespace != "" {
-		scope = "namespace"
-	}
-	if requestInfo.Name != "" {
-		scope = "resource"
-	}
+	namespace, resource, parts := requestInfo.Namespace, requestInfo.Resource, requestInfo.Parts
 
 	ctx = request.WithNamespace(ctx, namespace)
 	if len(parts) < 2 {
@@ -125,7 +111,6 @@ func (r *ProxyHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 		httpCode = http.StatusNotFound
 		return
 	}
-	apiResource = resource
 
 	gv := schema.GroupVersion{Group: requestInfo.APIGroup, Version: requestInfo.APIVersion}
 

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

@@ -19,6 +19,7 @@ go_library(
         "//vendor/github.com/emicklei/go-restful:go_default_library",
         "//vendor/github.com/prometheus/client_golang/prometheus:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/net:go_default_library",
+        "//vendor/k8s.io/apiserver/pkg/endpoints/request:go_default_library",
     ],
 )
 

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

@@ -27,7 +27,7 @@ import (
 	"time"
 
 	utilnet "k8s.io/apimachinery/pkg/util/net"
-	//utilruntime "k8s.io/apimachinery/pkg/util/runtime"
+	"k8s.io/apiserver/pkg/endpoints/request"
 
 	"github.com/emicklei/go-restful"
 	"github.com/prometheus/client_golang/prometheus"
@@ -82,22 +82,27 @@ func Register() {
 	prometheus.MustRegister(responseSizes)
 }
 
-// Monitor records a request to the apiserver endpoints that follow the Kubernetes API conventions.  verb must be
-// uppercase to be backwards compatible with existing monitoring tooling.
-func Monitor(verb, resource, subresource, scope, client, contentType string, httpCode, respSize int, reqStart time.Time) {
-	elapsed := float64((time.Since(reqStart)) / time.Microsecond)
-	requestCounter.WithLabelValues(verb, resource, subresource, scope, client, contentType, codeToString(httpCode)).Inc()
-	requestLatencies.WithLabelValues(verb, resource, subresource, scope).Observe(elapsed)
-	requestLatenciesSummary.WithLabelValues(verb, resource, subresource, scope).Observe(elapsed)
-	// We are only interested in response sizes of read requests.
-	if verb == "GET" || verb == "LIST" {
-		responseSizes.WithLabelValues(verb, resource, subresource, scope).Observe(float64(respSize))
+// Record records a single request to the standard metrics endpoints. For use by handlers that perform their own
+// processing. All API paths should use InstrumentRouteFunc implicitly. Use this instead of MonitorRequest if
+// you already have a RequestInfo object.
+func Record(req *http.Request, requestInfo *request.RequestInfo, contentType string, code int, responseSizeInBytes int, elapsed time.Duration) {
+	scope := "cluster"
+	if requestInfo.Namespace != "" {
+		scope = "namespace"
+	}
+	if requestInfo.Name != "" {
+		scope = "resource"
+	}
+	if requestInfo.IsResourceRequest {
+		MonitorRequest(req, strings.ToUpper(requestInfo.Verb), requestInfo.Resource, requestInfo.Subresource, contentType, scope, code, responseSizeInBytes, elapsed)
+	} else {
+		MonitorRequest(req, strings.ToUpper(requestInfo.Verb), "", requestInfo.Path, contentType, scope, code, responseSizeInBytes, elapsed)
 	}
 }
 
 // MonitorRequest handles standard transformations for client and the reported verb and then invokes Monitor to record
 // a request. verb must be uppercase to be backwards compatible with existing monitoring tooling.
-func MonitorRequest(request *http.Request, verb, resource, subresource, scope, contentType string, httpCode, respSize int, reqStart time.Time) {
+func MonitorRequest(request *http.Request, verb, resource, subresource, scope, contentType string, httpCode, respSize int, elapsed time.Duration) {
 	reportedVerb := verb
 	if verb == "LIST" {
 		// see apimachinery/pkg/runtime/conversion.go Convert_Slice_string_To_bool
@@ -113,7 +118,14 @@ func MonitorRequest(request *http.Request, verb, resource, subresource, scope, c
 	}
 
 	client := cleanUserAgent(utilnet.GetHTTPClient(request))
-	Monitor(reportedVerb, resource, subresource, scope, client, contentType, httpCode, respSize, reqStart)
+	elapsedMicroseconds := float64(elapsed / time.Microsecond)
+	requestCounter.WithLabelValues(reportedVerb, resource, subresource, scope, client, contentType, codeToString(httpCode)).Inc()
+	requestLatencies.WithLabelValues(reportedVerb, resource, subresource, scope).Observe(elapsedMicroseconds)
+	requestLatenciesSummary.WithLabelValues(reportedVerb, resource, subresource, scope).Observe(elapsedMicroseconds)
+	// We are only interested in response sizes of read requests.
+	if verb == "GET" || verb == "LIST" {
+		responseSizes.WithLabelValues(reportedVerb, resource, subresource, scope).Observe(float64(respSize))
+	}
 }
 
 func Reset() {
@@ -144,7 +156,7 @@ func InstrumentRouteFunc(verb, resource, subresource, scope string, routeFunc re
 
 		routeFunc(request, response)
 
-		MonitorRequest(request.Request, verb, resource, subresource, scope, delegate.Header().Get("Content-Type"), delegate.Status(), delegate.ContentLength(), now)
+		MonitorRequest(request.Request, verb, resource, subresource, scope, delegate.Header().Get("Content-Type"), delegate.Status(), delegate.ContentLength(), time.Now().Sub(now))
 	})
 }
 

staging/src/k8s.io/apiserver/pkg/server/filters/maxinflight.go

@@ -19,8 +19,6 @@ package filters
 import (
 	"fmt"
 	"net/http"
-	"strings"
-	"time"
 
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apiserver/pkg/authentication/user"
@@ -108,18 +106,7 @@ func WithMaxInFlightLimit(
 						}
 					}
 				}
-				scope := "cluster"
-				if requestInfo.Namespace != "" {
-					scope = "namespace"
-				}
-				if requestInfo.Name != "" {
-					scope = "resource"
-				}
-				if requestInfo.IsResourceRequest {
-					metrics.MonitorRequest(r, strings.ToUpper(requestInfo.Verb), requestInfo.Resource, requestInfo.Subresource, "", scope, http.StatusTooManyRequests, 0, time.Now())
-				} else {
-					metrics.MonitorRequest(r, strings.ToUpper(requestInfo.Verb), "", requestInfo.Path, "", scope, http.StatusTooManyRequests, 0, time.Now())
-				}
+				metrics.Record(r, requestInfo, "", http.StatusTooManyRequests, 0, 0)
 				tooManyRequests(r, w)
 			}
 		}

staging/src/k8s.io/apiserver/pkg/server/filters/timeout.go

@@ -22,7 +22,6 @@ import (
 	"fmt"
 	"net"
 	"net/http"
-	"strings"
 	"sync"
 	"time"
 
@@ -55,20 +54,8 @@ func WithTimeoutForNonLongRunningRequests(handler http.Handler, requestContextMa
 		if longRunning(req, requestInfo) {
 			return nil, nil, nil
 		}
-		now := time.Now()
 		metricFn := func() {
-			scope := "cluster"
-			if requestInfo.Namespace != "" {
-				scope = "namespace"
-			}
-			if requestInfo.Name != "" {
-				scope = "resource"
-			}
-			if requestInfo.IsResourceRequest {
-				metrics.MonitorRequest(req, strings.ToUpper(requestInfo.Verb), requestInfo.Resource, requestInfo.Subresource, "", scope, http.StatusGatewayTimeout, 0, now)
-			} else {
-				metrics.MonitorRequest(req, strings.ToUpper(requestInfo.Verb), "", requestInfo.Path, "", scope, http.StatusGatewayTimeout, 0, now)
-			}
+			metrics.Record(req, requestInfo, "", http.StatusGatewayTimeout, 0, 0)
 		}
 		return time.After(timeout), metricFn, apierrors.NewTimeoutError(fmt.Sprintf("request did not complete within %s", timeout), 0)
 	}