dapr / dotnet-sdk

Dapr SDK for .NET
Apache License 2.0
1.11k stars 335 forks source link

Dapr Client, serialization and multiple clients in dependency injection #654

Open modabas opened 3 years ago

modabas commented 3 years ago

I have been dabbling with dapr lately and I am amazed how dapr makes inter service integration much easier in a micro service based architecture. I would like to ask a few questions about future plans of dapr/dotnet sdk.

1.Serialization/Deserialization Currently, to communicate with a microservice behind dapr, dapr client in sdk is using Json serialization for some methods (InvokeMethod, PublishEvent) and protobuf serialization for others (InvokeMethodGrpc) without an option to use custom serializers. Are there any plans for dapr client in dapr/dotnet-sdk about exposing methods to implement and use custom serializers in the future that accepts and returns most primitive data types; byte[], span\<byte> or readonlyspan\<byte>?

2.Multiple dapr clients in dependency injection In current design, serialization and deserialization is bound to dapr client before client build operation. This includes json serialization options like camelCase in current iteration. There may be a need for multiple dapr clients to communicate with different microservices behind dapr if they are designed to use/accept different serialization options. AddDaprClient() extension method in Dapr.AspNetCore package doesn't allow multiple dapr clients to be added to dependency injection. Even if we don't use this extension method and add multiple dapr clients with IServiceCollection.AddSingleton, I couldn't find an easy way to get desired client from IEnumerable\<DaprClient> collection that a class gets from dependency injection. Ability to name clients during dependency injection registry (or some other method to easily distinguish them) would be very useful. Are there any plans to add functionality for supporting multiple dapr clients in dependency injection?

Again, I think dapr is an amazing project with so much potential. Thanks for all the hard work you put into it.

Regards,

rynowak commented 3 years ago

Hi @modabas - thanks for your questions

Are there any plans for dapr client in dapr/dotnet-sdk about exposing methods to implement and use custom serializers in the future that accepts and returns most primitive data types; byte[], span or readonlyspan?

There are no current plans to do this, but that doesn't mean that we never will. Ideally I'd really like to avoid introducing a serializer abstraction, and just make it clear what the methods do. If you have a need to use something other than JSON or protobuf today you should be able to build this as extension methods really easily.

If there's enough demand I'm open to hear proposals, but no one has brought forward anything concrete yet.

here may be a need for multiple dapr clients to communicate with different microservices behind dapr if they are designed to use/accept different serialization options. AddDaprClient() extension method in Dapr.AspNetCore package doesn't allow multiple dapr clients to be added to dependency injection.

This is a limitation of Microsoft.Extensions.DependencyInjection which we can't easily supplement from inside the Dapr .NET SDK. There's a pattern for this (named options) but it's esoteric and little-known. I don't want the solution to be more confusing than the problem.

This is also an example of why I don't want to introduce a replaceable serializer abstraction. If 1 DaprClient -> 1 Serializer then you need to manage different clients - if the choice of format is determined by the methods you call, then you don't need to manage multiple serializers.

In practice I think HttpContent/HttpRequestMessage/HttpResponseMessage has a great solution to these issues. You don't need a JsonClient or XmlClient, you just call the method you want.


If you have a concrete scenario that you're working on and want to give me some info then I'm happy to provide some advice and see what we can figure out.

modabas commented 3 years ago

Thanks for your reply @rynowak,

It just seemed natural to be able to change serialization method, because sooner or later one will encounter in a scenario to integrate with a legacy service. For example; that service expects to receive some different format (xml, protobuf or something else) from a message broker.

Even if all services in infrastructure is developed and maintained in-house, with dapr integration in mind; personally, if message doesn't have to be human readable "in transit" (on network or message broker), I prefer to use protobuf that makes integration easier (message class generation with .proto files), has smaller size, versioning support, etc.

Upon further reading, I came across a similar discussion (dapr/dapr#2905) under dapr project issues. It has broader coverage because it approaches serialization not only from dapr/dotnet-sdk perspective but from other language sdks as well. I think what I asked about is similar to this: "Every SDK must expose 'raw' functionality for each building block that lets users send/consume any format (bytes) " which is proposed on that issue.

If anyone is interested, About "injecting multiple clients and getting desired client" I asked about in first post, I implemented this workaround: (**Disclaimer: I didn't test thoroughly if injecting multiple dapr clients cause any issues.) -implemented a wrapper class NamedDaprClient around DaprClient. said wrapper class also has a name property;

    public interface INamedDaprClient
    {
        string GetName();
        DaprClient Instance {get;}
    }
    public class NamedDaprClient : INamedDaprClient
    {
        private readonly string _clientName;
        private readonly DaprClient _daprClient;

        public NamedDaprClient(string clientName, DaprClient daprClient)
        {
            _clientName = clientName;
            _daprClient = daprClient;
        }

        public string GetName() => _clientName;

        public DaprClient Instance { get { return _daprClient; } }
    }

-copied AddDaprClient() service extension method from Dapr.AspNetCore / DaprServiceCollectionExtensions.cs , removed DaprClientMarkerService injection and existence check that prevents multiple clients to be injected, and injected NamedDaprClient wrapper class instead of DaprClient

        public static void AddNamedDaprClient(this IServiceCollection services, string clientName, Action<Dapr.Client.DaprClientBuilder> configure = null)
        {
            if (services is null)
            {
                throw new ArgumentNullException(nameof(services));
            }

            if (clientName is null)
            {
                throw new ArgumentNullException(nameof(clientName));
            }

            services.AddSingleton<INamedDaprClient>(_ =>
            {
                var builder = new Dapr.Client.DaprClientBuilder();
                if (configure != null)
                {
                    configure.Invoke(builder);
                }

                return new NamedDaprClient(clientName, builder.Build());
            });
        }

-To get desired NamedDaprClient from dependency injection, any class do so in its constructor from NamedDaprClient collection provided by dependency injection

        private readonly ILogger<OnSayHelloHandler> _logger;
        private readonly INamedDaprClient _daprClient;
        private const string clientName = "default";
        public OnSayHelloHandler(ILogger<OnSayHelloHandler> logger, IEnumerable<INamedDaprClient> namedDaprClients)
        {
            _logger = logger;
            _daprClient = namedDaprClients?.FirstOrDefault(c => c.GetName().Equals(clientName));
        }

Regards,

rynowak commented 3 years ago

It just seemed natural to be able to change serialization method, because sooner or later one will encounter in a scenario to integrate with a legacy service. For example; that service expects to receive some different format (xml, protobuf or something else) from a message broker.

Thanks for the discussion. As mentioned I only plan take action here if there's a concrete scenario that's hard to accomplish without changes to the SDK.

In my experience, it's far easier and less error-prone when users can call a different method instead of having to grapple with multiple instances of the same class.

modabas commented 3 years ago

Sure, custom ser/des can be done before and after calling dapr sdk. Then different methods that operate on raw bytes and don't do json ser/des by default can work too, instead of multiple instances of dapr client with different serializations bound to them. Unfortunately all current overload methods for publish event, invoke binding, invoke method (non-grpc) do json ser/des and there is no way to disable it.

However, from comments, I understand that there are no plans to make such a change and current plan is to go forward with json serialization / deserialization only,

Since dapr is language agnostic, for all dapr enabled micro services deployed in same infrastructure but written in different languages to be able to interoperate, I presume this will be the case for all language sdks, not only dotnet sdk. Perhaps being language agnostic is the reason to support only json because any additional format should be supported or at least be processable by all language sdks one way or another.

Unfortunately, this makes introducing dapr enabled micro services into already existing micro service systems or integrating with legacy systems unlikely.

Nonetheless, this is a great project, and I look forward to use it to develop a brand new system from scratch.

Thanks for all the work,

Regards,

WhitWaldo commented 1 week ago

@modabas This is a bit of an older issue at this point, but starting in .NET 8 released last year, Microsoft.Extensions.DependencyInjection supports keyed services. We haven't yet implemented keyed support (haven't seen much of an ask for it outside of your issue here), but if you still need this capability, might I suggest something like the following as a temporary workaround to wrap our registration extension under a key of your choice?

public static class ServiceCollectionExtensions
{
  public static IServiceCollection AddKeyedDaprClient(this IServiceCollection services, string key)
  {
    var tempServices = new ServiceCollection();
    tempServices.AddDaprClient(); //Use the existing DaprClient registration extension
    var tempProvider = tempServices.BuildServiceProvider();
    var daprClient = tempProvider.GetRequiredService<DaprClient>();

    services.AddKeyedService<DaprClient>(key, provider => daprClient);
    return services;
  }
}

And if this is still something you're interested in, please let me know and we can see about adding it.

On the other issue, I've been thinking about serialization as well, so if you have any additional thoughts on that front, I'd welcome them as well.