remove client label from apiserver request count metric since it is unbounded
Change-Id: I3a9eacebc9d9dc9ed6347260d9378cdcb5743431 Kubernetes-commit: cc3190968b1f14ddf4067abef849fc41bd6068dc
This commit is contained in:
		
							parent
							
								
									b5db01569a
								
							
						
					
					
						commit
						fe6033d4a2
					
				|  | @ -31,7 +31,6 @@ import ( | ||||||
| 
 | 
 | ||||||
| 	"k8s.io/apimachinery/pkg/apis/meta/v1/validation" | 	"k8s.io/apimachinery/pkg/apis/meta/v1/validation" | ||||||
| 	"k8s.io/apimachinery/pkg/types" | 	"k8s.io/apimachinery/pkg/types" | ||||||
| 	utilnet "k8s.io/apimachinery/pkg/util/net" |  | ||||||
| 	utilsets "k8s.io/apimachinery/pkg/util/sets" | 	utilsets "k8s.io/apimachinery/pkg/util/sets" | ||||||
| 	"k8s.io/apiserver/pkg/endpoints/request" | 	"k8s.io/apiserver/pkg/endpoints/request" | ||||||
| 	"k8s.io/apiserver/pkg/features" | 	"k8s.io/apiserver/pkg/features" | ||||||
|  | @ -65,14 +64,14 @@ var ( | ||||||
| 	requestCounter = compbasemetrics.NewCounterVec( | 	requestCounter = compbasemetrics.NewCounterVec( | ||||||
| 		&compbasemetrics.CounterOpts{ | 		&compbasemetrics.CounterOpts{ | ||||||
| 			Name:           "apiserver_request_total", | 			Name:           "apiserver_request_total", | ||||||
| 			Help:           "Counter of apiserver requests broken out for each verb, dry run value, group, version, resource, scope, component, client, and HTTP response contentType and code.", | 			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, | 			StabilityLevel: compbasemetrics.ALPHA, | ||||||
| 		}, | 		}, | ||||||
| 		// The label_name contentType doesn't follow the label_name convention defined here:
 | 		// 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
 | 		// https://github.com/kubernetes/community/blob/master/contributors/devel/sig-instrumentation/instrumentation.md
 | ||||||
| 		// But changing it would break backwards compatibility. Future label_names
 | 		// But changing it would break backwards compatibility. Future label_names
 | ||||||
| 		// should be all lowercase and separated by underscores.
 | 		// should be all lowercase and separated by underscores.
 | ||||||
| 		[]string{"verb", "dry_run", "group", "version", "resource", "subresource", "scope", "component", "client", "contentType", "code"}, | 		[]string{"verb", "dry_run", "group", "version", "resource", "subresource", "scope", "component", "contentType", "code"}, | ||||||
| 	) | 	) | ||||||
| 	longRunningRequestGauge = compbasemetrics.NewGaugeVec( | 	longRunningRequestGauge = compbasemetrics.NewGaugeVec( | ||||||
| 		&compbasemetrics.GaugeOpts{ | 		&compbasemetrics.GaugeOpts{ | ||||||
|  | @ -256,9 +255,8 @@ func RecordLongRunning(req *http.Request, requestInfo *request.RequestInfo, comp | ||||||
| func MonitorRequest(req *http.Request, verb, group, version, resource, subresource, scope, component, contentType string, httpCode, respSize int, elapsed time.Duration) { | func MonitorRequest(req *http.Request, verb, group, version, resource, subresource, scope, component, contentType string, httpCode, respSize int, elapsed time.Duration) { | ||||||
| 	reportedVerb := cleanVerb(verb, req) | 	reportedVerb := cleanVerb(verb, req) | ||||||
| 	dryRun := cleanDryRun(req.URL) | 	dryRun := cleanDryRun(req.URL) | ||||||
| 	client := cleanUserAgent(utilnet.GetHTTPClient(req)) |  | ||||||
| 	elapsedSeconds := elapsed.Seconds() | 	elapsedSeconds := elapsed.Seconds() | ||||||
| 	requestCounter.WithLabelValues(reportedVerb, dryRun, group, version, resource, subresource, scope, component, client, contentType, codeToString(httpCode)).Inc() | 	requestCounter.WithLabelValues(reportedVerb, dryRun, group, version, resource, subresource, scope, component, contentType, codeToString(httpCode)).Inc() | ||||||
| 	requestLatencies.WithLabelValues(reportedVerb, dryRun, group, version, resource, subresource, scope, component).Observe(elapsedSeconds) | 	requestLatencies.WithLabelValues(reportedVerb, dryRun, group, version, resource, subresource, scope, component).Observe(elapsedSeconds) | ||||||
| 	// We are only interested in response sizes of read requests.
 | 	// We are only interested in response sizes of read requests.
 | ||||||
| 	if verb == "GET" || verb == "LIST" { | 	if verb == "GET" || verb == "LIST" { | ||||||
|  | @ -376,19 +374,6 @@ func cleanDryRun(u *url.URL) string { | ||||||
| 	return strings.Join(utilsets.NewString(dryRun...).List(), ",") | 	return strings.Join(utilsets.NewString(dryRun...).List(), ",") | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| func cleanUserAgent(ua string) string { |  | ||||||
| 	// We collapse all "web browser"-type user agents into one "browser" to reduce metric cardinality.
 |  | ||||||
| 	if strings.HasPrefix(ua, "Mozilla/") { |  | ||||||
| 		return "Browser" |  | ||||||
| 	} |  | ||||||
| 	// If an old "kubectl.exe" has passed us its full path, we discard the path portion.
 |  | ||||||
| 	if kubectlExeRegexp.MatchString(ua) { |  | ||||||
| 		// avoid an allocation
 |  | ||||||
| 		ua = kubectlExeRegexp.ReplaceAllString(ua, "$1") |  | ||||||
| 	} |  | ||||||
| 	return ua |  | ||||||
| } |  | ||||||
| 
 |  | ||||||
| // ResponseWriterDelegator interface wraps http.ResponseWriter to additionally record content-length, status-code, etc.
 | // ResponseWriterDelegator interface wraps http.ResponseWriter to additionally record content-length, status-code, etc.
 | ||||||
| type ResponseWriterDelegator struct { | type ResponseWriterDelegator struct { | ||||||
| 	http.ResponseWriter | 	http.ResponseWriter | ||||||
|  |  | ||||||
|  | @ -1,54 +0,0 @@ | ||||||
| /* |  | ||||||
| Copyright 2015 The Kubernetes Authors. |  | ||||||
| 
 |  | ||||||
| Licensed under the Apache License, Version 2.0 (the "License"); |  | ||||||
| you may not use this file except in compliance with the License. |  | ||||||
| You may obtain a copy of the License at |  | ||||||
| 
 |  | ||||||
|     http://www.apache.org/licenses/LICENSE-2.0
 |  | ||||||
| 
 |  | ||||||
| Unless required by applicable law or agreed to in writing, software |  | ||||||
| distributed under the License is distributed on an "AS IS" BASIS, |  | ||||||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. |  | ||||||
| See the License for the specific language governing permissions and |  | ||||||
| limitations under the License. |  | ||||||
| */ |  | ||||||
| 
 |  | ||||||
| package metrics |  | ||||||
| 
 |  | ||||||
| import "testing" |  | ||||||
| 
 |  | ||||||
| func TestCleanUserAgent(t *testing.T) { |  | ||||||
| 	panicBuf := []byte{198, 73, 129, 133, 90, 216, 104, 29, 13, 134, 209, 233, 30, 0, 22} |  | ||||||
| 
 |  | ||||||
| 	for _, tc := range []struct { |  | ||||||
| 		In  string |  | ||||||
| 		Out string |  | ||||||
| 	}{ |  | ||||||
| 		{ |  | ||||||
| 			In:  "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/52.0.2743.116 Safari/537.36", |  | ||||||
| 			Out: "Browser", |  | ||||||
| 		}, |  | ||||||
| 		{ |  | ||||||
| 			In:  "kubectl/v1.2.4", |  | ||||||
| 			Out: "kubectl/v1.2.4", |  | ||||||
| 		}, |  | ||||||
| 		{ |  | ||||||
| 			In:  `C:\Users\Kubernetes\kubectl.exe/v1.5.4`, |  | ||||||
| 			Out: "kubectl.exe/v1.5.4", |  | ||||||
| 		}, |  | ||||||
| 		{ |  | ||||||
| 			In:  `C:\Program Files\kubectl.exe/v1.5.4`, |  | ||||||
| 			Out: "kubectl.exe/v1.5.4", |  | ||||||
| 		}, |  | ||||||
| 		{ |  | ||||||
| 			// This malicious input courtesy of enisoc.
 |  | ||||||
| 			In:  string(panicBuf) + "kubectl.exe", |  | ||||||
| 			Out: "kubectl.exe", |  | ||||||
| 		}, |  | ||||||
| 	} { |  | ||||||
| 		if cleanUserAgent(tc.In) != tc.Out { |  | ||||||
| 			t.Errorf("Failed to clean User-Agent: %s", tc.In) |  | ||||||
| 		} |  | ||||||
| 	} |  | ||||||
| } |  | ||||||
		Loading…
	
		Reference in New Issue