grpc-dotnet icon indicating copy to clipboard operation
grpc-dotnet copied to clipboard

Can we set custom grpc settings?

Open AnashOommen opened this issue 1 year ago • 19 comments

I'm trying to address https://github.com/googleads/google-ads-dotnet/issues/445 and noticed that there's no support for custom grpc settings.

How do I set the the following settings?

* grpc.max_metadata_size
* grpc.http_proxy

@jtattermusch @jskeet FYI.

AnashOommen avatar Aug 10 '22 21:08 AnashOommen

I'm on holiday at the moment. I'll respond to this when I'm back.

jskeet avatar Aug 10 '22 21:08 jskeet

(There is support for custom settings via GrpcChannelOptions, but I don't know about these specific options in different implementations.)

jskeet avatar Aug 10 '22 21:08 jskeet

I thought we ignored custom options in Gax? https://github.com/googleapis/gax-dotnet/blob/main/Google.Api.Gax.Grpc/GrpcNetClientAdapter.cs#L67

AnashOommen avatar Aug 10 '22 21:08 AnashOommen

Btw, here's why we need grpc.max_metadata_size: https://github.com/googleads/google-ads-dotnet/issues/94

AnashOommen avatar Aug 10 '22 21:08 AnashOommen

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

jskeet avatar Aug 10 '22 21:08 jskeet

The proxy is customized on SocketsHttpHandler which is set on GrpcChannelOptions.HttpHandler.

I don't think SocketsHttpHandler has the equivalent of grpc.max_metadata_size. Are large headers/trailers a problem with Grpc.Net.Client?

JamesNK avatar Aug 10 '22 22:08 JamesNK

@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.

AnashOommen avatar Aug 11 '22 02:08 AnashOommen

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

JamesNK avatar Aug 11 '22 02:08 JamesNK

Exception & Stack trace

 {"Can't get the call trailers because the call has not completed successfully."} | System.InvalidOperationException


   at Grpc.Net.Client.Internal.GrpcCall`2.GetTrailers()
   at Grpc.Net.Client.Internal.HttpClientCallInvoker.Callbacks`2.<>c.<.cctor>b__4_2(Object state)
   at Grpc.Core.AsyncCallState.GetTrailers() in /var/local/git/grpc/src/csharp/Grpc.Core.Api/AsyncCallState.cs:line 81
   at Grpc.Core.AsyncUnaryCall`1.GetTrailers() in /var/local/git/grpc/src/csharp/Grpc.Core.Api/AsyncUnaryCall.cs:line 132
   at Google.Ads.GoogleAds.Logging.LoggingHandler.<HandleAsyncUnaryLogging>d__12`2.MoveNext() in C:\src\MyProjects\Common Library Base\simply-live\Google.Ads.GoogleAds.Core\src\Logging\LoggingHandler.cs:line 120
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() in /_/src/libraries/System.Private.CoreLib/src/System/Runtime/ExceptionServices/ExceptionDispatchInfo.cs:line 56
   at System.Threading.Tasks.Task.<>c.<ThrowAsync>b__140_1(Object state) in /_/src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs:line 1898
   at System.Threading.QueueUserWorkItemCallbackDefaultContext.Execute() in /_/src/libraries/System.Private.CoreLib/src/System/Threading/ThreadPool.cs:line 856
   at System.Threading.ThreadPoolWorkQueue.Dispatch() in /_/src/libraries/System.Private.CoreLib/src/System/Threading/ThreadPool.cs:line 655
   at System.Threading._ThreadPoolWaitCallback.PerformWaitCallback() in /_/src/coreclr/src/System.Private.CoreLib/src/System/Threading/ThreadPool.CoreCLR.cs:line 29

Logging

info: Grpc.Net.Client.Internal.GrpcCall[17]
      Error reading message.
      System.IO.IOException: The request was aborted.
       ---> System.Net.Http.HPack.HPackDecodingException: The HTTP headers length exceeded the set limit of 65536 bytes.         at System.Net.Http.HPack.HPackDecoder.OnStringLength(Int32 length, State nextState)
         at System.Net.Http.HPack.HPackDecoder.ParseHeaderValueLength(ReadOnlySpan`1 data, Int32& currentIndex, IHttpHeadersHandler handler)
         at System.Net.Http.HPack.HPackDecoder.ParseHeaderName(ReadOnlySpan`1 data, Int32& currentIndex, IHttpHeadersHandler handler)
         at System.Net.Http.HPack.HPackDecoder.ParseHeaderNameLength(ReadOnlySpan`1 data, Int32& currentIndex, IHttpHeadersHandler handler)
         at System.Net.Http.HPack.HPackDecoder.Parse(ReadOnlySpan`1 data, Int32& currentIndex, IHttpHeadersHandler handler)
         at System.Net.Http.HPack.HPackDecoder.DecodeInternal(ReadOnlySpan`1 data, IHttpHeadersHandler handler)
         at System.Net.Http.Http2Connection.ProcessHeadersFrame(FrameHeader frameHeader)
         at System.Net.Http.Http2Connection.ProcessIncomingFramesAsync()
         --- End of inner exception stack trace ---
         at System.Net.Http.Http2Connection.ThrowRequestAborted(Exception innerException)
         at System.Net.Http.Http2Connection.Http2Stream.CheckResponseBodyState()
         at System.Net.Http.Http2Connection.Http2Stream.TryReadFromBuffer(Span`1 buffer, Boolean partOfSyncRead)
         at System.Net.Http.Http2Connection.Http2Stream.ReadDataAsync(Memory`1 buffer, HttpResponseMessage responseMessage, CancellationToken cancellationToken)
         at Grpc.Net.Client.StreamExtensions.ReadMessageAsync[TResponse](Stream responseStream, GrpcCall call, Func`2 deserializer, String grpcEncoding, Boolean singleMessage, CancellationToken cancellationToken)
info: Grpc.Net.Client.Internal.GrpcCall[3]
      Call failed with gRPC error status. Status code: 'Unavailable', Message: 'Error starting gRPC call. IOException: The request was aborted. HPackDecodingException: The HTTP headers length exceeded the set limit of 65536 bytes.'.

AnashOommen avatar Aug 11 '22 02:08 AnashOommen

Ok. I think this can be increased by setting SocketsHttpHandler.MaxResponseHeadersLength.

JamesNK avatar Aug 11 '22 03:08 JamesNK

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

fail: Grpc.Net.Client.Internal.GrpcCall[6]
      Error starting gRPC call.
      System.Net.Http.HttpRequestException: An error occurred while sending the request.
       ---> System.IO.IOException: The request was aborted.
       ---> System.Net.Http.HttpRequestException: The HTTP response headers length exceeded the set limit of 21474836480 bytes.
         at System.Net.Http.Http2Connection.Http2Stream.OnHeader(ReadOnlySpan`1 name, ReadOnlySpan`1 value)
         at System.Net.Http.Http2Connection.Http2Stream.System.Net.Http.IHttpHeadersHandler.OnStaticIndexedHeader(Int32 index)
         at System.Net.Http.HPack.HPackDecoder.OnIndexedHeaderField(Int32 index, IHttpHeadersHandler handler)
         at System.Net.Http.HPack.HPackDecoder.Parse(ReadOnlySpan`1 data, Int32& currentIndex, IHttpHeadersHandler handler)
         at System.Net.Http.HPack.HPackDecoder.DecodeInternal(ReadOnlySpan`1 data, IHttpHeadersHandler handler)
         at System.Net.Http.Http2Connection.ProcessHeadersFrame(FrameHeader frameHeader)
         at System.Net.Http.Http2Connection.ProcessIncomingFramesAsync()
         --- End of inner exception stack trace ---
         at System.Net.Http.Http2Connection.ThrowRequestAborted(Exception innerException)
         at System.Net.Http.Http2Connection.Http2Stream.CheckResponseBodyState()
         at System.Net.Http.Http2Connection.Http2Stream.TryEnsureHeaders()
         at System.Net.Http.Http2Connection.Http2Stream.ReadResponseHeadersAsync(CancellationToken cancellationToken)
         at System.Net.Http.Http2Connection.SendAsync(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken)
         --- End of inner exception stack trace ---
         at System.Net.Http.Http2Connection.SendAsync(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken)
         at System.Net.Http.HttpConnectionPool.SendWithRetryAsync(HttpRequestMessage request, Boolean async, Boolean doRequestAuth, CancellationToken cancellationToken)
         at System.Net.Http.RedirectHandler.SendAsync(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken)
         at Grpc.Shared.TelemetryHeaderHandler.SendAsyncCore(HttpRequestMessage request, CancellationToken cancellationToken)
         at Grpc.Net.Client.Balancer.Internal.BalancerHttpHandler.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
         at Grpc.Net.Client.Internal.GrpcCall`2.RunCall(HttpRequestMessage request, Nullable`1 timeout)
info: Grpc.Net.Client.Internal.GrpcCall[3]
      Call failed with gRPC error status. Status code: 'Unavailable', Message: 'Error starting gRPC call. HttpRequestException: An error occurred while sending the request. IOException: The request was aborted. HttpRequestException: The HTTP response headers length exceeded the set limit of 21474836480 bytes.'.

I have no idea why the library is complaining about a 20GB limit.

AnashOommen avatar Aug 11 '22 14:08 AnashOommen

I think there is a 32-bit overflow inside the HTTP client.

MaxResponseHeadersLength is kilobytes so 20 megs (20 * 1024) is enough.

JamesNK avatar Aug 12 '22 14:08 JamesNK

Logged a bug with maintainers of HTTP/2 client - https://github.com/dotnet/runtime/issues/73848

Use a smaller value for now.

JamesNK avatar Aug 12 '22 14:08 JamesNK

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?

AnashOommen avatar Aug 12 '22 16:08 AnashOommen

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.

JamesNK avatar Aug 13 '22 01:08 JamesNK

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. I know there's https://docs.microsoft.com/en-us/aspnet/core/grpc/configuration?view=aspnetcore-6.0#configure-client-options, which is a good start, but I don't think we document mapping from gRPC channel arg names (e.g. grpc.max_receive_message_length to the GrpcChannelOptions field names - in this case it's MaxReceiveMessageSize). If we had such mapping, it would be clear which arguments are supported / unsupported / partially supported (and we could document the workarounds). I don't think we want to add every possible field to GrpcChannelOptions class, but if there isn't a corresponding field there, we could documents alternative ways of setting it.

@JamesNK WDYT?

jtattermusch avatar Aug 18 '22 13:08 jtattermusch

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 avatar Aug 18 '22 13:08 JamesNK

@JamesNK The reason why I asked to expose grpc.max_metadata_size and grpc.http_proxy on GrpcChannelOptions is because those are well-known gRPC channel settings and in my mind, the ideal place to expose those settings would be in GrpcChannelOptions.

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 GrpcChannelOptions instead. Even the Gax implementation isn't there yet in that sense.

The goal is definitely not to expose every setting on SocketHttpHandler on GrpcChannelOptions.

Thanks @jtattermusch for linking the correct documentation pages.

AnashOommen avatar Aug 18 '22 14:08 AnashOommen

Triage decision:

  • Until we have more customer feedback that the current way to configure these HttpClientHandler/SocketsHttpHandler options is too cumbersome, we don't think we should add API.

halter73 avatar Aug 23 '22 22:08 halter73