Merge pull request #99788 from logicalhan/stable-m

promote apiserver_request_total to STABLE status

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 <requestInfo>, 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 != "" {

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)
-			}
-		})
-	}
-}

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)

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

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) {