vanadium / issues

Vanadium issue tracker
1 stars 1 forks source link

longitudinal logging: integrate depcop logcalls and vtrace and maybe with vlog too #446

Closed cosnicolaou closed 8 years ago

cosnicolaou commented 9 years ago

Matt writes:

in some runtime implementation code:

fun PublicInterfaceThing(ctx *context.T, arg1, arg2, arg3) { defer vloggy.LogCall("PublicInterfaceThing", ctx, arg1, arg2, arg3)() }

In the vloggy package:

func LogCall(name string, ctx *context.T, args ...interface{}) { formatted := ... somehow format the args and name into a nice message ...

// Note DetailedLoggingRequested doesn't exist right now, but could be implemented very // easily by having some client up the chain say vtrace.RequestDetailedLogging(ctx). // So this allows users to control logging behavior with vtrace. if vlog.LogLevel >= 1 || vtrace.DetailedLoggingRequested(ctx) { vlog.Info(formatted) } // This part instead allows the user to attach this function call information to the existing // vtrace information. These things are useful in different circumstances. vtrace.GetSpan(ctx).Annotate(formatted) }

I'm also thinking about adding vtrace log levels so maybe the last line would be:

vtrace.GetSpan(ctx).AnnotateLevel(3, formatted)

Rather than placing this in the vlog package, I can create a new package in x.ref that implements this. I agree it'll be very useful to be able to trigger vlog calls 'longitudinally' like this. I wonder if we shouldn't consider adding something like this to vlog. Instead of vlog.VI(1).x, maybe vlog.T(flag).x, of course we can just use VLOG.VI(flag).x too, but I wonder if T would make it clearer to devs how to use it? Anyway. I'm certainly happy to make this change.

cosnicolaou commented 9 years ago

the new API is now in use. Matt has to implement the required support in context.T

ghost commented 8 years ago

We have a plan that would replace logging, vtrace, and logcall into one system. However it is not currently a priority to implement it:

https://docs.google.com/document/d/1-butkYedwzi6HWybMROvTQ68zigmZ3qGupxM9HFdqlg/edit#heading=h.wi7am98dudks

I don't think we'll add this to the existing system, so we're not going to do the specific thing mentioned in this bug. I'm closing it.