ziflex / lecho

Zerolog wrapper for Echo framework 🍅
MIT License
97 stars 11 forks source link

allow user-defined per-request log fields #18

Closed ryepup closed 1 year ago

ryepup commented 1 year ago

I'm using opentelemetry (otel) to trace my echo requests, and I would like the otel traceID to be a field in the logs made by lecho. Currently lecho has special handling for X-Request-ID, it would be great if that were extensible.

My current workaround is a partial fork of lecho using echo's middleware.RequestLoggerWithConfig, and it'd be nice to drop that.

One possible API would look something like:

e := echo.New()
logger := lecho.New(os.Stdout)
e.Logger = logger
e.Use(lecho.Middleware(
    lecho.Config{Logger: logger},
    lecho.WithPerRequestFields(func (c echo.Context, evt *zerolog.Event){
        // TODO: pull traceID off `c`, add to `evt`
    }),
))  

or maybe lump it into the logger itself:

e := echo.New()
logger := lecho.New(os.Stdout, lecho.WithPerRequestFields(func (c echo.Context, evt *zerolog.Event){
    // TODO: pull traceID off `c`, add to `evt`
}))
e.Logger = logger
e.Use(lecho.Middleware(lecho.Config{Logger: logger}))   

Would you be open to this kind of addition?

ziflex commented 1 year ago

hey, I'm open to the feature itself, but not sure I'm sold on the implementation proposal.

I will think about how to implement it.

ziflex commented 1 year ago

Hey, I added RequestIDHeader option to the Middleware configuration.

e.Use(lecho.Middleware(lecho.Config{
    Logger:          logger,
    RequestIDHeader: "traceID",
}))

Will it be sufficient for you? Or do you need both?

ziflex commented 1 year ago

Here is a way to enrich the logger with additional values: https://github.com/ziflex/lecho/pull/19

Let me know if it works for you.

ryepup commented 1 year ago

That looks great, very extensible.