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 43ccc2e8945..517411c5789 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/metrics/metrics.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/metrics/metrics.go @@ -49,7 +49,6 @@ type resettableCollector interface { const ( APIServerComponent string = "apiserver" - OtherContentType string = "other" OtherRequestMethod string = "other" ) @@ -76,14 +75,10 @@ var ( requestCounter = compbasemetrics.NewCounterVec( &compbasemetrics.CounterOpts{ Name: "apiserver_request_total", - Help: "Counter of apiserver requests broken out for each verb, dry run value, group, version, resource, scope, component, and HTTP response contentType and code.", - StabilityLevel: compbasemetrics.ALPHA, + Help: "Counter of apiserver requests broken out for each verb, dry run value, group, version, resource, scope, component, and HTTP response code.", + StabilityLevel: compbasemetrics.STABLE, }, - // The label_name contentType doesn't follow the label_name convention defined here: - // https://github.com/kubernetes/community/blob/master/contributors/devel/sig-instrumentation/instrumentation.md - // But changing it would break backwards compatibility. Future label_names - // should be all lowercase and separated by underscores. - []string{"verb", "dry_run", "group", "version", "resource", "subresource", "scope", "component", "contentType", "code"}, + []string{"verb", "dry_run", "group", "version", "resource", "subresource", "scope", "component", "code"}, ) longRunningRequestGauge = compbasemetrics.NewGaugeVec( &compbasemetrics.GaugeOpts{ @@ -235,19 +230,6 @@ var ( requestAbortsTotal, } - // these are the known (e.g. whitelisted/known) content types which we will report for - // request metrics. Any other RFC compliant content types will be aggregated under 'unknown' - knownMetricContentTypes = utilsets.NewString( - "application/apply-patch+yaml", - "application/json", - "application/json-patch+json", - "application/merge-patch+json", - "application/strategic-merge-patch+json", - "application/vnd.kubernetes.protobuf", - "application/vnd.kubernetes.protobuf;stream=watch", - "application/yaml", - "text/plain", - "text/plain;charset=utf-8") // these are the valid request methods which we report in our metrics. Any other request methods // will be aggregated under 'unknown' validRequestMethods = utilsets.NewString( @@ -392,7 +374,7 @@ func RecordLongRunning(req *http.Request, requestInfo *request.RequestInfo, comp // 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(req *http.Request, verb, group, version, resource, subresource, scope, component string, deprecated bool, removedRelease string, contentType string, httpCode, respSize int, elapsed time.Duration) { +func MonitorRequest(req *http.Request, verb, group, version, resource, subresource, scope, component string, deprecated bool, removedRelease string, httpCode, respSize int, elapsed time.Duration) { // We don't use verb from , as this may be propagated from // InstrumentRouteFunc which is registered in installer.go with predefined // list of verbs (different than those translated to RequestInfo). @@ -401,8 +383,7 @@ func MonitorRequest(req *http.Request, verb, group, version, resource, subresour dryRun := cleanDryRun(req.URL) elapsedSeconds := elapsed.Seconds() - cleanContentType := cleanContentType(contentType) - requestCounter.WithContext(req.Context()).WithLabelValues(reportedVerb, dryRun, group, version, resource, subresource, scope, component, cleanContentType, codeToString(httpCode)).Inc() + requestCounter.WithContext(req.Context()).WithLabelValues(reportedVerb, dryRun, group, version, resource, subresource, scope, component, codeToString(httpCode)).Inc() // MonitorRequest happens after authentication, so we can trust the username given by the request info, ok := request.UserFrom(req.Context()) if ok && info.GetName() == user.APIServerUser { @@ -446,7 +427,7 @@ func InstrumentRouteFunc(verb, group, version, resource, subresource, scope, com routeFunc(req, response) - MonitorRequest(req.Request, verb, group, version, resource, subresource, scope, component, deprecated, removedRelease, delegate.Header().Get("Content-Type"), delegate.Status(), delegate.ContentLength(), time.Since(requestReceivedTimestamp)) + MonitorRequest(req.Request, verb, group, version, resource, subresource, scope, component, deprecated, removedRelease, delegate.Status(), delegate.ContentLength(), time.Since(requestReceivedTimestamp)) }) } @@ -471,23 +452,10 @@ func InstrumentHandlerFunc(verb, group, version, resource, subresource, scope, c handler(w, req) - MonitorRequest(req, verb, group, version, resource, subresource, scope, component, deprecated, removedRelease, delegate.Header().Get("Content-Type"), delegate.Status(), delegate.ContentLength(), time.Since(requestReceivedTimestamp)) + MonitorRequest(req, verb, group, version, resource, subresource, scope, component, deprecated, removedRelease, delegate.Status(), delegate.ContentLength(), time.Since(requestReceivedTimestamp)) } } -// cleanContentType binds the contentType (for metrics related purposes) to a -// bounded set of known/expected content-types. -func cleanContentType(contentType string) string { - normalizedContentType := strings.ToLower(contentType) - if strings.HasSuffix(contentType, " stream=watch") || strings.HasSuffix(contentType, " charset=utf-8") { - normalizedContentType = strings.ReplaceAll(contentType, " ", "") - } - if knownMetricContentTypes.Has(normalizedContentType) { - return normalizedContentType - } - return OtherContentType -} - // CleanScope returns the scope of the request. func CleanScope(requestInfo *request.RequestInfo) string { if requestInfo.Namespace != "" { diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/metrics/metrics_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/metrics/metrics_test.go index 7f8791f6e2d..d620a44f423 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/metrics/metrics_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/metrics/metrics_test.go @@ -17,7 +17,6 @@ limitations under the License. package metrics import ( - "fmt" "net/http" "net/url" "testing" @@ -110,44 +109,3 @@ func TestCleanVerb(t *testing.T) { }) } } - -func TestContentType(t *testing.T) { - testCases := []struct { - rawContentType string - expectedContentType string - }{ - { - rawContentType: "application/json", - expectedContentType: "application/json", - }, - { - rawContentType: "image/svg+xml", - expectedContentType: "other", - }, - { - rawContentType: "text/plain; charset=utf-8", - expectedContentType: "text/plain;charset=utf-8", - }, - { - rawContentType: "application/json;foo=bar", - expectedContentType: "other", - }, - { - rawContentType: "application/json;charset=hancoding", - expectedContentType: "other", - }, - { - rawContentType: "unknownbutvalidtype", - expectedContentType: "other", - }, - } - - for _, tt := range testCases { - t.Run(fmt.Sprintf("parse %s", tt.rawContentType), func(t *testing.T) { - cleansedContentType := cleanContentType(tt.rawContentType) - if cleansedContentType != tt.expectedContentType { - t.Errorf("Got %s, but expected %s", cleansedContentType, tt.expectedContentType) - } - }) - } -} diff --git a/staging/src/k8s.io/apiserver/pkg/server/healthz/healthz_test.go b/staging/src/k8s.io/apiserver/pkg/server/healthz/healthz_test.go index ef5a7827c3e..0fb67c84570 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/healthz/healthz_test.go +++ b/staging/src/k8s.io/apiserver/pkg/server/healthz/healthz_test.go @@ -252,11 +252,11 @@ func TestMetrics(t *testing.T) { } expected := strings.NewReader(` - # HELP apiserver_request_total [ALPHA] Counter of apiserver requests broken out for each verb, dry run value, group, version, resource, scope, component, and HTTP response contentType and code. + # HELP apiserver_request_total [STABLE] Counter of apiserver requests broken out for each verb, dry run value, group, version, resource, scope, component, and HTTP response code. # TYPE apiserver_request_total counter - apiserver_request_total{code="200",component="",contentType="text/plain;charset=utf-8",dry_run="",group="",resource="",scope="",subresource="/healthz",verb="GET",version=""} 1 - apiserver_request_total{code="200",component="",contentType="text/plain;charset=utf-8",dry_run="",group="",resource="",scope="",subresource="/livez",verb="GET",version=""} 1 - apiserver_request_total{code="200",component="",contentType="text/plain;charset=utf-8",dry_run="",group="",resource="",scope="",subresource="/readyz",verb="GET",version=""} 1 + apiserver_request_total{code="200",component="",dry_run="",group="",resource="",scope="",subresource="/healthz",verb="GET",version=""} 1 + apiserver_request_total{code="200",component="",dry_run="",group="",resource="",scope="",subresource="/livez",verb="GET",version=""} 1 + apiserver_request_total{code="200",component="",dry_run="",group="",resource="",scope="",subresource="/readyz",verb="GET",version=""} 1 `) if err := testutil.GatherAndCompare(legacyregistry.DefaultGatherer, expected, "apiserver_request_total"); err != nil { t.Error(err) diff --git a/test/instrumentation/testdata/stable-metrics-list.yaml b/test/instrumentation/testdata/stable-metrics-list.yaml index e69de29bb2d..024feccd039 100644 --- a/test/instrumentation/testdata/stable-metrics-list.yaml +++ b/test/instrumentation/testdata/stable-metrics-list.yaml @@ -0,0 +1,15 @@ +- name: apiserver_request_total + help: Counter of apiserver requests broken out for each verb, dry run value, group, + version, resource, scope, component, and HTTP response code. + type: Counter + stabilityLevel: STABLE + labels: + - code + - component + - dry_run + - group + - resource + - scope + - subresource + - verb + - version diff --git a/test/integration/metrics/metrics_test.go b/test/integration/metrics/metrics_test.go index bdda9c0ce96..883925e90e6 100644 --- a/test/integration/metrics/metrics_test.go +++ b/test/integration/metrics/metrics_test.go @@ -287,42 +287,42 @@ func TestApiserverMetricsPods(t *testing.T) { executor: func() { callOrDie(c.Create(context.TODO(), makePod("foo"), metav1.CreateOptions{})) }, - want: `apiserver_request_total{code="201", component="apiserver", contentType="application/json", dry_run="", group="", resource="pods", scope="resource", subresource="", verb="POST", version="v1"}`, + want: `apiserver_request_total{code="201", component="apiserver", dry_run="", group="", resource="pods", scope="resource", subresource="", verb="POST", version="v1"}`, }, { name: "update pod", executor: func() { callOrDie(c.Update(context.TODO(), makePod("bar"), metav1.UpdateOptions{})) }, - want: `apiserver_request_total{code="200", component="apiserver", contentType="application/json", dry_run="", group="", resource="pods", scope="resource", subresource="", verb="PUT", version="v1"}`, + want: `apiserver_request_total{code="200", component="apiserver", dry_run="", group="", resource="pods", scope="resource", subresource="", verb="PUT", version="v1"}`, }, { name: "update pod status", executor: func() { callOrDie(c.UpdateStatus(context.TODO(), makePod("bar"), metav1.UpdateOptions{})) }, - want: `apiserver_request_total{code="200", component="apiserver", contentType="application/json", dry_run="", group="", resource="pods", scope="resource", subresource="status", verb="PUT", version="v1"}`, + want: `apiserver_request_total{code="200", component="apiserver", dry_run="", group="", resource="pods", scope="resource", subresource="status", verb="PUT", version="v1"}`, }, { name: "get pod", executor: func() { callOrDie(c.Get(context.TODO(), "foo", metav1.GetOptions{})) }, - want: `apiserver_request_total{code="200", component="apiserver", contentType="application/json", dry_run="", group="", resource="pods", scope="resource", subresource="", verb="GET", version="v1"}`, + want: `apiserver_request_total{code="200", component="apiserver", dry_run="", group="", resource="pods", scope="resource", subresource="", verb="GET", version="v1"}`, }, { name: "list pod", executor: func() { callOrDie(c.List(context.TODO(), metav1.ListOptions{})) }, - want: `apiserver_request_total{code="200", component="apiserver", contentType="application/json", dry_run="", group="", resource="pods", scope="namespace", subresource="", verb="LIST", version="v1"}`, + want: `apiserver_request_total{code="200", component="apiserver", dry_run="", group="", resource="pods", scope="namespace", subresource="", verb="LIST", version="v1"}`, }, { name: "delete pod", executor: func() { callOrDie(nil, c.Delete(context.TODO(), "foo", metav1.DeleteOptions{})) }, - want: `apiserver_request_total{code="200", component="apiserver", contentType="application/json", dry_run="", group="", resource="pods", scope="resource", subresource="", verb="DELETE", version="v1"}`, + want: `apiserver_request_total{code="200", component="apiserver", dry_run="", group="", resource="pods", scope="resource", subresource="", verb="DELETE", version="v1"}`, }, } { t.Run(tc.name, func(t *testing.T) { @@ -393,42 +393,42 @@ func TestApiserverMetricsNamespaces(t *testing.T) { executor: func() { callOrDie(c.Create(context.TODO(), makeNamespace("foo"), metav1.CreateOptions{})) }, - want: `apiserver_request_total{code="201", component="apiserver", contentType="application/json", dry_run="", group="", resource="namespaces", scope="resource", subresource="", verb="POST", version="v1"}`, + want: `apiserver_request_total{code="201", component="apiserver", dry_run="", group="", resource="namespaces", scope="resource", subresource="", verb="POST", version="v1"}`, }, { name: "update namespace", executor: func() { callOrDie(c.Update(context.TODO(), makeNamespace("bar"), metav1.UpdateOptions{})) }, - want: `apiserver_request_total{code="200", component="apiserver", contentType="application/json", dry_run="", group="", resource="namespaces", scope="resource", subresource="", verb="PUT", version="v1"}`, + want: `apiserver_request_total{code="200", component="apiserver", dry_run="", group="", resource="namespaces", scope="resource", subresource="", verb="PUT", version="v1"}`, }, { name: "update namespace status", executor: func() { callOrDie(c.UpdateStatus(context.TODO(), makeNamespace("bar"), metav1.UpdateOptions{})) }, - want: `apiserver_request_total{code="200", component="apiserver", contentType="application/json", dry_run="", group="", resource="namespaces", scope="resource", subresource="status", verb="PUT", version="v1"}`, + want: `apiserver_request_total{code="200", component="apiserver", dry_run="", group="", resource="namespaces", scope="resource", subresource="status", verb="PUT", version="v1"}`, }, { name: "get namespace", executor: func() { callOrDie(c.Get(context.TODO(), "foo", metav1.GetOptions{})) }, - want: `apiserver_request_total{code="200", component="apiserver", contentType="application/json", dry_run="", group="", resource="namespaces", scope="resource", subresource="", verb="GET", version="v1"}`, + want: `apiserver_request_total{code="200", component="apiserver", dry_run="", group="", resource="namespaces", scope="resource", subresource="", verb="GET", version="v1"}`, }, { name: "list namespace", executor: func() { callOrDie(c.List(context.TODO(), metav1.ListOptions{})) }, - want: `apiserver_request_total{code="200", component="apiserver", contentType="application/json", dry_run="", group="", resource="namespaces", scope="cluster", subresource="", verb="LIST", version="v1"}`, + want: `apiserver_request_total{code="200", component="apiserver", dry_run="", group="", resource="namespaces", scope="cluster", subresource="", verb="LIST", version="v1"}`, }, { name: "delete namespace", executor: func() { callOrDie(nil, c.Delete(context.TODO(), "foo", metav1.DeleteOptions{})) }, - want: `apiserver_request_total{code="200", component="apiserver", contentType="application/json", dry_run="", group="", resource="namespaces", scope="resource", subresource="", verb="DELETE", version="v1"}`, + want: `apiserver_request_total{code="200", component="apiserver", dry_run="", group="", resource="namespaces", scope="resource", subresource="", verb="DELETE", version="v1"}`, }, } { t.Run(tc.name, func(t *testing.T) {