More idiomatic Play integration

[ctoomey]

I lead a team of long-time Play Scala developers and we're about to start using grpc instead of REST for new APIs, as I've been discussing in #179.

Currently the auto-generated akka-grpc Play handler that wraps the grpc service method implementations returns a Future[HttpResponse] rather than an idiomatic Play EssentialAction. This means among other things that Play Filters don't get called on each request as they do for REST calls, and so cross-cutting Play functionality like access logging provided by filters doesn't work.

To gain back such functionality for grpc requests, developers would need to not only port their Play filters to some new home-grown filter-like interface (since a replacement isn't provided for Play filters by akka-grpc), but also design and implement a home-grown filter chain interface.

At the very least akka-grpc should provide a Play integration that supports the existing Play idioms in addition to one like the current that doesn't.

I've taken the existing auto-generated code and akka-grpc code base and modified both so as to have grpc service implementations wrapped to return EssentialActions and verified that our filters work as expected. I'd love to contribute these changes back either as a replacement for or additional option for Play users. What do you say?

[johanandren]

Sounds like it makes sense, please do PR so we can look at and discuss the details!

[ignasi35]

There are several possible approaches:

1. add a capability in akka-grpc for request filtering
   1. make it low-level (more familiar to akka-http users). Using Directives (as hinted in Server-side power API #179)
   2. make it high-level (more familiar to play users). In this case, I think we should still make it play-agnostic and part of the akka-grpc library. Maybe refactoring PlayRouter to inject akka-grpc filters?
2. improve Play's filtering code so all requests are filtered (not just EssentialAction's)

---

As discussed internally, when adding a filter on some akka-grpc router in a Play Application there would be some limited features:

• the filter MAY block or pass the request
• the filter MAY NOT alter the request (any modification will be ignored). The reason for this limitation is that the generated code in Akka-gRPC ignores the play request and uses the raw akka-http request.

---

The final concern, from a Play user point of view, is how would filters be enabled/injected. I haven't given any thought.

[ignasi35]

@ctoomey looking forward to your code.

[marcospereira]

Hey @ctoomey, thanks for starting this conversation.

I think it is not only about the filters, but all the other parts involved in how Play handles a request. There are filters involved, but also error handlers, request handlers and action composition. Having the code based on EssentialAction (or maybe even Action) will integrate grpc support with all these things.

Regarding what @ignasi35 said:

the filter MAY NOT alter the request (any modification will be ignored). The reason for this limitation is that the generated code in Akka-gRPC ignores the play request and uses the raw akka-http request.

This may result in an surprising API since in Play filters can do something like next(request.withHeader("My-Header", "Value"). What is the reason to not alter the request at all?

---

Disclaimer: I need to catch up with the discussion on #179.

[raboof]

I gave this some more thought. To answer @marcospereira's last question: I think in the current implementation altering the Play request in a Play filter will not have any effect, because the AkkaHttpHandler passes the akka-http HttpRequest directly to the handler, ignoring the taggedRequestHeader and requestBodySource in this case (https://github.com/playframework/playframework/blob/7f7ee5cdb53cc879b741a474148850458d348520/framework/src/play-akka-http-server/src/main/scala/play/core/server/AkkaHttpServer.scala#L300). I'm not completely familiar with the way Play works, but it looks like the taggedRequestHeader would contain the values possibly updated by filters?

So far we chose not to do a 'roundtrip' (start with Akka HTTP model, convert to Play model, allow filters to make changes to Play model, convert Play model back to Akka HTTP model, pass Akka HTTP model to route), partly because eventually the Akka HTTP model may be extended so that it can replace the current Play model entirely (akka/akka-http#1500) which would harmonize all this.

I see roughly 4 approaches here:

• Drop AkkaHttpHandler and write this code in terms of Play Actions instead. The disadvantage is that this forces us to always do the 'roundtrip' both for the request and for the response.
• Instead of writing a Play filter, write the equivalent code in terms of the Akka HTTP model and provide a way to put this around the (in the akka-grpc case generated) Akka HTTP code. This avoids the 'roundtrip'. For convenience we could provide an adapter to use a Play filter here after all, at the cost of doing the roundtrip in that case.
• Keep writing Play filters, and carrying over (at least some of) the values from the possibly-adapted Play model to the Akka HTTP request before passing it to the AkkaHttpHandler.
• Avoid implementing this feature until we can use the Akka HTTP model for everything and don't need a separate Play model at all anymore.

I guess we can't really get away with option '4' ;). I think we'd like to avoid always having to do the 'roundtrip' both for the request and the response, disqualifying option '1'.

This leaves option '2' and '3': '3' is the easiest to do right now, but I wonder if '2' perhaps does more to pave the way for future harmonization. WDYT?

[ctoomey]

Thanks all for weighing in on this. What I'd prototyped and planned on implementing was @raboof's option 1, since my objective was to be able to use existing Play filters and I knew how to make this work. There is conversion round-tripping involved, but that's the case for non-grpc Play requests too and we need a working solution sooner rather than later.

Option 2 would be ideal as it'd provide filtering for akka-grpc non-Play users as well as Play users, but it's beyond the scope of what we need and can afford to spend working on.

I'm not sure how option 3 would even work, since Play filters are defined in terms of EssentialActions

So I'm going to pursue option 1, and will look at giving users a choice between it and the existing Play integration as we're doing for the server power API in #179.

[marcospereira]

Thanks for further explaining things, @raboof.

I'm not completely familiar with the way Play works, but it looks like the taggedRequestHeader would contain the values possibly updated by filters?

It will. Here is the relevant code:

https://github.com/playframework/playframework/blob/da68fc546da305b8fc215d0b8ad53b644b9b843c/framework/src/play/src/main/scala/play/api/http/HttpRequestHandler.scala#L185-L199

So far we chose not to do a 'roundtrip' (start with Akka HTTP model, convert to Play model, allow filters to make changes to Play model, convert Play model back to Akka HTTP model, pass Akka HTTP model to route), partly because eventually the Akka HTTP model may be extended so that it can replace the current Play model entirely (akka/akka-http#1500) which would harmonize all this.

Just to set expectations appropriately, that this is something that we plan to do in Play 3 and won't be part of 2.7.

I guess we can't really get away with option '4' ;). I think we'd like to avoid always having to do the 'roundtrip' both for the request and the response, disqualifying option '1'.

While I agree that ideally, we should not do the roundtrip and instead harmonize Play and Akka HTTP models, we are already doing the roundtrip for every request. So in the current state of Play, it looks like it would make sense to do it for gRPC calls too. But I also like option 2, with the observation that this will work for filters, but not action composition (if we also want to support it). Option 1 would cover action composition too.

[raboof]

AFAIU, the roundtrip Play does for every request is: "Akka HTTP Request -> Play request -> Play response -> Akka HTTP Response". That is indeed inevitable and OK. However, if we would go for "option 1", it would be "Akka HTTP Request -> Play request -> Akka HTTP Request -> Akka HTTP Response -> Play Response -> Akka Http Response".

Of course @ctoomey might not want to have to wait until Play 3 :), and indeed perhaps we can find a way to do "option 1" without introducing too much churn as an intermediate solution for Play 2.x.

[ctoomey]

See PR #447 .

[raboof]

Super happy to have this in - thanks @ctoomey!!