[tracing-attributes] future-proof the #[instrument] integration with async-trait #1228

nightmared · 2021-02-06T21:29:50Z

This draft PR aims to resolve #1219.

Tell me what you think ;)

It works with both the old and new version of async-trait (except for one doc test that failed previously, and that works with the new version). One nice thing is that the code is simpler (e.g.g no self renaming to _self, which will enable some simplifications in the future).

A minor nitpick is that I disliked the deeply nested pattern matching in get_async_trait_kind (previously: get_async_trait_function), so I "flattened" that a bit.

nightmared · 2021-02-06T21:54:54Z

Mmh, I don't quite get the reason for this "unused variable" warning :/

The offending code:

#[async_trait]
impl Test for TestImpl {
    // check that self is correctly handled, even when using async_trait
    #[instrument(fields(val=self.foo(), test=%v+5))]
    async fn call(&mut self, v: usize) {}
}

The error:

warning: unused variable: `v`
   --> tracing-attributes/tests/async_fn.rs:176:34
    |
176 |         async fn call(&mut self, v: usize) {}
    |                                  ^ help: if this is intentional, prefix it with an underscore: `_v`
    |
    = note: `#[warn(unused_variables)]` on by default

warning: 1 warning emitted

Here is the expanded code (thanks to cargo-expand), and I really can't put my finger on the issue.

impl Test for TestImpl {
    #[allow(clippy::used_underscore_binding)]
    fn call<'life0, 'async_trait>(
        &'life0 mut self,
        v: usize,
    ) -> ::core::pin::Pin<
        Box<dyn ::core::future::Future<Output = ()> + ::core::marker::Send + 'async_trait>,
    >
    where
        'life0: 'async_trait,
        Self: 'async_trait,
    {
        ::std::boxed::Box::pin(async move {
            let __tracing_attr_span = {
                use ::tracing::__macro_support::{Callsite as _, Registration};
                static CALLSITE: ::tracing::__macro_support::MacroCallsite = {
                    use ::tracing::__macro_support::{MacroCallsite, Registration};
                    static META: ::tracing::Metadata<'static> = {
                        ::tracing_core::metadata::Metadata::new(
                            "call",
                            "async_fn",
                            tracing::Level::INFO,
                            Some("tracing-attributes/tests/async_fn.rs"),
                            Some(175u32),
                            Some("async_fn"),
                            ::tracing_core::field::FieldSet::new(
                                &["self", "v", "val", "test"],
                                ::tracing_core::callsite::Identifier(&CALLSITE),
                            ),
                            ::tracing::metadata::Kind::SPAN,
                        )
                    };
                    static REG: Registration = Registration::new(&CALLSITE);
                    MacroCallsite::new(&META, &REG)
                };
                let mut interest = ::tracing::collect::Interest::never();
                if tracing::Level::INFO <= ::tracing::level_filters::STATIC_MAX_LEVEL
                    && tracing::Level::INFO <= ::tracing::level_filters::LevelFilter::current()
                    && {
                        interest = CALLSITE.interest();
                        !interest.is_never()
                    }
                    && CALLSITE.is_enabled(interest)
                {
                    let meta = CALLSITE.metadata();
                    ::tracing::Span::new(meta, &{
                        #[allow(unused_imports)]
                        use ::tracing::field::{debug, display, Value};
                        let mut iter = meta.fields().iter();
                        meta.fields().value_set(&[
                            (
                                &iter.next().expect("FieldSet corrupted (this is a bug)"),
                                Some(&tracing::field::debug(&self) as &Value),
                            ),
                            (
                                &iter.next().expect("FieldSet corrupted (this is a bug)"),
                                Some(&tracing::field::debug(&v) as &Value),
                            ),
                            (
                                &iter.next().expect("FieldSet corrupted (this is a bug)"),
                                Some(&self.foo() as &Value),
                            ),
                            (
                                &iter.next().expect("FieldSet corrupted (this is a bug)"),
                                Some(&display(&(v + 5)) as &Value),
                            ),
                        ])
                    })
                } else {
                    let span = CALLSITE.disabled_span();
                    {};
                    span
                }
            };
            tracing::Instrument::instrument(
                async move {
                    {
                        let __ret: () = {
                            let mut __self = self;
                            let v = v;
                        };
                        #[allow(unreachable_code)]
                    __ret
                }
            },
            __tracing_attr_span,
        )
        .await
    })
}

EDIT: After sleeping on it, I guess the error probably comes from the let v = v; binding, and the compiler probably reports it at this position because of the span generated by async-trait.

EDIT2: I can confirm that hypothesis:

Input Function: ItemFn { attrs: [Attribute { pound_token: Pound, style: Outer, bracket_token: Bracket, path: Path { leading_colon: None, segments: [PathSegment { ident: Ident { ident: "allow", span: #533 bytes(2633..2647) }, arguments: None }] }, tokens: TokenStream [Group { delimiter: Parenthesis, stream: TokenStream [Ident { ident: "clippy", span: #533 bytes(2633..2647) }, Punct { ch: ':', spacing: Joint, span: #533 bytes(2633..2647) }, Punct { ch: ':', spacing: Alone, span: #533 bytes(2633..2647) }, Ident { ident: "used_underscore_binding", span: #533 bytes(2633..2647) }], span: #533 bytes(2633..2647) }] }], vis: Inherited, sig: Signature { constness: None, asyncness: None, unsafety: None, abi: None, fn_token: Fn, ident: Ident { ident: "foo", span: #0 bytes(2717..2720) }, generics: Generics { lt_token: Some(Lt), params: [Lifetime(LifetimeDef { attrs: [], lifetime: Lifetime { apostrophe: #533 bytes(2633..2647), ident: Ident { ident: "life0", span: #533 bytes(2633..2647) } }, colon_token: None, bounds: [] }), Comma, Lifetime(LifetimeDef { attrs: [], lifetime: Lifetime { apostrophe: #533 bytes(2633..2647), ident: Ident { ident: "async_trait", span: #533 bytes(2633..2647) } }, colon_token: None, bounds: [] })], gt_token: Some(Gt), where_clause: Some(WhereClause { where_token: Where, predicates: [Lifetime(PredicateLifetime { lifetime: Lifetime { apostrophe: #533 bytes(2633..2647), ident: Ident { ident: "life0", span: #533 bytes(2633..2647) } }, colon_token: Colon, bounds: [Lifetime { apostrophe: #533 bytes(2633..2647), ident: Ident { ident: "async_trait", span: #533 bytes(2633..2647) } }] }), Comma, Type(PredicateType { lifetimes: None, bounded_ty: Path(TypePath { qself: None, path: Path { leading_colon: None, segments: [PathSegment { ident: Ident { ident: "Self", span: #533 bytes(2633..2647) }, arguments: None }] } }), colon_token: Colon, bounds: [Lifetime(Lifetime { apostrophe: #533 bytes(2633..2647), ident: Ident { ident: "async_trait", span: #533 bytes(2633..2647) } })] })] }) }, paren_token: Paren, inputs: [Receiver(Receiver { attrs: [], reference: Some((And, Some(Lifetime { apostrophe: #533 bytes(2633..2647), ident: Ident { ident: "life0", span: #533 bytes(2633..2647) } }))), mutability: Some(Mut), self_token: SelfValue }), Comma, Typed(PatType { attrs: [], pat: Ident(PatIdent { attrs: [], by_ref: None, mutability: None, ident: Ident { ident: "v", span: #0 bytes(2732..2733) }, subpat: None }), colon_token: Colon, ty: Path(TypePath { qself: None, path: Path { leading_colon: None, segments: [PathSegment { ident: Ident { ident: "usize", span: #0 bytes(2735..2740) }, arguments: None }] } }) })], variadic: None, output: Type(RArrow, Path(TypePath { qself: None, path: Path { leading_colon: Some(Colon2), segments: [PathSegment { ident: Ident { ident: "core", span: #533 bytes(2633..2647) }, arguments: None }, Colon2, PathSegment { ident: Ident { ident: "pin", span: #533 bytes(2633..2647) }, arguments: None }, Colon2, PathSegment { ident: Ident { ident: "Pin", span: #533 bytes(2633..2647) }, arguments: AngleBracketed(AngleBracketedGenericArguments { colon2_token: None, lt_token: Lt, args: [Type(Path(TypePath { qself: None, path: Path { leading_colon: None, segments: [PathSegment { ident: Ident { ident: "Box", span: #533 bytes(2633..2647) }, arguments: AngleBracketed(AngleBracketedGenericArguments { colon2_token: None, lt_token: Lt, args: [Type(TraitObject(TypeTraitObject { dyn_token: Some(Dyn), bounds: [Trait(TraitBound { paren_token: None, modifier: None, lifetimes: None, path: Path { leading_colon: Some(Colon2), segments: [PathSegment { ident: Ident { ident: "core", span: #533 bytes(2633..2647) }, arguments: None }, Colon2, PathSegment { ident: Ident { ident: "future", span: #533 bytes(2633..2647) }, arguments: None }, Colon2, PathSegment { ident: Ident { ident: "Future", span: #533 bytes(2633..2647) }, arguments: AngleBracketed(AngleBracketedGenericArguments { colon2_token: None, lt_token: Lt, args: [Binding(Binding { ident: Ident { ident: "Output", span: #533 bytes(2633..2647) }, eq_token: Eq, ty: Tuple(TypeTuple { paren_token: Paren, elems: [] }) })], gt_token: Gt }) }] } }), Add, Trait(TraitBound { paren_token: None, modifier: None, lifetimes: None, path: Path { leading_colon: Some(Colon2), segments: [PathSegment { ident: Ident { ident: "core", span: #533 bytes(2633..2647) }, arguments: None }, Colon2, PathSegment { ident: Ident { ident: "marker", span: #533 bytes(2633..2647) }, arguments: None }, Colon2, PathSegment { ident: Ident { ident: "Send", span: #533 bytes(2633..2647) }, arguments: None }] } }), Add, Lifetime(Lifetime { apostrophe: #533 bytes(2633..2647), ident: Ident { ident: "async_trait", span: #533 bytes(2633..2647) } })] }))], gt_token: Gt }) }] } }))], gt_token: Gt }) }] } })) }, block: Block { brace_token: Brace, stmts: [Expr(Call(ExprCall { attrs: [], func: Path(ExprPath { attrs: [], qself: None, path: Path { leading_colon: None, segments: [PathSegment { ident: Ident { ident: "Box", span: #0 bytes(2742..2836) }, arguments: None }, Colon2, PathSegment { ident: Ident { ident: "pin", span: #0 bytes(2742..2836) }, arguments: None }] } }), paren_token: Paren, args: [Async(ExprAsync { attrs: [], async_token: Async, capture: Some(Move), block: Block { brace_token: Brace, stmts: [Local(Local { attrs: [], let_token: Let, pat: Type(PatType { attrs: [], pat: Ident(PatIdent { attrs: [], by_ref: None, mutability: None, ident: Ident { ident: "__ret", span: #0 bytes(2742..2836) }, subpat: None }), colon_token: Colon, ty: Tuple(TypeTuple { paren_token: Paren, elems: [] }) }), init: Some((Eq, Block(ExprBlock { attrs: [], label: None, block: Block { brace_token: Brace, stmts: [Local(Local { attrs: [], let_token: Let, pat: Ident(PatIdent { attrs: [], by_ref: None, mutability: Some(Mut), ident: Ident { ident: "__self", span: #0 bytes(2726..2730) }, subpat: None }), init: Some((Eq, Path(ExprPath { attrs: [], qself: None, path: Path { leading_colon: None, segments: [PathSegment { ident: Ident { ident: "self", span: #0 bytes(2726..2730) }, arguments: None }] } }))), semi_token: Semi }), Local(Local { attrs: [], let_token: Let, pat: Ident(PatIdent { attrs: [], by_ref: None, mutability: None, ident: Ident { ident: "v", span: #0 bytes(2732..2733) }, subpat: None }), init: Some((Eq, Path(ExprPath { attrs: [], qself: None, path: Path { leading_colon: None, segments: [PathSegment { ident: Ident { ident: "v", span: #0 bytes(2732..2733) }, arguments: None }] } }))), semi_token: Semi }), Semi(Await(ExprAwait { attrs: [], base: MethodCall(ExprMethodCall { attrs: [], receiver: Path(ExprPath { attrs: [], qself: None, path: Path { leading_colon: None, segments: [PathSegment { ident: Ident { ident: "__self", span: #0 bytes(2756..2760) }, arguments: None }] } }), dot_token: Dot, method: Ident { ident: "baz", span: #0 bytes(2761..2764) }, turbofish: None, paren_token: Paren, args: [] }), dot_token: Dot, await_token: Await }), Semi), Semi(Assign(ExprAssign { attrs: [], left: Field(ExprField { attrs: [], base: Path(ExprPath { attrs: [], qself: None, path: Path { leading_colon: None, segments: [PathSegment { ident: Ident { ident: "__self", span: #0 bytes(2786..2790) }, arguments: None }] } }), dot_token: Dot, member: Unnamed(Index { index: 0, span: #0 bytes(2791..2792) }) }), eq_token: Eq, right: Path(ExprPath { attrs: [], qself: None, path: Path { leading_colon: None, segments: [PathSegment { ident: Ident { ident: "v", span: #0 bytes(2795..2796) }, arguments: None }] } }) }), Semi), Expr(Await(ExprAwait { attrs: [], base: MethodCall(ExprMethodCall { attrs: [], receiver: Path(ExprPath { attrs: [], qself: None, path: Path { leading_colon: None, segments: [PathSegment { ident: Ident { ident: "__self", span: #0 bytes(2810..2814) }, arguments: None }] } }), dot_token: Dot, method: Ident { ident: "bar", span: #0 bytes(2815..2818) }, turbofish: None, paren_token: Paren, args: [] }), dot_token: Dot, await_token: Await }))] } }))), semi_token: Semi }), Expr(Path(ExprPath { attrs: [Attribute { pound_token: Pound, style: Outer, bracket_token: Bracket, path: Path { leading_colon: None, segments: [PathSegment { ident: Ident { ident: "allow", span: #0 bytes(2742..2836) }, arguments: None }] }, tokens: TokenStream [Group { delimiter: Parenthesis, stream: TokenStream [Ident { ident: "unreachable_code", span: #0 bytes(2742..2836) }], span: #0 bytes(2742..2836) }] }], qself: None, path: Path { leading_colon: None, segments: [PathSegment { ident: Ident { ident: "__ret", span: #0 bytes(2742..2836) }, arguments: None }] } }))] } })] }))] } }

Resulting async block: TokenStream [Ident { ident: "let", span: #0 bytes(2742..2836) }, Ident { ident: "__tracing_attr_span", span: #0 bytes(2742..2836) }, Punct { ch: '=', spacing: Alone, span: #0 bytes(2742..2836) }, Ident { ident: "tracing", span: #536 bytes(2686..2699) }, Punct { ch: ':', spacing: Joint, span: #536 bytes(2686..2699) }, Punct { ch: ':', spacing: Alone, span: #536 bytes(2686..2699) }, Ident { ident: "span", span: #536 bytes(2686..2699) }, Punct { ch: '!', spacing: Alone, span: #536 bytes(2686..2699) }, Group { delimiter: Parenthesis, stream: TokenStream [Ident { ident: "target", span: #536 bytes(2686..2699) }, Punct { ch: ':', spacing: Alone, span: #536 bytes(2686..2699) }, Ident { ident: "module_path", span: #536 bytes(2686..2699) }, Punct { ch: '!', spacing: Alone, span: #536 bytes(2686..2699) }, Group { delimiter: Parenthesis, stream: TokenStream [], span: #536 bytes(2686..2699) }, Punct { ch: ',', spacing: Alone, span: #536 bytes(2686..2699) }, Ident { ident: "tracing", span: #536 bytes(2686..2699) }, Punct { ch: ':', spacing: Joint, span: #536 bytes(2686..2699) }, Punct { ch: ':', spacing: Alone, span: #536 bytes(2686..2699) }, Ident { ident: "Level", span: #536 bytes(2686..2699) }, Punct { ch: ':', spacing: Joint, span: #536 bytes(2686..2699) }, Punct { ch: ':', spacing: Alone, span: #536 bytes(2686..2699) }, Ident { ident: "INFO", span: #536 bytes(2686..2699) }, Punct { ch: ',', spacing: Alone, span: #536 bytes(2686..2699) }, Literal { kind: Str, symbol: "foo", suffix: None, span: #536 bytes(2686..2699) }, Punct { ch: ',', spacing: Alone, span: #536 bytes(2686..2699) }, Ident { ident: "self", span: #0 bytes(2721..2730) }, Punct { ch: '=', spacing: Alone, span: #536 bytes(2686..2699) }, Ident { ident: "tracing", span: #536 bytes(2686..2699) }, Punct { ch: ':', spacing: Joint, span: #536 bytes(2686..2699) }, Punct { ch: ':', spacing: Alone, span: #536 bytes(2686..2699) }, Ident { ident: "field", span: #536 bytes(2686..2699) }, Punct { ch: ':', spacing: Joint, span: #536 bytes(2686..2699) }, Punct { ch: ':', spacing: Alone, span: #536 bytes(2686..2699) }, Ident { ident: "debug", span: #536 bytes(2686..2699) }, Group { delimiter: Parenthesis, stream: TokenStream [Punct { ch: '&', spacing: Alone, span: #536 bytes(2686..2699) }, Ident { ident: "self", span: #0 bytes(2721..2730) }], span: #536 bytes(2686..2699) }, Punct { ch: ',', spacing: Alone, span: #536 bytes(2686..2699) }, Ident { ident: "v", span: #0 bytes(2732..2733) }, Punct { ch: '=', spacing: Alone, span: #536 bytes(2686..2699) }, Ident { ident: "tracing", span: #536 bytes(2686..2699) }, Punct { ch: ':', spacing: Joint, span: #536 bytes(2686..2699) }, Punct { ch: ':', spacing: Alone, span: #536 bytes(2686..2699) }, Ident { ident: "field", span: #536 bytes(2686..2699) }, Punct { ch: ':', spacing: Joint, span: #536 bytes(2686..2699) }, Punct { ch: ':', spacing: Alone, span: #536 bytes(2686..2699) }, Ident { ident: "debug", span: #536 bytes(2686..2699) }, Group { delimiter: Parenthesis, stream: TokenStream [Punct { ch: '&', spacing: Alone, span: #536 bytes(2686..2699) }, Ident { ident: "v", span: #0 bytes(2732..2733) }], span: #536 bytes(2686..2699) }, Punct { ch: ',', spacing: Alone, span: #536 bytes(2686..2699) }], span: #536 bytes(2686..2699) }, Punct { ch: ';', spacing: Alone, span: #0 bytes(2742..2836) }, Ident { ident: "tracing", span: #0 bytes(2742..2836) }, Punct { ch: ':', spacing: Joint, span: #0 bytes(2742..2836) }, Punct { ch: ':', spacing: Alone, span: #0 bytes(2742..2836) }, Ident { ident: "Instrument", span: #0 bytes(2742..2836) }, Punct { ch: ':', spacing: Joint, span: #0 bytes(2742..2836) }, Punct { ch: ':', spacing: Alone, span: #0 bytes(2742..2836) }, Ident { ident: "instrument", span: #0 bytes(2742..2836) }, Group { delimiter: Parenthesis, stream: TokenStream [Ident { ident: "async", span: #0 bytes(2742..2836) }, Ident { ident: "move", span: #0 bytes(2742..2836) }, Group { delimiter: Brace, stream: TokenStream [Group { delimiter: Brace, stream: TokenStream [Ident { ident: "let", span: #0 bytes(2742..2836) }, Ident { ident: "__ret", span: #0 bytes(2742..2836) }, Punct { ch: ':', spacing: Alone, span: #0 bytes(2742..2836) }, Group { delimiter: Parenthesis, stream: TokenStream [], span: #0 bytes(2742..2836) }, Punct { ch: '=', spacing: Alone, span: #0 bytes(2742..2836) }, Group { delimiter: Brace, stream: TokenStream [Ident { ident: "let", span: #533 bytes(2633..2647) }, Ident { ident: "mut", span: #0 bytes(2722..2725) }, Ident { ident: "__self", span: #0 bytes(2726..2730) }, Punct { ch: '=', spacing: Alone, span: #533 bytes(2633..2647) }, Ident { ident: "self", span: #0 bytes(2726..2730) }, Punct { ch: ';', spacing: Alone, span: #533 bytes(2633..2647) }, Ident { ident: "let", span: #533 bytes(2633..2647) }, Ident { ident: "v", span: #0 bytes(2732..2733) }, Punct { ch: '=', spacing: Alone, span: #533 bytes(2633..2647) }, Ident { ident: "v", span: #0 bytes(2732..2733) }, Punct { ch: ';', spacing: Alone, span: #533 bytes(2633..2647) }, Ident { ident: "__self", span: #0 bytes(2756..2760) }, Punct { ch: '.', spacing: Alone, span: #0 bytes(2760..2761) }, Ident { ident: "baz", span: #0 bytes(2761..2764) }, Group { delimiter: Parenthesis, stream: TokenStream [], span: #0 bytes(2764..2766) }, Punct { ch: '.', spacing: Alone, span: #0 bytes(2766..2767) }, Ident { ident: "await", span: #0 bytes(2767..2772) }, Punct { ch: ';', spacing: Alone, span: #0 bytes(2772..2773) }, Ident { ident: "__self", span: #0 bytes(2786..2790) }, Punct { ch: '.', spacing: Alone, span: #0 bytes(2790..2791) }, Literal { kind: Integer, symbol: "0", suffix: None, span: #0 bytes(2791..2792) }, Punct { ch: '=', spacing: Alone, span: #0 bytes(2793..2794) }, Ident { ident: "v", span: #0 bytes(2795..2796) }, Punct { ch: ';', spacing: Alone, span: #0 bytes(2796..2797) }, Ident { ident: "__self", span: #0 bytes(2810..2814) }, Punct { ch: '.', spacing: Alone, span: #0 bytes(2814..2815) }, Ident { ident: "bar", span: #0 bytes(2815..2818) }, Group { delimiter: Parenthesis, stream: TokenStream [], span: #0 bytes(2818..2820) }, Punct { ch: '.', spacing: Alone, span: #0 bytes(2820..2821) }, Ident { ident: "await", span: #0 bytes(2821..2826) }], span: #0 bytes(2742..2836) }, Punct { ch: ';', spacing: Alone, span: #0 bytes(2742..2836) }, Punct { ch: '#', spacing: Alone, span: #0 bytes(2742..2836) }, Group { delimiter: Bracket, stream: TokenStream [Ident { ident: "allow", span: #0 bytes(2742..2836) }, Group { delimiter: Parenthesis, stream: TokenStream [Ident { ident: "unreachable_code", span: #0 bytes(2742..2836) }], span: #0 bytes(2742..2836) }], span: #0 bytes(2742..2836) }, Ident { ident: "__ret", span: #0 bytes(2742..2836) }], span: #0 bytes(2742..2836) }], span: #0 bytes(2742..2836) }, Punct { ch: ',', spacing: Alone, span: #0 bytes(2742..2836) }, Ident { ident: "__tracing_attr_span", span: #0 bytes(2742..2836) }], span: #0 bytes(2742..2836) }, Punct { ch: '.', spacing: Alone, span: #0 bytes(2742..2836) }, Ident { ident: "await", span: #0 bytes(2742..2836) }]

The span for the argument v is #0 bytes(2732..2733), and the span where v is redeclared (let v = v) is the same: Ident { ident: "let", span: #533 bytes(2633..2647) }, Ident { ident: "v", span: #0 bytes(2732..2733) }, Punct { ch: '=', spacing: Alone, span: #533 bytes(2633..2647) }, Ident { ident: "v", span: #0 bytes(2732..2733) }.

nightmared · 2021-02-11T21:56:18Z

WIP, I'm heavily reworking this, do not merge please ;)

nightmared · 2021-02-12T09:57:06Z

All right, this is working now! It only requires nightmared/async-trait@32b1573 to be added to the PR on the async-trait side and it should work.
Please tell me what you think of this pile of hacks ;)

nightmared · 2021-02-27T11:22:25Z

Hello, now that it appears quite stable in async-trait (see dtolnay/async-trait#143), could someone consider taking a look at this PR and giving me an opinion on the unused argument discussion at dtolnay/async-trait#143 (comment)?
Thanks!

nightmared · 2021-03-05T10:06:37Z

Considering that this feature is now merged in async-trait, but without the "__async_trait marker hack", I probably should delete that part in the code. Any opinion on this, before I start pruning this kludge?

hawkw · 2021-03-05T17:39:56Z

Okay, sorry I haven't had a chance to look at this one; I've been quite busy. Gonna try to get it reviewed today.

hawkw

Considering that this feature is now merged in async-trait, but without the "__async_trait marker hack", I probably should delete that part in the code. Any opinion on this, before I start pruning this kludge?

If async-trait doesn't ever generate the marker, yeah, we should probably remove code for handling it.

I've finally gotten the chance to give this a review; overall, the approach here seems solid. I had some suggestions for how we can clean up the implementation.

Also, it would be really nice to have tests with both pre-0.1.44 and post-0.1.44 versions of async-trait, so we can be more confident that this doesn't break older versions --- is that something we could add to this PR?

tracing-attributes/src/lib.rs

tracing-attributes/Cargo.toml

tracing-attributes/src/lib.rs

hawkw · 2021-03-06T18:13:01Z

tracing-attributes/src/lib.rs

+// of code (e.g. the "self" and "Self" tokens in user-supplied
+// fields expressions when the function is generated by an old
+// version of async-trait).
+struct KnifeReplacer<'a> {


why "KnifeReplacer"? that name doesn't really explain what this does, IMO.

Yeah, I'm really not good at naming I guess :/
I think when I wrote it I was thinking about swiss knives, but then that code didn't end up as generic or powerful or as I hoped 😖
Not sure the new naming (IdentAndTypesRenamer) is much clearer though :)

tracing-attributes/src/lib.rs

…async-trait

hawkw

okay, this looks really good — thanks for all the cleanup! i had a few last suggestions, let me know what you think? and then we can get this merged

tracing-attributes/Cargo.toml

hawkw · 2021-03-10T18:07:48Z

tracing-attributes/src/lib.rs

+                        // we use liballoc to allow for no_std environments
+                        out_stmts.push(quote! {
+                            extern crate alloc;
+                            alloc::boxed::Box::pin(#(#async_attrs) * async move { #instrumented_block })


hmm, i'm not actually sure if generating an extern crate statement for alloc is actually the best thing. looking at async-trait, they just generate an un-prefixed Box::pin: https://github.com/dtolnay/async-trait/blob/a4aab7b9285e4912d16f1f4733cfb99fc8aa37da/src/expand.rs#L385-L387

maybe this is more correct, since it doesn't silently add alloc to no-std projects which don't depend on alloc? it won't compile out of the box on core + alloc projects, but if users add a use alloc::boxed::Box at the top of the file, it will; otherwise, it'll use Box from the prelude when libstd is there.

i know i said otherwise earlier, but after giving it some more thought, I think maybe it's best to do "whatever async-trait does" in this case. maybe @dtolnay has input?

On that note, i'm trying to test in a no_std environement, but it's not taht easy considering the fact the the support module is using String and similar standard library features. Would you be interested in a PR that try to adress this? (Not really commiting to anything here, but I won't start working on this if this isn't interesting for the project)

Testing on no_std would be nice, and I'd happily accept a PR that adds it (you could depend on liballoc for types like String). However, it's not a huge priority, so I'd prefer to go ahead and merge this PR now; we can work on no_std testing in a follow-up branch if you like?

tracing-attributes/src/lib.rs

hawkw

okay, this looks great to me! thanks again @nightmared for all your hard work on this!

…1228) This backports #1228 from `master` to `v0.1.x`. It works with both the old and new version of async-trait (except for one doc test that failed previously, and that works with the new version). One nice thing is that the code is simpler (e.g.g no self renaming to _self, which will enable some simplifications in the future). A minor nitpick is that I disliked the deeply nested pattern matching in get_async_trait_kind (previously: get_async_trait_function), so I "flattened" that a bit. Fixes #1219.

…1228) (#1291) This backports #1228 from `master` to `v0.1.x`. It works with both the old and new version of async-trait (except for one doc test that failed previously, and that works with the new version). One nice thing is that the code is simpler (e.g.g no self renaming to _self, which will enable some simplifications in the future). A minor nitpick is that I disliked the deeply nested pattern matching in get_async_trait_kind (previously: get_async_trait_function), so I "flattened" that a bit. Fixes #1219. Co-authored-by: Simon THOBY <git@nightmared.fr>

@nightmared

Fixed - Compatibility between `#[instrument]` and `async-trait` v0.1.43 and newer ([#1228]) Thanks to @nightmared for lots of hard work on this fix! [#1228]: #1228

@nightmared

### Fixed - Compatibility between `#[instrument]` and `async-trait` v0.1.43 and newer ([#1228]) Thanks to @nightmared for lots of hard work on this fix! [#1228]: #1228

…utures (#1297) Fixes #1296. I had forgotten to use all the input statements in #1228, so we would delete nearly all the statement of sync functions that return a boxed future :/

…utures (#1297) This backports #1297 from `master`. Fixes #1296. I had forgotten to use all the input statements in #1228, so we would delete nearly all the statement of sync functions that return a boxed future :/

@Folyd

# 0.1.26 (April 30, 2021) ### Fixed - **attributes**: Compatibility between `#[instrument]` and `async-trait` v0.1.43 and newer ([#1228]) - Several documentation fixes ([#1305], [#1344]) ### Added - `Subscriber` impl for `Box<dyn Subscriber + Send + Sync + 'static>` ([#1358]) - `Subscriber` impl for `Arc<dyn Subscriber + Send + Sync + 'static>` ([#1374]) - Symmetric `From` impls for existing `Into` impls on `span::Current`, `Span`, and `Option<Id>` ([#1335], [#1338]) - `From<EnteredSpan>` implementation for `Option<Id>`, allowing `EnteredSpan` to be used in a `span!` macro's `parent:` field ([#1325]) - `Attributes::fields` accessor that returns the set of fields defined on a span's `Attributes` ([#1331]) Thanks to @Folyd, @nightmared, and new contributors @rmsc and @Fishrock123 for contributing to this release! [#1227]: #1228 [#1305]: #1305 [#1325]: #1325 [#1338]: #1338 [#1344]: #1344 [#1358]: #1358 [#1374]: #1374 [#1335]: #1335 [#1331]: #1331

@Folyd

# 0.1.26 (April 30, 2021) ### Fixed - **attributes**: Compatibility between `#[instrument]` and `async-trait` v0.1.43 and newer ([#1228]) - Several documentation fixes ([#1305], [#1344]) ### Added - `Subscriber` impl for `Box<dyn Subscriber + Send + Sync + 'static>` ([#1358]) - `Subscriber` impl for `Arc<dyn Subscriber + Send + Sync + 'static>` ([#1374]) - Symmetric `From` impls for existing `Into` impls on `span::Current`, `Span`, and `Option<Id>` ([#1335], [#1338]) - `From<EnteredSpan>` implementation for `Option<Id>`, allowing `EnteredSpan` to be used in a `span!` macro's `parent:` field ([#1325]) - `Attributes::fields` accessor that returns the set of fields defined on a span's `Attributes` ([#1331]) Thanks to @Folyd, @nightmared, and new contributors @rmsc and @Fishrock123 for contributing to this release! [#1227]: #1228 [#1305]: #1305 [#1325]: #1325 [#1338]: #1338 [#1344]: #1344 [#1358]: #1358 [#1374]: #1374 [#1335]: #1335 [#1331]: #1331

…okio-rs#1228) (tokio-rs#1291) This backports tokio-rs#1228 from `master` to `v0.1.x`. It works with both the old and new version of async-trait (except for one doc test that failed previously, and that works with the new version). One nice thing is that the code is simpler (e.g.g no self renaming to _self, which will enable some simplifications in the future). A minor nitpick is that I disliked the deeply nested pattern matching in get_async_trait_kind (previously: get_async_trait_function), so I "flattened" that a bit. Fixes tokio-rs#1219. Co-authored-by: Simon THOBY <git@nightmared.fr>

@nightmared

### Fixed - Compatibility between `#[instrument]` and `async-trait` v0.1.43 and newer ([tokio-rs#1228]) Thanks to @nightmared for lots of hard work on this fix! [tokio-rs#1228]: tokio-rs#1228

@Folyd

# 0.1.26 (April 30, 2021) ### Fixed - **attributes**: Compatibility between `#[instrument]` and `async-trait` v0.1.43 and newer ([tokio-rs#1228]) - Several documentation fixes ([tokio-rs#1305], [tokio-rs#1344]) ### Added - `Subscriber` impl for `Box<dyn Subscriber + Send + Sync + 'static>` ([tokio-rs#1358]) - `Subscriber` impl for `Arc<dyn Subscriber + Send + Sync + 'static>` ([tokio-rs#1374]) - Symmetric `From` impls for existing `Into` impls on `span::Current`, `Span`, and `Option<Id>` ([tokio-rs#1335], [tokio-rs#1338]) - `From<EnteredSpan>` implementation for `Option<Id>`, allowing `EnteredSpan` to be used in a `span!` macro's `parent:` field ([tokio-rs#1325]) - `Attributes::fields` accessor that returns the set of fields defined on a span's `Attributes` ([tokio-rs#1331]) Thanks to @Folyd, @nightmared, and new contributors @rmsc and @Fishrock123 for contributing to this release! [tokio-rs#1227]: tokio-rs#1228 [tokio-rs#1305]: tokio-rs#1305 [tokio-rs#1325]: tokio-rs#1325 [tokio-rs#1338]: tokio-rs#1338 [tokio-rs#1344]: tokio-rs#1344 [tokio-rs#1358]: tokio-rs#1358 [tokio-rs#1374]: tokio-rs#1374 [tokio-rs#1335]: tokio-rs#1335 [tokio-rs#1331]: tokio-rs#1331

nightmared force-pushed the async-trait-tracing-the-rematch branch from 1cd583f to b06f7d8 Compare February 6, 2021 21:35

nightmared marked this pull request as ready for review February 6, 2021 21:54

nightmared requested review from davidbarsky, hawkw and a team as code owners February 6, 2021 21:54

nightmared mentioned this pull request Feb 7, 2021

Transform blocks into async move for more compatibility dtolnay/async-trait#143

Merged

nightmared force-pushed the async-trait-tracing-the-rematch branch 4 times, most recently from 17bd89b to 46142dd Compare February 12, 2021 09:44

nightmared force-pushed the async-trait-tracing-the-rematch branch from 46142dd to b00c9f1 Compare February 27, 2021 11:08

nightmared force-pushed the async-trait-tracing-the-rematch branch from b00c9f1 to 7b49d8d Compare March 5, 2021 10:03

hawkw requested changes Mar 6, 2021

View reviewed changes

nightmared force-pushed the async-trait-tracing-the-rematch branch from 785c322 to 6b24f06 Compare March 6, 2021 20:19

[tracing-attributes] future-proof the #[instrument] integration with …

a71ad5b

…async-trait

nightmared force-pushed the async-trait-tracing-the-rematch branch from 6b24f06 to a71ad5b Compare March 7, 2021 08:57

hawkw mentioned this pull request Mar 8, 2021

Test async_fn_with_async_trait fails #1284

Closed

hawkw approved these changes Mar 10, 2021

View reviewed changes

review

02cf636

nightmared requested a review from hawkw March 10, 2021 19:08

dtolnay mentioned this pull request Mar 10, 2021

Build error when using tracing crate dtolnay/async-trait#156

Closed

hawkw approved these changes Mar 10, 2021

View reviewed changes

hawkw merged commit 2e4a4f3 into tokio-rs:master Mar 10, 2021

hawkw mentioned this pull request Mar 10, 2021

attributes: update #[instrument] to support async-trait 0.1.43+ #1291

Merged

nightmared deleted the async-trait-tracing-the-rematch branch March 10, 2021 20:30

hawkw added a commit that referenced this pull request Mar 10, 2021

attributes: prepare to release v0.1.14

98ad8ee

Fixed - Compatibility between `#[instrument]` and `async-trait` v0.1.43 and newer ([#1228]) Thanks to @nightmared for lots of hard work on this fix! [#1228]: #1228

This was referenced Mar 10, 2021

attributes: prepare to release v0.1.14 #1292

Merged

Add a public current_span() method for FmtContext #1290

Merged

hawkw added a commit that referenced this pull request Mar 10, 2021

attributes: prepare to release v0.1.14 (#1292)

88611cb

### Fixed - Compatibility between `#[instrument]` and `async-trait` v0.1.43 and newer ([#1228]) Thanks to @nightmared for lots of hard work on this fix! [#1228]: #1228

hawkw mentioned this pull request Mar 11, 2021

tracing-attributes 0.1.14 causes variables to be out of scope when they're in scope #1296

Closed

nightmared mentioned this pull request Mar 11, 2021

attributes: fix #[instrument] skipping code when returning pinned futures #1297

Merged

hawkw mentioned this pull request Apr 30, 2021

tracing: prepare to release 0.1.26 #1383

Merged

davidbarsky mentioned this pull request Sep 26, 2023

chore: backport roughly a year's worth of changes #2728

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[tracing-attributes] future-proof the #[instrument] integration with async-trait #1228

[tracing-attributes] future-proof the #[instrument] integration with async-trait #1228

nightmared commented Feb 6, 2021

nightmared commented Feb 6, 2021 •

edited

Loading

nightmared commented Feb 11, 2021

nightmared commented Feb 12, 2021

nightmared commented Feb 27, 2021

nightmared commented Mar 5, 2021

hawkw commented Mar 5, 2021 •

edited

Loading

hawkw left a comment

hawkw Mar 6, 2021

nightmared Mar 6, 2021

hawkw left a comment

hawkw Mar 10, 2021

nightmared Mar 10, 2021

hawkw Mar 10, 2021

hawkw left a comment

[tracing-attributes] future-proof the #[instrument] integration with async-trait #1228

[tracing-attributes] future-proof the #[instrument] integration with async-trait #1228

Conversation

nightmared commented Feb 6, 2021

nightmared commented Feb 6, 2021 • edited Loading

nightmared commented Feb 11, 2021

nightmared commented Feb 12, 2021

nightmared commented Feb 27, 2021

nightmared commented Mar 5, 2021

hawkw commented Mar 5, 2021 • edited Loading

hawkw left a comment

Choose a reason for hiding this comment

hawkw Mar 6, 2021

Choose a reason for hiding this comment

nightmared Mar 6, 2021

Choose a reason for hiding this comment

hawkw left a comment

Choose a reason for hiding this comment

hawkw Mar 10, 2021

Choose a reason for hiding this comment

nightmared Mar 10, 2021

Choose a reason for hiding this comment

hawkw Mar 10, 2021

Choose a reason for hiding this comment

hawkw left a comment

Choose a reason for hiding this comment

nightmared commented Feb 6, 2021 •

edited

Loading

hawkw commented Mar 5, 2021 •

edited

Loading