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

FR: disassociate target and client options #153

Closed
rauanmayemir opened this issue Jan 11, 2025 · 5 comments
Closed

FR: disassociate target and client options #153

rauanmayemir opened this issue Jan 11, 2025 · 5 comments

Comments

@rauanmayemir
Copy link

Currently, I can only specify which protocols my grpc server supports, but can't specify which protocols I want to support for the downstream (i.e clients).

If we assume that targets are usually only gRPC, then that leaves us with no way to specify client options and a lot of options for target which aren't really used.

From the source I see that serviceOptions doesn't have things like codecsMap or compressorsMap, and it's unclear how to specify options so that we can do this:

  1. Receive JSON request, decompress if needed
  2. Send request to upstream gRPC decompressed
  3. Collect uncompressed upstream response
  4. Compress payload for the downstream and return (add content-encoding headers)
@jhump
Copy link
Member

jhump commented Jan 11, 2025

The TranscoderOption factories -- like WithCodec and WithCompression -- are how you define the client-side support. There is no need to define which protocols are supported because they are all supported: the vanguard-go handler can accommodate clients using any of them: Connect, gRPC, gRPC-Web, or REST.

The ServiceOption factories are for configuring what protocols, codecs, and compressions the backend can actually handle, so that the vanguard-go handler can transform requests as necessary -- like if the client is using a protocol, codec, or compression that the backend doesn't understand.

The bullet list you mention is almost exactly what the vanguard-go handler does. For example, if you use the WithNoTargetCompression option (or use WithTargetCompression(...) but send a request that uses compression that is not in that target list), the handler will decompress the data and send the data, uncompressed, to the server.

Right now, I don't think it tries to compress responses if the backend doesn't compress them. Forcing compressed responses could be an option that we add (likely a transcoder option).

@rauanmayemir
Copy link
Author

Thank you. Indeed, I tried to specify WithCompression, but it didn't apply compression to uncompressed backend responses.

I understand the logic for e.g grpc client -> grpc server, but for connect/web/rest clien -> grpc server, unless it's a remote proxy, it doesn't make sense to compress the response just to uncompress and recompress it again in the same request lifecycle.

@rauanmayemir
Copy link
Author

There is no need to define which protocols are supported because they are all supported

I realized I was doing unnecessary work and it is indeed easier to just transparently and automagically support all of the protocols.

@rauanmayemir
Copy link
Author

I don’t think this issue is fixed.

I tried applying compression outside of transcoder as a middleware, but it wouldn’t work because transfer chunking has to happen after the response is compressed (it appears transcoder initiates the chunking).

@jhump
Copy link
Member

jhump commented Jan 21, 2025

I think you're describing something different from separating target and client options (which is the subject of this issue). I think you're talking about an enhancement to force compression on responses, even when the origin server has not provided a compressed response? If that's right, please open a new issue for that.

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

3 participants