Skip to content
This repository has been archived by the owner on Sep 15, 2022. It is now read-only.

Add http.time_ms to all logs #49

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
22 changes: 19 additions & 3 deletions logging/logrus/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,14 @@ var (
//
// All handlers will have a Logrus logger in their context, which can be fetched using `ctxlogrus.Extract`.
func Middleware(entry *logrus.Entry, opts ...Option) httpwares.Middleware {
entry.Logger.Hooks.Add(&timeHook{})
Copy link
Contributor

Choose a reason for hiding this comment

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

would prefer to just do this inline ... rather than adding hooks

Copy link
Author

Choose a reason for hiding this comment

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

From the PR context and offline discussion, I can't think of a good way of doing this without hooks.

return func(nextHandler http.Handler) http.Handler {
options := evaluateMiddlewareOpts(opts)
return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) {
wrappedResp := httpwares.WrapResponseWriter(resp)

requestFields := appendFields(newServerRequestFields(req), options.requestFieldExtractor(req))
startTime := time.Now()
requestFields := appendFields(newServerRequestFields(req, startTime), options.requestFieldExtractor(req))
Copy link
Contributor

Choose a reason for hiding this comment

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

whilst were in could we change this to use ctxLogrus the post call uses it but for some reason not pre call.

newEntry := entry.WithFields(requestFields)
newReq := req.WithContext(ctxlogrus.ToContext(req.Context(), newEntry))
var capture *responseCapture
Expand All @@ -36,7 +38,6 @@ func Middleware(entry *logrus.Entry, opts ...Option) httpwares.Middleware {
}
})

startTime := time.Now()
nextHandler.ServeHTTP(wrappedResp, newReq)
capture.finish() // captureResponse has a nil check, this can be nil

Expand Down Expand Up @@ -70,7 +71,7 @@ func responseFields(wrappedResp httpwares.WrappedResponseWriter, startTime time.
return postCallFields
}

func newServerRequestFields(req *http.Request) logrus.Fields {
func newServerRequestFields(req *http.Request, startTime time.Time) logrus.Fields {
host := req.URL.Host
if host == "" {
host = req.Host
Expand All @@ -86,6 +87,7 @@ func newServerRequestFields(req *http.Request) logrus.Fields {
"http.request.user_agent": req.Header.Get("User-Agent"),
"http.request.length_bytes": req.ContentLength,
"http.request.referer": req.Referer(),
"http.start_time": startTime,
Copy link
Contributor

Choose a reason for hiding this comment

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

Format(time.RFC3339)?

Copy link
Author

Choose a reason for hiding this comment

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

If we use time.Format, we have to then Parse it from the Hook, which could (?) yield an un-handable error. I'm not opposed to it but Time already has a String() function which is mostly human readable.

}

if addr := req.RemoteAddr; addr != "" {
Expand Down Expand Up @@ -129,3 +131,17 @@ func timeDiffToMilliseconds(then time.Time) float32 {
}
return float32(sub/1000) / 1000.0
}

type timeHook struct{}

func (*timeHook) Levels() []logrus.Level {
return logrus.AllLevels
}

func (*timeHook) Fire(entry *logrus.Entry) error {
if startTime, ok := entry.Data["http.start_time"]; ok {
entry.Data["http.time_ms"] = timeDiffToMilliseconds(startTime.(time.Time))
}

return nil
}