[C#] FlightClient integration with Grpc.Net.ClientFactory

[robcao]

Describe the enhancement requested

Request

I would like the FlightClient to support standard integration with the Grpc.Net.ClientFactory library.

Background

It is standard in .NET web applications to manage gRPC clients using the Grpc.Net.ClientFactory, particularly when using the ASP.NET Core framework.

Currently the FlightClient does not work out of the box with the Grpc.Net.ClientFactory library. This is because the FlightClient is a thin abstraction over the raw .NET gRPC client, and does not have all of the same constructors as the raw gRPC client. Specifically, the Grpc.Net.ClientFactory expects a constructor that accepts a CallInvoker.

Steps to reproduce:

Update the Apache.Arrow.Flight.Tests package with the following library: dotnet add package Grpc.Net.ClientFactory.

Add the following test class, which attempts to instantiate a FlightClient instance using the Grpc.Net.ClientFactory library, and view the test fails with the following error.

using System;
using Apache.Arrow.Flight.Client;
using Microsoft.Extensions.DependencyInjection;
using Xunit;

namespace Apache.Arrow.Flight.Tests
{
    public class GrpcNetClientFactoryTests
    {
        [Fact]
        public void CanResolve_FlightClient_UsingGrpcClientFactory()
        {
            IServiceCollection services = new ServiceCollection();

            services.AddGrpcClient<FlightClient>(grpc => grpc.Address = new("http://localhost:50051"))
                .ConfigureChannel(channel => channel.UnsafeUseInsecureChannelCallCredentials = true);

            IServiceProvider provider = services.BuildServiceProvider();

            provider.GetRequiredService<FlightClient>();
        }

    }
}

System.InvalidOperationException
A suitable constructor for type 'Apache.Arrow.Flight.Client.FlightClient' could not be located. Ensure the type is concrete and all parameters of a public constructor are either registered as services or passed as arguments. Also ensure no extraneous arguments are provided.
   at Microsoft.Extensions.DependencyInjection.ActivatorUtilities.FindApplicableConstructor(Type instanceType, Type[] argumentTypes, ConstructorInfo& matchingConstructor, Nullable`1[]& matchingParameterMap)
   at Microsoft.Extensions.DependencyInjection.ActivatorUtilities.CreateFactoryInternal(Type instanceType, Type[] argumentTypes, ParameterExpression& provider, ParameterExpression& argumentArray, Expression& factoryExpressionBody)
   at Microsoft.Extensions.DependencyInjection.ActivatorUtilities.CreateFactory(Type instanceType, Type[] argumentTypes)
   at Grpc.Net.ClientFactory.Internal.DefaultClientActivator`1.<>c.<.cctor>b__9_0()
   at System.Threading.LazyInitializer.EnsureInitializedCore[T](T& target, Boolean& initialized, Object& syncLock, Func`1 valueFactory)
   at Grpc.Net.ClientFactory.Internal.DefaultClientActivator`1.get_Activator()
   at Grpc.Net.ClientFactory.Internal.DefaultClientActivator`1.CreateClient(CallInvoker callInvoker)
   at Grpc.Net.ClientFactory.Internal.DefaultGrpcClientFactory.CreateClient[TClient](String name)
   at Microsoft.Extensions.DependencyInjection.GrpcClientServiceExtensions.<>c__DisplayClass7_0`1.<AddGrpcHttpClient>b__0(IServiceProvider s)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSiteMain(ServiceCallSite callSite, TArgument argument)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitDisposeCache(ServiceCallSite transientCallSite, RuntimeResolverContext context)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSite(ServiceCallSite callSite, TArgument argument)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.Resolve(ServiceCallSite callSite, ServiceProviderEngineScope scope)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.DynamicServiceProviderEngine.<>c__DisplayClass2_0.<RealizeService>b__0(ServiceProviderEngineScope scope)
   at Microsoft.Extensions.DependencyInjection.ServiceProvider.GetService(ServiceIdentifier serviceIdentifier, ServiceProviderEngineScope serviceProviderEngineScope)
   at Microsoft.Extensions.DependencyInjection.ServiceProvider.GetService(Type serviceType)
   at Microsoft.Extensions.DependencyInjection.ServiceProviderServiceExtensions.GetRequiredService(IServiceProvider provider, Type serviceType)
   at Microsoft.Extensions.DependencyInjection.ServiceProviderServiceExtensions.GetRequiredService[T](IServiceProvider provider)
   at Apache.Arrow.Flight.Tests.GrpcNetClientFactoryTests.CanResolve_FlightClient_UsingGrpcClientFactory() in C:\Sourcecode\arrow\csharp\test\Apache.Arrow.Flight.Tests\GrpcNetClientFactoryTests.cs:line 24
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)

Proposed Solution

This can be achieved by adding another constructor to the FlightClient like below. I would be happy to open a pull request for this (along with test coverage) if this proposal is accepted.

public FlightClient(CallInvoker callInvoker)
{
    _client = new FlightService.FlightServiceClient(callInvoker);
}

Another solution could be to create an internal constructor that accepts an instance of FlightService.FlightServiceClient, and have a client extension library, which has more decoupling between the FlightClient and Grpc.Net.ClientFactory, but has a larger surface area. However, gRPC interceptors are attached at the CallInvoker level, so it is probably desirable to accept a constructor with a CallInvoker regardless.

Component(s)

C#

[adamreeve]

Hi @robcao, your proposed solution looks very reasonable to me. Please do make a pull request for this.

[robcao]

Thank you!

Opened a pull request here: #45458