Improve #[instrument(ret, err)] handling for tracing-opentelemetry

tracing-attributes/src/attr.rs

@@ -163,28 +163,31 @@ impl Parse for EventArgs {
         let content;
         let _ = syn::parenthesized!(content in input);
         let mut result = Self::default();
-        let mut parse_one_arg =
-            || {
-                let lookahead = content.lookahead1();
-                if lookahead.peek(kw::level) {
-                    if result.level.is_some() {
-                        return Err(content.error("expected only a single `level` argument"));
-                    }
-                    result.level = Some(content.parse()?);
-                } else if result.mode != FormatMode::default() {
-                    return Err(content.error("expected only a single format argument"));
-                } else if let Some(ident) = content.parse::<Option<Ident>>()? {
-                    match ident.to_string().as_str() {
-                        "Debug" => result.mode = FormatMode::Debug,
-                        "Display" => result.mode = FormatMode::Display,
-                        _ => return Err(syn::Error::new(
+        let mut parse_one_arg = || {
+            let lookahead = content.lookahead1();
+            if lookahead.peek(kw::level) {
+                if result.level.is_some() {
+                    return Err(content.error("expected only a single `level` argument"));
+                }
+                result.level = Some(content.parse()?);
+            } else if result.mode != FormatMode::default() {
+                return Err(content.error("expected only a single format argument"));
+            } else if let Some(ident) = content.parse::<Option<Ident>>()? {
+                match ident.to_string().as_str().into() {
+                    FormatMode::Default => {
+                        return Err(syn::Error::new(
                             ident.span(),
-                            "unknown event formatting mode, expected either `Debug` or `Display`",
-                        )),
+                            format!(
+                                "unknown event formatting mode, must be one of: {}",
+                                FormatMode::options_list()
+                            ),
+                        ))
                     }
+                    mode => result.mode = mode,
                 }
-                Ok(())
-            };
+            }
+            Ok(())
+        };
         parse_one_arg()?;
         if !content.is_empty() {
             if content.lookahead1().peek(Token![,]) {
@@ -260,6 +263,7 @@ pub(crate) enum FormatMode {
     Default,
     Display,
     Debug,
+    Value,
 }
 
 impl Default for FormatMode {
@@ -268,6 +272,58 @@ impl Default for FormatMode {
     }
 }
 
+impl FormatMode {
+    /// A list of the valid options for the format mode. This should be updated
+    /// when new modes are added to ensure that parser error messages are formatted correctly.
+    const OPTIONS: [(&str, FormatMode);3] = [
+        ("Debug", FormatMode::Debug),
+        ("Display", FormatMode::Display),
+        ("Value", FormatMode::Value),
+    ];
+
+    fn options_list() -> String {
+        Self::OPTIONS.iter().map(|(name, _)| *name).collect::<Vec<_>>().join(", ")
+    }
+}
+
+impl<S> From<S> for FormatMode
+where
+    S: AsRef<str>,
+{
+    fn from(s: S) -> Self {
+        for (name, value) in Self::OPTIONS {
+            if s.as_ref() == name {
+                 return value;
+            }
+        }
+
+        FormatMode::default()
+    }
+}
+
+impl Parse for FormatMode {
+    fn parse(input: ParseStream<'_>) -> syn::Result<Self> {
+        if !input.peek(syn::token::Paren) {
+            return Ok(FormatMode::default());
+        }
+        let content;
+        let _ = syn::parenthesized!(content in input);
+        let maybe_mode: Option<Ident> = content.parse()?;
+        maybe_mode.map_or(Ok(FormatMode::default()), |ident| {
+            match ident.to_string().as_str().into() {
+                FormatMode::Default => Err(syn::Error::new(
+                    ident.span(),
+                    format!(
+                        "unknown error mode, must be one of: {}",
+                        FormatMode::options_list()
+                    ),
+                )),
+                mode => Ok(mode),
+            }
+        })
+    }
+}
+
 #[derive(Clone, Debug)]
 pub(crate) struct Fields(pub(crate) Punctuated<Field, Token![,]>);
 

tracing-attributes/src/expand.rs

@@ -241,10 +241,13 @@ fn gen_block<B: ToTokens>(
             let level_tokens = event_args.level(Level::Error);
             match event_args.mode {
                 FormatMode::Default | FormatMode::Display => Some(quote!(
-                    tracing::event!(target: #target, #level_tokens, error = %e)
+                    tracing::event!(target: #target, #level_tokens, error = %e, "{}", e)
                 )),
                 FormatMode::Debug => Some(quote!(
-                    tracing::event!(target: #target, #level_tokens, error = ?e)
+                    tracing::event!(target: #target, #level_tokens, error = ?e, "{:?}", e)
+                )),
+                FormatMode::Value => Some(quote!(
+                    tracing::event!(target: #target, #level_tokens, error = e, "{}", e)
                 )),
             }
         }
@@ -256,10 +259,13 @@ fn gen_block<B: ToTokens>(
             let level_tokens = event_args.level(args_level);
             match event_args.mode {
                 FormatMode::Display => Some(quote!(
-                    tracing::event!(target: #target, #level_tokens, return = %x)
+                    tracing::event!(target: #target, #level_tokens, return = %x, "{}", x)
                 )),
                 FormatMode::Default | FormatMode::Debug => Some(quote!(
-                    tracing::event!(target: #target, #level_tokens, return = ?x)
+                    tracing::event!(target: #target, #level_tokens, return = ?x, "{:?}", x)
+                )),
+                FormatMode::Value => Some(quote!(
+                    tracing::event!(target: #target, #level_tokens, return = x, "{}", x)
                 )),
             }
         }

tracing-attributes/src/lib.rs

@@ -244,7 +244,7 @@ mod expand;
 /// }
 /// ```
 ///
-/// **Note**:  if the function returns a `Result<T, E>`, `ret` will record returned values if and
+/// **Note**:  if the function returns a [`std::result::Result<T, E>`], `ret` will record returned values if and
 /// only if the function returns [`Result::Ok`].
 ///
 /// By default, returned values will be recorded using their [`std::fmt::Debug`] implementations.
@@ -259,7 +259,18 @@ mod expand;
 /// }
 /// ```
 ///
-/// If the function returns a `Result<T, E>` and `E` implements `std::fmt::Display`, adding
+/// If the value being returned implements [`tracing::field::Value`], it can be recorded using its
+/// `Value` implementation directly, using  `ret(Value)`:
+///
+/// ```
+/// # use tracing_attributes::instrument;
+/// #[instrument(ret(Value))]
+/// fn my_function() -> i32 {
+///     42
+/// }
+/// ```
+///
+/// If the function returns a [`std::result::Result<T, E>`] and `E` implements [`std::fmt::Display`], adding
 /// `err` or `err(Display)` will emit error events when the function returns `Err`:
 ///
 /// ```
@@ -283,8 +294,8 @@ mod expand;
 /// }
 /// ```
 ///
-/// By default, error values will be recorded using their `std::fmt::Display` implementations.
-/// If an error implements `std::fmt::Debug`, it can be recorded using its `Debug` implementation
+/// By default, error values will be recorded using their [`std::fmt::Display`] implementations.
+/// If an error implements `std::fmt::Debug`, it can be recorded using its [`std::fmt::Debug`] implementation
 /// instead by writing `err(Debug)`:
 ///
 /// ```
@@ -295,6 +306,18 @@ mod expand;
 /// }
 /// ```
 ///
+/// If a function returns a [`std::result::Result<T, E>`] and `E` implements [`std::error::Error`], the raw
+/// error value can be recorded using `err(Value)`. This is useful when paired with a subscriber
+/// which specially handles [`std::error::Error`]s, like [`tracing_opentelemetry`]:
+///
+/// ```
+/// # use tracing_attributes::instrument;
+/// #[instrument(err(Value))]
+/// fn my_function(arg: usize) -> Result<(), Box<dyn std::error::Error>> {
+///    Ok(())
+/// }
+/// ```
+///
 /// If a `target` is specified, both the `ret` and `err` arguments will emit outputs to
 /// the declared target (or the default channel if `target` is not specified).
 ///
@@ -355,6 +378,8 @@ mod expand;
 /// [span]: https://docs.rs/tracing/latest/tracing/span/index.html
 /// [`follows_from`]: https://docs.rs/tracing/latest/tracing/struct.Span.html#method.follows_from
 /// [`tracing`]: https://github.com/tokio-rs/tracing
+/// [`tracing_opentelemetry`]: https://docs.rs/tracing-opentelemetry
+/// [`tracing::field::Value`]: https://docs.rs/tracing/latest/tracing/field/trait.Value.html
 /// [`fmt::Debug`]: std::fmt::Debug
 /// [`Level`]: https://docs.rs/tracing/latest/tracing/struct.Level.html
 /// [`Level::TRACE`]: https://docs.rs/tracing/latest/tracing/struct.Level.html#associatedconstant.TRACE

tracing-attributes/tests/err.rs

@@ -6,6 +6,7 @@ use tracing_subscriber::filter::EnvFilter;
 use tracing_subscriber::subscribe::CollectExt;
 
 use std::convert::TryFrom;
+use std::error::Error;
 use std::num::TryFromIntError;
 
 #[instrument(err)]
@@ -205,6 +206,36 @@ fn test_err_dbg() {
     handle.assert_finished();
 }
 
+#[instrument(err(Value))]
+fn err_value() -> Result<u8, Box<dyn Error + 'static>> {
+    Err("oh no".into())
+}
+
+#[test]
+fn test_err_raw() {
+    let err: Box<dyn Error + 'static> = "oh no".into();
+    let span = expect::span().named("err_value");
+    let (collector, handle) = collector::mock()
+        .new_span(span.clone())
+        .enter(span.clone())
+        .event(
+            expect::event().at_level(Level::ERROR).with_fields(
+                expect::field("error")
+                    // use the actual error value that will be emitted, so
+                    // that this test doesn't break if the standard library
+                    // changes the `fmt::Debug` output from the error type
+                    // in the future.
+                    .with_value(&err),
+            ),
+        )
+        .exit(span.clone())
+        .drop_span(span)
+        .only()
+        .run_with_handle();
+    with_default(collector, || err_value().ok());
+    handle.assert_finished();
+}
+
 #[test]
 fn test_err_display_default() {
     let span = expect::span().named("err");

tracing-attributes/tests/ret.rs

@@ -214,6 +214,8 @@ fn test_ret_and_err() {
                 .with_fields(
                     expect::field("error")
                         .with_value(&tracing::field::display(u8::try_from(1234).unwrap_err()))
+                        .and(expect::field("message")
+                            .with_value(&tracing::field::display(u8::try_from(1234).unwrap_err())))
                         .only(),
                 )
                 .at_level(Level::ERROR),
@@ -243,6 +245,8 @@ fn test_ret_and_ok() {
                 .with_fields(
                     expect::field("return")
                         .with_value(&tracing::field::debug(u8::try_from(123).unwrap()))
+                        .and(expect::field("message")
+                            .with_value(&tracing::field::debug(123)))
                         .only(),
                 )
                 .at_level(Level::INFO),