Skip to content

Commit

Permalink
tsweb: add QuietLogging option (tailscale#12838)
Browse files Browse the repository at this point in the history
Allows the use of tsweb.LogHandler exclusively for callbacks describing the
handler HTTP requests.

Fixes tailscale#12837

Signed-off-by: Paul Scott <paul@tailscale.com>
  • Loading branch information
icio authored Jul 29, 2024
1 parent c5623e0 commit 1bf7ed0
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 1 deletion.
6 changes: 5 additions & 1 deletion tsweb/tsweb.go
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,10 @@ type LogOptions struct {
// Now is a function giving the current time. Defaults to [time.Now].
Now func() time.Time

// QuietLogging suppresses all logging of handled HTTP requests, even if
// there are errors or status codes considered unsuccessful. Use this option
// to add your own logging in OnCompletion.
QuietLogging bool
// QuietLoggingIfSuccessful suppresses logging of handled HTTP requests
// where the request's response status code is 200 or 304.
QuietLoggingIfSuccessful bool
Expand Down Expand Up @@ -569,7 +573,7 @@ func (h logHandler) logRequest(r *http.Request, lw *loggingResponseWriter, msg A
}
}

if !h.opts.QuietLoggingIfSuccessful || (msg.Code != http.StatusOK && msg.Code != http.StatusNotModified) {
if !h.opts.QuietLogging && !(h.opts.QuietLoggingIfSuccessful && (msg.Code == http.StatusOK || msg.Code == http.StatusNotModified)) {
h.opts.Logf("%s", msg)
}

Expand Down
56 changes: 56 additions & 0 deletions tsweb/tsweb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1021,6 +1021,62 @@ func TestStdHandler_OnErrorPanic(t *testing.T) {
res.Body.Close()
}

func TestLogHandler_QuietLogging(t *testing.T) {
now := time.Now()
var logs []string
logf := func(format string, args ...any) {
logs = append(logs, fmt.Sprintf(format, args...))
}

var done bool
onComp := func(r *http.Request, alr AccessLogRecord) {
if done {
t.Fatal("expected only one OnCompletion call")
}
done = true

want := AccessLogRecord{
Time: now,
RemoteAddr: "192.0.2.1:1234",
Proto: "HTTP/1.1",
Host: "example.com",
Method: "GET",
RequestURI: "/",
Code: 200,
}
if diff := cmp.Diff(want, alr); diff != "" {
t.Fatalf("unexpected OnCompletion AccessLogRecord (-want +got):\n%s", diff)
}
}

LogHandler(
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(200)
w.WriteHeader(201) // loggingResponseWriter will write a warning.
}),
LogOptions{
Logf: logf,
OnCompletion: onComp,
QuietLogging: true,
Now: func() time.Time { return now },
},
).ServeHTTP(
httptest.NewRecorder(),
httptest.NewRequest("GET", "/", nil),
)

if !done {
t.Fatal("OnCompletion call didn't happen")
}

wantLogs := []string{
"[unexpected] HTTP handler set statusCode twice (200 and 201)",
}
if diff := cmp.Diff(wantLogs, logs); diff != "" {
t.Fatalf("logs (-want +got):\n%s", diff)
}
}

func TestErrorHandler_Panic(t *testing.T) {
// errorHandler should panic when not wrapped in logHandler.
defer func() {
Expand Down

0 comments on commit 1bf7ed0

Please sign in to comment.