Context logging #39
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| // Package ctxlog allows logging data stored with a context. | ||
| package ctxlog | ||
|
|
||
| import ( | ||
| "context" | ||
| "sync" | ||
|
|
||
| "github.com/micromdm/nanomdm/log" | ||
| ) | ||
|
|
||
| // CtxKVFunc creates logger key-value pairs from a context. | ||
| // CtxKVFuncs should aim to be be as efficient as possible—ideally only | ||
| // doing the minimum to read context values and generate KV pairs. Each | ||
| // associated CtxKVFunc is called every time we adapt a logger with | ||
| // Logger. | ||
| type CtxKVFunc func(context.Context) []interface{} | ||
|
|
||
| // ctxKeyFuncs is the context key for storing and retriveing | ||
| // a funcs{} struct on a context. | ||
| type ctxKeyFuncs struct{} | ||
|
|
||
| // funcs holds the associated CtxKVFunc functions to run. | ||
| type funcs struct { | ||
| sync.RWMutex | ||
| funcs []CtxKVFunc | ||
| } | ||
|
|
||
| // AddFunc associates a new CtxKVFunc function to a context. | ||
| func AddFunc(ctx context.Context, f CtxKVFunc) context.Context { | ||
| if ctx == nil { | ||
| return ctx | ||
| } | ||
| ctxFuncs, ok := ctx.Value(ctxKeyFuncs{}).(*funcs) | ||
| if !ok || ctxFuncs == nil { | ||
| ctxFuncs = &funcs{} | ||
| } | ||
| ctxFuncs.Lock() | ||
| ctxFuncs.funcs = append(ctxFuncs.funcs, f) | ||
| ctxFuncs.Unlock() | ||
| return context.WithValue(ctx, ctxKeyFuncs{}, ctxFuncs) | ||
| } | ||
|
|
||
| // Logger runs the associated CtxKVFunc functions and returns a new | ||
| // logger with the results. | ||
| func Logger(ctx context.Context, logger log.Logger) log.Logger { | ||
| if ctx == nil { | ||
| return logger | ||
| } | ||
|
There was a problem hiding this comment. ... Do you have benchmarks for how long it takes (on average) to log a line? This seems expensive for logging, and I'm wondering if this can be precalculated |
||
| ctxFuncs, ok := ctx.Value(ctxKeyFuncs{}).(*funcs) | ||
| if !ok || ctxFuncs == nil { | ||
| return logger | ||
| } | ||
| var acc []interface{} | ||
| ctxFuncs.RLock() | ||
| for _, f := range ctxFuncs.funcs { | ||
|
There was a problem hiding this comment. Similarly in the annals of "premature optimization", is it possible to avoid doing this if the message won't actually be logged because the current log level is too high for the message? (or am I totally missing how this is implemented?) |
||
| acc = append(acc, f(ctx)...) | ||
| } | ||
| ctxFuncs.RUnlock() | ||
| return logger.With(acc...) | ||
| } | ||
|
|
||
| // SimpleStringFunc is a helper that generates a simple CtxKVFunc that | ||
| // returns a key-value pair if found on the context. | ||
| func SimpleStringFunc(logKey string, ctxKey interface{}) CtxKVFunc { | ||
| return func(ctx context.Context) (out []interface{}) { | ||
| v, _ := ctx.Value(ctxKey).(string) | ||
| if v != "" { | ||
| out = []interface{}{logKey, v} | ||
| } | ||
| return | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to make this configurable for testing purposes?
Also, is it possible to get a separately-seeded/used sequence, so that this will only be used for the trace-ids?
(If you need cryptographic strength here, this isn't a good enough seed, and also not a good enough package...but it doesn't look like you need it)
(looks like you can create your own source/random object... just can't be easily used concurrently)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
main.go is the "reference" nanoMDM implementation afaik. Any other implementer who uses the NanoMDM library could do this in a separate way, so I don't think this is an issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a testability consideration, not anything else.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(As far as I can tell, the only security implication of UUIDs is preventing collisions. So ... any seeding is "good enough", and use of a cryptographic PRNG is unnecessary. There's no harm if someone can guess a UUID.
So I'm just asking for "creation of unit test" purposes: if you can control the PRNG seed, you can get repeatable sequences out which makes testing easier.
If this isn't where anyone would actually test this, then ... yeah. Nevermind/drop)