Merge pull request #104161 from wojtek-t/fix_metrics_reporting_2

Fix metrics reporting for the deprecated watch path
This commit is contained in:
Kubernetes Prow Robot 2021-08-05 16:37:18 -07:00 committed by GitHub
commit 1658e13cae
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 40 additions and 10 deletions

View File

@ -372,7 +372,7 @@ func RecordRequestAbort(req *http.Request, requestInfo *request.RequestInfo) {
}
scope := CleanScope(requestInfo)
reportedVerb := cleanVerb(CanonicalVerb(strings.ToUpper(req.Method), scope), req)
reportedVerb := cleanVerb(CanonicalVerb(strings.ToUpper(req.Method), scope), "", req)
resource := requestInfo.Resource
subresource := requestInfo.Subresource
group := requestInfo.APIGroup
@ -395,7 +395,7 @@ func RecordRequestTermination(req *http.Request, requestInfo *request.RequestInf
// InstrumentRouteFunc which is registered in installer.go with predefined
// list of verbs (different than those translated to RequestInfo).
// However, we need to tweak it e.g. to differentiate GET from LIST.
reportedVerb := cleanVerb(CanonicalVerb(strings.ToUpper(req.Method), scope), req)
reportedVerb := cleanVerb(CanonicalVerb(strings.ToUpper(req.Method), scope), "", req)
if requestInfo.IsResourceRequest {
requestTerminationsTotal.WithContext(req.Context()).WithLabelValues(reportedVerb, requestInfo.APIGroup, requestInfo.APIVersion, requestInfo.Resource, requestInfo.Subresource, scope, component, codeToString(code)).Inc()
@ -417,7 +417,7 @@ func RecordLongRunning(req *http.Request, requestInfo *request.RequestInfo, comp
// InstrumentRouteFunc which is registered in installer.go with predefined
// list of verbs (different than those translated to RequestInfo).
// However, we need to tweak it e.g. to differentiate GET from LIST.
reportedVerb := cleanVerb(CanonicalVerb(strings.ToUpper(req.Method), scope), req)
reportedVerb := cleanVerb(CanonicalVerb(strings.ToUpper(req.Method), scope), "", req)
if requestInfo.IsResourceRequest {
g = longRunningRequestGauge.WithContext(req.Context()).WithLabelValues(reportedVerb, requestInfo.APIGroup, requestInfo.APIVersion, requestInfo.Resource, requestInfo.Subresource, scope, component)
@ -436,7 +436,7 @@ func MonitorRequest(req *http.Request, verb, group, version, resource, subresour
// InstrumentRouteFunc which is registered in installer.go with predefined
// list of verbs (different than those translated to RequestInfo).
// However, we need to tweak it e.g. to differentiate GET from LIST.
reportedVerb := cleanVerb(CanonicalVerb(strings.ToUpper(req.Method), scope), req)
reportedVerb := cleanVerb(CanonicalVerb(strings.ToUpper(req.Method), scope), verb, req)
dryRun := cleanDryRun(req.URL)
elapsedSeconds := elapsed.Seconds()
@ -566,8 +566,15 @@ func CleanVerb(verb string, request *http.Request) string {
}
// cleanVerb additionally ensures that unknown verbs don't clog up the metrics.
func cleanVerb(verb string, request *http.Request) string {
func cleanVerb(verb, suggestedVerb string, request *http.Request) string {
reportedVerb := CleanVerb(verb, request)
// CanonicalVerb (being an input for this function) doesn't handle correctly the
// deprecated path pattern for watch of:
// GET /api/{version}/watch/{resource}
// We correct it manually based on the pass verb from the installer.
if suggestedVerb == "WATCH" || suggestedVerb == "WATCHLIST" {
reportedVerb = "WATCH"
}
if validRequestMethods.Has(reportedVerb) {
return reportedVerb
}

View File

@ -26,10 +26,11 @@ import (
func TestCleanVerb(t *testing.T) {
testCases := []struct {
desc string
initialVerb string
request *http.Request
expectedVerb string
desc string
initialVerb string
suggestedVerb string
request *http.Request
expectedVerb string
}{
{
desc: "An empty string should be designated as unknown",
@ -63,6 +64,28 @@ func TestCleanVerb(t *testing.T) {
},
expectedVerb: "LIST",
},
{
desc: "LIST is transformed to WATCH for the old pattern watch",
initialVerb: "LIST",
suggestedVerb: "WATCH",
request: &http.Request{
URL: &url.URL{
RawQuery: "/api/v1/watch/pods",
},
},
expectedVerb: "WATCH",
},
{
desc: "LIST is transformed to WATCH for the old pattern watchlist",
initialVerb: "LIST",
suggestedVerb: "WATCHLIST",
request: &http.Request{
URL: &url.URL{
RawQuery: "/api/v1/watch/pods",
},
},
expectedVerb: "WATCH",
},
{
desc: "WATCHLIST should be transformed to WATCH",
initialVerb: "WATCHLIST",
@ -104,7 +127,7 @@ func TestCleanVerb(t *testing.T) {
if tt.request != nil {
req = tt.request
}
cleansedVerb := cleanVerb(tt.initialVerb, req)
cleansedVerb := cleanVerb(tt.initialVerb, tt.suggestedVerb, req)
if cleansedVerb != tt.expectedVerb {
t.Errorf("Got %s, but expected %s", cleansedVerb, tt.expectedVerb)
}