Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion _example/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ require (
github.com/go-chi/chi/v5 v5.1.0
github.com/go-chi/httplog/v3 v3.0.0-00010101000000-000000000000
github.com/go-chi/traceid v0.3.0
github.com/golang-cz/devslog v0.0.14
github.com/golang-cz/devslog v0.0.15
)

require github.com/google/uuid v1.6.0 // indirect
6 changes: 2 additions & 4 deletions _example/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,7 @@ github.com/go-chi/chi/v5 v5.1.0 h1:acVI1TYaD+hhedDJ3r54HyA6sExp3HfXq7QWEEY/xMw=
github.com/go-chi/chi/v5 v5.1.0/go.mod h1:DslCQbL2OYiznFReuXYUmQ2hGd1aDpCnlMNITLSKoi8=
github.com/go-chi/traceid v0.3.0 h1:BYITxMnIeQasU7/U7+InZWANWxVZeCUBGTAqWleQOeg=
github.com/go-chi/traceid v0.3.0/go.mod h1:XFfEEYZjqgML4ySh+wYBU29eqJkc2um7oEzgIc63e74=
github.com/golang-cz/devslog v0.0.9 h1:GU59V626sUccMjytKhewgN3teduXtx/itoQs78NaPdE=
github.com/golang-cz/devslog v0.0.9/go.mod h1:bSe5bm0A7Nyfqtijf1OMNgVJHlWEuVSXnkuASiE1vV8=
github.com/golang-cz/devslog v0.0.14 h1:hZY6VuZ/+MmG4djP9X1YDSmX/z5zPDDVgFlO0fyb+CY=
github.com/golang-cz/devslog v0.0.14/go.mod h1:bSe5bm0A7Nyfqtijf1OMNgVJHlWEuVSXnkuASiE1vV8=
github.com/golang-cz/devslog v0.0.15 h1:ejoBLTCwJHWGbAmDf2fyTJJQO3AkzcPjw8SC9LaOQMI=
github.com/golang-cz/devslog v0.0.15/go.mod h1:bSe5bm0A7Nyfqtijf1OMNgVJHlWEuVSXnkuASiE1vV8=
github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0=
github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
4 changes: 4 additions & 0 deletions _example/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,10 @@ func main() {
}
return nil
},

LogFormat: func(r *http.Request, statusCode int, d time.Duration) string {
return fmt.Sprintf("%s %s => HTTP %v", r.Method, r.URL, statusCode)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

out of curiosity, what's the format you're thinking of for yourself?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just constant string, like "request processed".

Because all info about request already stored in slog attrs, and variance part in message field sort of difficult to work with aggregations (for example, % of uniq clients were affected by a particular error in this operation).

I thought about just putting the template string in the params, but the function seems more flexible, in general.

},
}))

// Set request log attribute from within middleware.
Expand Down
7 changes: 6 additions & 1 deletion middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ func RequestLogger(logger *slog.Logger, o *Options) func(http.Handler) http.Hand
s = SchemaECS
}

logFormat := o.LogFormat
if logFormat == nil {
logFormat = defaultLogFormat
}
Comment on lines +38 to +41
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
logFormat := o.LogFormat
if logFormat == nil {
logFormat = defaultLogFormat
}
if o.LogFormat == nil {
o.LogFormat = defaultLogFormat
}


return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
ctx := context.WithValue(r.Context(), ctxKeyLogAttrs{}, &[]slog.Attr{})
Expand Down Expand Up @@ -166,7 +171,7 @@ func RequestLogger(logger *slog.Logger, o *Options) func(http.Handler) http.Hand
logAttrs = groupAttrs(logAttrs, s.GroupDelimiter)
}

msg := fmt.Sprintf("%s %s => HTTP %v (%v)", r.Method, r.URL, statusCode, duration)
msg := logFormat(r, statusCode, duration)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
msg := logFormat(r, statusCode, duration)
msg := o.LogFormat(r, statusCode, duration)

logger.LogAttrs(ctx, lvl, msg, logAttrs...)
}()

Expand Down
12 changes: 12 additions & 0 deletions options.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package httplog

import (
"fmt"
"time"

"log/slog"
"net/http"
)
Expand Down Expand Up @@ -95,6 +98,10 @@ type Options struct {
//
// WARNING: Be careful not to leak any sensitive information in the logs.
LogExtraAttrs func(req *http.Request, reqBody string, respStatus int) []slog.Attr

// LogFormat is a optional function that lets you control the format of the log message
// If not provided, default format will be used
LogFormat func(*http.Request, int, time.Duration) string
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you see people needing more than statusCode and duration in the future? Should we pass the arguments via an extendable struct?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, let's do it via struct, so we don't make breaking changes in future.
Also add parameter names in signature of function please.

type LogFormatParameters struct {
	StatusCode int
	Duration   time.Duration
}
Suggested change
LogFormat func(*http.Request, int, time.Duration) string
LogFormat func(req *http.Request, parameters *LogFormatParameters) string

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this @david-littlefarmer. Perhaps we can call it "attrs"?

How feasible would it be to pass the raw logAttrs?

Suggested change
LogFormat func(*http.Request, int, time.Duration) string
LogFormat func(req *http.Request, attrs []slog.Attr) string

Would that be developer friendly?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are already populating attributes of the log. This should just build the message for log. So it don't makes sense to me to pass same parameter again to message.

Also every log we would have to iterate through slice of attributes and find the wanted one.

But the flexibility is nice, no argues against it.

So what about this? We can pass the most wanted parameters directly, so they can be used without any unnecessary iterations and also provide the slice of attributes.

type LogFormatParameters struct {
	StatusCode int
	Duration   time.Duration
	Attributes []slog.Attr
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'm fine with this.

But remember that we're in the hotpath and every extra attr copy counts (N * number of HTTP requests).

So better if we can avoid the extra copying just to alter the format.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And as I mentioned the above, I realized it'd be probably better to print the default message without calling the defaultLogFormatter function. IDK if Go can inline a function that is dynamic, I guess not? Meaning we'd copy the attrs on function stack for no reason.

var msg string
if opts.LogFormat != nil {
      msg = logFormat(r, LogFormatArgs{...})
} else {
      msg = fmt.Sprintf("%s %s => HTTP %v (%v)", r.Method, r.URL, statusCode, duration)
}

How bad would it be for users to go though []log.Attrs? It should be a straight forward loop; with no allocations, right?

TL;DR: I'm trying to avoid extra allocations per each request.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that this would be the best approach.

}

var defaultOptions = Options{
Expand All @@ -105,4 +112,9 @@ var defaultOptions = Options{
LogResponseHeaders: []string{"Content-Type"},
LogBodyContentTypes: []string{"application/json", "application/xml", "text/plain", "text/csv", "application/x-www-form-urlencoded", ""},
LogBodyMaxLen: 1024,
LogFormat: defaultLogFormat,
}

func defaultLogFormat(r *http.Request, statusCode int, duration time.Duration) string {
return fmt.Sprintf("%s %s => HTTP %v (%v)", r.Method, r.URL, statusCode, duration)
}