Skip to content

Replace lambda_runtime::Handler with tower::Service? #374

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

Closed
nmoutschen opened this issue Nov 26, 2021 · 6 comments · Fixed by #401
Closed

Replace lambda_runtime::Handler with tower::Service? #374

nmoutschen opened this issue Nov 26, 2021 · 6 comments · Fixed by #401

Comments

@nmoutschen
Copy link
Contributor

Hey folks!

After looking at how to wrap lambda_runtime::Handler compared to tower's layers (thanks to @davidbarsky), and @Mdrbhatti's recent PR on making self mutable for Handler, I'm wondering if we wouldn't benefit from using tower::Service instead.

The signatures today are pretty close already, with the two differences between the addition of a Response type and poll_ready method.

Why should we do this?

We would then benefit from being able to reuse existing (and well-tested) libraries on that side. For me, I'd like to implement a few tools to help with Rust functions on Lambda similar to what we've done for Python and Java, such as creating structured logs, handling tracing, etc. Being able to use layers to wrap the function itself would be quite helpful there.

It could make some of the existing tools such as warp_lambda or poem-lambda simpler too.

Why shouldn't we do this?

I could write my own implementation of layers for my use case. However, before committing to a specific implementation, I want to double-check we're not missing on something by not doing this.

This is also a pretty significant change for the libraries that depends on lambda-runtime or lambda-http. However, We could minimize the impact for most users of this runtime since they usually interact only with lambda_runtime::run, lambda_runtime::handler_fn, or lambda_http::handler. We could keep the same function names but change their signatures.

What would the experience look like?

The least impactful change would be to implement tower::Service for HandlerFn, and make run accept a service.

We'll probably need to create a new type that would handle both the event payload and context at the same time, as Handler.call take two arguments while tower::Service.call only takes one.

If people are interested in this, I'm happy to create a proof of concept.

@davidbarsky
Copy link
Contributor

While I no longer work for Amazon, I'd like to voice my support for switching to tower::Service in this runtime. I think there are a few benefits:

  1. Hyper and the AWS SDK for Rust already use tower:Service; this isn't adding a new dependency.
  2. This runtime would benefit from the existing Tower ecosystem.
  3. tower::make::Shared and tower::service_fn can and should be reexported by this runtime. It should also provide similar ergonomics to what is availible today.

@brainstorm
Copy link

Yes please! Is it more lightweight than bringing in opentelemetry, btw?

@nmoutschen
Copy link
Contributor Author

@brainstorm this change is not directly related to tracing/observability, but it makes it easier to do so in the future.

If I were to compare the features available in Lambda Powertools for Python, using tower::Service and the wider tower capabilities will make it easier to:

These are all possible today, but with an interface that matches other libraries, we could speed up development and/or reuse existing ones.

@nmoutschen
Copy link
Contributor Author

nmoutschen commented Nov 27, 2021

I've made a quick POC to showcase what the developer experience would look like. For anyone not interacting with HandlerFn/Handler directly, this shouldn't change anything.

If you want to try for your own code, you can do so by making the following changes in your Cargo.toml file:

# For lambda_http
lambda_http = { git = "https://github.com/nmoutschen/aws-lambda-rust-runtime", package = "lambda_http", branch = "tower-service" }
# For lambda_runtime
lambda_runtime = { git = "https://github.com/nmoutschen/aws-lambda-rust-runtime", package = "lambda_runtime", branch = "tower-service" }

Please note that this is just a POC, so don't build anything that would depend on that in the long term.

@rimutaka
Copy link
Contributor

rimutaka commented Dec 4, 2021

I built both, the current and the proposed versions using basic.rs for 'main' and got identical executable size of 1,717,768 bytes, which is strange. Rechecked a few times. Build params: stable-x86_64-unknown-linux-gnu, rustc 1.56.1, cargo build --release followed by strip. I expected at least a few bytes difference. FYI.

@nmoutschen
Copy link
Contributor Author

nmoutschen commented Dec 4, 2021

@rimutaka that's interesting, but sounds plausible.

The Handler trait already looks very close to the Service trait. The only main differences are:

  1. Handler takes a generic B parameter as output, while Service uses a placeholder type instead; and
  2. The PR uses a call function that takes a LambdaRequest as input, while Handler takes a value and event.

I wouldn't be surprised if the compiler interprets them both as the same thing. Let me check if I can reproduce that on my side.


UPDATE: after a quick check, I do get different versions.

I used cross build -p lambda_runtime --example basic --target aarch64-unknown-linux-gnu --release followed by aarch64-unknown-linux-strip for the PR branch, the master branch, and the release tag. Here are the values for du and sha256sum:

1584    basic-master
1552    basic-release
1568    basic-tower
6785767df536a2bdc3c07dc1db388739b8cc620b7c274ebfccfaaa646a648bd9  basic-master
c1be583bec7a75b4ad5347b8fbc45a71082a0d2934c09852ab6ba1b60fcacf0e  basic-release
29b968ab15cac5affafe8ce1193982ea187749ebdce046acfd1644d0613787df  basic-tower

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

Successfully merging a pull request may close this issue.

4 participants