Add ability to print error `Result` using `err` via `Debug` instead of only `Display`

[ciuncan]

Feature Request

Crates

• tracing-attributes

Motivation

Currently instrument attribute macro's err meta item makes the expanded code to print the Err result only using its Display implementation. Some Error types could be more beneficial to be printed using their Debug implementation. For example eyre::Result contains an error chain. When printed in via Display, it shows the most recent error message. But if you print it via the Debug implementation it shows the error with the error chain.

Proposal

Being able to write #[insturment(?err, ...)] would be in line with trace! et. al macro's fields, but it appears this isn't compatible with meta item syntax. Maybe two alternatives can be provided as:

• #[instrument(err, ...)]: print the Error using Display implementation so that existing codes aren't affected.
• #[instrument(err_dbg, ...)]: print the Error using Debug implementation.

I am willing to work on this and open a PR. Let me know if you can think of a better alternative syntax.

Alternatives

The only alternative I can think of is to remove err meta item from the attribute, catch the error somehow and print manually with "{:?}" format string. But this defeats the purpose of #[instrument] attribute (ergonomy) as it could easily implement this behavior.

[hawkw]

Maybe two alternatives can be provided as:

• #[instrument(err, ...)]: print the Error using Display implementation so that existing codes aren't affected.
• #[instrument(err_dbg, ...)]: print the Error using Debug implementation.

Syntactically, what do you think about using #[instrument(err)] and #[instrument(err(Debug)] or something, instead of adding an additional argument? IMO this feels more consistent with other attribute macros. Also, if we wanted to also permit recording the errors as dyn Error values in the future, we could add an #[instrument(err(Error))] option to use the error trait...

[ciuncan]

Hmm, #[instrument(err(Debug))] sounds good to me, it looks like as in the #[derive(...)] attribute macro. 👍 I'll try and push it as another commit.

But I am not sure if I understood #[instrument(err(Error))] one. How would that change the tracing::error! macro invocation? (Currently it would be invoked as error!(error = ?e); when err(Debug) and as error!(error = %e); when err only). Since Error trait implements both Debug and Display, it may create an ambiguity. I might be misunderstanding though.

[ciuncan]

hi @hawkw , I have implemented err(Debug) feel free to take a look at my PR https://github.com/tokio-rs/tracing/pull/1631/files#diff-74a47b15914675cd4c2a0c370bbec4a3d2d6e50d939dda795497ec67de050e8fR178-R181.

As an alternative, how would you regard err(?)/err(%)/err instead of err(Debug)/err(Display)/err? That would be in line with how fields are declared (these symbols are prefixed to field names, or their values if they are different, e.g. fields(?field1, %field2, field3 = ?value3, field4 = ?value4)).

[ciuncan]

Another idea: err = ?/err = %/err. We probably also extend the decision we make here to the proposed return meta in #1281.

[hawkw]

But I am not sure if I understood #[instrument(err(Error))] one. How would that change the tracing::error! macro invocation? (Currently it would be invoked as error!(error = ?e); when err(Debug) and as error!(error = %e); when err only). Since Error trait implements both Debug and Display, it may create an ambiguity. I might be misunderstanding though.

The Error trait (in the form of dyn Error + 'static trait objects) is one of tracing's primitive Value types. Visitors can opt in to recording errors as dyn Error values rather than as Display or Debug values. If they implement record_error, a Visit implementation can choose to record a dyn Error using Debug or Display, and can also call Error methods like source and downcast_ref to inspect the error in a more structured way.

I think it would be good to support this as well, but it's only applicable when the error type being returned implements the Error trait, which is not always the case.

[ciuncan]

Oh, see now. I also understand why we would need to specify Error (or Value maybe?), since the behavior would be changing (using Value implementation rather than the Display implementation as existing behavior).

Is it ok, if we cover this later, in a separate issue/PR? I'd be willing to implement this as well.

[hawkw]

Oh, see now. I also understand why we would need to specify Error (or Value maybe?), since the behavior would be changing (using Value implementation rather than the Display implementation as existing behavior).

Yeah, that's right! Sorry if that wasn't clear from my original comment!

Is it ok, if we cover this later, in a separate issue/PR? I'd be willing to implement this as well.

Of course. I think this should definitely be a follow-up change once the Debug PR is merged. :)

[hawkw]

Closed by #1631!