philippseith / signalr

SignalR server and client in go
MIT License
133 stars 39 forks source link

Better logging interoperability #126

Closed kirides closed 2 years ago

kirides commented 2 years ago

The current StructuredLogger interface is too basic and requires manually filtering some data, especially the levels

Suggestion:

currently:

type signalRLogger struct {
    zap *zap.Logger
}

func (l *signalRLogger) Log(keyVals ...interface{}) error {
    if l.zap == nil {
        var err error
        l.zap, err = zap.NewDevelopment()
        if err != nil {
            return err
        }
    }
    lvl := level.DebugValue()
    logger := l.zap.Sugar()
    for i := 0; i < len(keyVals); i += 2 {
        _, isStr := keyVals[i].(string)
        if !isStr {
            // wtf
            continue
        }
        if keyVals[i] == "ts" {
            continue
        }
        if keyVals[i] == level.Key() {
            if gokit, ok := keyVals[i+1].(level.Value); ok {
                lvl = gokit
            }
            continue
        }
        logger = logger.With(keyVals[i], keyVals[i+1])
    }

    switch lvl {
    case level.DebugValue():
        logger.Debug()
    case level.InfoValue():
        logger.Info()
    case level.WarnValue():
        logger.Warn()
    case level.ErrorValue():
        logger.Error()
    default:
        logger.Error()
    }

    return nil
}

proposed:

type StructuredLogger interface {
    Debug(message string, keyVals interface{}) error
    Info(message string, keyVals interface{}) error
    Warn(message string, keyVals interface{}) error
    Error(message string, keyVals interface{}) error
}

// example implementation with zap

func (l *signalRLogger) Debug(message string, keyVals ...interface{}) error {
    l.zap.Sugar().Debugw(message, keyVals...)
    return nil
}

func (l *signalRLogger) Info(message string, keyVals ...interface{}) error {
    l.zap.Sugar().Infow(message, keyVals...)
    return nil
}

func (l *signalRLogger) Warn(message string, keyVals ...interface{}) error {
    l.zap.Sugar().Warnw(message, keyVals...)
    return nil
}

func (l *signalRLogger) Error(message string, keyVals ...interface{}) error {
    l.zap.Sugar().Errorw(message, keyVals...)
    return nil
}

// example in go-kit/log

func (l *signalRLogger) Debug(message string, keyVals ...interface{}) error {
    return gokit.With(level.Debug(gokitLogger), "message", message).Log(keyVals...)
}

func (l *signalRLogger) Info(message string, keyVals ...interface{}) error {
    return gokit.With(level.Info(gokitLogger), "message", message).Log(keyVals...)
}

func (l *signalRLogger) Warn(message string, keyVals ...interface{}) error {
    return gokit.With(level.Warn(gokitLogger), "message", message).Log(keyVals...)
}

func (l *signalRLogger) Error(message string, keyVals ...interface{}) error {
    return gokit.With(level.Error(gokitLogger), "message", message).Log(keyVals...)
}
philippseith commented 2 years ago

See https://github.com/go-kit/log#readme why the interface is as it is

kirides commented 2 years ago

i guess that's fine. Will have to live with that quirky workaround for now ;)