From 5d61e18a4343773105e39c36b944b4f223e70eaf Mon Sep 17 00:00:00 2001 From: lala123912 Date: Mon, 23 Nov 2020 16:27:05 +0800 Subject: [PATCH] Code optimization for add additional information to log trace in api server --- .../apiserver/pkg/endpoints/handlers/BUILD | 1 + .../pkg/endpoints/handlers/create.go | 2 +- .../pkg/endpoints/handlers/delete.go | 4 +-- .../apiserver/pkg/endpoints/handlers/get.go | 4 +-- .../pkg/endpoints/handlers/helpers.go | 15 +++++++++ .../pkg/endpoints/handlers/helpers_test.go | 14 ++++++++ .../apiserver/pkg/endpoints/handlers/patch.go | 2 +- .../pkg/endpoints/handlers/trace_util.go | 32 +++++++++++++++++++ .../pkg/endpoints/handlers/update.go | 2 +- 9 files changed, 69 insertions(+), 7 deletions(-) create mode 100644 staging/src/k8s.io/apiserver/pkg/endpoints/handlers/trace_util.go diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/BUILD b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/BUILD index 3b299e4dfe3..6525941ebc6 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/BUILD @@ -59,6 +59,7 @@ go_library( "patch.go", "response.go", "rest.go", + "trace_util.go", "update.go", "watch.go", ], 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 631914f36aa..0b950c72dd4 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go @@ -49,7 +49,7 @@ var namespaceGVK = schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Name func createHandler(r rest.NamedCreater, scope *RequestScope, admit admission.Interface, includeName bool) http.HandlerFunc { return func(w http.ResponseWriter, req *http.Request) { // For performance tracking purposes. - trace := utiltrace.New("Create", utiltrace.Field{Key: "url", Value: req.URL.Path}, utiltrace.Field{Key: "user-agent", Value: &lazyTruncatedUserAgent{req}}, utiltrace.Field{Key: "client", Value: &lazyClientIP{req}}) + trace := utiltrace.New("Create", traceFields(req)...) defer trace.LogIfLong(500 * time.Millisecond) if isDryRun(req.URL) && !utilfeature.DefaultFeatureGate.Enabled(features.DryRun) { 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 892aaf4a0d2..498eeee5f31 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go @@ -46,7 +46,7 @@ import ( func DeleteResource(r rest.GracefulDeleter, allowsOptions bool, scope *RequestScope, admit admission.Interface) http.HandlerFunc { return func(w http.ResponseWriter, req *http.Request) { // For performance tracking purposes. - trace := utiltrace.New("Delete", utiltrace.Field{Key: "url", Value: req.URL.Path}, utiltrace.Field{Key: "user-agent", Value: &lazyTruncatedUserAgent{req}}, utiltrace.Field{Key: "client", Value: &lazyClientIP{req}}) + trace := utiltrace.New("Delete", traceFields(req)...) defer trace.LogIfLong(500 * time.Millisecond) if isDryRun(req.URL) && !utilfeature.DefaultFeatureGate.Enabled(features.DryRun) { @@ -164,7 +164,7 @@ func DeleteResource(r rest.GracefulDeleter, allowsOptions bool, scope *RequestSc // DeleteCollection returns a function that will handle a collection deletion func DeleteCollection(r rest.CollectionDeleter, checkBody bool, scope *RequestScope, admit admission.Interface) http.HandlerFunc { return func(w http.ResponseWriter, req *http.Request) { - trace := utiltrace.New("Delete", utiltrace.Field{"url", req.URL.Path}) + trace := utiltrace.New("Delete", traceFields(req)...) defer trace.LogIfLong(500 * time.Millisecond) if isDryRun(req.URL) && !utilfeature.DefaultFeatureGate.Enabled(features.DryRun) { diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/get.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/get.go index c3f6e4cbe13..da227e7aeda 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/get.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/get.go @@ -51,7 +51,7 @@ type getterFunc func(ctx context.Context, name string, req *http.Request, trace // passed-in getterFunc to perform the actual get. func getResourceHandler(scope *RequestScope, getter getterFunc) http.HandlerFunc { return func(w http.ResponseWriter, req *http.Request) { - trace := utiltrace.New("Get", utiltrace.Field{Key: "url", Value: req.URL.Path}, utiltrace.Field{Key: "user-agent", Value: &lazyTruncatedUserAgent{req}}, utiltrace.Field{Key: "client", Value: &lazyClientIP{req}}) + trace := utiltrace.New("Get", traceFields(req)...) defer trace.LogIfLong(500 * time.Millisecond) namespace, name, err := scope.Namer.Name(req) @@ -168,7 +168,7 @@ func getRequestOptions(req *http.Request, scope *RequestScope, into runtime.Obje func ListResource(r rest.Lister, rw rest.Watcher, scope *RequestScope, forceWatch bool, minRequestTimeout time.Duration) http.HandlerFunc { return func(w http.ResponseWriter, req *http.Request) { // For performance tracking purposes. - trace := utiltrace.New("List", utiltrace.Field{Key: "url", Value: req.URL.Path}, utiltrace.Field{Key: "user-agent", Value: &lazyTruncatedUserAgent{req}}, utiltrace.Field{Key: "client", Value: &lazyClientIP{req}}) + trace := utiltrace.New("List", traceFields(req)...) namespace, err := scope.Namer.Namespace(req) if err != nil { diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/helpers.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/helpers.go index 82170e050ec..244a3fd0a51 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/helpers.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/helpers.go @@ -58,3 +58,18 @@ func (lazy *lazyClientIP) String() string { } return "unknown" } + +// lazyAccept implements String() string and it will +// calls http.Request Header.Get() lazily only when required. +type lazyAccept struct { + req *http.Request +} + +func (lazy *lazyAccept) String() string { + if lazy.req != nil { + accept := lazy.req.Header.Get("Accept") + return accept + } + + return "unknown" +} diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/helpers_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/helpers_test.go index e55b40a1c45..3f7e3dc106e 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/helpers_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/helpers_test.go @@ -57,3 +57,17 @@ func TestLazyClientIP(t *testing.T) { clientIPWithoutReq := &lazyClientIP{} assert.Equal(t, "unknown", fmt.Sprintf("%v", clientIPWithoutReq)) } + +func TestLazyAccept(t *testing.T) { + req := &http.Request{} + req.Header = http.Header{} + + accept := "application/json" + req.Header.Set("Accept", accept) + + acceptWithReq := &lazyAccept{req} + assert.Equal(t, accept, fmt.Sprintf("%v", acceptWithReq)) + + acceptWithoutReq := &lazyAccept{} + assert.Equal(t, "unknown", fmt.Sprintf("%v", acceptWithoutReq)) +} 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 096330a4ae9..5fe8cedc28d 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go @@ -61,7 +61,7 @@ const ( func PatchResource(r rest.Patcher, scope *RequestScope, admit admission.Interface, patchTypes []string) http.HandlerFunc { return func(w http.ResponseWriter, req *http.Request) { // For performance tracking purposes. - trace := utiltrace.New("Patch", utiltrace.Field{Key: "url", Value: req.URL.Path}, utiltrace.Field{Key: "user-agent", Value: &lazyTruncatedUserAgent{req}}, utiltrace.Field{Key: "client", Value: &lazyClientIP{req}}) + trace := utiltrace.New("Patch", traceFields(req)...) defer trace.LogIfLong(500 * time.Millisecond) if isDryRun(req.URL) && !utilfeature.DefaultFeatureGate.Enabled(features.DryRun) { diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/trace_util.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/trace_util.go new file mode 100644 index 00000000000..69b41fac4e3 --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/trace_util.go @@ -0,0 +1,32 @@ +/* +Copyright 2020 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 handlers + +import ( + "net/http" + + utiltrace "k8s.io/utils/trace" +) + +func traceFields(req *http.Request) []utiltrace.Field { + return []utiltrace.Field{ + {Key: "url", Value: req.URL.Path}, + {Key: "user-agent", Value: &lazyTruncatedUserAgent{req: req}}, + {Key: "client", Value: &lazyClientIP{req: req}}, + {Key: "accept", Value: &lazyAccept{req: req}}, + {Key: "protocol", Value: req.Proto}} +} 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 f0473323f99..1c46e7f0dbc 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go @@ -46,7 +46,7 @@ import ( func UpdateResource(r rest.Updater, scope *RequestScope, admit admission.Interface) http.HandlerFunc { return func(w http.ResponseWriter, req *http.Request) { // For performance tracking purposes. - trace := utiltrace.New("Update", utiltrace.Field{Key: "url", Value: req.URL.Path}, utiltrace.Field{Key: "user-agent", Value: &lazyTruncatedUserAgent{req}}, utiltrace.Field{Key: "client", Value: &lazyClientIP{req}}) + trace := utiltrace.New("Update", traceFields(req)...) defer trace.LogIfLong(500 * time.Millisecond) if isDryRun(req.URL) && !utilfeature.DefaultFeatureGate.Enabled(features.DryRun) {