-
Notifications
You must be signed in to change notification settings - Fork 781
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
Can we set custom grpc settings? #1842
Comments
I'm on holiday at the moment. I'll respond to this when I'm back. |
(There is support for custom settings via GrpcChannelOptions, but I don't know about these specific options in different implementations.) |
I thought we ignored custom options in Gax? https://github.com/googleapis/gax-dotnet/blob/main/Google.Api.Gax.Grpc/GrpcNetClientAdapter.cs#L67 |
Btw, here's why we need |
Yes, quite possibly. I'm on holiday, so haven't actually checked. The proxy is normally set in other ways - I don't know if Grpc.Net has support for maximum metadata size |
The proxy is customized on SocketsHttpHandler which is set on GrpcChannelOptions.HttpHandler. I don't think SocketsHttpHandler has the equivalent of |
@JamesNK yes, it is. I can replicate this at my end by uploading ~200 keywords with policy errors in Google Ads API. If you can reach out to me at [email protected], I'd be happy to share some source code to replicate the issue. This requires credentials to make call to the Google Ads API server, so I can't post it here. |
What is the exception? Can you add the message + stack trace to this issue? gRPC removes some of the exception detail. To get the original error you might need to configure logging: https://docs.microsoft.com/en-us/aspnet/core/grpc/diagnostics?view=aspnetcore-6.0#grpc-client-logging |
Exception & Stack trace
Logging
|
Ok. I think this can be increased by setting |
Out of curiosity, I tried this out: GrpcChannelOptions options = new GrpcChannelOptions();
options.MaxReceiveMessageSize = 256 * 1024 * 1204;
options.Credentials = credentials;
ILoggerFactory factory = LoggerFactory.Create(builder =>builder.AddConsole());
options.LoggerFactory = factory;
#if NET5_0_OR_GREATER
options.HttpHandler = new SocketsHttpHandler()
{
MaxResponseHeadersLength = 20 * 1024 * 1024
};
#endif
return GrpcChannel.ForAddress(uri, options); And I get this log
I have no idea why the library is complaining about a 20GB limit. |
I think there is a 32-bit overflow inside the HTTP client. MaxResponseHeadersLength is kilobytes so 20 megs (20 * 1024) is enough. |
Logged a bug with maintainers of HTTP/2 client - dotnet/runtime#73848 Use a smaller value for now. |
Thanks @JamesNK. I wonder, is there a chance to expose this in GrpcChannelOptions instead of doing this on GrpcChannelOptions::HttpHandler? Seems there's some logic being done here to create different types of handlers. I'd prefer if I don't have to replicate all that logic in my code just to be able to set a few custom settings. Any other alternatives if this isn't an option? |
There are too many settings on SocketsHttpHandler to expose them all. And settings don't apply to handler instances. e.g. the handler in WASM either doesn't have MaxResponseHeadersLength or Proxy, or will ignore them if set. |
We should probably document better what are the Grpc.Net.Client alternatives for well-known channel arguments (e.g. see https://github.com/grpc/grpc/blob/f268659bf183a39857064593b141f313845282d9/include/grpc/impl/codegen/grpc_types.h) when configuring the client/server. @JamesNK WDYT? |
Adding a mapping between channel arg names and settings on HttpClientHandler/SocketsHttpHandler makes sense. It could be added here: https://docs.microsoft.com/en-us/aspnet/core/grpc/migration?view=aspnetcore-6.0 |
@JamesNK The reason why I asked to expose Of course, there are situations where those settings are not supported at all / there are alternate ways to achieve the same effect, and the documentation will go a long way towards achieving that goal. Then we can have discussions around whether the alternative workaround is too implementation-specific (and a burden on end user) and whether it should be promoted to The goal is definitely not to expose every setting on Thanks @jtattermusch for linking the correct documentation pages. |
Triage decision:
|
Hey all, I'm running into a problem using a local proxy (ProxyMan) that I think may be related to this. According to the result of this issue, replacing the handler in the channel options should resolve the problem, but I cannot find a way to specify that using Any suggestions or workarounds? |
@dkapellusch: See https://github.com/googleapis/google-cloud-dotnet/blob/main/issues/Issue12318/Workaround/Program.cs for an example of replacing the handler when working with GrpcNetClientAdapter. |
Thank you @jskeet! internal static void ModifyGrpcChannelOptions(Grpc.Net.Client.GrpcChannelOptions options)
{
options.HttpHandler = new LocalBypassProxyHandler(new SocketsHttpHandler
{
EnableMultipleHttp2Connections = true,
Proxy = null,
UseProxy = false,
});
} |
I'm trying to address googleads/google-ads-dotnet#445 and noticed that there's no support for custom grpc settings.
How do I set the the following settings?
@jtattermusch @jskeet FYI.
The text was updated successfully, but these errors were encountered: