From 07020ab42e8f1a2e9d6fe4969c01d0f971324ae1 Mon Sep 17 00:00:00 2001 From: Han Kang Date: Wed, 24 Aug 2022 08:54:51 -0700 Subject: [PATCH 1/2] add metric and test Change-Id: Ic2bcf39caef791b2e13448a97d2c3203ed1d94b9 --- .../pkg/endpoints/handlers/metrics/OWNERS | 4 + .../pkg/endpoints/handlers/metrics/metrics.go | 36 ++++++++ .../pkg/endpoints/handlers/rest_test.go | 92 +++++++++++++++++++ 3 files changed, 132 insertions(+) create mode 100644 staging/src/k8s.io/apiserver/pkg/endpoints/handlers/metrics/OWNERS create mode 100644 staging/src/k8s.io/apiserver/pkg/endpoints/handlers/metrics/metrics.go diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/metrics/OWNERS b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/metrics/OWNERS new file mode 100644 index 00000000000..433e84aa3e4 --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/metrics/OWNERS @@ -0,0 +1,4 @@ +# See the OWNERS docs at https://go.k8s.io/owners + +approvers: + - logicalhan diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/metrics/metrics.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/metrics/metrics.go new file mode 100644 index 00000000000..694ff53b25e --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/metrics/metrics.go @@ -0,0 +1,36 @@ +/* +Copyright 2022 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 ( + "k8s.io/component-base/metrics" +) + +var ( + RequestBodySizes = metrics.NewHistogramVec( + &metrics.HistogramOpts{ + Subsystem: "apiserver", + Name: "request_body_sizes", + Help: "Apiserver request body sizes broken out by size.", + // we use 0.05 KB as the smallest bucket with 0.1 KB increments up to the + // apiserver limit. + Buckets: metrics.LinearBuckets(50000, 100000, 31), + StabilityLevel: metrics.ALPHA, + }, + []string{"resource", "verb"}, + ) +) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest_test.go index 52d169a86e4..5a7e39979ba 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest_test.go @@ -20,6 +20,7 @@ import ( "context" "errors" "fmt" + "io" "net/http" "reflect" "strings" @@ -45,9 +46,12 @@ import ( "k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/apis/example" examplev1 "k8s.io/apiserver/pkg/apis/example/v1" + "k8s.io/apiserver/pkg/endpoints/handlers/metrics" "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/rest" clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "k8s.io/component-base/metrics/legacyregistry" + "k8s.io/component-base/metrics/testutil" utiltrace "k8s.io/utils/trace" ) @@ -107,6 +111,94 @@ func TestPatchAnonymousField(t *testing.T) { } } +func TestLimitedReadBody(t *testing.T) { + defer legacyregistry.Reset() + legacyregistry.Register(metrics.RequestBodySizes) + + testcases := []struct { + desc string + requestBody io.Reader + limit int64 + expectedMetrics string + expectedErr bool + }{ + { + desc: "aaaa with limit 1", + requestBody: strings.NewReader("aaaa"), + limit: 1, + expectedMetrics: "", + expectedErr: true, + }, + { + desc: "aaaa with limit 5", + requestBody: strings.NewReader("aaaa"), + limit: 5, + expectedMetrics: ` + # HELP apiserver_request_body_sizes [ALPHA] Apiserver request body sizes broken out by size. + # TYPE apiserver_request_body_sizes histogram + apiserver_request_body_sizes_bucket{resource="resource.group",verb="create",le="50000"} 1 + apiserver_request_body_sizes_bucket{resource="resource.group",verb="create",le="150000"} 1 + apiserver_request_body_sizes_bucket{resource="resource.group",verb="create",le="250000"} 1 + apiserver_request_body_sizes_bucket{resource="resource.group",verb="create",le="350000"} 1 + apiserver_request_body_sizes_bucket{resource="resource.group",verb="create",le="450000"} 1 + apiserver_request_body_sizes_bucket{resource="resource.group",verb="create",le="550000"} 1 + apiserver_request_body_sizes_bucket{resource="resource.group",verb="create",le="650000"} 1 + apiserver_request_body_sizes_bucket{resource="resource.group",verb="create",le="750000"} 1 + apiserver_request_body_sizes_bucket{resource="resource.group",verb="create",le="850000"} 1 + apiserver_request_body_sizes_bucket{resource="resource.group",verb="create",le="950000"} 1 + apiserver_request_body_sizes_bucket{resource="resource.group",verb="create",le="1.05e+06"} 1 + apiserver_request_body_sizes_bucket{resource="resource.group",verb="create",le="1.15e+06"} 1 + apiserver_request_body_sizes_bucket{resource="resource.group",verb="create",le="1.25e+06"} 1 + apiserver_request_body_sizes_bucket{resource="resource.group",verb="create",le="1.35e+06"} 1 + apiserver_request_body_sizes_bucket{resource="resource.group",verb="create",le="1.45e+06"} 1 + apiserver_request_body_sizes_bucket{resource="resource.group",verb="create",le="1.55e+06"} 1 + apiserver_request_body_sizes_bucket{resource="resource.group",verb="create",le="1.65e+06"} 1 + apiserver_request_body_sizes_bucket{resource="resource.group",verb="create",le="1.75e+06"} 1 + apiserver_request_body_sizes_bucket{resource="resource.group",verb="create",le="1.85e+06"} 1 + apiserver_request_body_sizes_bucket{resource="resource.group",verb="create",le="1.95e+06"} 1 + apiserver_request_body_sizes_bucket{resource="resource.group",verb="create",le="2.05e+06"} 1 + apiserver_request_body_sizes_bucket{resource="resource.group",verb="create",le="2.15e+06"} 1 + apiserver_request_body_sizes_bucket{resource="resource.group",verb="create",le="2.25e+06"} 1 + apiserver_request_body_sizes_bucket{resource="resource.group",verb="create",le="2.35e+06"} 1 + apiserver_request_body_sizes_bucket{resource="resource.group",verb="create",le="2.45e+06"} 1 + apiserver_request_body_sizes_bucket{resource="resource.group",verb="create",le="2.55e+06"} 1 + apiserver_request_body_sizes_bucket{resource="resource.group",verb="create",le="2.65e+06"} 1 + apiserver_request_body_sizes_bucket{resource="resource.group",verb="create",le="2.75e+06"} 1 + apiserver_request_body_sizes_bucket{resource="resource.group",verb="create",le="2.85e+06"} 1 + apiserver_request_body_sizes_bucket{resource="resource.group",verb="create",le="2.95e+06"} 1 + apiserver_request_body_sizes_bucket{resource="resource.group",verb="create",le="3.05e+06"} 1 + apiserver_request_body_sizes_bucket{resource="resource.group",verb="create",le="+Inf"} 1 + apiserver_request_body_sizes_sum{resource="resource.group",verb="create"} 3 + apiserver_request_body_sizes_count{resource="resource.group",verb="create"} 1 +`, + expectedErr: false, + }, + } + + for _, tc := range testcases { + t.Run(tc.desc, func(t *testing.T) { + // reset metrics + defer metrics.RequestBodySizes.Reset() + defer legacyregistry.Reset() + + req, err := http.NewRequest("POST", "/", tc.requestBody) + if err != nil { + t.Errorf("err not expected: got %v", err) + } + _, err = limitedReadBody(context.Background(), req, tc.limit, "resource.group", "create") + if tc.expectedErr { + if err == nil { + t.Errorf("err expected: got nil") + } + return + } + if err = testutil.GatherAndCompare(legacyregistry.DefaultGatherer, strings.NewReader(tc.expectedMetrics), "apiserver_request_body_sizes"); err != nil { + t.Errorf("unexpected err: %v", err) + } + }) + } +} + func TestStrategicMergePatchInvalid(t *testing.T) { testGV := schema.GroupVersion{Group: "", Version: "v"} scheme.AddKnownTypes(testGV, &testPatchType{}) From 43c95cbf0682895cf5bb79452b1f011123ac4513 Mon Sep 17 00:00:00 2001 From: Han Kang Date: Wed, 24 Aug 2022 09:15:23 -0700 Subject: [PATCH 2/2] Add request body size metric Change-Id: Ica5d9b5457d4f844c4500b2c05b2f0631c27454c --- .../apiserver/pkg/endpoints/handlers/create.go | 3 ++- .../apiserver/pkg/endpoints/handlers/delete.go | 3 ++- .../pkg/endpoints/handlers/metrics/metrics.go | 14 ++++++++++++++ .../apiserver/pkg/endpoints/handlers/patch.go | 5 +++-- .../apiserver/pkg/endpoints/handlers/rest.go | 10 ++++++++++ .../apiserver/pkg/endpoints/handlers/rest_test.go | 4 ++-- .../apiserver/pkg/endpoints/handlers/update.go | 3 ++- vendor/modules.txt | 1 + 8 files changed, 36 insertions(+), 7 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go index 1f3ce809448..c40ec854add 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go @@ -37,6 +37,7 @@ import ( "k8s.io/apiserver/pkg/audit" "k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager" "k8s.io/apiserver/pkg/endpoints/handlers/finisher" + requestmetrics "k8s.io/apiserver/pkg/endpoints/handlers/metrics" "k8s.io/apiserver/pkg/endpoints/handlers/negotiation" "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/features" @@ -93,7 +94,7 @@ func createHandler(r rest.NamedCreater, scope *RequestScope, admit admission.Int return } - body, err := limitedReadBody(req, scope.MaxRequestBodyBytes) + body, err := limitedReadBodyWithRecordMetric(ctx, req, scope.MaxRequestBodyBytes, scope.Resource.GroupResource().String(), requestmetrics.Create) trace.Step("limitedReadBody done", utiltrace.Field{"len", len(body)}, utiltrace.Field{"err", err}) if err != nil { scope.err(err, w, req) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go index a7712f115fa..b9536d77a72 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go @@ -33,6 +33,7 @@ import ( "k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/audit" "k8s.io/apiserver/pkg/endpoints/handlers/finisher" + requestmetrics "k8s.io/apiserver/pkg/endpoints/handlers/metrics" "k8s.io/apiserver/pkg/endpoints/handlers/negotiation" "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/features" @@ -77,7 +78,7 @@ func DeleteResource(r rest.GracefulDeleter, allowsOptions bool, scope *RequestSc options := &metav1.DeleteOptions{} if allowsOptions { - body, err := limitedReadBody(req, scope.MaxRequestBodyBytes) + body, err := limitedReadBodyWithRecordMetric(ctx, req, scope.MaxRequestBodyBytes, scope.Resource.GroupResource().String(), requestmetrics.Patch) if err != nil { scope.err(err, w, req) return diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/metrics/metrics.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/metrics/metrics.go index 694ff53b25e..a870f3db418 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/metrics/metrics.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/metrics/metrics.go @@ -17,9 +17,19 @@ limitations under the License. package metrics import ( + "context" "k8s.io/component-base/metrics" ) +type RequestBodyVerb string + +const ( + Patch RequestBodyVerb = "patch" + Delete RequestBodyVerb = "delete" + Update RequestBodyVerb = "update" + Create RequestBodyVerb = "create" +) + var ( RequestBodySizes = metrics.NewHistogramVec( &metrics.HistogramOpts{ @@ -34,3 +44,7 @@ var ( []string{"resource", "verb"}, ) ) + +func RecordRequestBodySize(ctx context.Context, resource string, verb RequestBodyVerb, size int) { + RequestBodySizes.WithContext(ctx).WithLabelValues(resource, string(verb)).Observe(float64(size)) +} diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go index a0a8fb6ca77..504f41233cf 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go @@ -23,9 +23,9 @@ import ( "strings" "time" + jsonpatch "github.com/evanphx/json-patch" kjson "sigs.k8s.io/json" - jsonpatch "github.com/evanphx/json-patch" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metainternalversionscheme "k8s.io/apimachinery/pkg/apis/meta/internalversion/scheme" @@ -45,6 +45,7 @@ import ( "k8s.io/apiserver/pkg/authorization/authorizer" "k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager" "k8s.io/apiserver/pkg/endpoints/handlers/finisher" + requestmetrics "k8s.io/apiserver/pkg/endpoints/handlers/metrics" "k8s.io/apiserver/pkg/endpoints/handlers/negotiation" "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/features" @@ -105,7 +106,7 @@ func PatchResource(r rest.Patcher, scope *RequestScope, admit admission.Interfac return } - patchBytes, err := limitedReadBody(req, scope.MaxRequestBodyBytes) + patchBytes, err := limitedReadBodyWithRecordMetric(ctx, req, scope.MaxRequestBodyBytes, scope.Resource.GroupResource().String(), requestmetrics.Patch) trace.Step("limitedReadBody done", utiltrace.Field{"len", len(patchBytes)}, utiltrace.Field{"err", err}) if err != nil { scope.err(err, w, req) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go index e413661e760..7f005a37167 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go @@ -41,6 +41,7 @@ import ( "k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/authorization/authorizer" "k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager" + requestmetrics "k8s.io/apiserver/pkg/endpoints/handlers/metrics" "k8s.io/apiserver/pkg/endpoints/handlers/responsewriters" "k8s.io/apiserver/pkg/endpoints/metrics" "k8s.io/apiserver/pkg/endpoints/request" @@ -390,6 +391,15 @@ func limitedReadBody(req *http.Request, limit int64) ([]byte, error) { return data, nil } +func limitedReadBodyWithRecordMetric(ctx context.Context, req *http.Request, limit int64, resourceGroup string, verb requestmetrics.RequestBodyVerb) ([]byte, error) { + readBody, err := limitedReadBody(req, limit) + if err == nil { + // only record if we've read successfully + requestmetrics.RecordRequestBodySize(ctx, resourceGroup, verb, len(readBody)) + } + return readBody, err +} + func isDryRun(url *url.URL) bool { return len(url.Query()["dryRun"]) != 0 } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest_test.go index 5a7e39979ba..5518839523b 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest_test.go @@ -168,7 +168,7 @@ func TestLimitedReadBody(t *testing.T) { apiserver_request_body_sizes_bucket{resource="resource.group",verb="create",le="2.95e+06"} 1 apiserver_request_body_sizes_bucket{resource="resource.group",verb="create",le="3.05e+06"} 1 apiserver_request_body_sizes_bucket{resource="resource.group",verb="create",le="+Inf"} 1 - apiserver_request_body_sizes_sum{resource="resource.group",verb="create"} 3 + apiserver_request_body_sizes_sum{resource="resource.group",verb="create"} 4 apiserver_request_body_sizes_count{resource="resource.group",verb="create"} 1 `, expectedErr: false, @@ -185,7 +185,7 @@ func TestLimitedReadBody(t *testing.T) { if err != nil { t.Errorf("err not expected: got %v", err) } - _, err = limitedReadBody(context.Background(), req, tc.limit, "resource.group", "create") + _, err = limitedReadBodyWithRecordMetric(context.Background(), req, tc.limit, "resource.group", metrics.Create) if tc.expectedErr { if err == nil { t.Errorf("err expected: got nil") diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go index cb0ba5d7de8..3d77b1c5818 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go @@ -35,6 +35,7 @@ import ( "k8s.io/apiserver/pkg/authorization/authorizer" "k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager" "k8s.io/apiserver/pkg/endpoints/handlers/finisher" + requestmetrics "k8s.io/apiserver/pkg/endpoints/handlers/metrics" "k8s.io/apiserver/pkg/endpoints/handlers/negotiation" "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/features" @@ -76,7 +77,7 @@ func UpdateResource(r rest.Updater, scope *RequestScope, admit admission.Interfa return } - body, err := limitedReadBody(req, scope.MaxRequestBodyBytes) + body, err := limitedReadBodyWithRecordMetric(ctx, req, scope.MaxRequestBodyBytes, scope.Resource.GroupResource().String(), requestmetrics.Update) trace.Step("limitedReadBody done", utiltrace.Field{"len", len(body)}, utiltrace.Field{"err", err}) if err != nil { scope.err(err, w, req) diff --git a/vendor/modules.txt b/vendor/modules.txt index 1b99b6847ed..b245ea5c6e7 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -1600,6 +1600,7 @@ k8s.io/apiserver/pkg/endpoints/handlers k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal k8s.io/apiserver/pkg/endpoints/handlers/finisher +k8s.io/apiserver/pkg/endpoints/handlers/metrics k8s.io/apiserver/pkg/endpoints/handlers/negotiation k8s.io/apiserver/pkg/endpoints/handlers/responsewriters k8s.io/apiserver/pkg/endpoints/metrics