Merge pull request #106911 from aojea/client_go_metrics

Update client-go latency metrics bucket

staging/src/k8s.io/client-go/rest/request.go

@@ -496,84 +496,6 @@ func (r *Request) URL() *url.URL {
 	return finalURL
 }
 
-// finalURLTemplate is similar to URL(), but will make all specific parameter values equal
-// - instead of name or namespace, "{name}" and "{namespace}" will be used, and all query
-// parameters will be reset. This creates a copy of the url so as not to change the
-// underlying object.
-func (r Request) finalURLTemplate() url.URL {
-	newParams := url.Values{}
-	v := []string{"{value}"}
-	for k := range r.params {
-		newParams[k] = v
-	}
-	r.params = newParams
-	url := r.URL()
-
-	segments := strings.Split(url.Path, "/")
-	groupIndex := 0
-	index := 0
-	trimmedBasePath := ""
-	if url != nil && r.c.base != nil && strings.Contains(url.Path, r.c.base.Path) {
-		p := strings.TrimPrefix(url.Path, r.c.base.Path)
-		if !strings.HasPrefix(p, "/") {
-			p = "/" + p
-		}
-		// store the base path that we have trimmed so we can append it
-		// before returning the URL
-		trimmedBasePath = r.c.base.Path
-		segments = strings.Split(p, "/")
-		groupIndex = 1
-	}
-	if len(segments) <= 2 {
-		return *url
-	}
-
-	const CoreGroupPrefix = "api"
-	const NamedGroupPrefix = "apis"
-	isCoreGroup := segments[groupIndex] == CoreGroupPrefix
-	isNamedGroup := segments[groupIndex] == NamedGroupPrefix
-	if isCoreGroup {
-		// checking the case of core group with /api/v1/... format
-		index = groupIndex + 2
-	} else if isNamedGroup {
-		// checking the case of named group with /apis/apps/v1/... format
-		index = groupIndex + 3
-	} else {
-		// this should not happen that the only two possibilities are /api... and /apis..., just want to put an
-		// outlet here in case more API groups are added in future if ever possible:
-		// https://kubernetes.io/docs/concepts/overview/kubernetes-api/#api-groups
-		// if a wrong API groups name is encountered, return the {prefix} for url.Path
-		url.Path = "/{prefix}"
-		url.RawQuery = ""
-		return *url
-	}
-	//switch segLength := len(segments) - index; segLength {
-	switch {
-	// case len(segments) - index == 1:
-	// resource (with no name) do nothing
-	case len(segments)-index == 2:
-		// /$RESOURCE/$NAME: replace $NAME with {name}
-		segments[index+1] = "{name}"
-	case len(segments)-index == 3:
-		if segments[index+2] == "finalize" || segments[index+2] == "status" {
-			// /$RESOURCE/$NAME/$SUBRESOURCE: replace $NAME with {name}
-			segments[index+1] = "{name}"
-		} else {
-			// /namespace/$NAMESPACE/$RESOURCE: replace $NAMESPACE with {namespace}
-			segments[index+1] = "{namespace}"
-		}
-	case len(segments)-index >= 4:
-		segments[index+1] = "{namespace}"
-		// /namespace/$NAMESPACE/$RESOURCE/$NAME: replace $NAMESPACE with {namespace},  $NAME with {name}
-		if segments[index+3] != "finalize" && segments[index+3] != "status" {
-			// /$RESOURCE/$NAME/$SUBRESOURCE: replace $NAME with {name}
-			segments[index+3] = "{name}"
-		}
-	}
-	url.Path = path.Join(trimmedBasePath, path.Join(segments...))
-	return *url
-}
-
 func (r *Request) tryThrottleWithInfo(ctx context.Context, retryInfo string) error {
 	if r.rateLimiter == nil {
 		return nil
@@ -601,7 +523,7 @@ func (r *Request) tryThrottleWithInfo(ctx context.Context, retryInfo string) err
 		// but we use a throttled logger to prevent spamming.
 		globalThrottledLogger.Infof("%s", message)
 	}
-	metrics.RateLimiterLatency.Observe(ctx, r.verb, r.finalURLTemplate(), latency)
+	metrics.RateLimiterLatency.Observe(ctx, r.verb, *r.URL(), latency)
 
 	return err
 }
@@ -929,7 +851,7 @@ func (r *Request) request(ctx context.Context, fn func(*http.Request, *http.Resp
 	//Metrics for total request latency
 	start := time.Now()
 	defer func() {
-		metrics.RequestLatency.Observe(ctx, r.verb, r.finalURLTemplate(), time.Since(start))
+		metrics.RequestLatency.Observe(ctx, r.verb, *r.URL(), time.Since(start))
 	}()
 
 	if r.err != nil {

staging/src/k8s.io/client-go/rest/request_test.go

@@ -337,206 +337,6 @@ func TestResultIntoWithNoBodyReturnsErr(t *testing.T) {
 	}
 }
 
-func TestURLTemplate(t *testing.T) {
-	uri, _ := url.Parse("http://localhost/some/base/url/path")
-	uriSingleSlash, _ := url.Parse("http://localhost/")
-	testCases := []struct {
-		Request          *Request
-		ExpectedFullURL  string
-		ExpectedFinalURL string
-	}{
-		{
-			// non dynamic client
-			Request: NewRequestWithClient(uri, "", ClientContentConfig{GroupVersion: schema.GroupVersion{Group: "test"}}, nil).Verb("POST").
-				Prefix("api", "v1").Resource("r1").Namespace("ns").Name("nm").Param("p0", "v0"),
-			ExpectedFullURL:  "http://localhost/some/base/url/path/api/v1/namespaces/ns/r1/nm?p0=v0",
-			ExpectedFinalURL: "http://localhost/some/base/url/path/api/v1/namespaces/%7Bnamespace%7D/r1/%7Bname%7D?p0=%7Bvalue%7D",
-		},
-		{
-			// non dynamic client with wrong api group
-			Request: NewRequestWithClient(uri, "", ClientContentConfig{GroupVersion: schema.GroupVersion{Group: "test"}}, nil).Verb("POST").
-				Prefix("pre1", "v1").Resource("r1").Namespace("ns").Name("nm").Param("p0", "v0"),
-			ExpectedFullURL:  "http://localhost/some/base/url/path/pre1/v1/namespaces/ns/r1/nm?p0=v0",
-			ExpectedFinalURL: "http://localhost/%7Bprefix%7D",
-		},
-		{
-			// dynamic client with core group + namespace + resourceResource (with name)
-			// /api/$RESOURCEVERSION/namespaces/$NAMESPACE/$RESOURCE/%NAME
-			Request: NewRequestWithClient(uri, "", ClientContentConfig{GroupVersion: schema.GroupVersion{Group: "test"}}, nil).Verb("DELETE").
-				Prefix("/api/v1/namespaces/ns/r1/name1"),
-			ExpectedFullURL:  "http://localhost/some/base/url/path/api/v1/namespaces/ns/r1/name1",
-			ExpectedFinalURL: "http://localhost/some/base/url/path/api/v1/namespaces/%7Bnamespace%7D/r1/%7Bname%7D",
-		},
-		{
-			// dynamic client with named group + namespace + resourceResource (with name)
-			// /apis/$NAMEDGROUPNAME/$RESOURCEVERSION/namespaces/$NAMESPACE/$RESOURCE/%NAME
-			Request: NewRequestWithClient(uri, "", ClientContentConfig{GroupVersion: schema.GroupVersion{Group: "test"}}, nil).Verb("DELETE").
-				Prefix("/apis/g1/v1/namespaces/ns/r1/name1"),
-			ExpectedFullURL:  "http://localhost/some/base/url/path/apis/g1/v1/namespaces/ns/r1/name1",
-			ExpectedFinalURL: "http://localhost/some/base/url/path/apis/g1/v1/namespaces/%7Bnamespace%7D/r1/%7Bname%7D",
-		},
-		{
-			// dynamic client with core group + namespace + resourceResource (with NO name)
-			// /api/$RESOURCEVERSION/namespaces/$NAMESPACE/$RESOURCE
-			Request: NewRequestWithClient(uri, "", ClientContentConfig{GroupVersion: schema.GroupVersion{Group: "test"}}, nil).Verb("DELETE").
-				Prefix("/api/v1/namespaces/ns/r1"),
-			ExpectedFullURL:  "http://localhost/some/base/url/path/api/v1/namespaces/ns/r1",
-			ExpectedFinalURL: "http://localhost/some/base/url/path/api/v1/namespaces/%7Bnamespace%7D/r1",
-		},
-		{
-			// dynamic client with named group + namespace + resourceResource (with NO name)
-			// /apis/$NAMEDGROUPNAME/$RESOURCEVERSION/namespaces/$NAMESPACE/$RESOURCE
-			Request: NewRequestWithClient(uri, "", ClientContentConfig{GroupVersion: schema.GroupVersion{Group: "test"}}, nil).Verb("DELETE").
-				Prefix("/apis/g1/v1/namespaces/ns/r1"),
-			ExpectedFullURL:  "http://localhost/some/base/url/path/apis/g1/v1/namespaces/ns/r1",
-			ExpectedFinalURL: "http://localhost/some/base/url/path/apis/g1/v1/namespaces/%7Bnamespace%7D/r1",
-		},
-		{
-			// dynamic client with core group + resourceResource (with name)
-			// /api/$RESOURCEVERSION/$RESOURCE/%NAME
-			Request: NewRequestWithClient(uri, "", ClientContentConfig{GroupVersion: schema.GroupVersion{Group: "test"}}, nil).Verb("DELETE").
-				Prefix("/api/v1/r1/name1"),
-			ExpectedFullURL:  "http://localhost/some/base/url/path/api/v1/r1/name1",
-			ExpectedFinalURL: "http://localhost/some/base/url/path/api/v1/r1/%7Bname%7D",
-		},
-		{
-			// dynamic client with named group + resourceResource (with name)
-			// /apis/$NAMEDGROUPNAME/$RESOURCEVERSION/$RESOURCE/%NAME
-			Request: NewRequestWithClient(uri, "", ClientContentConfig{GroupVersion: schema.GroupVersion{Group: "test"}}, nil).Verb("DELETE").
-				Prefix("/apis/g1/v1/r1/name1"),
-			ExpectedFullURL:  "http://localhost/some/base/url/path/apis/g1/v1/r1/name1",
-			ExpectedFinalURL: "http://localhost/some/base/url/path/apis/g1/v1/r1/%7Bname%7D",
-		},
-		{
-			// dynamic client with named group + namespace + resourceResource (with name) + subresource
-			// /apis/$NAMEDGROUPNAME/$RESOURCEVERSION/namespaces/$NAMESPACE/$RESOURCE/%NAME/$SUBRESOURCE
-			Request: NewRequestWithClient(uri, "", ClientContentConfig{GroupVersion: schema.GroupVersion{Group: "test"}}, nil).Verb("DELETE").
-				Prefix("/apis/namespaces/namespaces/namespaces/namespaces/namespaces/namespaces/finalize"),
-			ExpectedFullURL:  "http://localhost/some/base/url/path/apis/namespaces/namespaces/namespaces/namespaces/namespaces/namespaces/finalize",
-			ExpectedFinalURL: "http://localhost/some/base/url/path/apis/namespaces/namespaces/namespaces/%7Bnamespace%7D/namespaces/%7Bname%7D/finalize",
-		},
-		{
-			// dynamic client with named group + namespace + resourceResource (with name)
-			// /apis/$NAMEDGROUPNAME/$RESOURCEVERSION/namespaces/$NAMESPACE/$RESOURCE/%NAME
-			Request: NewRequestWithClient(uri, "", ClientContentConfig{GroupVersion: schema.GroupVersion{Group: "test"}}, nil).Verb("DELETE").
-				Prefix("/apis/namespaces/namespaces/namespaces/namespaces/namespaces/namespaces"),
-			ExpectedFullURL:  "http://localhost/some/base/url/path/apis/namespaces/namespaces/namespaces/namespaces/namespaces/namespaces",
-			ExpectedFinalURL: "http://localhost/some/base/url/path/apis/namespaces/namespaces/namespaces/%7Bnamespace%7D/namespaces/%7Bname%7D",
-		},
-		{
-			// dynamic client with named group + namespace + resourceResource (with NO name) + subresource
-			// /apis/$NAMEDGROUPNAME/$RESOURCEVERSION/namespaces/$NAMESPACE/$RESOURCE/%SUBRESOURCE
-			Request: NewRequestWithClient(uri, "", ClientContentConfig{GroupVersion: schema.GroupVersion{Group: "test"}}, nil).Verb("DELETE").
-				Prefix("/apis/namespaces/namespaces/namespaces/namespaces/namespaces/finalize"),
-			ExpectedFullURL:  "http://localhost/some/base/url/path/apis/namespaces/namespaces/namespaces/namespaces/namespaces/finalize",
-			ExpectedFinalURL: "http://localhost/some/base/url/path/apis/namespaces/namespaces/namespaces/%7Bnamespace%7D/namespaces/finalize",
-		},
-		{
-			// dynamic client with named group + namespace + resourceResource (with NO name) + subresource
-			// /apis/$NAMEDGROUPNAME/$RESOURCEVERSION/namespaces/$NAMESPACE/$RESOURCE/%SUBRESOURCE
-			Request: NewRequestWithClient(uri, "", ClientContentConfig{GroupVersion: schema.GroupVersion{Group: "test"}}, nil).Verb("DELETE").
-				Prefix("/apis/namespaces/namespaces/namespaces/namespaces/namespaces/status"),
-			ExpectedFullURL:  "http://localhost/some/base/url/path/apis/namespaces/namespaces/namespaces/namespaces/namespaces/status",
-			ExpectedFinalURL: "http://localhost/some/base/url/path/apis/namespaces/namespaces/namespaces/%7Bnamespace%7D/namespaces/status",
-		},
-		{
-			// dynamic client with named group + namespace + resourceResource (with no name)
-			// /apis/$NAMEDGROUPNAME/$RESOURCEVERSION/namespaces/$NAMESPACE/$RESOURCE/%NAME
-			Request: NewRequestWithClient(uri, "", ClientContentConfig{GroupVersion: schema.GroupVersion{Group: "test"}}, nil).Verb("DELETE").
-				Prefix("/apis/namespaces/namespaces/namespaces/namespaces/namespaces"),
-			ExpectedFullURL:  "http://localhost/some/base/url/path/apis/namespaces/namespaces/namespaces/namespaces/namespaces",
-			ExpectedFinalURL: "http://localhost/some/base/url/path/apis/namespaces/namespaces/namespaces/%7Bnamespace%7D/namespaces",
-		},
-		{
-			// dynamic client with named group + resourceResource (with name) + subresource
-			// /apis/$NAMEDGROUPNAME/$RESOURCEVERSION/namespaces/$NAMESPACE/$RESOURCE/%NAME
-			Request: NewRequestWithClient(uri, "", ClientContentConfig{GroupVersion: schema.GroupVersion{Group: "test"}}, nil).Verb("DELETE").
-				Prefix("/apis/namespaces/namespaces/namespaces/namespaces/finalize"),
-			ExpectedFullURL:  "http://localhost/some/base/url/path/apis/namespaces/namespaces/namespaces/namespaces/finalize",
-			ExpectedFinalURL: "http://localhost/some/base/url/path/apis/namespaces/namespaces/namespaces/%7Bname%7D/finalize",
-		},
-		{
-			// dynamic client with named group + resourceResource (with name) + subresource
-			// /apis/$NAMEDGROUPNAME/$RESOURCEVERSION/namespaces/$NAMESPACE/$RESOURCE/%NAME
-			Request: NewRequestWithClient(uri, "", ClientContentConfig{GroupVersion: schema.GroupVersion{Group: "test"}}, nil).Verb("DELETE").
-				Prefix("/apis/namespaces/namespaces/namespaces/namespaces/status"),
-			ExpectedFullURL:  "http://localhost/some/base/url/path/apis/namespaces/namespaces/namespaces/namespaces/status",
-			ExpectedFinalURL: "http://localhost/some/base/url/path/apis/namespaces/namespaces/namespaces/%7Bname%7D/status",
-		},
-		{
-			// dynamic client with named group + resourceResource (with name)
-			// /apis/$NAMEDGROUPNAME/$RESOURCEVERSION/$RESOURCE/%NAME
-			Request: NewRequestWithClient(uri, "", ClientContentConfig{GroupVersion: schema.GroupVersion{Group: "test"}}, nil).Verb("DELETE").
-				Prefix("/apis/namespaces/namespaces/namespaces/namespaces"),
-			ExpectedFullURL:  "http://localhost/some/base/url/path/apis/namespaces/namespaces/namespaces/namespaces",
-			ExpectedFinalURL: "http://localhost/some/base/url/path/apis/namespaces/namespaces/namespaces/%7Bname%7D",
-		},
-		{
-			// dynamic client with named group + resourceResource (with no name)
-			// /apis/$NAMEDGROUPNAME/$RESOURCEVERSION/$RESOURCE/%NAME
-			Request: NewRequestWithClient(uri, "", ClientContentConfig{GroupVersion: schema.GroupVersion{Group: "test"}}, nil).Verb("DELETE").
-				Prefix("/apis/namespaces/namespaces/namespaces"),
-			ExpectedFullURL:  "http://localhost/some/base/url/path/apis/namespaces/namespaces/namespaces",
-			ExpectedFinalURL: "http://localhost/some/base/url/path/apis/namespaces/namespaces/namespaces",
-		},
-		{
-			// dynamic client with wrong api group + namespace + resourceResource (with name) + subresource
-			// /apis/$NAMEDGROUPNAME/$RESOURCEVERSION/namespaces/$NAMESPACE/$RESOURCE/%NAME/$SUBRESOURCE
-			Request: NewRequestWithClient(uri, "", ClientContentConfig{GroupVersion: schema.GroupVersion{Group: "test"}}, nil).Verb("DELETE").
-				Prefix("/pre1/namespaces/namespaces/namespaces/namespaces/namespaces/namespaces/finalize"),
-			ExpectedFullURL:  "http://localhost/some/base/url/path/pre1/namespaces/namespaces/namespaces/namespaces/namespaces/namespaces/finalize",
-			ExpectedFinalURL: "http://localhost/%7Bprefix%7D",
-		},
-		{
-			// dynamic client with core group + namespace + resourceResource (with name) where baseURL is a single /
-			// /api/$RESOURCEVERSION/namespaces/$NAMESPACE/$RESOURCE/%NAME
-			Request: NewRequestWithClient(uriSingleSlash, "", ClientContentConfig{GroupVersion: schema.GroupVersion{Group: "test"}}, nil).Verb("DELETE").
-				Prefix("/api/v1/namespaces/ns/r2/name1"),
-			ExpectedFullURL:  "http://localhost/api/v1/namespaces/ns/r2/name1",
-			ExpectedFinalURL: "http://localhost/api/v1/namespaces/%7Bnamespace%7D/r2/%7Bname%7D",
-		},
-		{
-			// dynamic client with core group + namespace + resourceResource (with name) where baseURL is 'some/base/url/path'
-			// /api/$RESOURCEVERSION/namespaces/$NAMESPACE/$RESOURCE/%NAME
-			Request: NewRequestWithClient(uri, "", ClientContentConfig{GroupVersion: schema.GroupVersion{Group: "test"}}, nil).Verb("DELETE").
-				Prefix("/api/v1/namespaces/ns/r3/name1"),
-			ExpectedFullURL:  "http://localhost/some/base/url/path/api/v1/namespaces/ns/r3/name1",
-			ExpectedFinalURL: "http://localhost/some/base/url/path/api/v1/namespaces/%7Bnamespace%7D/r3/%7Bname%7D",
-		},
-		{
-			// dynamic client where baseURL is a single /
-			// /
-			Request: NewRequestWithClient(uriSingleSlash, "", ClientContentConfig{GroupVersion: schema.GroupVersion{Group: "test"}}, nil).Verb("DELETE").
-				Prefix("/"),
-			ExpectedFullURL:  "http://localhost/",
-			ExpectedFinalURL: "http://localhost/",
-		},
-		{
-			// dynamic client where baseURL is a single /
-			// /version
-			Request: NewRequestWithClient(uriSingleSlash, "", ClientContentConfig{GroupVersion: schema.GroupVersion{Group: "test"}}, nil).Verb("DELETE").
-				Prefix("/version"),
-			ExpectedFullURL:  "http://localhost/version",
-			ExpectedFinalURL: "http://localhost/version",
-		},
-	}
-	for i, testCase := range testCases {
-		r := testCase.Request
-		full := r.URL()
-		if full.String() != testCase.ExpectedFullURL {
-			t.Errorf("%d: unexpected initial URL: %s %s", i, full, testCase.ExpectedFullURL)
-		}
-		actualURL := r.finalURLTemplate()
-		actual := actualURL.String()
-		if actual != testCase.ExpectedFinalURL {
-			t.Errorf("%d: unexpected URL template: %s %s", i, actual, testCase.ExpectedFinalURL)
-		}
-		if r.URL().String() != full.String() {
-			t.Errorf("%d, creating URL template changed request: %s -> %s", i, full.String(), r.URL().String())
-		}
-	}
-}
-
 func TestTransformResponse(t *testing.T) {
 	invalid := []byte("aaaaa")
 	uri, _ := url.Parse("http://localhost")

staging/src/k8s.io/component-base/metrics/prometheus/restclient/metrics.go

@@ -35,7 +35,7 @@ var (
 		&k8smetrics.HistogramOpts{
 			Name:    "rest_client_request_duration_seconds",
 			Help:    "Request latency in seconds. Broken down by verb, and host.",
-			Buckets: k8smetrics.ExponentialBuckets(0.001, 2, 10),
+			Buckets: []float64{0.005, 0.025, 0.1, 0.25, 0.5, 1.0, 2.0, 4.0, 8.0, 15.0, 30.0, 60.0},
 		},
 		[]string{"verb", "host"},
 	)
@@ -44,7 +44,7 @@ var (
 		&k8smetrics.HistogramOpts{
 			Name:    "rest_client_rate_limiter_duration_seconds",
 			Help:    "Client side rate limiter latency in seconds. Broken down by verb, and host.",
-			Buckets: k8smetrics.ExponentialBuckets(0.001, 2, 10),
+			Buckets: []float64{0.005, 0.025, 0.1, 0.25, 0.5, 1.0, 2.0, 4.0, 8.0, 15.0, 30.0, 60.0},
 		},
 		[]string{"verb", "host"},
 	)