Allow running handler with a callback

[purkhusid]

What?

This is a pretty naive approach that allows the end user to supply a callback that will be invoked once a single event loop has been finished.

Why?

Because currently there is no good way(that I know of) to flush logs/traces/metrics. By having a callback that is called after all event handling is finished allows the end user to do a cleanup at the end of each lambda invocation.

See #691 for further context

[greenwoodcm]

Thanks for the submission! In general I like this change, I think this is functionality we want to support. The current approach feels a bit disjointed - the callback is passed as an Arc<impl Fn()> and exists outside of the "awareness" of the Service impl the developer provides. I'd like to find a way to make the callback more of a first-class construct down at that level of the type system. the goal here for me is allowing the developer to reason over the function handler and the cleanup callback closer together in code. A few alternatives I can think of:

1. right now the Service impl must return back a response of type B: Serialize, which is wired up as Service::Future: Future<Output = Result<B, F::Error>>. we could make the response type optionally be a wrapper type that includes both the response payload (still of type Serialize) and additionally includes a callback Fn(). in that model the function handler is responsible for returning the callback on every invoke response, the contract being "here's the response, and here's the way to call me back to clean up after you've responded".
2. there's the concept of middleware and layering in tower. this is the idea that you can layer one service on top of another. the example they give in the docs is to build a timeout service that wraps another service and enforces a timeout. we could make the callback infrastructure essentially middleware around the existing Service impl. then what I would do is construct a Service using service_fn, then wrap it in a callback_fn that returns it's wrapped Service. something like that.

thoughts? my initial impression is that i like option two better.

[greenwoodcm]

can you do the following to avoid the clone?

if let Some(ref callback) = callback {
    ...
}

[calavera]

This looks reasonable, but I'd like to have some time to see if we can find a solution that doesn't involve adding an extra function. Since functions gets translated into Tower services, there might be an opportunity to design this as another Tower integration that doesn't require any extra functionality in the runtime.

[BMorinDrifter]

See MetricsService in https://github.com/BMorinDrifter/metrics-cloudwatch-embedded/blob/main/src/lambda.rs for how I did this for the embedded metrics crate.

That said, working with Tower ended up being quite a chore between figuring out how to write MetricsServiceFuture and dealing with metaprogramming because the tower service is coupled to a response that's either a thing for serde to serialize or an http response depending on if you're using lambda_http or not.

[BMorinDrifter]

2. there's the concept of middleware and layering in tower. this is the idea that you can layer one service on top of another.

Given that the current interface makes everyone make a call to build a tower service, it makes sense to solve this problem within the framework of Tower. The problem I see is the lack of examples and perhaps Tower being awkward to work with given the amount of metaprogramming (maybe this gets better when Async traits hit stable???.) Getting a where clause that worked was a lot of trial and error because I don't have a deep background in Rust language details.

[greenwoodcm]

Yeah I started prototyping what I had in mind, and one of the annoying things is that you can't build Tower middleware with standard async/await semantics. so we'd probably end up doing our own Future stuff like you did, @BMorinDrifter . i don't think we have the response type problem you refer to, as I think where I'd take this is the Service that run uses has a response type of (). that service internally would be responsible not just for invoking the handler, but also for posting the response back to the runtime API. it would essentially terminate the response type itself because the response isn't needed after it's serialized and sent. this would simplify the type signature and the body of Runtime::run and would leave the door open for something to wrap that new Service in a middleware that does callbacks.

[BMorinDrifter]

Yeah I started prototyping what I had in mind, and one of the annoying things is that you can't build Tower middleware with standard async/await semantics. so we'd probably end up doing our own Future stuff like you did, @BMorinDrifter .

My bad, I had that backwards. Yeah the hook after the lambda function is what required writing my own Future. So yeah, doing this right now does require quite a bit of code. There might be a an easier way with tower::ServiceBuilder however I was unable to get that to work at the library scope.

[calavera]

I don't think this is the direction that we want to take. We probably need to think a little bit more about how to layer this with Tower.

I'm going to close this PR. Thanks a lot for the contribution. I hope we can make some improvements in this area in the future.