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

Syntax for recording errors in events and spans #3067

Open
ziutech opened this issue Aug 27, 2024 · 3 comments
Open

Syntax for recording errors in events and spans #3067

ziutech opened this issue Aug 27, 2024 · 3 comments

Comments

@ziutech
Copy link

ziutech commented Aug 27, 2024

Feature Request

Crates

tracing-core, tracing

Motivation

Tracing supports recording a type implementing std::error::Error with a method Visit::record_error. Currently, one way to record such type in event or span macros is by writing something similar to

error!(error = &error as &dyn std::error::Error);

One could bring std::error::Error into scope as StdError, but even then it can be considered cumbersome to write.

Also, users might have to use different syntax depending on type of error:

some_fallible_fn().inspect_err(|error: &Error| {
    // notice the missing '&'
    error!(error = error as &dyn std::error::Error); 
});

if let Err(error) = some_fallible_fn() {
    // here '&' is required
    error!(error = &error as &dyn std::error::Error); 
}

I consider this too explicit. I do not see any reason why Debug and Display would have a dedicated syntax, while Error requires spelling out the type cast every time, especially considering that errors are added to traces as frequently as any other value (at least for me that's the case).

It might be valuable to gather data on whether recording errors with ... as &dyn std::error::Error is the most common way, or are there any popular alternatives.

Why not just use the ? or % for errors?

In my opinion, if Visit has a method specifically for recording errors, then that feature should be available for users in span and event macros, which are the basic blocks for defining spans and traces. For logging subscribers this is probably not that big of a difference, but for other subscribers that might collect statistics or other data, this distinction might be important.

Proposal

I propose to introduce syntax for recording errors in event and span macros in tracing and add ErrorValue to tracing-core as an analogous to tracing_core::field::DebugValue and tracing_core::field::DisplayValue.

Possible syntax candidates are:

Sigil #, aka pound sign/hashtag/octothorp

error!(#error, "foo failed"); 
error!(some.error = #error, "foo failed"); 

Sigil ^, aka caret

error!(^error, "foo failed"); 
error!(some.error = ^error, "foo failed"); 

Sigil @, aka at sign

error!(@error, "foo failed"); 
error!(some.error = @error, "foo failed"); 

Keyword

There is a limited number of symbols available in rust. One alternative is to use a keyword like err or e.

error!(err error, "foo failed"); 
error!(some.error = err error, "foo failed"); 
error!(e error, "foo failed"); 
error!(some.error = e error, "foo failed"); 

Keyword and a sigil

Mixing is also an option, e.g. e%:

error!(e%error, "foo failed"); 
error!(some.error = e%error, "foo failed"); 

Other, most likely invalid candidates

  • ~, aka tilde, is currently disallowed in Rust when writing ~expr to help users who might think this is a unary bitwise not operator, recommending to use ! instead, thus making error!(~err) fail before macro expansion,
  • !, aka exclamation mark, might clash with Rust's negation operator in error!(error = !error)

Alternatives

Don't add anything

Currently there is one other way to record an error using the record_error method - using the instrument macro.
Edit: I was wrong, there's an open PR to implement that #3057.

Thus, one alternative is to not add additional support for recording errors in event and span macros.

Function error_as_dyn or trait ErrorExt

tracing could recommend users to cast the error to &dyn StdError with a function. This option results in less jarring syntax than manually casting, but still not as concise as a single sigil or keyword.

// name up for debate
fn error_as_dyn<'a, T>(
    error: &'a (dyn std::error::Error + 'static),
) -> &'a (dyn std::error::Error + 'static) {
    error
}

// example usage
error!(error = error_as_dyn(&error));

It could also be possible to define trait ErrorExt with method as_dyn, but I failed at figuring out the lifetime bounds. As far as I know there's not such trait or utility function in standard library.

Wait until valuable is stabilized

Personally, I've never used the valuable feature, but from what I understand it could remove the need for a special syntax by allowing users to implement valuable::Valuable for their own error's, or implement it for all errors (if possible).

Docs

Write examples, blog posts, documentation, etc. on how to structure programs to better utilize current tracing features for logging errors. Of course, this is a good thing to do, regardless of the future of this issue.

@thomaseizinger
Copy link
Contributor

sentry-tracing is one of these integrations that would greatly benefit from this feature: https://docs.rs/sentry-tracing/latest/sentry_tracing/#tracking-errors

Instead of a special symbol, could we perhaps specialise on the error name itself and require it to be the first field that is mentioned?

Another idea could be a wrapper struct. Anything that implements Value can just be captured, right? Could we not create an extension trait for anything that implements std::error::Error which wraps it in a newtype that implements Value?

@ziutech
Copy link
Author

ziutech commented Oct 20, 2024

Instead of a special symbol, could we perhaps specialise on the error name itself and require it to be the first field that is mentioned?

This assumes that no one ever will want two errors in one event.

Another idea could be a wrapper struct.

This is how ? and % are implemented today (for ? take a look at tracing_core::field::DebugValue and the debug fn). So a corresponding ErrorValue and error would have to be implemented, independent of the way this is presented to users of the macros.

And I just realized there's a problem with that since tracing supports no-std projects and core::error::Error was added only recently (in 1.81) . This means that ErrorValue would either:

  • require a MSRV bump to 1.81 for both no-std and std,
  • require a MSRV bump to 1.81 only for no-std,
  • be a std only feature.
    Edit: I'm wrong, recording error's is already std-only

@thomaseizinger
Copy link
Contributor

Another idea could be a wrapper struct.

This is how ? and % are implemented today (for ? take a look at tracing_core::field::DebugValue and the debug fn). So a corresponding ErrorValue and error would have to be implemented, independent of the way this is presented to users of the macros.

Perhaps that could be a first step into this direction to make it easier to use and not require these obscure casts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants