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

log/slog's LogAttrs (and other methods) now requires a context as their first argument #858

Closed
mhghw opened this issue Oct 17, 2023 · 3 comments

Comments

@mhghw
Copy link

mhghw commented Oct 17, 2023

Hi, I was looking around the examples and in the logging example I found this struct that satisfies the middleware.LogEntry interface:

type StructuredLoggerEntry struct {
	Logger *slog.Logger
}

func (l *StructuredLoggerEntry) Write(status, bytes int, header http.Header, elapsed time.Duration, extra interface{}) {
	l.Logger.LogAttrs(slog.LevelInfo, "request complete",
		slog.Int("resp_status", status),
		slog.Int("resp_byte_length", bytes),
		slog.Float64("resp_elapsed_ms", float64(elapsed.Nanoseconds())/1000000.0),
	)
}

func (l *StructuredLoggerEntry) Panic(v interface{}, stack []byte) {
	l.Logger.LogAttrs(slog.LevelInfo, "",
		slog.String("stack", string(stack)),
		slog.String("panic", fmt.Sprintf("%+v", v)),
	)
}

Since its using log/slog as its logger, the LogAttrs method for the log/slog is changed from
func (l *Logger) LogAttrs(level Level, msg string, attrs ...Attr)

to

func (l *Logger) LogAttrs(ctx context.Context,level Level, msg string, attrs ...Attr)

and well, your example is not working anymore.

I read the source code and I couldn't figure out why there is a context passing around but it seemed that we can pass context.Background() to the LogAttr method (or even nil) and log/slog package handles it.

Can you please clarify the usage of context in here for me and see if it's possible to change the middleware.LogEntry interface that we could send the request context as an argument to use in log/slog methods.

Thank you very much.

@mhghw mhghw changed the title log/slog now requires a context as their first argument log/slog's LogAttrs (and other methods) now requires a context as their first argument Oct 17, 2023
@VojtechVitek
Copy link
Contributor

slog support was added in #771

@ydarb mind taking this one please?

@mcosta74
Copy link

I just want to add that

slogJSONHandler := slog.HandlerOptions{
    // Remove default time slog.Attr, we create our own later
    ReplaceAttr: func(groups []string, a slog.Attr) slog.Attr {
        if a.Key == slog.TimeKey {
            return slog.Attr{}
	}
	return a
    },
}.NewJSONHandler(os.Stdout)

is no longer valid. The following is the right syntax

slogJSONHandler := slog.NewJSONHandler(os.Stdout, &slog.HandlerOptions{
    // Remove default time slog.Attr, we create our own later
    ReplaceAttr: func(groups []string, a slog.Attr) slog.Attr {
        if a.Key == slog.TimeKey {
	    return slog.Attr{}
	}
	return a
    },
})

ydarb added a commit to ydarb/chi that referenced this issue Oct 19, 2023
chore: Fix logging example Panic() method to pretty print stack trace

Ref: go-chi#858
ydarb added a commit to ydarb/chi that referenced this issue Oct 19, 2023
Adds bool to flip example between a TextHandler and a JSONHandler.
Update Panic() to pretty print stack trace if log handler is a
TextHandler, otherwise it will log the stack trace as a JSON value
string with a key of "stack".

Ref: go-chi#858
@pkieltyka
Copy link
Member

https://github.com/go-chi/httplog/releases/tag/v2.0.0 is out built on "log/slog" and has lots of little nice features including a pretty logger when printing to stdout tty

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