async-backtrace icon indicating copy to clipboard operation
async-backtrace copied to clipboard

Can we add frame for a function that reutrns `Poll<T>`?

Open Xuanwo opened this issue 2 years ago • 6 comments

Hi, thanks for creating this lib! OpenDAL is working on for adding native integration with this lib to allow users to dump the async backtrace: https://github.com/apache/incubator-opendal/pull/2765.

But we met a problem that we have some trait like AsyncRead returns Poll<T>. Can we add frame for them?

Xuanwo avatar Aug 03 '23 11:08 Xuanwo

Hi @Xuanwo! I apologize for the slow response — I was on an extended leave from work.

I think this is possible. I'll look into it.

jswrenn avatar Sep 20 '23 15:09 jswrenn

Can you point me to a type in OpenDAL that implements AsyncRead where you might like to include poll_read in frames?

I think I have a solution that could work for this use-case, but I'd like to be sure.

jswrenn avatar Sep 26 '23 18:09 jswrenn

Can you point me to a type in OpenDAL that implements AsyncRead where you might like to include poll_read in frames?

Sure!

Here is the code where we added async-backtrace: https://github.com/apache/incubator-opendal/blob/main/core/src/layers/async_backtrace.rs.

Currently, we have only added framed for async functions, and we expect to have them on our Readers just like we do for logging: https://github.com/apache/incubator-opendal/blob/main/core/src/layers/logging.rs#L992-L1037.

We plan to return an AsyncBacktraceWrapper<R> and add frame for it's poll_read, poll_seek functions.

Xuanwo avatar Sep 27 '23 02:09 Xuanwo

I'm experimenting with approaches on the more-frames branch: https://github.com/tokio-rs/async-backtrace/tree/more-frames

I've started by making our Frame type public, which makes it possible to crate your own backtrace frames that aren't in the context of a Future, like so (using tokio's Take as an example):

pin_project! {
    #[must_use = "streams do nothing unless you `.await` or poll them"]
    pub struct Take<R> {
        #[pin]
        inner: R,
        // Add a `Frame` field to your type.
        #[pin]
        frame: async_backtrace::Frame,
        limit: u64,
    }
}

impl<R: AsyncRead> AsyncRead for Take<R> {
    fn poll_read(
        self: Pin<&mut Self>,
        cx: &mut Context<'_>,
        buf: &mut ReadBuf<'_>,
    ) -> Poll<Result<(), std::io::Error>> {
        // And run your async routines `in_scope` of `Frame`.
        let this = self.project();
        this.frame.in_scope(|| {
            unimplemented!("method internals go here")  
        })
    }
}

Does this work for you?

This is fine for AsyncRead (which requires that its receiver is pinned), but I see oio::Read doesn't require a pinned receiver. I'm not yet sure how to support non-pinned frames — pinning is fundamental to how this crate is able to keep track of the structure of futures without allocating.

jswrenn avatar Sep 28 '23 17:09 jswrenn

I'm experimenting with approaches on the more-frames branch: https://github.com/tokio-rs/async-backtrace/tree/more-frames

Cool! I will give it a try.

This is fine for AsyncRead (which requires that its receiver is pinned), but I see oio::Read doesn't require a pinned receiver.

oio::Read requires to be Unpin so we don't require a pinned receiver.

Xuanwo avatar Oct 07 '23 07:10 Xuanwo

Hi @jswrenn, thanks for your work! This solution works for us. However, we've migrated to an async trait design and no longer need this feature.

Xuanwo avatar May 16 '24 10:05 Xuanwo