log icon indicating copy to clipboard operation
log copied to clipboard

add WithFieldFunc() for dynamic fields at log invocation with fixed calldepth

Open arussellsaw opened this issue 6 years ago • 7 comments

Hi!

we've been scratching our heads for a way to implement line numbering in a non fragile way that won't break when new wrapper handlers are added, and this is what i've come up with. i appreciate i've changed quite a few things so i welcome feedback, or if you'd rather not that's fine too!

thanks

example line number implementation:

func GetAppLogContext() *alog.Entry {
	log := log.WithFieldFunc(func() alog.Fielder {
		_, file, line, ok := runtime.Caller(alog.FieldFuncCallDepth)
		if ok {
			return alog.Fields{
				"caller_file": file,
				"caller_line": strconv.Itoa(line),
			}
		}
		return nil
	})
	return log
}

arussellsaw avatar Jan 23 '18 12:01 arussellsaw

looks like the build failure is down to a dependency

arussellsaw avatar Jan 26 '18 14:01 arussellsaw

bruh

arussellsaw avatar Feb 09 '18 15:02 arussellsaw

It's a little hacky due to how pkg/errors exposes the filename/lineno, but there's support for pkg/errors which may help a bit if you can use that.

I'm not against it but I'd definitely prefer to keep the API smaller if possible. Maybe a FielderFunc similar to http.HandlerFunc

tj avatar Feb 12 '18 22:02 tj

hey, thanks for getting back to me!

we're currently moving over to pkg/errors so hopefully will have that soon, however we wanted to be able to attach additional info for non-error logs as well, i think this would be useful for attaching extra dynamic info without having to register an entire new log.Handler to wrap all logs, such as only attaching something like memory/runtime info to log lines in a certain area of code. I'm not sure what you mean by the http.HandlerFunc equivalent, as the WithFieldFunc() already accepts a FielderFunc, so the api is about as terse as i can see it being

arussellsaw avatar Feb 13 '18 10:02 arussellsaw

Something like:

  // The HandlerFunc type is an adapter to allow the use of
  // ordinary functions as HTTP handlers. If f is a function
  // with the appropriate signature, HandlerFunc(f) is a
  // Handler that calls f.
  type HandlerFunc func(ResponseWriter, *Request)
  
  // ServeHTTP calls f(w, r).
  func (f HandlerFunc) ServeHTTP(w ResponseWriter, r *Request) {
  	f(w, r)
  }

What are some use-cases for deferring the invocation? I've only ever really needed ctx = ctx.WithFields(...), not saying it's a bad idea or anything but good to know :D

tj avatar Feb 13 '18 18:02 tj

i think i get what ya mean now! i think i'll remove the CallDepth const as that kinda pollutes the api too, i'll just add a mention of the expected calldepth in godoc.

one of the main use cases i can think of is how it can simplify the flow of your logging in many situations, instead of having to call WithFields() to reattach new/updated data for each log line, you simply attach a FieldFunc to the root context for that area of code, and you can log knowing that each log line will have totally up to date data. the kind of things i'd attach to this are runtime/memory/gc stats, current reads of counters from instrumentation, cpu load, LOC/package

arussellsaw avatar Feb 14 '18 10:02 arussellsaw

Seems like something that could maybe be delegated with a little util func, like ctx = util.LogContext(ctx) in the app somewhere adding those details

tj avatar Feb 17 '18 12:02 tj