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.

Sign up for GitHub

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

Jump to bottom

Support key-value annotations on wrap #36

Open
abhinav opened this issue Nov 27, 2023 · 4 comments
Open

Support key-value annotations on wrap #36

abhinav opened this issue Nov 27, 2023 · 4 comments

Comments

@abhinav
Copy link
Contributor

abhinav commented Nov 27, 2023

Based on @prashantv's suggestions.
In the future, we should consider adding support for errtrace.Wrap to include key-value pairs as part of wrapping.
These values can be shown next to the function call in the trace, e.g.

example.com/main.getUser name="foo"
    path/to/file:42

The API for this is undecided at this time.
@akshayjshah
Copy link
Contributor

Is it reasonable to use slog fields for this, or would we want to design our own API for key-value pairs?

@abhinav
Copy link
Contributor Author

abhinav commented Nov 27, 2023

This was a bit fuzzy in terms of details, but re-using slog fields was one of the options I was considering.
I'm keeping this as a low priority item, though, so as not to increase scope of this project at this time.

@StevenACoffman StevenACoffman mentioned this issue May 5, 2024
Merged
@StevenACoffman
Copy link
Contributor

cockroachdb/errors has a ton of useful functionality... but is a very heavyweight and complicated library.

In #106 I added an interface so that the main value proposition of this library is composable with external custom error types.

I hope that allows this particular project to maintain a tighter focus.

@abhinav
Copy link
Contributor Author

abhinav commented May 5, 2024

@StevenACoffman Agreed! We're hoping to keep this library lightweight and focused, and not turn it into a general "error utils" library.

Whether this feature goes into this library is still up for discussion, and it would have to be lightweight and composable (similarly to #106) if it did. An argument can be made that this is a logging-specific concern and it should go in a logerr package; nothing about this in set in stone at this time.

abhinav added a commit that referenced this issue May 6, 2024
@StevenACoffman @abhinav
Add TraceCaller interface for extensibility for #104 (#106)
eaa936c 
Fixes #104

This PR extends errtrace functionality to accommodate custom errors that
implement the `TracePC() uintptr` method.

For instance, a custom error can have arbitrary fields as described in
#36 and as long as it also contains a `TracePC()` method,
it can be included in the errtrace.

```
type myTraceableError struct {
	// Fields are key-value pairs
	attrs    []slog.Attr

	// Wrapped Error
	err error

	// PC is the program counter for the location in this frame.
	// For a frame that calls another frame, this will be the
	// program counter of a call instruction. Because of inlining,
	// multiple frames may have the same PC value, but different
	// symbolic information.
	pc  uintptr
}

func (e *myTraceableError) TracePC() uintptr {
	return e.pc
}
```

---------

Signed-off-by: Steve Coffman <steve@khanacademy.org>
Co-authored-by: Abhinav Gupta <mail@abhinavg.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@abhinav @akshayjshah @StevenACoffman