Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add witnesses #53

Merged
merged 2 commits into from
Aug 16, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
44 changes: 43 additions & 1 deletion clues.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,49 @@ func AddCommentTo(
}

// ---------------------------------------------------------------------------
// hooks
// witness
// ---------------------------------------------------------------------------

// AddWitness adds a witness with a given name to the contex. The caller can
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: without knowing what a witness is (and having no documentation on an exported Witness type, which itself is fine as it fits into the larger project architecture), having this described as adding a witness to something seems unhelpful

Suggested change
// AddWitness adds a witness with a given name to the contex. The caller can
// AddWitness adds a witness with a given name to the context. The caller can

Copy link
Contributor Author

Choose a reason for hiding this comment

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

check, will update.

// pass the witness clues directly in a downstream instance to add those clues
// to the current clues node. This can be handy in a certain set of uncommon
// cases where retrieving clues is otherwise difficult to do, such as working
// with middleware that doesn't allow control over error creation.
//
// When retreiving clues from a context, each witness will produce its own
// namespaced set of values
func AddWitness(
ctx context.Context,
name string,
) context.Context {
nc := nodeFromCtx(ctx, defaultNamespace)
nn := nc.addWitness(name)

return setDefaultNodeInCtx(ctx, nn)
}

// Relay adds all key-value pairs to the provided witness. The witness will
// record those values to the dataNode in which it was created. All relayed
// values are namespaced to the owning witness.
func Relay(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not my favorite name. I'm open to suggestions. Preferably puns about a detective getting clues from a witness.

ctx context.Context,
witness string,
vs ...any,
) {
nc := nodeFromCtx(ctx, defaultNamespace)
wit, ok := nc.witnesses[witness]

if !ok {
return
}

// set values, not add. We don't want witnesses
// to own a full clues tree.
wit.data.setValues(normalize(vs...))
}

// ---------------------------------------------------------------------------
// error label counter
// ---------------------------------------------------------------------------

// AddLabelCounter embeds an Adder interface into this context. Any already
Expand Down
78 changes: 77 additions & 1 deletion clues_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,19 @@ import (
"golang.org/x/exp/slices"
)

func mapEquals(
t *testing.T,
ctx context.Context,
expect msa,
expectCluesTrace bool,
) {
mustEquals(
t,
expect,
clues.In(ctx).Map(),
expectCluesTrace)
}

func mustEquals[K comparable, V any](
t *testing.T,
expect, got map[K]V,
Expand All @@ -21,7 +34,9 @@ func mustEquals[K comparable, V any](

if hasCluesTrace && len(g) > 0 {
if _, ok := g["clues_trace"]; !ok {
t.Error("expected map to contain key [clues_trace]")
t.Errorf(
"expected map to contain key [clues_trace]\ngot: %+v",
g)
}
delete(g, "clues_trace")
}
Expand Down Expand Up @@ -683,3 +698,64 @@ func TestAddComment_trace(t *testing.T) {

commentMatches(t, expected, stack)
}

func TestAddWitness(t *testing.T) {
ctx := context.Background()
ctx = clues.Add(ctx, "one", 1)

mapEquals(t, ctx, msa{
"one": 1,
}, true)

ctxWithWit := clues.AddWitness(ctx, "wit")
clues.Relay(ctx, "wit", "zero", 0)
clues.Relay(ctxWithWit, "wit", "two", 2)

mapEquals(t, ctx, msa{
"one": 1,
}, true)

mapEquals(t, ctxWithWit, msa{
"one": 1,
"witnessed": map[string]map[string]any{
"wit": {
"two": 2,
},
},
}, true)

ctxWithTim := clues.AddWitness(ctxWithWit, "tim")
clues.Relay(ctxWithTim, "tim", "three", 3)

mapEquals(t, ctx, msa{
"one": 1,
}, true)

mapEquals(t, ctxWithTim, msa{
"one": 1,
"witnessed": map[string]map[string]any{
"wit": {
"two": 2,
},
"tim": {
"three": 3,
},
},
}, true)

ctxWithBob := clues.AddWitness(ctx, "bob")
clues.Relay(ctxWithBob, "bob", "four", 4)

mapEquals(t, ctx, msa{
"one": 1,
}, true)

mapEquals(t, ctxWithBob, msa{
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: could you please also make sure that the first two contexts with witnesses aren't changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure!

"one": 1,
"witnessed": map[string]map[string]any{
"bob": {
"four": 4,
},
},
}, true)
}
114 changes: 93 additions & 21 deletions datanode.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,24 @@ type dataNode struct {
// Errors will only utilize the first labelCounter they find. The tree is searched
// from leaf to root when looking for populated labelCounters.
labelCounter Adder

// witnesses act as proxy dataNodes that can gather specific, intentional data
// additions. They're namespaced so that additions to the witness don't accidentally
// clobber other values in the dataNode. This also allows witnesses to protect
// variations of data from each other, in case users need to compare differences
// on the same keys. That's not the goal for witnesses, exactly, but it is capable.
witnesses map[string]*witness
}

// spawnDescendant generates a new dataNode that is a descendant of the current
// node. A descendant maintains a pointer to its parent, and carries any genetic
// necessities (ie, copies of fields) that must be present for continued functionality.
func (dn *dataNode) spawnDescendant() *dataNode {
return &dataNode{
parent: dn,
labelCounter: dn.labelCounter,
witnesses: maps.Clone(dn.witnesses),
Copy link
Contributor

Choose a reason for hiding this comment

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

you may want to test this on different machine architectures. IIRC, I was having trouble with this function returning nil if the input was nil on Mac, which would eventually lead to NPEs when I tried to assign to the map

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good call, thank you

}
}

// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -126,12 +144,24 @@ func (dn *dataNode) addValues(m map[string]any) *dataNode {
m = map[string]any{}
}

return &dataNode{
parent: dn,
id: makeNodeID(),
values: maps.Clone(m),
labelCounter: dn.labelCounter,
spawn := dn.spawnDescendant()
spawn.id = makeNodeID()
spawn.setValues(m)

return spawn
}

// setValues is a helper called by addValues.
func (dn *dataNode) setValues(m map[string]any) {
if len(m) == 0 {
return
}

if len(dn.values) == 0 {
dn.values = map[string]any{}
}

maps.Copy(dn.values, m)
}

// trace adds a new leaf containing a trace ID and no other values.
Expand All @@ -140,12 +170,10 @@ func (dn *dataNode) trace(name string) *dataNode {
name = makeNodeID()
}

return &dataNode{
parent: dn,
id: name,
values: map[string]any{},
labelCounter: dn.labelCounter,
}
spawn := dn.spawnDescendant()
spawn.id = name

return spawn
}

// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -184,24 +212,36 @@ func InNamespace(ctx context.Context, namespace string) *dataNode {
// take priority over ancestors in cases of collision.
func (dn *dataNode) Map() map[string]any {
var (
m = map[string]any{}
idsl = []string{}
m = map[string]any{}
nodeIDs = []string{}
)

dn.lineage(func(id string, vs map[string]any) {
if len(id) > 0 {
idsl = append(idsl, id)
nodeIDs = append(nodeIDs, id)
}

for k, v := range vs {
m[k] = v
}
})

if len(idsl) > 0 {
m["clues_trace"] = strings.Join(idsl, ",")
if len(nodeIDs) > 0 {
m["clues_trace"] = strings.Join(nodeIDs, ",")
}

if len(dn.witnesses) == 0 {
return m
}

witnessVals := map[string]map[string]any{}

for _, witness := range dn.witnesses {
witnessVals[witness.id] = witness.data.Map()
}

m["witnessed"] = witnessVals

return m
}

Expand Down Expand Up @@ -266,11 +306,11 @@ func (dn *dataNode) addComment(
return dn
}

return &dataNode{
parent: dn,
labelCounter: dn.labelCounter,
comment: newComment(depth+1, msg, vs...),
}
spawn := dn.spawnDescendant()
spawn.id = makeNodeID()
spawn.comment = newComment(depth+1, msg, vs...)

return spawn
}

// comments allows us to put a stringer on a slice of comments.
Expand Down Expand Up @@ -317,6 +357,38 @@ func (dn *dataNode) Comments() comments {
return result
}

// ---------------------------------------------------------------------------
// witnesses
// ---------------------------------------------------------------------------

type witness struct {
// the name of the witness
id string

// dataNode is used here instead of a basic value map so that
// we can extend the usage of witnesses in the future by allowing
// the full set of dataNode behavior. We'll need a builder for that,
// but we'll get there eventually.
data *dataNode
}

// addWitness adds a new named witness to the dataNode.
func (dn *dataNode) addWitness(name string) *dataNode {
spawn := dn.spawnDescendant()

if len(spawn.witnesses) == 0 {
spawn.witnesses = map[string]*witness{}
}

spawn.witnesses[name] = &witness{
id: name,
// no spawn here, this needs an isolated node
data: &dataNode{},
}

return spawn
}

// ---------------------------------------------------------------------------
// ctx handling
// ---------------------------------------------------------------------------
Expand Down
20 changes: 8 additions & 12 deletions err.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,8 @@ func newErr(
file: file,
caller: getCaller(traceDepth + 1),
msg: msg,
data: &dataNode{
id: makeNodeID(),
values: m,
},
// no ID needed for err data nodes
data: &dataNode{values: m},
}
}

Expand Down Expand Up @@ -106,10 +104,8 @@ func toStack(
file: file,
caller: getCaller(traceDepth + 1),
stack: stack,
data: &dataNode{
id: makeNodeID(),
values: map[string]any{},
},
// no ID needed for err dataNodes
data: &dataNode{},
}
}

Expand Down Expand Up @@ -224,7 +220,7 @@ func stackAncestorsOntoSelf(err error) []error {
// comply with.
func InErr(err error) *dataNode {
if isNilErrIface(err) {
return &dataNode{values: map[string]any{}}
return &dataNode{}
}

return &dataNode{values: inErr(err)}
Expand Down Expand Up @@ -252,7 +248,7 @@ func inErr(err error) map[string]any {
// data take least priority.
func (err *Err) Values() *dataNode {
if isNilErrIface(err) {
return &dataNode{values: map[string]any{}}
return &dataNode{}
}

return &dataNode{values: err.values()}
Expand Down Expand Up @@ -1026,7 +1022,7 @@ func WithSkipCaller(err error, depth int) *Err {
// appearance and prefixed by the file and line in which they appeared. This
// means comments are always added to the error and never clobber each other,
// regardless of their location.
func (err *Err) WithComment(msg string, vs ...any) *Err {
func (err *Err) Comment(msg string, vs ...any) *Err {
if isNilErrIface(err) {
return nil
}
Expand All @@ -1052,7 +1048,7 @@ func (err *Err) WithComment(msg string, vs ...any) *Err {
// appearance and prefixed by the file and line in which they appeared. This
// means comments are always added to the error and never clobber each other,
// regardless of their location.
func WithComment(err error, msg string, vs ...any) *Err {
func Comment(err error, msg string, vs ...any) *Err {
if isNilErrIface(err) {
return nil
}
Expand Down
Loading
Loading