feat: allow to customize log message

[sad-fat-cat]

New option LogFormat, that lets you control the format of the log messages

[VojtechVitek]

Looks good

[VojtechVitek]

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

[sad-fat-cat]

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.

[sad-fat-cat]

Anything news? Can we merge this?

[david-littlefarmer]

Hi, thanks for PR, this change makes sense to me.
I have just one comment, so we have same pattern for options in whole middleware.

[david-littlefarmer]

Suggested change

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

[david-littlefarmer]

Suggested change

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

[VojtechVitek]

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

[david-littlefarmer]

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

[VojtechVitek]

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?

[david-littlefarmer]

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
}

[VojtechVitek]

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.

[VojtechVitek]

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.

[david-littlefarmer]

I agree that this would be the best approach.