From 7c68cadb0da30be49bcbfe4786ef655a4268c4f8 Mon Sep 17 00:00:00 2001 From: Keepers Date: Tue, 16 Jul 2024 17:48:19 -0600 Subject: [PATCH] some quick cleanups (#48) - adds some missing namespace coverage (comment, labelcounter). - renames WithTrace to SkipCaller for clearer readability. - updates the comment headers on SkipCaller funcs. --- clues.go | 85 +++++++++++++++++++++++++++++++++++++++++---------- clues_test.go | 14 ++++----- err.go | 56 +++++++++++++++++++-------------- err_test.go | 4 +-- 4 files changed, 111 insertions(+), 48 deletions(-) diff --git a/clues.go b/clues.go index fe40ee7..922e4ab 100644 --- a/clues.go +++ b/clues.go @@ -64,9 +64,12 @@ func AddMapTo[K comparable, V any]( // used to correlate and isolate logs to certain trace branches. // AddTrace is only needed for layers that don't otherwise call Add() or // similar functions, since those funcs already attach a new node. -func AddTrace(ctx context.Context) context.Context { +func AddTrace( + ctx context.Context, + traceID string, +) context.Context { nc := nodeFromCtx(ctx, defaultNamespace) - return setDefaultNodeInCtx(ctx, nc.trace("")) + return setDefaultNodeInCtx(ctx, nc.trace(traceID)) } // AddTraceTo stacks a clues node onto this context within the specified @@ -75,17 +78,17 @@ func AddTrace(ctx context.Context) context.Context { // certain trace branches. AddTraceTo is only needed for layers that don't // otherwise call AddTo() or similar functions, since those funcs already // attach a new node. -func AddTraceTo(ctx context.Context, namespace string) context.Context { +func AddTraceTo(ctx context.Context, traceID, namespace string) context.Context { nc := nodeFromCtx(ctx, ctxKey(namespace)) - return setNodeInCtx(ctx, namespace, nc.trace("")) + return setNodeInCtx(ctx, namespace, nc.trace(traceID)) } -// AddTraceName stacks a clues node onto this context and uses the provided -// name for the trace id, instead of a randomly generated hash. AddTraceName +// AddTraceWith stacks a clues node onto this context and uses the provided +// name for the trace id, instead of a randomly generated hash. AddTraceWith // can be called without additional values if you only want to add a trace marker. -func AddTraceName( +func AddTraceWith( ctx context.Context, - name string, + traceID string, kvs ...any, ) context.Context { nc := nodeFromCtx(ctx, defaultNamespace) @@ -93,20 +96,20 @@ func AddTraceName( var node *dataNode if len(kvs) > 0 { node = nc.addValues(normalize(kvs...)) - node.id = name + node.id = traceID } else { - node = nc.trace(name) + node = nc.trace(traceID) } return setDefaultNodeInCtx(ctx, node) } -// AddTraceNameTo stacks a clues node onto this context and uses the provided -// name for the trace id, instead of a randomly generated hash. AddTraceNameTo +// AddTraceWithTo stacks a clues node onto this context and uses the provided +// name for the trace id, instead of a randomly generated hash. AddTraceWithTo // can be called without additional values if you only want to add a trace marker. -func AddTraceNameTo( +func AddTraceWithTo( ctx context.Context, - name, namespace string, + traceID, namespace string, kvs ...any, ) context.Context { nc := nodeFromCtx(ctx, ctxKey(namespace)) @@ -114,9 +117,9 @@ func AddTraceNameTo( var node *dataNode if len(kvs) > 0 { node = nc.addValues(normalize(kvs...)) - node.id = name + node.id = traceID } else { - node = nc.trace(name) + node = nc.trace(traceID) } return setNodeInCtx(ctx, namespace, node) @@ -126,6 +129,8 @@ func AddTraceNameTo( // comments // --------------------------------------------------------------------------- +// AddComment adds a long form comment to the clues. +// // Comments are special case additions to the context. They're here to, well, // let you add comments! Why? Because sometimes it's not sufficient to have a // log let you know that a line of code was reached. Even a bunch of clues to @@ -156,6 +161,38 @@ func AddComment( return setDefaultNodeInCtx(ctx, nn) } +// AddCommentTo adds a long form comment to the clues in a certain namespace. +// +// Comments are special case additions to the context. They're here to, well, +// let you add comments! Why? Because sometimes it's not sufficient to have a +// log let you know that a line of code was reached. Even a bunch of clues to +// describe system state may not be enough. Sometimes what you need in order +// to debug the situation is a long-form explanation (you do already add that +// to your code, don't you?). Or, even better, a linear history of long-form +// explanations, each one building on the prior (which you can't easily do in +// code). +// +// Should you transfer all your comments to clues? Absolutely not. But in +// cases where extra explantion is truly important to debugging production, +// when all you've got are some logs and (maybe if you're lucky) a span trace? +// Those are the ones you want. +// +// Unlike other additions, which are added as top-level key:value pairs to the +// context, comments are all held as a single array of additions, persisted in +// order of appearance, and prefixed by the file and line in which they appeared. +// This means comments are always added to the context and never clobber each +// other, regardless of their location. IE: don't add them to a loop. +func AddCommentTo( + ctx context.Context, + namespace, msg string, + vs ...any, +) context.Context { + nc := nodeFromCtx(ctx, ctxKey(namespace)) + nn := nc.addComment(1, msg, vs...) + + return setNodeInCtx(ctx, namespace, nn) +} + // --------------------------------------------------------------------------- // hooks // --------------------------------------------------------------------------- @@ -171,3 +208,19 @@ func AddLabelCounter(ctx context.Context, counter Adder) context.Context { return setDefaultNodeInCtx(ctx, nn) } + +// AddLabelCounterTo embeds an Adder interface into this context. Any already +// embedded Adder will get replaced. When adding Labels to a clues.Err the +// LabelCounter will use the label as the key for the Add call, and increment +// the count of that label by one. +func AddLabelCounterTo( + ctx context.Context, + namespace string, + counter Adder, +) context.Context { + nc := nodeFromCtx(ctx, ctxKey(namespace)) + nn := nc.addValues(nil) + nn.labelCounter = counter + + return setNodeInCtx(ctx, namespace, nn) +} diff --git a/clues_test.go b/clues_test.go index 94a0ccd..56a0bea 100644 --- a/clues_test.go +++ b/clues_test.go @@ -246,7 +246,7 @@ func TestAddTrace(t *testing.T) { check := msa{} mustEquals(t, check, clues.In(ctx).Map(), true) - ctx = clues.AddTrace(ctx) + ctx = clues.AddTrace(ctx, "") assert( t, ctx, "", @@ -278,7 +278,7 @@ func TestAddTraceName(t *testing.T) { mustEquals(t, msa{}, clues.In(ctx).Map(), true) for _, name := range test.names { - ctx = clues.AddTraceName(ctx, name, test.kvs...) + ctx = clues.AddTraceWith(ctx, name, test.kvs...) } assert( @@ -376,7 +376,7 @@ func TestAddTraceTo(t *testing.T) { check := msa{} mustEquals(t, check, clues.InNamespace(ctx, "ns").Map(), true) - ctx = clues.AddTraceTo(ctx, "ns") + ctx = clues.AddTraceTo(ctx, "", "ns") assert( t, ctx, "ns", @@ -408,7 +408,7 @@ func TestAddTraceNameTo(t *testing.T) { mustEquals(t, msa{}, clues.InNamespace(ctx, "ns").Map(), true) for _, name := range test.names { - ctx = clues.AddTraceNameTo(ctx, name, "ns", test.kvs...) + ctx = clues.AddTraceWithTo(ctx, name, "ns", test.kvs...) } assert( @@ -630,7 +630,7 @@ func TestAddComment(t *testing.T) { } } -func addCommentTo(ctx context.Context, msg string) context.Context { +func addCommentToCtx(ctx context.Context, msg string) context.Context { return clues.AddComment(ctx, msg) } @@ -670,7 +670,7 @@ func commentMatches( func TestAddComment_trace(t *testing.T) { ctx := context.Background() ctx = clues.AddComment(ctx, "one") - ctx = addCommentTo(ctx, "two") + ctx = addCommentToCtx(ctx, "two") ctx = clues.AddComment(ctx, "three") dn := clues.In(ctx) @@ -678,7 +678,7 @@ func TestAddComment_trace(t *testing.T) { stack := comments.String() expected := commentRE( "TestAddComment_trace", "clues_test.go", "one", - "addCommentTo", "clues_test.go", "two", + "addCommentToCtx", "clues_test.go", "two", "TestAddComment_trace", "clues_test.go", `three$`) commentMatches(t, expected, stack) diff --git a/err.go b/err.go index 40feb00..782d348 100644 --- a/err.go +++ b/err.go @@ -134,14 +134,15 @@ func makeStack( // TODO: transform all this to comply with a standard interface // ------------------------------------------------------------ -// TODO: this will need some cleanup in a follow-up PR. -// // ancestors builds out the ancestor lineage of this // particular error. This follows standard layout rules // already established elsewhere: // * the first entry is the oldest ancestor, the last is // the current error. // * Stacked errors get visited before wrapped errors. +// +// TODO: get other ancestor stack builders to work off of this +// func instead of spreading that handling everywhere. func ancestors(err error) []error { return stackAncestorsOntoSelf(err) } @@ -847,20 +848,25 @@ func WithMap(err error, m map[string]any) *Err { // builders - tracing // ------------------------------------------------------------ -// WithTrace sets the error trace to a certain depth. -// A depth of 0 traces to the func where WithTrace is -// called. 1 sets the trace to its parent, etc. -// Error traces are already generated for the location -// where clues.Wrap or clues.Stack was called. This -// call is for cases where Wrap or Stack calls are handled -// in a helper func and are not reporting the actual -// error origin. -// TODO: rename to `SkipCaller`. -func (err *Err) WithTrace(depth int) *Err { +// SkipCaller skips callers when constructing the +// error trace stack. The caller is the file, line, and func +// where the *clues.Err was generated. +// +// A depth of 0 performs no skips, and returns the same +// caller info as if SkipCaller was not called. 1 skips the +// immediate parent, etc. +// +// Error traces are already generated for the location where +// clues.Wrap or clues.Stack was called. This func is for +// cases where Wrap or Stack calls are handled in a helper +// func and are not reporting the actual error origin. +func (err *Err) SkipCaller(depth int) *Err { if isNilErrIface(err) { return nil } + // needed both here and in withTrace() to + // correct for the extra call depth. if depth < 0 { depth = 0 } @@ -870,18 +876,22 @@ func (err *Err) WithTrace(depth int) *Err { return err } -// WithTrace sets the error trace to a certain depth. -// A depth of 0 traces to the func where WithTrace is -// called. 1 sets the trace to its parent, etc. -// Error traces are already generated for the location -// where clues.Wrap or clues.Stack was called. This -// call is for cases where Wrap or Stack calls are handled -// in a helper func and are not reporting the actual -// error origin. +// SkipCaller skips callers when constructing the +// error trace stack. The caller is the file, line, and func +// where the *clues.Err was generated. +// +// A depth of 0 performs no skips, and returns the same +// caller info as if SkipCaller was not called. 1 skips the +// immediate parent, etc. +// +// Error traces are already generated for the location where +// clues.Wrap or clues.Stack was called. This func is for +// cases where Wrap or Stack calls are handled in a helper +// func and are not reporting the actual error origin. +// // If err is not an *Err intance, returns the error wrapped // into an *Err struct. -// TODO: rename to `SkipCaller`. -func WithTrace(err error, depth int) *Err { +func WithSkipCaller(err error, depth int) *Err { if isNilErrIface(err) { return nil } @@ -897,7 +907,7 @@ func WithTrace(err error, depth int) *Err { return newErr(err, "", map[string]any{}, depth+1) } - return e.WithTrace(depth + 1) + return e.SkipCaller(depth + 1) } // ------------------------------------------------------------ diff --git a/err_test.go b/err_test.go index fa1e8bf..07bee87 100644 --- a/err_test.go +++ b/err_test.go @@ -1340,11 +1340,11 @@ func TestLabelCounter_variadic_noCluesInErr(t *testing.T) { // --------------------------------------------------------------------------- func withTraceWrapper(err error, depth int) error { - return clues.WithTrace(err, depth) + return clues.WithSkipCaller(err, depth) } func cluesWithTraceWrapper(err *clues.Err, depth int) error { - return err.WithTrace(depth) + return err.SkipCaller(depth) } func withCommentWrapper(