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

Allow extending ServerContext and/or ServerRequest with additional information #2159

Open
sergiocampama opened this issue Jan 15, 2025 · 7 comments

Comments

@sergiocampama
Copy link
Contributor

I'm implementing a Vapor server where I can process grpc-web and regular API routes. While I have the entire request/response flow working (custom transport implementation specific for grpc-web), I'm missing passing in more context into the gRPC services in relation to the request itself. Vapor's Request object is fairly complete with much context needed to process any kind of request, while ServerRequest from grpc-swift doesn't have much going on other than metadata.

Would it align with this project to extend these objects so that we can pass arbitrary contexts into the requests themselves, so that they can be injected along the request path into the GRPCService itself?

@sergiocampama
Copy link
Contributor Author

Perhaps something like swift-service-context in ServerRequest and ServerContext would do the trick? Or is that a bad fit?

@glbrntt
Copy link
Collaborator

glbrntt commented Jan 15, 2025

I'm implementing a Vapor server where I can process grpc-web and regular API routes. While I have the entire request/response flow working (custom transport implementation specific for grpc-web),

That's awesome! Is this available somewhere?

I'm missing passing in more context into the gRPC services in relation to the request itself. Vapor's Request object is fairly complete with much context needed to process any kind of request, while ServerRequest from grpc-swift doesn't have much going on other than metadata.

Would it align with this project to extend these objects so that we can pass arbitrary contexts into the requests themselves, so that they can be injected along the request path into the GRPCService itself?

Possibly. I don't think we have a clear position on this yet. We'd like to hold off on adding a grab-bag style context in which you can stick anything, but we're open to the possibility of adding more things to the existing client/server contexts. What objects from Vapor's Request object would you like to see in gRPC?

We are also still figuring out where to draw the line between adding to the context vs. making things available via task locals. Part of me wonders whether task locals should entirely replace that grab-bag style of context.

@sergiocampama
Copy link
Contributor Author

That's awesome! Is this available somewhere?

For now just my private repo haha, but could clean it up a little and offer it up to OSS. I took this transport approach since it felt like the easiest way to hook in without modifying the grpc-swift sources, but ideally I would like to be able to use RPCRouter directly from a Vapor middleware, but the RPCRouter.handle method is not public. With that I wouldn't have to keep a running instance of a GRPCServer in a separate task during the life of the vapor server.

What objects from Vapor's Request object would you like to see in gRPC?

For my particular scenario, I would like to forward Vapor's Request entirely. I have another set of authentication middleware that sets the Request's Authenticatable user object (id coming in from the JWT credentials) and keeps a reference for the lifecycle of the request.

Without being able to forward the request instance to access the db and authenticated user, I'm forced to keep a reference of Vapor's app in the service implementations (to access the db from the RPC methods, along with other context relative references like logging), and to reload the user from the db for every RPC call (tracing/audit purposes of my product).

Those are the most important in my opinion, but I foresee wanting to use other things from Request that are specific to that invocation.

Again I realize this is not the intended use case of grpc-swift, but it's sooo close to being really easy to hook into Vapor.

@sergiocampama
Copy link
Contributor Author

sergiocampama commented Jan 15, 2025

Just as FYI, decided to see what changes I would need for what I want to do, and got this: sergiocampama@8f0dec9

And my implementation of the gRPC-web middleware is here: https://gist.github.com/sergiocampama/bddf5a92c56cf6204660c1263593d5ea

@glbrntt
Copy link
Collaborator

glbrntt commented Jan 16, 2025

For now just my private repo haha, but could clean it up a little and offer it up to OSS.

Fair enough, it was more out of curiosity.

I took this transport approach since it felt like the easiest way to hook in without modifying the grpc-swift sources, but ideally I would like to be able to use RPCRouter directly from a Vapor middleware, but the RPCRouter.handle method is not public. With that I wouldn't have to keep a running instance of a GRPCServer in a separate task during the life of the vapor server.

The transport is the right abstraction here (this is the kind of use case we had in mind for it). The router will change slightly in the next beta (it'll become generic over a transport) so you'll need to continue using the transport I'm afraid. (The reason for this is so that we can avoid copying between [UInt8] and whatever bag-of-bytes type the transport uses, so the transport will grow an associated bag-of-bytes type which the router will need to know about.)

Without being able to forward the request instance to access the db and authenticated user, I'm forced to keep a reference of Vapor's app in the service implementations (to access the db from the RPC methods, along with other context relative references like logging), and to reload the user from the db for every RPC call (tracing/audit purposes of my product).

Did you consider using a task local here instead?

@sergiocampama
Copy link
Contributor Author

yeah, the ergonomics of using transport are just a bit cumbersome, since it requires keeping a grpc server instance waiting, which also means having to deal with cancellation correctly (in the transport layer and the group task that keeps the server looping)... being able to pass in some bytes and having the router process that and return the result makes it much easier...

I wasn't familiar with TaskLocal, but was able to use it correctly to avoid the service context layer, thanks for that pointer!

I'll look at the next beta to figure out how I can make it easier on myself to support this frankenstein of a project

@glbrntt
Copy link
Collaborator

glbrntt commented Jan 16, 2025

The lifecycle of your transport is tied to Vapor, so that run loop for the transport is effectively "don't return until the Vapor server shuts down". I assume you can get that info from Vapor and then just emit that as an event on an AsyncStream?

Also wondering whether you can run the transport via Vapor's lifecycle mechanism? (I'm not too familiar with Vapor.)

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