want way to ask #[instrument] to trace the return value

[ijackson]

Feature Request

Crates

tracing-attributes

Motivation

Tracing's #[instrument] is very nice for printf-style debugging as well as post-hoc analysis. But as far as I can tell it does not print the return value of the annotated function (although you can use err to print the Err of a Result).

Proposal

I suggest #[instrument(return)] to make it print the return value on function exit.

Alternatives

Adding a dbg! call to the call sites seems the most obvious workaround but is far less convenient.

[hawkw]

Something like this would definitely be nice, and should be doable (using the same techniques we use for #[instrument(err)]). Are you interested in writing a PR? I'm happy to give some pointers.

[ijackson]

Something like this would definitely be nice, and should be doable (using the same techniques we use for #[instrument(err)]). Are you interested in writing a PR? I'm happy to give some pointers.

Thanks. I've put it on my list but it might be a while before it gets to the top.

[ciuncan]

hi @hawkw , @ijackson , I recently opened an issue #1630 and a PR #1631 for adding err_dbg to print errors with Debug impls. Maybe this could also benefit from the same (Display vs. Debug dichotomy), maybe via return_dbg? I am willing to open a PR for the issue described here as well as the return_dbg addition, if no one else is working on this.

[hkmatsumoto]

I want this feature very much so will work on this next week.

In order to implement #[instrument(return)] I reckon I need to add return field to InstrumentArgs and some code for parsing the arguments. Am I on the right track @hawkw ?

tracing/tracing-attributes/src/attr.rs

Lines 10 to 19 in b439705

	pub(crate) struct InstrumentArgs {
	level: Option<Level>,
	pub(crate) name: Option<LitStr>,
	target: Option<LitStr>,
	pub(crate) skips: HashSet<Ident>,
	pub(crate) fields: Option<Fields>,
	pub(crate) err_mode: Option<ErrorMode>,
	/// Errors describing any unrecognized parse inputs that we skipped.
	parse_warnings: Vec<syn::Error>,
	}

[ciuncan]

I want this feature very much so will work on this next week.

In order to implement #[instrument(return)] I reckon I need to add return field to InstrumentArgs and some code for parsing the arguments. Am I on the right track @hawkw ?

tracing/tracing-attributes/src/attr.rs

Lines 10 to 19 in b439705

	pub(crate) struct InstrumentArgs {
	level: Option<Level>,
	pub(crate) name: Option<LitStr>,
	target: Option<LitStr>,
	pub(crate) skips: HashSet<Ident>,
	pub(crate) fields: Option<Fields>,
	pub(crate) err_mode: Option<ErrorMode>,
	/// Errors describing any unrecognized parse inputs that we skipped.
	parse_warnings: Vec<syn::Error>,
	}

Hi @hkmatsumoto , allow me to jump in. Yes you are on right track. That struct is the representation of the parsed arguments of instrument macro. So having a return or rather return_mode field makes sense. It may be a bool representing its presence but I suggest you make it similar to err_mode field, since you may want to support emitting events with return value presented via either Display or Debug implementation. In my recent PR, as per @hawkw 's suggestion, we made it err(Debug), err(Display) or just err (existing way, prior to my PR, which is the same as err(Display)). You can follow the same here for consistency.

Feel free to check the changes and discussion in #1631, there will be lots of inspiration points. Basically you'll need to determine presence and mode of return keyword in macro invocation (at Parse impl of InsturmentArgs), save in InstrumentArgs, then at gen_block you should evaluate this field and emit event if necessary. You also need to handle async and non-async case.

To debug your implementation, I suggest using VS Code's "expand macro recursively" feature (after a code change you may need to close and reopen VSCode due to some limitation, though), possibly on a test example function you'll add https://github.com/tokio-rs/tracing/pull/1631/files#diff-74a47b15914675cd4c2a0c370bbec4a3d2d6e50d939dda795497ec67de050e8fR178-R181 and examine the expansion output.