valyala / fasthttp

Fast HTTP package for Go. Tuned for high performance. Zero memory allocations in hot paths. Up to 10x faster than net/http
MIT License
21.91k stars 1.76k forks source link

RequestCtx forces the usage of a logger #1867

Closed gaby closed 2 months ago

gaby commented 2 months ago

Looking at the Init() code from RequestCtx if nil is passes it defaults to using golangs native logger which adds latency.

Here https://github.com/valyala/fasthttp/blob/master/server.go#L2720

func (ctx *RequestCtx) Init(req *Request, remoteAddr net.Addr, logger Logger) {
    if remoteAddr == nil {
        remoteAddr = zeroTCPAddr
    }
    c := &fakeAddrer{
        laddr: zeroTCPAddr,
        raddr: remoteAddr,
    }
    if logger == nil {
        logger = defaultLogger
    }
    ctx.Init2(c, logger, true)
    req.CopyTo(&ctx.Request)
}

Is this something that could be removed or avoided?

Related to https://github.com/gofiber/fiber/issues/3130

gaby commented 2 months ago

This can be avoided using:

type NOOPLogger struct{}

func (NOOPLogger) Printf(ctx context.Context, format string, v ...interface{}) {
    // Do nothing here
}

But it adds overhead to the RequestCtx for no reason. Anything we can do about this ? @erikdubbelboer

erikdubbelboer commented 2 months ago

Changing this would in theory break backwards compatibility. It's such a minor thing and your NOOPLogger is a good solution that I think we should just leave it as is.