Can not use TraceLayer from tower_http crate

[yotaro-shimose]

Hi.
I'm enjoying this library to create http server using lambda with rust's low memory consumption and speed.
I'm new to rust so this might be stupid question.

I want to log any (successful or failed) response in cloud watch using layer like we can in actix_web just by adding a line of code like

App::new()
    .wrap(Logger::default())
    .route("/path, web::get().to(my_handler))

So I tried

use lambda_http::{self, tower::ServiceBuilder, Error};
use lambda_http::{Request, Response};
use tower_http::trace::TraceLayer;

#[tokio::main]
async fn main() -> Result<(), Error> {
    std::env::set_var("RUST_LOG", "info");
    env_logger::init();
    let service = ServiceBuilder::new()
        .layer(TraceLayer::new_for_http())
        .service_fn(handler);
    lambda_http::run(service).await?;
    Ok(())
}

async fn handler(req: Request) -> Result<Response<String>, Error> {
    Ok(Response::new("Success".to_string()))
}

but this code does not compile.

The compiler says

the trait bound `lambda_http::Body: std::convert::From<tower_http::trace::ResponseBody<std::string::String, tower_http::classify::NeverClassifyEos<tower_http::classify::ServerErrorsFailureClass>, tower_http::trace::DefaultOnBodyChunk, tower_http::trace::DefaultOnEos, tower_http::trace::DefaultOnFailure>>` is not satisfied
the following implementations were found:
  <lambda_http::Body as std::convert::From<&'a [u8]>>
  <lambda_http::Body as std::convert::From<&'a str>>
  <lambda_http::Body as std::convert::From<()>>
  <lambda_http::Body as std::convert::From<std::borrow::Cow<'static, [u8]>>>
and 3 others
required because of the requirements on the impl of `std::convert::Into<lambda_http::Body>` for `tower_http::trace::ResponseBody<std::string::String, tower_http::classify::NeverClassifyEos<tower_http::classify::ServerErrorsFailureClass>, tower_http::trace::DefaultOnBodyChunk, tower_http::trace::DefaultOnEos, tower_http::trace::DefaultOnFailure>`
required because of the requirements on the impl of `lambda_http::IntoResponse` for `lambda_http::Response<tower_http::trace::ResponseBody<std::string::String, tower_http::classify::NeverClassifyEos<tower_http::classify::ServerErrorsFailureClass>, tower_http::trace::DefaultOnBodyChunk, tower_http::trace::DefaultOnEos, tower_http::trace::DefaultOnFailure>>`

I'm not confident but it seems the Trace service's call method's return type is not compatible with lambda_http::run;
It doesn't make sense to me because the reason we replaced handler_fn with service_fn is to reutilize existing tower services and layers.

So question is

1. Is there any way to utilize existing logging response layer in lambda_http?
2. Any workaround to accomplish logging response? I saw this workaround and i liked it. But I think there might be cleaner and more abstract implementation like tower's Layer.
   Any idea or suggestion is really appriciated.

[calavera]

We need to modify the Body enum that we use for responses to implement this trait:

std::convert::From<tower_http::trace::ResponseBody<std::string::String, tower_http::classify::NeverClassifyEos<tower_http::classify::ServerErrorsFailureClass>, tower_http::trace::DefaultOnBodyChunk, tower_http::trace::DefaultOnEos, tower_http::trace::DefaultOnFailure>>

The source is here: https://github.com/LegNeato/aws-lambda-events/blob/master/aws_lambda_events%2Fsrc%2Fencodings.rs#L177

[nmoutschen]

tower_http::trace::ResponseBody implements http_body::Body, so we could solve this issue in a more generic way. I see two options for this:

1. implement From<http_body::Body> for aws_lambda_events::encodings::Body and ensure that lambda_http::IntoResponse would work there.
2. implement http_body::Body for aws_lambda_events::encodings::Body and accept any http_body::Body.

The 2nd solution is probably neater, but would make it hard to know if a payload is the Text or Binary variant.

[nmoutschen]

aws_lambda_events::encodings::Body already implements http_body::Body (see the code here).

We could transform lambda_http::run to accept any Request<http_body::Body>, but there are a few gotchas we need to think about:

1. Because aws_lambda_events::encodings::Body implements http_body::Body, we cannot implement our own From<http_body::Body> because we'd conflict with the From<T> for T implementation.
2. Because we want to send back a serialized LambdaResponse, we still need to convert it into an aws_lambda_events::encodings::Body at the end of the day.
3. If we accept any http_body::Body, it would be hard to know which variant to use in the final representation.
4. We need to read from http_body::Body asynchronously. This breaks the current assumptions that the body would always be synchronous.

PS: this is waayy too many Body to be readable 😆

[calavera]

I wonder if it'd be easier if we offered our own trace layer that behaves like the one in tower-http, but with our semantics.

[nmoutschen]

To update here, this would work by changing what we accept as a handler response (see #466), and potentially more layers in the tower ecosystem.

[calavera]

I'd be onboard with being more compatible with the ecosystem in general, even if it means changing some basic behaviors. I doubt anyone use many "OK".into() as response messages in real applications.

[calavera]

This has been fixed in #491. I will release a new version of lambda_http this week, but until then, you can use the main branch from the repo.