Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Idea: rethink/rework #[instrument] codegen #1874

Open
Swatinem opened this issue Jan 29, 2022 · 1 comment
Open

Idea: rethink/rework #[instrument] codegen #1874

Swatinem opened this issue Jan 29, 2022 · 1 comment

Comments

@Swatinem
Copy link
Contributor

Feature Request

Crates

tracing-attributes

Motivation

Primarily, the codegen right now does not correctly support hybrid-async functions (fn() -> impl Future that has both sync code, and returns an async block) #1863
The codegen also generates weird code in certain situations and can trigger some lints, for example #1831
It also does not support more than one return position for fn() -> Box<dyn Future>, as in #1840

We have a couple of patterns the codegen should cover, and apart from that there is optional support to log the return value which we have to cater for.

As for the input code patterns:

fn normal_sync_fn() -> Result<A, B> {
    if whatever()? {
        return Ok(A);
    }
    Err(B)
}

async fn normal_async_fn() -> Result<A, B> {
    if whatever().await? {
        return Ok(A);
    }
    Err(B)
}

fn manual_async_fn() -> impl Future<Output = Result<A, B>> {
    async move {
        if whatever().await? {
            return Ok(A);
        }
        Err(B)
    }
}

fn manual_boxed_async_block() -> Box<dyn Future<Output = Result<A, B>>> {
    Box::pin(async move {
        if whatever().await? {
            return Ok(A);
        }
        Err(B)
    })
}

fn manual_boxed_async_inner() -> Box<dyn Future<Output = Result<A, B>>> {
    async fn inner() -> Result<A, B> {
        if whatever().await? {
            return Ok(A);
        }
        Err(B)
    }
    Box::pin(inner())
}


fn manual_hybrid_fn() -> impl Future<Output = Result<A, B>> {
    let val = whatever();
    async move {
        if val.await? {
            return Ok(A);
        }
        Err(B)
    }
}

fn manual_hybrid_boxed_fn() -> Box<dyn Future<Output = Result<A, B>>> {
    let val = whatever();
    Box::pin(async move {
        if val.await? {
            return Ok(A);
        }
        Err(B)
    })
}

fn unbalanced_boxed_fn() -> Box<dyn Future<Output = Result<A, B>>> {
    if whatever() {
        return Box::pin(futureA());
    }
    Box::pin(futureB())
}

And then there is also the case that returns a named future, but I don’t think we can do anything in those, because its

  1. impossible to wrap them in Instrumented
  2. possibly impossible to detect without type checking that the return value is actually an impl Future.

Nice to haves

  • Generate code that is possible to completely optimize out when compile-time level are defined
  • Ideally avoid creating enter-guards when the span is disabled
  • Avoid excessive async block wrapping when possible

Proposal

No concrete fleshed out proposal yet, just some brainstorming ;-)

Especially the unbalanced_boxed_fn case, and the cases involving return-value inspection are causing me headaches right now.

@Alfred-Mountfield
Copy link

Reworking #[instrument] could also possibly address #1841

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants