github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-31 23:37:01 +00:00
Code Issues Projects Releases Wiki Activity

fix a number of unbounded dimensions in request metrics (#89451)

Browse Source 
* fix a number of unbounded dimensions in request metrics

* add test suite for cleanVerb and cleanContentType

* Properly validate that the content-type and charset (if applicable) are RFC compliant

* add additional test case

* truncate list of content-types

Change-Id: Ia5fe0d2e2c602e4def4b8e0849cc19f3f9251818
This commit is contained in:
Han Kang 2020-05-29 01:05:15 -07:00 committed by GitHub
parent c8ceeed698
commit 6c588c3f44
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 219 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
staging/src/k8s.io/apiserver/pkg/endpoints/metrics/BUILD
View File

@ -3,6 +3,7 @@ package(default_visibility = ["//visibility:public"])
load(
"@io_bazel_rules_go//go:def.bzl",
"go_library",
"go_test",
)
go_library(
@ -35,3 +36,9 @@ filegroup(
srcs = [":package-srcs"],
tags = ["automanaged"],
)
go_test(
name = "go_default_test",
srcs = ["metrics_test.go"],
embed = [":go_default_library"],
)

59
staging/src/k8s.io/apiserver/pkg/endpoints/metrics/metrics.go
View File

@ -28,7 +28,6 @@ import (
"time"
restful "github.com/emicklei/go-restful"
"k8s.io/apimachinery/pkg/apis/meta/v1/validation"
"k8s.io/apimachinery/pkg/types"
utilsets "k8s.io/apimachinery/pkg/util/sets"
@ -48,6 +47,8 @@ type resettableCollector interface {
const (
APIServerComponent string = "apiserver"
OtherContentType string = "other"
OtherRequestMethod string = "other"
)
/*
@ -172,6 +173,37 @@ var (
currentInflightRequests,
requestTerminationsTotal,
}
// 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(
"APPLY",
"CONNECT",
"CREATE",
"DELETE",
"DELETECOLLECTION",
"GET",
"LIST",
"PATCH",
"POST",
"PROXY",
"PUT",
"UPDATE",
"WATCH",
"WATCHLIST")
)
const (
@ -219,6 +251,10 @@ func RecordRequestTermination(req *http.Request, requestInfo *request.RequestInf
// translated to RequestInfo).
// However, we need to tweak it e.g. to differentiate GET from LIST.
verb := canonicalVerb(strings.ToUpper(req.Method), scope)
// set verbs to a bounded set of known and expected verbs
if !validRequestMethods.Has(verb) {
verb = OtherRequestMethod
}
if requestInfo.IsResourceRequest {
requestTerminationsTotal.WithLabelValues(cleanVerb(verb, req), requestInfo.APIGroup, requestInfo.APIVersion, requestInfo.Resource, requestInfo.Subresource, scope, component, codeToString(code)).Inc()
} else {
@ -256,7 +292,8 @@ func MonitorRequest(req *http.Request, verb, group, version, resource, subresour
reportedVerb := cleanVerb(verb, req)
dryRun := cleanDryRun(req.URL)
elapsedSeconds := elapsed.Seconds()
requestCounter.WithLabelValues(reportedVerb, dryRun, group, version, resource, subresource, scope, component, contentType, codeToString(httpCode)).Inc()
cleanContentType := cleanContentType(contentType)
requestCounter.WithLabelValues(reportedVerb, dryRun, group, version, resource, subresource, scope, component, cleanContentType, codeToString(httpCode)).Inc()
requestLatencies.WithLabelValues(reportedVerb, dryRun, group, version, resource, subresource, scope, component).Observe(elapsedSeconds)
// We are only interested in response sizes of read requests.
if verb == "GET" || verb == "LIST" {
@ -311,6 +348,19 @@ func InstrumentHandlerFunc(verb, group, version, resource, subresource, scope, c
}
}
// 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 != "" {
@ -355,7 +405,10 @@ func cleanVerb(verb string, request *http.Request) string {
if verb == "PATCH" && request.Header.Get("Content-Type") == string(types.ApplyPatchType) && utilfeature.DefaultFeatureGate.Enabled(features.ServerSideApply) {
reportedVerb = "APPLY"
}
return reportedVerb
if validRequestMethods.Has(reportedVerb) {
return reportedVerb
}
return OtherRequestMethod
}
func cleanDryRun(u *url.URL) string {

153
staging/src/k8s.io/apiserver/pkg/endpoints/metrics/metrics_test.go Normal file
View File

@ -0,0 +1,153 @@
/*
Copyright 2019 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 (
"fmt"
"net/http"
"net/url"
"testing"
)
func TestCleanVerb(t *testing.T) {
testCases := []struct {
desc string
initialVerb string
request *http.Request
expectedVerb string
}{
{
desc: "An empty string should be designated as unknown",
initialVerb: "",
request: nil,
expectedVerb: "other",
},
{
desc: "LIST should normally map to LIST",
initialVerb: "LIST",
request: nil,
expectedVerb: "LIST",
},
{
desc: "LIST should be transformed to WATCH if we have the right query param on the request",
initialVerb: "LIST",
request: &http.Request{
URL: &url.URL{
RawQuery: "watch=true",
},
},
expectedVerb: "WATCH",
},
{
desc: "LIST isn't transformed to WATCH if we have query params that do not include watch",
initialVerb: "LIST",
request: &http.Request{
URL: &url.URL{
RawQuery: "blah=asdf&something=else",
},
},
expectedVerb: "LIST",
},
{
desc: "WATCHLIST should be transformed to WATCH",
initialVerb: "WATCHLIST",
request: nil,
expectedVerb: "WATCH",
},
{
desc: "PATCH should be transformed to APPLY with the right content type",
initialVerb: "PATCH",
request: &http.Request{
Header: http.Header{
"Content-Type": []string{"application/apply-patch+yaml"},
},
},
expectedVerb: "APPLY",
},
{
desc: "PATCH shouldn't be transformed to APPLY without the right content type",
initialVerb: "PATCH",
request: nil,
expectedVerb: "PATCH",
},
{
desc: "WATCHLIST should be transformed to WATCH",
initialVerb: "WATCHLIST",
request: nil,
expectedVerb: "WATCH",
},
{
desc: "unexpected verbs should be designated as unknown",
initialVerb: "notValid",
request: nil,
expectedVerb: "other",
},
}
for _, tt := range testCases {
t.Run(tt.initialVerb, func(t *testing.T) {
req := &http.Request{URL: &url.URL{}}
if tt.request != nil {
req = tt.request
}
cleansedVerb := cleanVerb(tt.initialVerb, req)
if cleansedVerb != tt.expectedVerb {
t.Errorf("Got %s, but expected %s", cleansedVerb, tt.expectedVerb)
}
})
}
}
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)
}
})
}
}

6
staging/src/k8s.io/apiserver/pkg/server/healthz/healthz_test.go
View File

@ -256,9 +256,9 @@ 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.
# 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="",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
`)
if err := testutil.GatherAndCompare(legacyregistry.DefaultGatherer, expected, "apiserver_request_total"); err != nil {
t.Error(err)