Merge pull request #43377 from deads2k/server-09-logging

Automatic merge from submit-queue

only log stacks on server errors

Stack traces on http logging statements can be useful, but you  usually only need them for server errors not for things like redirects or even client errors since the server won't be fixing those.

This tightens the "log stack" behavior to only happen for server errors.  Messages will still be logged, but the automatic stacks will only come out for server errors.

@enj @mfojtik @smarterclayton
@kubernetes/sig-api-machinery-misc

pkg/kubeapiserver/server/insecure_handler.go

@@ -36,7 +36,7 @@ import (
 func BuildInsecureHandlerChain(apiHandler http.Handler, c *server.Config) http.Handler {
 	handler := genericapifilters.WithAudit(apiHandler, c.RequestContextMapper, c.AuditWriter)
 	handler = genericfilters.WithCORS(handler, c.CorsAllowedOriginList, nil, nil, nil, "true")
-	handler = genericfilters.WithPanicRecovery(handler, c.RequestContextMapper)
+	handler = genericfilters.WithPanicRecovery(handler)
 	handler = genericfilters.WithTimeoutForNonLongRunningRequests(handler, c.RequestContextMapper, c.LongRunningFunc)
 	handler = genericfilters.WithMaxInFlightLimit(handler, c.MaxRequestsInFlight, c.MaxMutatingRequestsInFlight, c.RequestContextMapper, c.LongRunningFunc)
 	handler = genericapifilters.WithRequestInfo(handler, server.NewRequestInfoResolver(c), c.RequestContextMapper)

staging/src/k8s.io/apiserver/pkg/server/config.go

@@ -492,7 +492,7 @@ func DefaultBuildHandlerChain(apiHandler http.Handler, c *Config) http.Handler {
 	handler = genericapifilters.WithAudit(handler, c.RequestContextMapper, c.AuditWriter)
 	handler = genericapifilters.WithAuthentication(handler, c.RequestContextMapper, c.Authenticator, genericapifilters.Unauthorized(c.SupportsBasicAuth))
 	handler = genericfilters.WithCORS(handler, c.CorsAllowedOriginList, nil, nil, nil, "true")
-	handler = genericfilters.WithPanicRecovery(handler, c.RequestContextMapper)
+	handler = genericfilters.WithPanicRecovery(handler)
 	handler = genericfilters.WithTimeoutForNonLongRunningRequests(handler, c.RequestContextMapper, c.LongRunningFunc)
 	handler = genericfilters.WithMaxInFlightLimit(handler, c.MaxRequestsInFlight, c.MaxMutatingRequestsInFlight, c.RequestContextMapper, c.LongRunningFunc)
 	handler = genericapifilters.WithRequestInfo(handler, NewRequestInfoResolver(c), c.RequestContextMapper)

staging/src/k8s.io/apiserver/pkg/server/filters/wrap.go

@@ -22,14 +22,12 @@ import (
 
 	"github.com/golang/glog"
 
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/util/runtime"
-	apirequest "k8s.io/apiserver/pkg/endpoints/request"
 	"k8s.io/apiserver/pkg/server/httplog"
 )
 
 // WithPanicRecovery wraps an http Handler to recover and log panics.
-func WithPanicRecovery(handler http.Handler, requestContextMapper apirequest.RequestContextMapper) http.Handler {
+func WithPanicRecovery(handler http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
 		defer runtime.HandleCrash(func(err interface{}) {
 			http.Error(w, "This request caused apisever to panic. Look in log for details.", http.StatusInternalServerError)
@@ -37,37 +35,6 @@ func WithPanicRecovery(handler http.Handler, requestContextMapper apirequest.Req
 		})
 
 		logger := httplog.NewLogged(req, &w)
-
-		var requestInfo *apirequest.RequestInfo
-		ctx, ok := requestContextMapper.Get(req)
-		if !ok {
-			glog.Errorf("no context found for request, handler chain must be wrong")
-		} else {
-			requestInfo, ok = apirequest.RequestInfoFrom(ctx)
-			if !ok {
-				glog.Errorf("no RequestInfo found in context, handler chain must be wrong")
-			}
-		}
-
-		if !ok || requestInfo.Verb != "proxy" {
-			logger.StacktraceWhen(
-				httplog.StatusIsNot(
-					http.StatusOK,
-					http.StatusCreated,
-					http.StatusAccepted,
-					http.StatusBadRequest,
-					http.StatusMovedPermanently,
-					http.StatusTemporaryRedirect,
-					http.StatusConflict,
-					http.StatusNotFound,
-					http.StatusUnauthorized,
-					http.StatusForbidden,
-					http.StatusNotModified,
-					apierrors.StatusUnprocessableEntity,
-					http.StatusSwitchingProtocols,
-				),
-			)
-		}
 		defer logger.Log()
 
 		// Dispatch to the internal handler