grpc-ecosystem / go-grpc-middleware

Golang gRPC Middlewares: interceptor chaining, auth, logging, retries and more.
Apache License 2.0
6.26k stars 690 forks source link

ctxlogrus.Extract returns a useless logrus.Entry #271

Open drewwells opened 4 years ago

drewwells commented 4 years ago

Use Case: My package instantiates a global logger. I have an http Handler in this package where I want to see if a logger is passed. If no logger is passed, use the global logger.

Problem ctxlogrus.Entry was made safe by always returning a logrus.Entry even if this entry sends all output to ioutil.Discard. However, this makes it misleading that you actually retrieved a functional logger and finding out if it's a logger requires digging into the Entry object.

Workaround

func checkLogger(logger *logrus.Entry) (*logrus.Entry, bool) {
        if logger == nil {
                return logger, false
        } else if logger.Logger.Out == ioutil.Discard {
                return logger, false
        }
        return logger, true
}

Proposals

  1. Return a nil *logrus.Entry from Extract when no logrus is found in the context. Breaking change
  2. Add a new method that returns a boolean to indicate that a logrus.Entry was found
package ctxlogrus

func FromContext(ctx context.Context) (*logrus.Entry, bool) {
    l, ok := ctx.Value(ctxLoggerKey).(*ctxLogger)
    if !ok {
        return nil, ok
    }
    return Extract(ctx)
}
johanbrandhorst commented 4 years ago

Hi @drewwells, thanks for the comprehensive issue report! I think we're very keen to keep backwards compatibility, so option #1 is out, but I think we would be happy to add a new method that solves your use case. Would you be happy to contribute this?