Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit81e1527

Browse files
committed
refactor after code review
1 parent8f550c5 commit81e1527

File tree

2 files changed

+29
-41
lines changed

2 files changed

+29
-41
lines changed

‎coderd/httpmw/logger.go‎

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -54,11 +54,7 @@ func Logger(log slog.Logger) func(next http.Handler) http.Handler {
5454
)
5555
}
5656

57-
// We already capture most of this information in the span (minus
58-
// the response body which we don't want to capture anyways).
59-
tracing.RunWithoutSpan(r.Context(),func(ctx context.Context) {
60-
logContext.WriteLog(ctx,sw.Status)
61-
})
57+
logContext.WriteLog(r.Context(),sw.Status)
6258
})
6359
}
6460
}
@@ -68,27 +64,29 @@ type RequestLogger interface {
6864
WriteLog(ctx context.Context,statusint)
6965
}
7066

71-
typeRequestContextLoggerstruct {
67+
typeSlogRequestLoggerstruct {
7268
log slog.Logger
7369
writtenbool
7470
messagestring
7571
start time.Time
7672
}
7773

74+
var_RequestLogger=&SlogRequestLogger{}
75+
7876
funcNewRequestLogger(log slog.Logger,messagestring,start time.Time)RequestLogger {
79-
return&RequestContextLogger{
77+
return&SlogRequestLogger{
8078
log:log,
8179
written:false,
8280
message:message,
8381
start:start,
8482
}
8583
}
8684

87-
func (c*RequestContextLogger)WithFields(fields...slog.Field) {
85+
func (c*SlogRequestLogger)WithFields(fields...slog.Field) {
8886
c.log=c.log.With(fields...)
8987
}
9088

91-
func (c*RequestContextLogger)WriteLog(ctx context.Context,statusint) {
89+
func (c*SlogRequestLogger)WriteLog(ctx context.Context,statusint) {
9290
ifc.written {
9391
return
9492
}
@@ -100,14 +98,18 @@ func (c *RequestContextLogger) WriteLog(ctx context.Context, status int) {
10098
slog.F("status_code",status),
10199
slog.F("latency_ms",float64(end.Sub(c.start)/time.Millisecond)),
102100
)
103-
// We should not log at level ERROR for 5xx status codes because 5xx
104-
// includes proxy errors etc. It also causes slogtest to fail
105-
// instantly without an error message by default.
106-
ifstatus>=http.StatusInternalServerError {
107-
logger.Warn(ctx,c.message)
108-
}else {
109-
logger.Debug(ctx,c.message)
110-
}
101+
// We already capture most of this information in the span (minus
102+
// the response body which we don't want to capture anyways).
103+
tracing.RunWithoutSpan(ctx,func(ctx context.Context) {
104+
// We should not log at level ERROR for 5xx status codes because 5xx
105+
// includes proxy errors etc. It also causes slogtest to fail
106+
// instantly without an error message by default.
107+
ifstatus>=http.StatusInternalServerError {
108+
logger.Warn(ctx,c.message)
109+
}else {
110+
logger.Debug(ctx,c.message)
111+
}
112+
})
111113
}
112114

113115
typelogContextKeystruct{}

‎coderd/httpmw/logger_internal_test.go‎

Lines changed: 10 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import (
88
"testing"
99
"time"
1010

11+
"github.com/stretchr/testify/require"
12+
1113
"cdr.dev/slog"
1214
"github.com/coder/coder/v2/coderd/tracing"
1315
"github.com/coder/coder/v2/testutil"
@@ -31,24 +33,16 @@ func TestRequestLogger_WriteLog(t *testing.T) {
3133
// Write log for 200 status
3234
logCtx.WriteLog(ctx,http.StatusOK)
3335

34-
iflen(sink.entries)!=1 {
35-
t.Fatalf("expected 1 log entry, got %d",len(sink.entries))
36-
}
36+
require.Len(t,sink.entries,1,"log was written twice")
3737

38-
ifsink.entries[0].Message!="GET" {
39-
t.Errorf("expected log message to be 'GET', got '%s'",sink.entries[0].Message)
40-
}
38+
require.Equal(t,sink.entries[0].Message,"GET","log message should be GET")
4139

42-
ifsink.entries[0].Fields[0].Value!="custom_value" {
43-
t.Errorf("expected a custom_field with value custom_value, got '%s'",sink.entries[0].Fields[0].Value)
44-
}
40+
require.Equal(t,sink.entries[0].Fields[0].Value,"custom_value","custom_field should be custom_value")
4541

4642
// Attempt to write again (should be skipped).
4743
logCtx.WriteLog(ctx,http.StatusInternalServerError)
4844

49-
iflen(sink.entries)!=1 {
50-
t.Fatalf("expected 1 log entry after second write, got %d",len(sink.entries))
51-
}
45+
require.Len(t,sink.entries,1,"log was written twice")
5246
}
5347

5448
funcTestLoggerMiddleware_SingleRequest(t*testing.T) {
@@ -79,13 +73,9 @@ func TestLoggerMiddleware_SingleRequest(t *testing.T) {
7973
// Serve the request
8074
wrappedHandler.ServeHTTP(sw,req)
8175

82-
iflen(sink.entries)!=1 {
83-
t.Fatalf("expected 1 log entry, got %d",len(sink.entries))
84-
}
76+
require.Len(t,sink.entries,1,"log was written twice")
8577

86-
ifsink.entries[0].Message!="GET" {
87-
t.Errorf("expected log message to be 'GET', got '%s'",sink.entries[0].Message)
88-
}
78+
require.Equal(t,sink.entries[0].Message,"GET","log message should be GET")
8979
}
9080

9181
funcTestLoggerMiddleware_WebSocket(t*testing.T) {
@@ -134,13 +124,9 @@ func TestLoggerMiddleware_WebSocket(t *testing.T) {
134124
}
135125
deferconn.Close(websocket.StatusNormalClosure,"")
136126
wg.Wait()
137-
iflen(sink.entries)!=1 {
138-
t.Fatalf("expected 1 log entry, got %d",len(sink.entries))
139-
}
127+
require.Len(t,sink.entries,1,"log was written twice")
140128

141-
ifsink.entries[0].Message!="GET" {
142-
t.Errorf("expected log message to be 'GET', got '%s'",sink.entries[0].Message)
143-
}
129+
require.Equal(t,sink.entries[0].Message,"GET","log message should be GET")
144130
}
145131

146132
typefakeSinkstruct {

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp