Skip to content

Commit c2c139d

Browse files
ploxilnploxiln
committed
use app.logf() for internal packages for nsqd nsqlookupd nsqadmin
Instead of setting a Logger for github.com/nsqio/nsq/internal packages, pass a logf() function, so it is called with and honors a LogLevel. * internal/clusterinfo/ * internal/http_api/ * internal/protocol/ nsqd lookupPeer also needed to be converted Get rid of interal.app.Logger type, but internal/test/ needs its own Logger definition to avoid circular import with internal/lg/ tests.
1 parent 2b4a610 commit c2c139d

File tree

15 files changed

+64
-70
lines changed

15 files changed

+64
-70
lines changed

‎internal/app/logger.go

-5
This file was deleted.

‎internal/clusterinfo/data.go

+5-9
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111

1212
"github.com/blang/semver"
1313
"github.com/nsqio/nsq/internal/http_api"
14+
"github.com/nsqio/nsq/internal/lg"
1415
"github.com/nsqio/nsq/internal/stringy"
1516
)
1617

@@ -33,27 +34,22 @@ func (l ErrList) Errors() []error {
3334
return l
3435
}
3536

36-
type logger interface {
37-
Output(maxdepth int, s string) error
38-
}
39-
4037
type ClusterInfo struct {
41-
log logger
38+
log lg.AppLogFunc
4239
client *http_api.Client
4340
}
4441

45-
func New(log logger, client *http_api.Client) *ClusterInfo {
42+
func New(log lg.AppLogFunc, client *http_api.Client) *ClusterInfo {
4643
return &ClusterInfo{
4744
log: log,
4845
client: client,
4946
}
5047
}
5148

5249
func (c *ClusterInfo) logf(f string, args ...interface{}) {
53-
if c.log == nil {
54-
return
50+
if c.log != nil {
51+
c.log(lg.INFO, f, args...)
5552
}
56-
c.log.Output(2, fmt.Sprintf(f, args...))
5753
}
5854

5955
// GetVersion returns a semver.Version object by querying /info

‎internal/http_api/api_response.go

+11-11
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import (
88
"time"
99

1010
"github.com/julienschmidt/httprouter"
11-
"github.com/nsqio/nsq/internal/app"
11+
"github.com/nsqio/nsq/internal/lg"
1212
)
1313

1414
type Decorator func(APIHandler) APIHandler
@@ -112,7 +112,7 @@ func Decorate(f APIHandler, ds ...Decorator) httprouter.Handle {
112112
}
113113
}
114114

115-
func Log(l app.Logger) Decorator {
115+
func Log(logf lg.AppLogFunc) Decorator {
116116
return func(f APIHandler) APIHandler {
117117
return func(w http.ResponseWriter, req *http.Request, ps httprouter.Params) (interface{}, error) {
118118
start := time.Now()
@@ -122,34 +122,34 @@ func Log(l app.Logger) Decorator {
122122
if e, ok := err.(Err); ok {
123123
status = e.Code
124124
}
125-
l.Output(2, fmt.Sprintf("%d %s %s (%s) %s",
126-
status, req.Method, req.URL.RequestURI(), req.RemoteAddr, elapsed))
125+
logf(lg.INFO, "%d %s %s (%s) %s",
126+
status, req.Method, req.URL.RequestURI(), req.RemoteAddr, elapsed)
127127
return response, err
128128
}
129129
}
130130
}
131131

132-
func LogPanicHandler(l app.Logger) func(w http.ResponseWriter, req *http.Request, p interface{}) {
132+
func LogPanicHandler(logf lg.AppLogFunc) func(w http.ResponseWriter, req *http.Request, p interface{}) {
133133
return func(w http.ResponseWriter, req *http.Request, p interface{}) {
134-
l.Output(2, fmt.Sprintf("ERROR: panic in HTTP handler - %s", p))
134+
logf(lg.ERROR, "panic in HTTP handler - %s", p)
135135
Decorate(func(w http.ResponseWriter, req *http.Request, ps httprouter.Params) (interface{}, error) {
136136
return nil, Err{500, "INTERNAL_ERROR"}
137-
}, Log(l), V1)(w, req, nil)
137+
}, Log(logf), V1)(w, req, nil)
138138
}
139139
}
140140

141-
func LogNotFoundHandler(l app.Logger) http.Handler {
141+
func LogNotFoundHandler(logf lg.AppLogFunc) http.Handler {
142142
return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
143143
Decorate(func(w http.ResponseWriter, req *http.Request, ps httprouter.Params) (interface{}, error) {
144144
return nil, Err{404, "NOT_FOUND"}
145-
}, Log(l), V1)(w, req, nil)
145+
}, Log(logf), V1)(w, req, nil)
146146
})
147147
}
148148

149-
func LogMethodNotAllowedHandler(l app.Logger) http.Handler {
149+
func LogMethodNotAllowedHandler(logf lg.AppLogFunc) http.Handler {
150150
return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
151151
Decorate(func(w http.ResponseWriter, req *http.Request, ps httprouter.Params) (interface{}, error) {
152152
return nil, Err{405, "METHOD_NOT_ALLOWED"}
153-
}, Log(l), V1)(w, req, nil)
153+
}, Log(logf), V1)(w, req, nil)
154154
})
155155
}

‎internal/http_api/http_server.go

+8-9
Original file line numberDiff line numberDiff line change
@@ -1,36 +1,35 @@
11
package http_api
22

33
import (
4-
"fmt"
54
"log"
65
"net"
76
"net/http"
87
"strings"
98

10-
"github.com/nsqio/nsq/internal/app"
9+
"github.com/nsqio/nsq/internal/lg"
1110
)
1211

1312
type logWriter struct {
14-
app.Logger
13+
logf lg.AppLogFunc
1514
}
1615

1716
func (l logWriter) Write(p []byte) (int, error) {
18-
l.Logger.Output(2, string(p))
17+
l.logf(lg.WARN, "%s", string(p))
1918
return len(p), nil
2019
}
2120

22-
func Serve(listener net.Listener, handler http.Handler, proto string, l app.Logger) {
23-
l.Output(2, fmt.Sprintf("%s: listening on %s", proto, listener.Addr()))
21+
func Serve(listener net.Listener, handler http.Handler, proto string, logf lg.AppLogFunc) {
22+
logf(lg.INFO, "%s: listening on %s", proto, listener.Addr())
2423

2524
server := &http.Server{
2625
Handler: handler,
27-
ErrorLog: log.New(logWriter{l}, "", 0),
26+
ErrorLog: log.New(logWriter{logf}, "", 0),
2827
}
2928
err := server.Serve(listener)
3029
// theres no direct way to detect this error because it is not exposed
3130
if err != nil && !strings.Contains(err.Error(), "use of closed network connection") {
32-
l.Output(2, fmt.Sprintf("ERROR: http.Serve() - %s", err))
31+
logf(lg.ERROR, "http.Serve() - %s", err)
3332
}
3433

35-
l.Output(2, fmt.Sprintf("%s: closing %s", proto, listener.Addr()))
34+
logf(lg.INFO, "%s: closing %s", proto, listener.Addr())
3635
}

‎internal/lg/lg.go

+2
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ const (
1616
FATAL = LogLevel(5)
1717
)
1818

19+
type AppLogFunc func(lvl LogLevel, f string, args ...interface{})
20+
1921
type Logger interface {
2022
Output(maxdepth int, s string) error
2123
}

‎internal/protocol/tcp_server.go

+6-7
Original file line numberDiff line numberDiff line change
@@ -1,37 +1,36 @@
11
package protocol
22

33
import (
4-
"fmt"
54
"net"
65
"runtime"
76
"strings"
87

9-
"github.com/nsqio/nsq/internal/app"
8+
"github.com/nsqio/nsq/internal/lg"
109
)
1110

1211
type TCPHandler interface {
1312
Handle(net.Conn)
1413
}
1514

16-
func TCPServer(listener net.Listener, handler TCPHandler, l app.Logger) {
17-
l.Output(2, fmt.Sprintf("TCP: listening on %s", listener.Addr()))
15+
func TCPServer(listener net.Listener, handler TCPHandler, logf lg.AppLogFunc) {
16+
logf(lg.INFO, "TCP: listening on %s", listener.Addr())
1817

1918
for {
2019
clientConn, err := listener.Accept()
2120
if err != nil {
2221
if nerr, ok := err.(net.Error); ok && nerr.Temporary() {
23-
l.Output(2, fmt.Sprintf("NOTICE: temporary Accept() failure - %s", err))
22+
logf(lg.WARN, "temporary Accept() failure - %s", err)
2423
runtime.Gosched()
2524
continue
2625
}
2726
// theres no direct way to detect this error because it is not exposed
2827
if !strings.Contains(err.Error(), "use of closed network connection") {
29-
l.Output(2, fmt.Sprintf("ERROR: listener.Accept() - %s", err))
28+
logf(lg.ERROR, "listener.Accept() - %s", err)
3029
}
3130
break
3231
}
3332
go handler.Handle(clientConn)
3433
}
3534

36-
l.Output(2, fmt.Sprintf("TCP: closing %s", listener.Addr()))
35+
logf(lg.INFO, "TCP: closing %s", listener.Addr())
3736
}

‎internal/test/logger.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
package test
22

3-
import (
4-
"github.com/nsqio/nsq/internal/app"
5-
)
3+
type Logger interface {
4+
Output(maxdepth int, s string) error
5+
}
66

77
type tbLog interface {
88
Log(...interface{})
@@ -17,6 +17,6 @@ func (tl *testLogger) Output(maxdepth int, s string) error {
1717
return nil
1818
}
1919

20-
func NewTestLogger(tbl tbLog) app.Logger {
20+
func NewTestLogger(tbl tbLog) Logger {
2121
return &testLogger{tbl}
2222
}

‎nsqadmin/http.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -55,21 +55,21 @@ type httpServer struct {
5555
}
5656

5757
func NewHTTPServer(ctx *Context) *httpServer {
58-
log := http_api.Log(ctx.nsqadmin.getOpts().Logger)
58+
log := http_api.Log(ctx.nsqadmin.logf)
5959

6060
client := http_api.NewClient(ctx.nsqadmin.httpClientTLSConfig, ctx.nsqadmin.getOpts().HTTPClientConnectTimeout,
6161
ctx.nsqadmin.getOpts().HTTPClientRequestTimeout)
6262

6363
router := httprouter.New()
6464
router.HandleMethodNotAllowed = true
65-
router.PanicHandler = http_api.LogPanicHandler(ctx.nsqadmin.getOpts().Logger)
66-
router.NotFound = http_api.LogNotFoundHandler(ctx.nsqadmin.getOpts().Logger)
67-
router.MethodNotAllowed = http_api.LogMethodNotAllowedHandler(ctx.nsqadmin.getOpts().Logger)
65+
router.PanicHandler = http_api.LogPanicHandler(ctx.nsqadmin.logf)
66+
router.NotFound = http_api.LogNotFoundHandler(ctx.nsqadmin.logf)
67+
router.MethodNotAllowed = http_api.LogMethodNotAllowedHandler(ctx.nsqadmin.logf)
6868
s := &httpServer{
6969
ctx: ctx,
7070
router: router,
7171
client: client,
72-
ci: clusterinfo.New(ctx.nsqadmin.getOpts().Logger, client),
72+
ci: clusterinfo.New(ctx.nsqadmin.logf, client),
7373
}
7474

7575
router.Handle("GET", "/ping", http_api.Decorate(s.pingHandler, log, http_api.PlainText))

‎nsqadmin/nsqadmin.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ func (n *NSQAdmin) Main() {
179179
n.Unlock()
180180
httpServer := NewHTTPServer(&Context{n})
181181
n.waitGroup.Wrap(func() {
182-
http_api.Serve(n.httpListener, http_api.CompressHandler(httpServer), "HTTP", n.getOpts().Logger)
182+
http_api.Serve(n.httpListener, http_api.CompressHandler(httpServer), "HTTP", n.logf)
183183
})
184184
n.waitGroup.Wrap(func() { n.handleAdminActions() })
185185
}

‎nsqd/http.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,13 @@ type httpServer struct {
3232
}
3333

3434
func newHTTPServer(ctx *context, tlsEnabled bool, tlsRequired bool) *httpServer {
35-
log := http_api.Log(ctx.nsqd.getOpts().Logger)
35+
log := http_api.Log(ctx.nsqd.logf)
3636

3737
router := httprouter.New()
3838
router.HandleMethodNotAllowed = true
39-
router.PanicHandler = http_api.LogPanicHandler(ctx.nsqd.getOpts().Logger)
40-
router.NotFound = http_api.LogNotFoundHandler(ctx.nsqd.getOpts().Logger)
41-
router.MethodNotAllowed = http_api.LogMethodNotAllowedHandler(ctx.nsqd.getOpts().Logger)
39+
router.PanicHandler = http_api.LogPanicHandler(ctx.nsqd.logf)
40+
router.NotFound = http_api.LogNotFoundHandler(ctx.nsqd.logf)
41+
router.MethodNotAllowed = http_api.LogMethodNotAllowedHandler(ctx.nsqd.logf)
4242
s := &httpServer{
4343
ctx: ctx,
4444
tlsEnabled: tlsEnabled,

‎nsqd/lookup.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ func (n *NSQD) lookupLoop() {
7070
continue
7171
}
7272
n.logf(LOG_INFO, "LOOKUP(%s): adding peer", host)
73-
lookupPeer := newLookupPeer(host, n.getOpts().MaxBodySize, n.getOpts().Logger,
73+
lookupPeer := newLookupPeer(host, n.getOpts().MaxBodySize, n.logf,
7474
connectCallback(n, hostname, syncTopicChan))
7575
lookupPeer.Command(nil) // start the connection
7676
lookupPeers = append(lookupPeers, lookupPeer)

‎nsqd/lookup_peer.go

+5-4
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"time"
99

1010
"github.com/nsqio/go-nsq"
11+
"github.com/nsqio/nsq/internal/lg"
1112
)
1213

1314
// lookupPeer is a low-level type for connecting/reading/writing to nsqlookupd
@@ -16,7 +17,7 @@ import (
1617
// gracefully (i.e. it is all handled by the library). Clients can simply use the
1718
// Command interface to perform a round-trip.
1819
type lookupPeer struct {
19-
l Logger
20+
logf lg.AppLogFunc
2021
addr string
2122
conn net.Conn
2223
state int32
@@ -36,9 +37,9 @@ type peerInfo struct {
3637
// newLookupPeer creates a new lookupPeer instance connecting to the supplied address.
3738
//
3839
// The supplied connectCallback will be called *every* time the instance connects.
39-
func newLookupPeer(addr string, maxBodySize int64, l Logger, connectCallback func(*lookupPeer)) *lookupPeer {
40+
func newLookupPeer(addr string, maxBodySize int64, l lg.AppLogFunc, connectCallback func(*lookupPeer)) *lookupPeer {
4041
return &lookupPeer{
41-
l: l,
42+
logf: l,
4243
addr: addr,
4344
state: stateDisconnected,
4445
maxBodySize: maxBodySize,
@@ -48,7 +49,7 @@ func newLookupPeer(addr string, maxBodySize int64, l Logger, connectCallback fun
4849

4950
// Connect will Dial the specified address, with timeouts
5051
func (lp *lookupPeer) Connect() error {
51-
lp.l.Output(2, fmt.Sprintf("LOOKUP connecting to %s", lp.addr))
52+
lp.logf(lg.INFO, "LOOKUP connecting to %s", lp.addr)
5253
conn, err := net.DialTimeout("tcp", lp.addr, time.Second)
5354
if err != nil {
5455
return err

‎nsqd/nsqd.go

+6-4
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,11 @@ func New(opts *Options) *NSQD {
8787
exitChan: make(chan int),
8888
notifyChan: make(chan interface{}),
8989
optsNotificationChan: make(chan struct{}, 1),
90-
ci: clusterinfo.New(opts.Logger, http_api.NewClient(nil, opts.HTTPClientConnectTimeout, opts.HTTPClientRequestTimeout)),
9190
dl: dirlock.New(dataPath),
9291
}
92+
httpcli := http_api.NewClient(nil, opts.HTTPClientConnectTimeout, opts.HTTPClientRequestTimeout)
93+
n.ci = clusterinfo.New(n.logf, httpcli)
94+
9395
n.swapOpts(opts)
9496
n.errValue.Store(errStore{})
9597

@@ -226,7 +228,7 @@ func (n *NSQD) Main() {
226228
n.Unlock()
227229
tcpServer := &tcpServer{ctx: ctx}
228230
n.waitGroup.Wrap(func() {
229-
protocol.TCPServer(n.tcpListener, tcpServer, n.getOpts().Logger)
231+
protocol.TCPServer(n.tcpListener, tcpServer, n.logf)
230232
})
231233

232234
if n.tlsConfig != nil && n.getOpts().HTTPSAddress != "" {
@@ -240,7 +242,7 @@ func (n *NSQD) Main() {
240242
n.Unlock()
241243
httpsServer := newHTTPServer(ctx, true, true)
242244
n.waitGroup.Wrap(func() {
243-
http_api.Serve(n.httpsListener, httpsServer, "HTTPS", n.getOpts().Logger)
245+
http_api.Serve(n.httpsListener, httpsServer, "HTTPS", n.logf)
244246
})
245247
}
246248
httpListener, err = net.Listen("tcp", n.getOpts().HTTPAddress)
@@ -253,7 +255,7 @@ func (n *NSQD) Main() {
253255
n.Unlock()
254256
httpServer := newHTTPServer(ctx, false, n.getOpts().TLSRequired == TLSRequired)
255257
n.waitGroup.Wrap(func() {
256-
http_api.Serve(n.httpListener, httpServer, "HTTP", n.getOpts().Logger)
258+
http_api.Serve(n.httpListener, httpServer, "HTTP", n.logf)
257259
})
258260

259261
n.waitGroup.Wrap(func() { n.queueScanLoop() })

‎nsqlookupd/http.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,13 @@ type httpServer struct {
1818
}
1919

2020
func newHTTPServer(ctx *Context) *httpServer {
21-
log := http_api.Log(ctx.nsqlookupd.opts.Logger)
21+
log := http_api.Log(ctx.nsqlookupd.logf)
2222

2323
router := httprouter.New()
2424
router.HandleMethodNotAllowed = true
25-
router.PanicHandler = http_api.LogPanicHandler(ctx.nsqlookupd.opts.Logger)
26-
router.NotFound = http_api.LogNotFoundHandler(ctx.nsqlookupd.opts.Logger)
27-
router.MethodNotAllowed = http_api.LogMethodNotAllowedHandler(ctx.nsqlookupd.opts.Logger)
25+
router.PanicHandler = http_api.LogPanicHandler(ctx.nsqlookupd.logf)
26+
router.NotFound = http_api.LogNotFoundHandler(ctx.nsqlookupd.logf)
27+
router.MethodNotAllowed = http_api.LogMethodNotAllowedHandler(ctx.nsqlookupd.logf)
2828
s := &httpServer{
2929
ctx: ctx,
3030
router: router,

0 commit comments

Comments
 (0)