Skip to content

Commit 129a61a

Browse files
DylanGuedesDylanGuedes
authored
Make DisableRequestSuccessLog configurable. (weaveworks#284)
- NewLogMiddleware is a public method. Adding a new parameter would make this PR a breaking change one. - However, the behavior is the same: whatever is configured for the existing DisableRequestSuccessLog, it will be used by the log middleware. - Test option to not log successful requests.
1 parent 3344856 commit 129a61a

File tree

3 files changed

+75
-15
lines changed

3 files changed

+75
-15
lines changed

‎middleware/logging.go‎

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,12 @@ import (
1414

1515
// Log middleware logs http requests
1616
type Log struct {
17-
Log logging.Interface
18-
LogRequestHeaders bool // LogRequestHeaders true -> dump http headers at debug log level
19-
LogRequestAtInfoLevel bool // LogRequestAtInfoLevel true -> log requests at info log level
20-
SourceIPs *SourceIPExtractor
21-
HttpHeadersToExclude map[string]bool
17+
Log logging.Interface
18+
DisableRequestSuccessLog bool
19+
LogRequestHeaders bool // LogRequestHeaders true -> dump http headers at debug log level
20+
LogRequestAtInfoLevel bool // LogRequestAtInfoLevel true -> log requests at info log level
21+
SourceIPs *SourceIPExtractor
22+
HttpHeadersToExclude map[string]bool
2223
}
2324

2425
var defaultExcludedHeaders = map[string]bool{
@@ -94,20 +95,27 @@ func (l Log) Wrap(next http.Handler) http.Handler {
9495

9596
return
9697
}
97-
if 100 <= statusCode && statusCode < 500 || statusCode == http.StatusBadGateway || statusCode == http.StatusServiceUnavailable {
98+
99+
switch {
100+
// success and shouldn't log successful requests.
101+
case statusCode >= 200 && statusCode < 300 && l.DisableRequestSuccessLog:
102+
return
103+
104+
case 100 <= statusCode && statusCode < 500 || statusCode == http.StatusBadGateway || statusCode == http.StatusServiceUnavailable:
98105
if l.LogRequestAtInfoLevel {
99106
requestLog.Infof("%s %s (%d) %s", r.Method, uri, statusCode, time.Since(begin))
100-
} else {
101-
requestLog.Debugf("%s %s (%d) %s", r.Method, uri, statusCode, time.Since(begin))
102-
}
103-
if l.LogRequestHeaders && headers != nil {
104-
if l.LogRequestAtInfoLevel {
107+
108+
if l.LogRequestHeaders && headers != nil {
105109
requestLog.Infof("ws: %v; %s", IsWSHandshakeRequest(r), string(headers))
106-
} else {
107-
requestLog.Debugf("ws: %v; %s", IsWSHandshakeRequest(r), string(headers))
108110
}
111+
return
112+
}
113+
114+
requestLog.Debugf("%s %s (%d) %s", r.Method, uri, statusCode, time.Since(begin))
115+
if l.LogRequestHeaders && headers != nil {
116+
requestLog.Debugf("ws: %v; %s", IsWSHandshakeRequest(r), string(headers))
109117
}
110-
} else {
118+
default:
111119
requestLog.Warnf("%s %s (%d) %s Response: %q ws: %v; %s",
112120
r.Method, uri, statusCode, time.Since(begin), buf.Bytes(), IsWSHandshakeRequest(r), headers)
113121
}

‎middleware/logging_test.go‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,55 @@ func TestBadWriteLogging(t *testing.T) {
5757
}
5858
}
5959

60+
func TestDisabledSuccessfulRequestsLogging(t *testing.T) {
61+
for _, tc := range []struct {
62+
err error
63+
disableLog bool
64+
logContains string
65+
}{
66+
{
67+
err: nil,
68+
disableLog: false,
69+
}, {
70+
err: nil,
71+
disableLog: true,
72+
logContains: "",
73+
},
74+
} {
75+
buf := bytes.NewBuffer(nil)
76+
logrusLogger := logrus.New()
77+
logrusLogger.Out = buf
78+
logrusLogger.Level = logrus.DebugLevel
79+
80+
loggingMiddleware := Log{
81+
Log: logging.Logrus(logrusLogger),
82+
DisableRequestSuccessLog: tc.disableLog,
83+
}
84+
85+
handler := func(w http.ResponseWriter, r *http.Request) {
86+
io.WriteString(w, "<html><body>Hello World!</body></html>") //nolint:errcheck
87+
}
88+
loggingHandler := loggingMiddleware.Wrap(http.HandlerFunc(handler))
89+
90+
req := httptest.NewRequest("GET", "http://example.com/foo", nil)
91+
recorder := httptest.NewRecorder()
92+
93+
w := errorWriter{
94+
err: tc.err,
95+
w: recorder,
96+
}
97+
loggingHandler.ServeHTTP(w, req)
98+
content := buf.String()
99+
100+
if !tc.disableLog {
101+
require.Contains(t, content, "GET http://example.com/foo (200)")
102+
} else {
103+
require.NotContains(t, content, "(200)")
104+
require.Empty(t, content)
105+
}
106+
}
107+
}
108+
60109
func TestLoggingRequestsAtInfoLevel(t *testing.T) {
61110
for _, tc := range []struct {
62111
err error

‎server/server.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -428,12 +428,15 @@ func New(cfg Config) (*Server, error) {
428428
}
429429
}
430430

431+
defaultLogMiddleware := middleware.NewLogMiddleware(log, cfg.LogRequestHeaders, cfg.LogRequestAtInfoLevel, sourceIPs, strings.Split(cfg.LogRequestExcludeHeadersList, ","))
432+
defaultLogMiddleware.DisableRequestSuccessLog = cfg.DisableRequestSuccessLog
433+
431434
defaultHTTPMiddleware := []middleware.Interface{
432435
middleware.Tracer{
433436
RouteMatcher: router,
434437
SourceIPs: sourceIPs,
435438
},
436-
middleware.NewLogMiddleware(log, cfg.LogRequestHeaders, cfg.LogRequestAtInfoLevel, sourceIPs, strings.Split(cfg.LogRequestExcludeHeadersList, ",")),
439+
defaultLogMiddleware,
437440
middleware.Instrument{
438441
RouteMatcher: router,
439442
Duration: requestDuration,

0 commit comments

Comments
 (0)