From f27de9a80412242434d6b6c8c526a59e679b887b Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Thu, 4 Feb 2016 19:29:49 -0800 Subject: [PATCH] Fixed logging Signed-off-by: Vishal Rana --- context.go | 8 ++-- echo.go | 31 ++++---------- engine/fasthttp/server.go | 14 +++---- engine/standard/response.go | 7 +++- engine/standard/server.go | 14 ++++--- glide.lock | 12 +++--- logger/logger.go | 21 ++++++++++ middleware/logger.go | 4 +- middleware/logger_test.go | 3 +- response.go | 84 ------------------------------------- response_test.go | 66 ----------------------------- test/http.go | 3 +- 12 files changed, 64 insertions(+), 203 deletions(-) create mode 100644 logger/logger.go delete mode 100644 response.go delete mode 100644 response_test.go diff --git a/context.go b/context.go index ff7e1a98..f327adc7 100644 --- a/context.go +++ b/context.go @@ -8,7 +8,7 @@ import ( "time" "github.com/labstack/echo/engine" - "github.com/labstack/gommon/log" + "github.com/labstack/echo/logger" "net/url" @@ -46,7 +46,7 @@ type ( NoContent(int) error Redirect(int, string) error Error(err error) - Logger() *log.Logger + Logger() logger.Logger Context() *context } @@ -303,7 +303,7 @@ func (c *context) Error(err error) { } // Logger returns the `Logger` instance. -func (c *context) Logger() *log.Logger { +func (c *context) Logger() logger.Logger { return c.echo.logger } @@ -314,8 +314,6 @@ func (c *context) Context() *context { func (c *context) reset(req engine.Request, res engine.Response, e *Echo) { c.request = req - // TODO: v2 - // c.response.reset(res, e) c.response = res c.query = nil c.store = nil diff --git a/echo.go b/echo.go index 2a7d607f..d4821944 100644 --- a/echo.go +++ b/echo.go @@ -13,13 +13,13 @@ import ( "runtime" "strings" "sync" - "time" "encoding/xml" "github.com/labstack/echo/engine" "github.com/labstack/echo/engine/fasthttp" "github.com/labstack/echo/engine/standard" + "github.com/labstack/echo/logger" "github.com/labstack/gommon/log" ) @@ -41,7 +41,7 @@ type ( engineType engine.Type engine engine.Engine router *Router - logger *log.Logger + logger logger.Logger } Route struct { @@ -184,8 +184,6 @@ var ( methodNotAllowedHandler = func(c Context) error { return NewHTTPError(http.StatusMethodNotAllowed) } - - unixEpochTime = time.Unix(0, 0) ) // New creates an instance of Echo. @@ -222,7 +220,6 @@ func New() (e *Echo) { // Logger e.logger = log.New("echo") - e.logger.SetLevel(log.INFO) return } @@ -232,23 +229,13 @@ func (e *Echo) Router() *Router { return e.router } -// SetLogPrefix sets the prefix for the logger. Default value is `echo`. -func (e *Echo) SetLogPrefix(prefix string) { - e.logger.SetPrefix(prefix) -} - -// SetLogOutput sets the output destination for the logger. Default value is `os.Stdout` -func (e *Echo) SetLogOutput(w io.Writer) { - e.logger.SetOutput(w) -} - -// SetLogLevel sets the log level for the logger. Default value is `log.INFO`. -func (e *Echo) SetLogLevel(l log.Level) { - e.logger.SetLevel(l) +// SetLogger sets the logger instance. +func (e *Echo) SetLogger(logger logger.Logger) { + e.logger = logger } // Logger returns the logger instance. -func (e *Echo) Logger() *log.Logger { +func (e *Echo) Logger() logger.Logger { return e.logger } @@ -592,13 +579,13 @@ func (e *Echo) RunWithConfig(config *engine.Config) { e.pool.Put(c) } - e.engine = standard.NewServer(config, handler) switch e.engineType { case engine.FastHTTP: - e.engine = fasthttp.NewServer(config, handler) + e.engine = fasthttp.NewServer(config, handler, e.logger) + default: + e.engine = standard.NewServer(config, handler, e.logger) } - e.engine.Start() } diff --git a/engine/fasthttp/server.go b/engine/fasthttp/server.go index a4b0ad28..1f2ee558 100644 --- a/engine/fasthttp/server.go +++ b/engine/fasthttp/server.go @@ -3,10 +3,8 @@ package fasthttp import ( "net/http" - "github.com/labstack/gommon/log" -) -import ( "github.com/labstack/echo/engine" + "github.com/labstack/echo/logger" "github.com/valyala/fasthttp" ) @@ -15,14 +13,16 @@ type ( *http.Server config *engine.Config handler engine.HandlerFunc + logger logger.Logger } ) -func NewServer(config *engine.Config, handler engine.HandlerFunc) *Server { +func NewServer(c *engine.Config, h engine.HandlerFunc, l logger.Logger) *Server { return &Server{ Server: new(http.Server), - config: config, - handler: handler, + config: c, + handler: h, + logger: l, } } @@ -39,5 +39,5 @@ func (s *Server) Start() { } s.handler(req, res) }) - log.Fatal(s.ListenAndServe()) + s.logger.Fatal(s.ListenAndServe()) } diff --git a/engine/standard/response.go b/engine/standard/response.go index 0da2c3d5..d0de29ab 100644 --- a/engine/standard/response.go +++ b/engine/standard/response.go @@ -5,6 +5,7 @@ import ( "net/http" "github.com/labstack/echo/engine" + "github.com/labstack/echo/logger" ) type ( @@ -15,14 +16,16 @@ type ( size int64 committed bool writer io.Writer + logger logger.Logger } ) -func NewResponse(w http.ResponseWriter) *Response { +func NewResponse(w http.ResponseWriter, l logger.Logger) *Response { return &Response{ response: w, header: &Header{w.Header()}, writer: w, + logger: l, } } @@ -32,7 +35,7 @@ func (r *Response) Header() engine.Header { func (r *Response) WriteHeader(code int) { if r.committed { - // r.echo.Logger().Warn("response already committed") + r.logger.Warn("response already committed") return } r.status = code diff --git a/engine/standard/server.go b/engine/standard/server.go index 9c527d0b..d33736ce 100644 --- a/engine/standard/server.go +++ b/engine/standard/server.go @@ -1,11 +1,11 @@ package standard import ( - "log" "net/http" "sync" "github.com/labstack/echo/engine" + "github.com/labstack/echo/logger" ) type ( @@ -14,6 +14,7 @@ type ( config *engine.Config handler engine.HandlerFunc pool *Pool + logger logger.Logger } Pool struct { @@ -24,11 +25,11 @@ type ( } ) -func NewServer(config *engine.Config, handler engine.HandlerFunc) *Server { +func NewServer(c *engine.Config, h engine.HandlerFunc, l logger.Logger) *Server { return &Server{ Server: new(http.Server), - config: config, - handler: handler, + config: c, + handler: h, pool: &Pool{ request: sync.Pool{ New: func() interface{} { @@ -37,7 +38,7 @@ func NewServer(config *engine.Config, handler engine.HandlerFunc) *Server { }, response: sync.Pool{ New: func() interface{} { - return &Response{} + return &Response{logger: l} }, }, header: sync.Pool{ @@ -51,6 +52,7 @@ func NewServer(config *engine.Config, handler engine.HandlerFunc) *Server { }, }, }, + logger: l, } } @@ -78,5 +80,5 @@ func (s *Server) Start() { s.pool.response.Put(res) s.pool.header.Put(resHdr) }) - log.Fatal(s.ListenAndServe()) + s.logger.Fatal(s.ListenAndServe()) } diff --git a/glide.lock b/glide.lock index 110c0bd7..41ddc21a 100644 --- a/glide.lock +++ b/glide.lock @@ -1,8 +1,8 @@ hash: 57f60c6f58e40de3ea10123144095336abe2a4024e256feea9721e4e89ad38be -updated: 2016-01-28T19:14:46.587206305-08:00 +updated: 2016-02-04T19:09:01.247772095-08:00 imports: - name: github.com/labstack/gommon - version: 25c3ce2d2abe76d8dd935305f1a8c1063dd3aac3 + version: 28bf9248c3e81039c18bff6a1cf2a7f0841caf35 subpackages: - /color - log @@ -11,17 +11,15 @@ imports: - name: github.com/mattn/go-isatty version: 56b76bdf51f7708750eac80fa38b952bb9f32639 - name: github.com/valyala/fasthttp - version: 359b3a1dd826fd4f7a4375f7072e7314ddad2400 + version: df213349e25e909911691ffe752149440d969641 - name: golang.org/x/crypto version: 1f22c0103821b9390939b6776727195525381532 - repo: https://golang.org/x/crypto - name: golang.org/x/net - version: 04b9de9b512f58addf28c9853d50ebef61c3953e + version: 493a26246902f2887349f625a5f846bf0286af49 subpackages: - /context - http2 - websocket - name: golang.org/x/text - version: 6d3c22c4525a4da167968fa2479be5524d2e8bd0 - repo: https://golang.org/x/text + version: cd1d59e467ef512633026c0f15282e108e41d453 devImports: [] diff --git a/logger/logger.go b/logger/logger.go new file mode 100644 index 00000000..540874ad --- /dev/null +++ b/logger/logger.go @@ -0,0 +1,21 @@ +package logger + +type ( + // Logger is the interface that declares Echo's logging system. + Logger interface { + Debug(...interface{}) + Debugf(string, ...interface{}) + + Info(...interface{}) + Infof(string, ...interface{}) + + Warn(...interface{}) + Warnf(string, ...interface{}) + + Error(...interface{}) + Errorf(string, ...interface{}) + + Fatal(...interface{}) + Fatalf(string, ...interface{}) + } +) diff --git a/middleware/logger.go b/middleware/logger.go index e98a3e9d..b224fb47 100644 --- a/middleware/logger.go +++ b/middleware/logger.go @@ -29,7 +29,7 @@ func Logger() echo.MiddlewareFunc { c.Error(err) } stop := time.Now() - method := req.Method + method := req.Method() path := req.URL().Path() if path == "" { path = "/" @@ -47,7 +47,7 @@ func Logger() echo.MiddlewareFunc { code = color.Cyan(n) } - logger.Info("%s %s %s %s %s %d", remoteAddr, method, path, code, stop.Sub(start), size) + logger.Infof("%s %s %s %s %s %d", remoteAddr, method, path, code, stop.Sub(start), size) return nil } } diff --git a/middleware/logger_test.go b/middleware/logger_test.go index b485ce95..bd739e62 100644 --- a/middleware/logger_test.go +++ b/middleware/logger_test.go @@ -8,6 +8,7 @@ import ( "github.com/labstack/echo" "github.com/labstack/echo/test" + "github.com/labstack/gommon/log" "github.com/stretchr/testify/assert" ) @@ -56,7 +57,7 @@ func TestLoggerIPAddress(t *testing.T) { rec := test.NewResponseRecorder() c := echo.NewContext(req, rec, e) buf := new(bytes.Buffer) - e.Logger().SetOutput(buf) + e.Logger().(*log.Logger).SetOutput(buf) ip := "127.0.0.1" h := func(c echo.Context) error { return c.String(http.StatusOK, "test") diff --git a/response.go b/response.go deleted file mode 100644 index 1e43df9e..00000000 --- a/response.go +++ /dev/null @@ -1,84 +0,0 @@ -package echo - -import ( - "bufio" - "net" - "net/http" -) - -type ( - Response struct { - writer http.ResponseWriter - status int - size int64 - committed bool - echo *Echo - } -) - -func NewResponse(w http.ResponseWriter, e *Echo) *Response { - return &Response{writer: w, echo: e} -} - -func (r *Response) SetWriter(w http.ResponseWriter) { - r.writer = w -} - -func (r *Response) Header() http.Header { - return r.writer.Header() -} - -func (r *Response) Writer() http.ResponseWriter { - return r.writer -} - -func (r *Response) WriteHeader(code int) { - if r.committed { - r.echo.Logger().Warn("response already committed") - return - } - r.status = code - r.writer.WriteHeader(code) - r.committed = true -} - -func (r *Response) Write(b []byte) (n int, err error) { - n, err = r.writer.Write(b) - r.size += int64(n) - return n, err -} - -// Flush wraps response writer's Flush function. -func (r *Response) Flush() { - r.writer.(http.Flusher).Flush() -} - -// Hijack wraps response writer's Hijack function. -func (r *Response) Hijack() (net.Conn, *bufio.ReadWriter, error) { - return r.writer.(http.Hijacker).Hijack() -} - -// CloseNotify wraps response writer's CloseNotify function. -func (r *Response) CloseNotify() <-chan bool { - return r.writer.(http.CloseNotifier).CloseNotify() -} - -func (r *Response) Status() int { - return r.status -} - -func (r *Response) Size() int64 { - return r.size -} - -func (r *Response) Committed() bool { - return r.committed -} - -func (r *Response) reset(w http.ResponseWriter, e *Echo) { - r.writer = w - r.size = 0 - r.status = http.StatusOK - r.committed = false - r.echo = e -} diff --git a/response_test.go b/response_test.go deleted file mode 100644 index 26643637..00000000 --- a/response_test.go +++ /dev/null @@ -1,66 +0,0 @@ -package echo - -import ( - "net/http" - "net/http/httptest" - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestResponse(t *testing.T) { - e := New() - w := httptest.NewRecorder() - r := NewResponse(w, e) - - // SetWriter - r.SetWriter(w) - - // Writer - assert.Equal(t, w, r.Writer()) - - // Header - assert.NotNil(t, r.Header()) - - // WriteHeader - r.WriteHeader(http.StatusOK) - assert.Equal(t, http.StatusOK, r.status) - - // Committed - assert.True(t, r.committed) - - // Already committed - r.WriteHeader(http.StatusTeapot) - assert.NotEqual(t, http.StatusTeapot, r.Status()) - - // Status - r.status = http.StatusOK - assert.Equal(t, http.StatusOK, r.Status()) - - // Write - s := "echo" - _, err := r.Write([]byte(s)) - assert.NoError(t, err) - - // Flush - r.Flush() - - // Size - assert.EqualValues(t, len(s), r.Size()) - - // Committed - assert.Equal(t, true, r.Committed()) - - // Hijack - assert.Panics(t, func() { - r.Hijack() - }) - - // CloseNotify - assert.Panics(t, func() { - r.CloseNotify() - }) - - // reset - r.reset(httptest.NewRecorder(), New()) -} diff --git a/test/http.go b/test/http.go index 6d89f57d..2053abfc 100644 --- a/test/http.go +++ b/test/http.go @@ -8,6 +8,7 @@ import ( "github.com/labstack/echo/engine" "github.com/labstack/echo/engine/standard" + "github.com/labstack/gommon/log" ) type ( @@ -25,7 +26,7 @@ func NewRequest(method, url string, body io.Reader) engine.Request { func NewResponseRecorder() *ResponseRecorder { r := httptest.NewRecorder() return &ResponseRecorder{ - Response: standard.NewResponse(r), + Response: standard.NewResponse(r, log.New("test")), Body: r.Body, } }