Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

integrate with log/slog #35

Open
jba opened this issue Aug 4, 2023 · 4 comments
Open

integrate with log/slog #35

jba opened this issue Aug 4, 2023 · 4 comments

Comments

@jba
Copy link

jba commented Aug 4, 2023

The log/slog package will be in Go 1.21.
Consider adding a Logger that wraps a slog.Handler.

@peterbourgon
Copy link
Member

It's not clear to me how to build a Go kit Logger that wraps a slog.Handler. I hope you can provide some guidance!

I guess it would have to look something like the following.

type slogLogger struct {
    handler slog.Handler
}

func NewSlogLogger(h slog.Handler) Logger {
    return &slogLogger{handler: h}
}

func (sl *slogLogger) Log(keyvals ...any) error {
    ctx := context.Background() // I think this is right
    rec := ???
    return sl.handler.Handle(ctx, rec)
}

What isn't clear to me is how to construct a slog.Record. The slog.NewRecord constructor takes

  • A time argument, which we can produce easily enough.
  • A level argument, which is tricky — the Go kit logger implements levels via package level which basically models them as a key=val pair in the log event, same as any other. Log events are not guaranteed to have a level at all, and even when a level does exist, it won't necessarily map to the slog.Level definitions. I guess a path forward here would be to assume that each Go kit log event either has a level (via some specific key) or, if it doesn't, it will be assigned a "default" level. That's somewhat unsatisfying; is there a better approach?
  • A msg argument, which has no obvious solution — the Go kit logger doesn't mandate that log events include any specific key, so there's no way to reliably get a "msg" from a set of keyvals. I guess a path forward here would be to simply assume that each Go kit log event has a "msg" key, and parse/use the corresponding val, even if it is the empty string. That's somewhat unsatisfying; is there a better approach?
  • A pc uintptr argument, which we can definitely just assign — but there are many subtle issues with that approach, specifically with helpers like log.With, or, more generally, with any kind of Logger decorator. Do you have some guidance on producing a pc uintptr which is robust to these use cases?

Looking forward to any thoughts you might have.

@jba
Copy link
Author

jba commented Aug 4, 2023

  • level: if you see a level.Key(), map the value. It looks like the four levels you name are the same that slog names, so the mapping seems straightforward. I would use Info if no level is provided. Most people think of it as the default.
  • msg: your suggestion seems as good as any.
  • pc: I may not understand your concern with log.With and other decorators. I think the pc should point to the Log call, which is the line that actually pushes out the log event. With slog, the code
    logger := logger.With("a", 1)
    logger.Info("msg")
    
    would create a source attribute pointing to the second line; nothing about the first line is recorded anywhere.

I'm not too worried about getting the mapping perfect, or even great. I think there will be a lot of value in just having the code in your repo, so go-kit/log users will see how it's done. For example, if someone consistently uses "message" instead of "msg", they can copy your code, or maybe suggest adding an option to NewSlogLogger. You may want to consider adding an options struct at the beginning, or functional options if that's what you prefer.

@sunjayaali
Copy link

sunjayaali commented Jan 12, 2024

What about wrapping the *slog.Logger.<LogFunc> method? Something like..

type logFunc func(msg string, keysAndValues ...interface{})

func (l logFunc) Log(keyvals ...interface{}) error {
	l("", keyvals...)
	return nil
}

type config struct {
	level slog.Level
}

type Option func(*config)

func WithLevel(level slog.Level) Option {
	return func(l *config) {
		l.level = level
	}
}

func NewSlogLogger(logger *slog.Logger, options ...Option) kitlog.Logger {
	c := &config{}
	for _, option := range options {
		option(c)
	}

	var logFunc logFunc
	switch c.level {
	case slog.LevelDebug:
		logFunc = slog.Debug
	case slog.LevelInfo:
		logFunc = slog.Info
	case slog.LevelWarn:
		logFunc = slog.Warn
	case slog.LevelError:
		logFunc = slog.Error
	default:
		logFunc = slog.Info
	}

	return logFunc
}

@isauran
Copy link

isauran commented Mar 27, 2024

example with log/slog: https://github.com/isauran/logger

@xyluet thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants