From bfa778d1905ce34fe88545a5b86728008f06c619 Mon Sep 17 00:00:00 2001 From: "M. J. Fromberger" Date: Tue, 26 Oct 2021 13:43:12 -0700 Subject: [PATCH] Rework the text logger interface. Instead of using a standard log.Logger directly, define Logger as a function to receive the formatted log string. Add a helper (StdLogger) that adapts a standard logger to the new interface, to simplify migration of existing use. Replace: Logger: log.New(...) with: Logger: jrpc2.StdLogger(log.New(...)) to get the equivalent behaviour. The StdLogger function also accepts nil as an argument to write to the default standard log. --- client.go | 2 +- opts.go | 37 ++++++++++++++++++++++++++++--------- server.go | 26 ++++++++++++-------------- server/local_test.go | 28 ++++++++++------------------ 4 files changed, 51 insertions(+), 42 deletions(-) diff --git a/client.go b/client.go index 85a0ba9..a2d00ff 100644 --- a/client.go +++ b/client.go @@ -37,7 +37,7 @@ type Client struct { func NewClient(ch channel.Channel, opts *ClientOptions) *Client { c := &Client{ done: new(sync.WaitGroup), - log: opts.logger(), + log: opts.logFunc(), allow1: opts.allowV1(), enctx: opts.encodeContext(), snote: opts.handleNotification(), diff --git a/opts.go b/opts.go index 33cd578..a62ac7f 100644 --- a/opts.go +++ b/opts.go @@ -17,7 +17,7 @@ import ( // It is safe to share server options among multiple server instances. type ServerOptions struct { // If not nil, send debug text logs here. - Logger *log.Logger + Logger Logger // If not nil, the methods of this value are called to log each request // received and each response or error returned. @@ -72,12 +72,11 @@ type ServerOptions struct { StartTime time.Time } -func (s *ServerOptions) logger() logger { +func (s *ServerOptions) logFunc() func(string, ...interface{}) { if s == nil || s.Logger == nil { return func(string, ...interface{}) {} } - logger := s.Logger - return func(msg string, args ...interface{}) { logger.Output(2, fmt.Sprintf(msg, args...)) } + return s.Logger.Printf } func (s *ServerOptions) allowV1() bool { return s != nil && s.AllowV1 } @@ -142,8 +141,8 @@ func (s *ServerOptions) rpcLog() RPCLogger { // ClientOptions control the behaviour of a client created by NewClient. // A nil *ClientOptions provides sensible defaults. type ClientOptions struct { - // If not nil, send debug logs here. - Logger *log.Logger + // If not nil, send debug text logs here. + Logger Logger // Instructs the client to tolerate responses that do not include the // required "jsonrpc" version marker. @@ -180,12 +179,11 @@ type ClientOptions struct { OnCancel func(cli *Client, rsp *Response) } -func (c *ClientOptions) logger() logger { +func (c *ClientOptions) logFunc() func(string, ...interface{}) { if c == nil || c.Logger == nil { return func(string, ...interface{}) {} } - logger := c.Logger - return func(msg string, args ...interface{}) { logger.Output(2, fmt.Sprintf(msg, args...)) } + return c.Logger.Printf } func (c *ClientOptions) allowV1() bool { return c != nil && c.AllowV1 } @@ -267,6 +265,27 @@ func panicToError(f func() (interface{}, error)) (v interface{}, err error) { return f() } +// A Logger records text logs from a server or a client. A nil logger discards +// text log input. +type Logger func(text string) + +// Printf writes a formatted message to the logger. If lg == nil, the message +// is discarded. +func (lg Logger) Printf(msg string, args ...interface{}) { + if lg != nil { + lg(fmt.Sprintf(msg, args...)) + } +} + +// StdLogger adapts a *log.Logger to a Logger. If logger == nil, the returned +// function sends logs to the default logger . +func StdLogger(logger *log.Logger) Logger { + if logger == nil { + return func(text string) { log.Output(2, text) } + } + return func(text string) { logger.Output(2, text) } +} + // An RPCLogger receives callbacks from a server to record the receipt of // requests and the delivery of responses. These callbacks are invoked // synchronously with the processing of the request. diff --git a/server.go b/server.go index 6496364..8b1f617 100644 --- a/server.go +++ b/server.go @@ -17,8 +17,6 @@ import ( "golang.org/x/sync/semaphore" ) -type logger = func(string, ...interface{}) - // A Server is a JSON-RPC 2.0 server. The server receives requests and sends // responses on a channel.Channel provided by the caller, and dispatches // requests to user-defined Handlers. @@ -28,17 +26,17 @@ type Server struct { sem *semaphore.Weighted // bounds concurrent execution (default 1) // Configurable settings - allow1 bool // allow v1 requests with no version marker - allowP bool // allow server notifications to the client - log logger // write debug logs here - rpcLog RPCLogger // log RPC requests and responses here - newctx func() context.Context // create a new base request context - dectx decoder // decode context from request - ckreq verifier // request checking hook - expctx bool // whether to expect request context - metrics *metrics.M // metrics collected during execution - start time.Time // when Start was called - builtin bool // whether built-in rpc.* methods are enabled + allow1 bool // allow v1 requests with no version marker + allowP bool // allow server notifications to the client + log func(string, ...interface{}) // write debug logs here + rpcLog RPCLogger // log RPC requests and responses here + newctx func() context.Context // create a new base request context + dectx decoder // decode context from request + ckreq verifier // request checking hook + expctx bool // whether to expect request context + metrics *metrics.M // metrics collected during execution + start time.Time // when Start was called + builtin bool // whether built-in rpc.* methods are enabled mu *sync.Mutex // protects the fields below @@ -75,7 +73,7 @@ func NewServer(mux Assigner, opts *ServerOptions) *Server { sem: semaphore.NewWeighted(opts.concurrency()), allow1: opts.allowV1(), allowP: opts.allowPush(), - log: opts.logger(), + log: opts.logFunc(), rpcLog: opts.rpcLog(), newctx: opts.newContext(), dectx: dc, diff --git a/server/local_test.go b/server/local_test.go index 096d469..f368bd8 100644 --- a/server/local_test.go +++ b/server/local_test.go @@ -3,8 +3,6 @@ package server_test import ( "context" "flag" - "log" - "os" "sync" "testing" @@ -13,25 +11,20 @@ import ( "github.com/creachadair/jrpc2/server" ) -var ( - doDebug = flag.Bool("debug", false, "Enable server and client debugging logs") +var doDebug = flag.Bool("debug", false, "Enable server and client debugging logs") - testOpts *server.LocalOptions - once sync.Once -) - -func setup() { - if *doDebug { - testOpts = &server.LocalOptions{ - Client: &jrpc2.ClientOptions{Logger: log.New(os.Stderr, "[local client] ", 0)}, - Server: &jrpc2.ServerOptions{Logger: log.New(os.Stderr, "[local server] ", 0)}, - } +func testOpts(t *testing.T) *server.LocalOptions { + if !*doDebug { + return nil + } + return &server.LocalOptions{ + Client: &jrpc2.ClientOptions{Logger: func(s string) { t.Log(s) }}, + Server: &jrpc2.ServerOptions{Logger: func(s string) { t.Log(s) }}, } } func TestLocal(t *testing.T) { - once.Do(setup) - loc := server.NewLocal(make(handler.Map), testOpts) + loc := server.NewLocal(make(handler.Map), testOpts(t)) ctx := context.Background() si, err := jrpc2.RPCServerInfo(ctx, loc.Client) if err != nil { @@ -54,10 +47,9 @@ func TestLocal(t *testing.T) { // Test that concurrent callers to a local service do not deadlock. func TestLocalConcurrent(t *testing.T) { - once.Do(setup) loc := server.NewLocal(handler.Map{ "Test": handler.New(func(context.Context) error { return nil }), - }, testOpts) + }, testOpts(t)) const numCallers = 20