Merge pull request #102934 from lavalamp/verblog

Make logged verbs match metric-reported verbs
This commit is contained in:
Kubernetes Prow Robot 2021-06-18 02:18:17 -07:00 committed by GitHub
commit f62b9dbd92
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 30 additions and 8 deletions

View File

@ -370,7 +370,7 @@ func RecordRequestAbort(req *http.Request, requestInfo *request.RequestInfo) {
} }
scope := CleanScope(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 resource := requestInfo.Resource
subresource := requestInfo.Subresource subresource := requestInfo.Subresource
group := requestInfo.APIGroup group := requestInfo.APIGroup
@ -393,7 +393,7 @@ func RecordRequestTermination(req *http.Request, requestInfo *request.RequestInf
// InstrumentRouteFunc which is registered in installer.go with predefined // InstrumentRouteFunc which is registered in installer.go with predefined
// list of verbs (different than those translated to RequestInfo). // list of verbs (different than those translated to RequestInfo).
// However, we need to tweak it e.g. to differentiate GET from LIST. // 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 { if requestInfo.IsResourceRequest {
requestTerminationsTotal.WithContext(req.Context()).WithLabelValues(reportedVerb, requestInfo.APIGroup, requestInfo.APIVersion, requestInfo.Resource, requestInfo.Subresource, scope, component, codeToString(code)).Inc() requestTerminationsTotal.WithContext(req.Context()).WithLabelValues(reportedVerb, requestInfo.APIGroup, requestInfo.APIVersion, requestInfo.Resource, requestInfo.Subresource, scope, component, codeToString(code)).Inc()
@ -415,7 +415,7 @@ func RecordLongRunning(req *http.Request, requestInfo *request.RequestInfo, comp
// InstrumentRouteFunc which is registered in installer.go with predefined // InstrumentRouteFunc which is registered in installer.go with predefined
// list of verbs (different than those translated to RequestInfo). // list of verbs (different than those translated to RequestInfo).
// However, we need to tweak it e.g. to differentiate GET from LIST. // 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 { if requestInfo.IsResourceRequest {
g = longRunningRequestGauge.WithContext(req.Context()).WithLabelValues(reportedVerb, requestInfo.APIGroup, requestInfo.APIVersion, requestInfo.Resource, requestInfo.Subresource, scope, component) g = longRunningRequestGauge.WithContext(req.Context()).WithLabelValues(reportedVerb, requestInfo.APIGroup, requestInfo.APIVersion, requestInfo.Resource, requestInfo.Subresource, scope, component)
@ -434,7 +434,7 @@ func MonitorRequest(req *http.Request, verb, group, version, resource, subresour
// InstrumentRouteFunc which is registered in installer.go with predefined // InstrumentRouteFunc which is registered in installer.go with predefined
// list of verbs (different than those translated to RequestInfo). // list of verbs (different than those translated to RequestInfo).
// However, we need to tweak it e.g. to differentiate GET from LIST. // 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)
dryRun := cleanDryRun(req.URL) dryRun := cleanDryRun(req.URL)
elapsedSeconds := elapsed.Seconds() elapsedSeconds := elapsed.Seconds()
@ -526,7 +526,9 @@ func CleanScope(requestInfo *request.RequestInfo) string {
return "" return ""
} }
func canonicalVerb(verb string, scope string) string { // CanonicalVerb distinguishes LISTs from GETs (and HEADs). It assumes verb is
// UPPERCASE.
func CanonicalVerb(verb string, scope string) string {
switch verb { switch verb {
case "GET", "HEAD": case "GET", "HEAD":
if scope != "resource" && scope != "" { if scope != "resource" && scope != "" {
@ -538,7 +540,9 @@ func canonicalVerb(verb string, scope string) string {
} }
} }
func cleanVerb(verb string, request *http.Request) string { // CleanVerb returns a normalized verb, so that it is easy to tell WATCH from
// LIST and APPLY from PATCH.
func CleanVerb(verb string, request *http.Request) string {
reportedVerb := verb reportedVerb := verb
if verb == "LIST" { if verb == "LIST" {
// see apimachinery/pkg/runtime/conversion.go Convert_Slice_string_To_bool // see apimachinery/pkg/runtime/conversion.go Convert_Slice_string_To_bool
@ -555,6 +559,12 @@ func cleanVerb(verb string, request *http.Request) string {
if verb == "PATCH" && request.Header.Get("Content-Type") == string(types.ApplyPatchType) && utilfeature.DefaultFeatureGate.Enabled(features.ServerSideApply) { if verb == "PATCH" && request.Header.Get("Content-Type") == string(types.ApplyPatchType) && utilfeature.DefaultFeatureGate.Enabled(features.ServerSideApply) {
reportedVerb = "APPLY" reportedVerb = "APPLY"
} }
return reportedVerb
}
// cleanVerb additionally ensures that unknown verbs don't clog up the metrics.
func cleanVerb(verb string, request *http.Request) string {
reportedVerb := CleanVerb(verb, request)
if validRequestMethods.Has(reportedVerb) { if validRequestMethods.Has(reportedVerb) {
return reportedVerb return reportedVerb
} }

View File

@ -765,7 +765,6 @@ func DefaultBuildHandlerChain(apiHandler http.Handler, c *Config) http.Handler {
handler = genericapifilters.WithRequestDeadline(handler, c.AuditBackend, c.AuditPolicyChecker, handler = genericapifilters.WithRequestDeadline(handler, c.AuditBackend, c.AuditPolicyChecker,
c.LongRunningFunc, c.Serializer, c.RequestTimeout) c.LongRunningFunc, c.Serializer, c.RequestTimeout)
handler = genericfilters.WithWaitGroup(handler, c.LongRunningFunc, c.HandlerChainWaitGroup) handler = genericfilters.WithWaitGroup(handler, c.LongRunningFunc, c.HandlerChainWaitGroup)
handler = genericapifilters.WithRequestInfo(handler, c.RequestInfoResolver)
if c.SecureServing != nil && !c.SecureServing.DisableHTTP2 && c.GoawayChance > 0 { if c.SecureServing != nil && !c.SecureServing.DisableHTTP2 && c.GoawayChance > 0 {
handler = genericfilters.WithProbabilisticGoaway(handler, c.GoawayChance) handler = genericfilters.WithProbabilisticGoaway(handler, c.GoawayChance)
} }
@ -774,6 +773,7 @@ func DefaultBuildHandlerChain(apiHandler http.Handler, c *Config) http.Handler {
handler = genericapifilters.WithCacheControl(handler) handler = genericapifilters.WithCacheControl(handler)
handler = genericfilters.WithHSTS(handler, c.HSTSDirectives) handler = genericfilters.WithHSTS(handler, c.HSTSDirectives)
handler = genericfilters.WithHTTPLogging(handler) handler = genericfilters.WithHTTPLogging(handler)
handler = genericapifilters.WithRequestInfo(handler, c.RequestInfoResolver)
handler = genericapifilters.WithRequestReceivedTimestamp(handler) handler = genericapifilters.WithRequestReceivedTimestamp(handler)
handler = genericfilters.WithPanicRecovery(handler, c.RequestInfoResolver) handler = genericfilters.WithPanicRecovery(handler, c.RequestInfoResolver)
handler = genericapifilters.WithAuditID(handler) handler = genericapifilters.WithAuditID(handler)

View File

@ -26,6 +26,7 @@ import (
"strings" "strings"
"time" "time"
"k8s.io/apiserver/pkg/endpoints/metrics"
"k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/endpoints/request"
"k8s.io/klog/v2" "k8s.io/klog/v2"
) )
@ -184,8 +185,19 @@ func AddInfof(ctx context.Context, format string, data ...interface{}) {
func (rl *respLogger) Log() { func (rl *respLogger) Log() {
latency := time.Since(rl.startTime) latency := time.Since(rl.startTime)
auditID := request.GetAuditIDTruncated(rl.req) auditID := request.GetAuditIDTruncated(rl.req)
verb := rl.req.Method
if requestInfo, ok := request.RequestInfoFrom(rl.req.Context()); ok {
// If we can find a requestInfo, we can get a scope, and then
// we can convert GETs to LISTs when needed.
scope := metrics.CleanScope(requestInfo)
verb = metrics.CanonicalVerb(strings.ToUpper(verb), scope)
}
// mark APPLY requests and WATCH requests correctly.
verb = metrics.CleanVerb(verb, rl.req)
keysAndValues := []interface{}{ keysAndValues := []interface{}{
"verb", rl.req.Method, "verb", verb,
"URI", rl.req.RequestURI, "URI", rl.req.RequestURI,
"latency", latency, "latency", latency,
"userAgent", rl.req.UserAgent(), "userAgent", rl.req.UserAgent(),