Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add timeout message in addition to metric #96424

Merged
merged 1 commit into from Nov 12, 2020
Merged

add timeout message in addition to metric #96424

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 10 additions & 2 deletions staging/src/k8s.io/apiserver/pkg/server/filters/timeout_test.go
View file
Open in desktop
Expand Up @@ -244,8 +244,12 @@ func TestErrConnKilled(t *testing.T) {
t.Fatal("expected to receive an error")
}

// we should only get one line for this, not the big stack from before
capturedOutput := readStdErr()
if len(capturedOutput) > 0 {
if strings.Count(capturedOutput, "\n") != 1 {
t.Errorf("unexpected output captured actual = %v", capturedOutput)
}
if !strings.Contains(capturedOutput, `timeout or abort while handling: GET "/"`) {
t.Errorf("unexpected output captured actual = %v", capturedOutput)
}
}
Expand Down Expand Up @@ -330,8 +334,12 @@ func TestErrConnKilledHTTP2(t *testing.T) {
t.Fatal("expected to receive an error")
}

// we should only get one line for this, not the big stack from before
capturedOutput := readStdErr()
if len(capturedOutput) > 0 {
if strings.Count(capturedOutput, "\n") != 1 {
t.Errorf("unexpected output captured actual = %v", capturedOutput)
}
if !strings.Contains(capturedOutput, `timeout or abort while handling: GET "/"`) {
t.Errorf("unexpected output captured actual = %v", capturedOutput)
}

Expand Down
16 changes: 10 additions & 6 deletions staging/src/k8s.io/apiserver/pkg/server/filters/wrap.go
View file
Open in desktop
Expand Up @@ -17,13 +17,14 @@ limitations under the License.
package filters

import (
"k8s.io/apiserver/pkg/endpoints/request"
"k8s.io/klog/v2"
"fmt"
"net/http"

"k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/apiserver/pkg/endpoints/metrics"
"k8s.io/apiserver/pkg/endpoints/request"
"k8s.io/apiserver/pkg/server/httplog"
"k8s.io/klog/v2"
)

// WithPanicRecovery wraps an http Handler to recover and log panics (except in the special case of http.ErrAbortHandler panics, which suppress logging).
Expand All @@ -42,12 +43,15 @@ func WithPanicRecovery(handler http.Handler, resolver request.RequestInfoResolve
//
// Note that the ReallyCrash variable controls the behaviour of the HandleCrash function
// So it might actually crash, after calling the handlers
info, err := resolver.NewRequestInfo(req)
if err != nil {
if info, err := resolver.NewRequestInfo(req); err != nil {
metrics.RecordRequestAbort(req, nil)
return
} else {
metrics.RecordRequestAbort(req, info)
}
metrics.RecordRequestAbort(req, info)
// This call can have different handlers, but the default chain rate limits. Call it after the metrics are updated
// in case the rate limit delays it. If you outrun the rate for this one timed out requests, something has gone
// seriously wrong with your server, but generally having a logging signal for timeouts is useful.
runtime.HandleError(fmt.Errorf("timeout or abort while handling: %v %q", req.Method, req.URL.Path))
return
}
http.Error(w, "This request caused apiserver to panic. Look in the logs for details.", http.StatusInternalServerError)
Expand Down