From ad431c454c1306fdcc2134a3626444984d350f46 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Wed, 24 May 2017 11:34:24 -0400 Subject: [PATCH] Subresources are not included in apiserver prometheus metrics Subresources are very often completely different code paths and errors generated on those code paths are important to distinguish. --- .../apiserver/pkg/endpoints/handlers/proxy.go | 6 ++--- .../apiserver/pkg/endpoints/installer.go | 22 +++++++++---------- .../pkg/endpoints/metrics/metrics.go | 18 +++++++-------- test/e2e/framework/metrics_util.go | 22 ++++++++++--------- 4 files changed, 35 insertions(+), 33 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/proxy.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/proxy.go index 11853b4eec..d856440a83 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/proxy.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/proxy.go @@ -56,11 +56,11 @@ func (r *ProxyHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { proxyHandlerTraceID := rand.Int63() var verb string - var apiResource string + var apiResource, subresource string var httpCode int reqStart := time.Now() defer func() { - metrics.Monitor(&verb, &apiResource, + metrics.Monitor(&verb, &apiResource, &subresource, net.GetHTTPClient(req), w.Header().Get("Content-Type"), httpCode, reqStart) @@ -85,7 +85,7 @@ func (r *ProxyHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { return } verb = requestInfo.Verb - namespace, resource, parts := requestInfo.Namespace, requestInfo.Resource, requestInfo.Parts + namespace, resource, subresource, parts := requestInfo.Namespace, requestInfo.Resource, requestInfo.Subresource, requestInfo.Parts ctx = request.WithNamespace(ctx, namespace) if len(parts) < 2 { diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go b/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go index fae9acd385..f7344bdb91 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go @@ -572,7 +572,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag } else { handler = restfulGetResource(getter, exporter, reqScope) } - handler = metrics.InstrumentRouteFunc(action.Verb, resource, handler) + handler = metrics.InstrumentRouteFunc(action.Verb, resource, subresource, handler) doc := "read the specified " + kind if hasSubresource { doc = "read " + subresource + " of the specified " + kind @@ -601,7 +601,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag if hasSubresource { doc = "list " + subresource + " of objects of kind " + kind } - handler := metrics.InstrumentRouteFunc(action.Verb, resource, restfulListResource(lister, watcher, reqScope, false, a.minRequestTimeout)) + handler := metrics.InstrumentRouteFunc(action.Verb, resource, subresource, restfulListResource(lister, watcher, reqScope, false, a.minRequestTimeout)) route := ws.GET(action.Path).To(handler). Doc(doc). Param(ws.QueryParameter("pretty", "If 'true', then the output is pretty printed.")). @@ -633,7 +633,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag if hasSubresource { doc = "replace " + subresource + " of the specified " + kind } - handler := metrics.InstrumentRouteFunc(action.Verb, resource, restfulUpdateResource(updater, reqScope, a.group.Typer, admit)) + handler := metrics.InstrumentRouteFunc(action.Verb, resource, subresource, restfulUpdateResource(updater, reqScope, a.group.Typer, admit)) route := ws.PUT(action.Path).To(handler). Doc(doc). Param(ws.QueryParameter("pretty", "If 'true', then the output is pretty printed.")). @@ -649,7 +649,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag if hasSubresource { doc = "partially update " + subresource + " of the specified " + kind } - handler := metrics.InstrumentRouteFunc(action.Verb, resource, restfulPatchResource(patcher, reqScope, admit, mapping.ObjectConvertor)) + handler := metrics.InstrumentRouteFunc(action.Verb, resource, subresource, restfulPatchResource(patcher, reqScope, admit, mapping.ObjectConvertor)) route := ws.PATCH(action.Path).To(handler). Doc(doc). Param(ws.QueryParameter("pretty", "If 'true', then the output is pretty printed.")). @@ -668,7 +668,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag } else { handler = restfulCreateResource(creater, reqScope, a.group.Typer, admit) } - handler = metrics.InstrumentRouteFunc(action.Verb, resource, handler) + handler = metrics.InstrumentRouteFunc(action.Verb, resource, subresource, handler) article := getArticleForNoun(kind, " ") doc := "create" + article + kind if hasSubresource { @@ -690,7 +690,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag if hasSubresource { doc = "delete " + subresource + " of" + article + kind } - handler := metrics.InstrumentRouteFunc(action.Verb, resource, restfulDeleteResource(gracefulDeleter, isGracefulDeleter, reqScope, admit)) + handler := metrics.InstrumentRouteFunc(action.Verb, resource, subresource, restfulDeleteResource(gracefulDeleter, isGracefulDeleter, reqScope, admit)) route := ws.DELETE(action.Path).To(handler). Doc(doc). Param(ws.QueryParameter("pretty", "If 'true', then the output is pretty printed.")). @@ -711,7 +711,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag if hasSubresource { doc = "delete collection of " + subresource + " of a " + kind } - handler := metrics.InstrumentRouteFunc(action.Verb, resource, restfulDeleteCollection(collectionDeleter, isCollectionDeleter, reqScope, admit)) + handler := metrics.InstrumentRouteFunc(action.Verb, resource, subresource, restfulDeleteCollection(collectionDeleter, isCollectionDeleter, reqScope, admit)) route := ws.DELETE(action.Path).To(handler). Doc(doc). Param(ws.QueryParameter("pretty", "If 'true', then the output is pretty printed.")). @@ -730,7 +730,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag if hasSubresource { doc = "watch changes to " + subresource + " of an object of kind " + kind } - handler := metrics.InstrumentRouteFunc(action.Verb, resource, restfulListResource(lister, watcher, reqScope, true, a.minRequestTimeout)) + handler := metrics.InstrumentRouteFunc(action.Verb, resource, subresource, restfulListResource(lister, watcher, reqScope, true, a.minRequestTimeout)) route := ws.GET(action.Path).To(handler). Doc(doc). Param(ws.QueryParameter("pretty", "If 'true', then the output is pretty printed.")). @@ -749,7 +749,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag if hasSubresource { doc = "watch individual changes to a list of " + subresource + " of " + kind } - handler := metrics.InstrumentRouteFunc(action.Verb, resource, restfulListResource(lister, watcher, reqScope, true, a.minRequestTimeout)) + handler := metrics.InstrumentRouteFunc(action.Verb, resource, subresource, restfulListResource(lister, watcher, reqScope, true, a.minRequestTimeout)) route := ws.GET(action.Path).To(handler). Doc(doc). Param(ws.QueryParameter("pretty", "If 'true', then the output is pretty printed.")). @@ -780,7 +780,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag if hasSubresource { doc = "connect " + method + " requests to " + subresource + " of " + kind } - handler := metrics.InstrumentRouteFunc(action.Verb, resource, restfulConnectResource(connecter, reqScope, admit, path, hasSubresource)) + handler := metrics.InstrumentRouteFunc(action.Verb, resource, subresource, restfulConnectResource(connecter, reqScope, admit, path, hasSubresource)) route := ws.Method(method).Path(action.Path). To(handler). Doc(doc). @@ -841,7 +841,7 @@ func buildProxyRoute(ws *restful.WebService, method string, prefix string, path if hasSubresource { doc = "proxy " + method + " requests to " + subresource + " of " + kind } - handler := metrics.InstrumentRouteFunc("PROXY", resource, routeFunction(proxyHandler)) + handler := metrics.InstrumentRouteFunc("PROXY", resource, subresource, routeFunction(proxyHandler)) proxyRoute := ws.Method(method).Path(path).To(handler). Doc(doc). Operation("proxy" + strings.Title(method) + namespaced + kind + strings.Title(subresource) + operationSuffix). diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/metrics/metrics.go b/staging/src/k8s.io/apiserver/pkg/endpoints/metrics/metrics.go index 88a363785d..2b5a3e0fd4 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/metrics/metrics.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/metrics/metrics.go @@ -39,7 +39,7 @@ var ( Name: "apiserver_request_count", Help: "Counter of apiserver requests broken out for each verb, API resource, client, and HTTP response contentType and code.", }, - []string{"verb", "resource", "client", "contentType", "code"}, + []string{"verb", "resource", "subresource", "client", "contentType", "code"}, ) requestLatencies = prometheus.NewHistogramVec( prometheus.HistogramOpts{ @@ -48,7 +48,7 @@ var ( // Use buckets ranging from 125 ms to 8 seconds. Buckets: prometheus.ExponentialBuckets(125000, 2.0, 7), }, - []string{"verb", "resource"}, + []string{"verb", "resource", "subresource"}, ) requestLatenciesSummary = prometheus.NewSummaryVec( prometheus.SummaryOpts{ @@ -57,7 +57,7 @@ var ( // Make the sliding window of 1h. MaxAge: time.Hour, }, - []string{"verb", "resource"}, + []string{"verb", "resource", "subresource"}, ) kubectlExeRegexp = regexp.MustCompile(`^.*((?i:kubectl\.exe))`) ) @@ -69,11 +69,11 @@ func Register() { prometheus.MustRegister(requestLatenciesSummary) } -func Monitor(verb, resource *string, client, contentType string, httpCode int, reqStart time.Time) { +func Monitor(verb, resource, subresource *string, client, contentType string, httpCode int, reqStart time.Time) { elapsed := float64((time.Since(reqStart)) / time.Microsecond) - requestCounter.WithLabelValues(*verb, *resource, client, contentType, codeToString(httpCode)).Inc() - requestLatencies.WithLabelValues(*verb, *resource).Observe(elapsed) - requestLatenciesSummary.WithLabelValues(*verb, *resource).Observe(elapsed) + requestCounter.WithLabelValues(*verb, *resource, *subresource, client, contentType, codeToString(httpCode)).Inc() + requestLatencies.WithLabelValues(*verb, *resource, *subresource).Observe(elapsed) + requestLatenciesSummary.WithLabelValues(*verb, *resource, *subresource).Observe(elapsed) } func Reset() { @@ -84,7 +84,7 @@ func Reset() { // InstrumentRouteFunc works like Prometheus' InstrumentHandlerFunc but wraps // the go-restful RouteFunction instead of a HandlerFunc -func InstrumentRouteFunc(verb, resource string, routeFunc restful.RouteFunction) restful.RouteFunction { +func InstrumentRouteFunc(verb, resource, subresource string, routeFunc restful.RouteFunction) restful.RouteFunction { return restful.RouteFunction(func(request *restful.Request, response *restful.Response) { now := time.Now() @@ -107,7 +107,7 @@ func InstrumentRouteFunc(verb, resource string, routeFunc restful.RouteFunction) if verb == "LIST" && strings.ToLower(request.QueryParameter("watch")) == "true" { reportedVerb = "WATCH" } - Monitor(&reportedVerb, &resource, cleanUserAgent(utilnet.GetHTTPClient(request.Request)), rw.Header().Get("Content-Type"), delegate.status, now) + Monitor(&reportedVerb, &resource, &subresource, cleanUserAgent(utilnet.GetHTTPClient(request.Request)), rw.Header().Get("Content-Type"), delegate.status, now) }) } diff --git a/test/e2e/framework/metrics_util.go b/test/e2e/framework/metrics_util.go index ce8e0133ad..71d4b4621e 100644 --- a/test/e2e/framework/metrics_util.go +++ b/test/e2e/framework/metrics_util.go @@ -188,10 +188,11 @@ type SaturationTime struct { } type APICall struct { - Resource string `json:"resource"` - Verb string `json:"verb"` - Latency LatencyMetric `json:"latency"` - Count int `json:"count"` + Resource string `json:"resource"` + Subresource string `json:"subresource"` + Verb string `json:"verb"` + Latency LatencyMetric `json:"latency"` + Count int `json:"count"` } type APIResponsiveness struct { @@ -221,14 +222,14 @@ func (a *APIResponsiveness) Less(i, j int) bool { // Set request latency for a particular quantile in the APICall metric entry (creating one if necessary). // 0 <= quantile <=1 (e.g. 0.95 is 95%tile, 0.5 is median) // Only 0.5, 0.9 and 0.99 quantiles are supported. -func (a *APIResponsiveness) addMetricRequestLatency(resource, verb string, quantile float64, latency time.Duration) { +func (a *APIResponsiveness) addMetricRequestLatency(resource, subresource, verb string, quantile float64, latency time.Duration) { for i, apicall := range a.APICalls { if apicall.Resource == resource && apicall.Verb == verb { a.APICalls[i] = setQuantileAPICall(apicall, quantile, latency) return } } - apicall := setQuantileAPICall(APICall{Resource: resource, Verb: verb}, quantile, latency) + apicall := setQuantileAPICall(APICall{Resource: resource, Subresource: subresource, Verb: verb}, quantile, latency) a.APICalls = append(a.APICalls, apicall) } @@ -252,14 +253,14 @@ func setQuantile(metric *LatencyMetric, quantile float64, latency time.Duration) } // Add request count to the APICall metric entry (creating one if necessary). -func (a *APIResponsiveness) addMetricRequestCount(resource, verb string, count int) { +func (a *APIResponsiveness) addMetricRequestCount(resource, subresource, verb string, count int) { for i, apicall := range a.APICalls { if apicall.Resource == resource && apicall.Verb == verb { a.APICalls[i].Count += count return } } - apicall := APICall{Resource: resource, Verb: verb, Count: count} + apicall := APICall{Resource: resource, Subresource: subresource, Verb: verb, Count: count} a.APICalls = append(a.APICalls, apicall) } @@ -290,6 +291,7 @@ func readLatencyMetrics(c clientset.Interface) (*APIResponsiveness, error) { } resource := string(sample.Metric["resource"]) + subresource := string(sample.Metric["subresource"]) verb := string(sample.Metric["verb"]) if ignoredResources.Has(resource) || ignoredVerbs.Has(verb) { continue @@ -302,10 +304,10 @@ func readLatencyMetrics(c clientset.Interface) (*APIResponsiveness, error) { if err != nil { return nil, err } - a.addMetricRequestLatency(resource, verb, quantile, time.Duration(int64(latency))*time.Microsecond) + a.addMetricRequestLatency(resource, subresource, verb, quantile, time.Duration(int64(latency))*time.Microsecond) case "apiserver_request_count": count := sample.Value - a.addMetricRequestCount(resource, verb, int(count)) + a.addMetricRequestCount(resource, subresource, verb, int(count)) } }