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

Enhance DaprClientBuilder to override gRPC and Http Client #683

Open
subash89 opened this issue Feb 4, 2022 · 4 comments
Open

Enhance DaprClientBuilder to override gRPC and Http Client #683

subash89 opened this issue Feb 4, 2022 · 4 comments
Labels

Comments

@subash89
Copy link

subash89 commented Feb 4, 2022

The gRPC client and http client creation inside DaprClient is not flexible to outside users. For example when creating a grpc client, all the impactful configuration resides inside grpc ManagedChannel where you can set a sizable executor pool, timeouts etc. Same with OkHttpClient.

I locally made changes to cater this requirement. Following is the final look from the dapr client builder point of view.
DaprClientBuilder daprClientBuilder = new DaprClientBuilder(); ManagedChannelBuilder managedChannelBuilder = Grpc.newChannelBuilder("localhost:8000", TlsChannelCredentials.create()) .executor(Executors.newFixedThreadPool(20)); DaprClient client = daprClientBuilder.withManagedGrpcChannel(managedChannelBuilder.build()).build();

Same goes for OkHttpClient.

DaprClientBuilder daprClientBuilder = new DaprClientBuilder(); DaprClient client = daprClientBuilder.withOkHttpClient(new OkHttpClient.Builder().readTimeout(10, TimeUnit.SECONDS)).build();

I specifically added these methods to DaprClientBuilder directly(even though it's not aware of grpc or http specifically from the interface), to keep the implementation and current code remain same and make this change backward compatible.

Please let me know your thoughts, I can send the official PR based on your feedbacl.

@mthmulders
Copy link
Contributor

The fact that Dapr completely configures and manages the underlying OkHttpClient and gRPC client also prevents frameworks like Spring, OpenTelemetry etc. to do automatic instrumentation of those clients. I guess that being able to configure a DaprClient with an underlying client provided by the user might also make distributed tracing a lot easier....

@artursouza
Copy link
Member

First, we plan to remove dependency on OKHTTP to avoid version conflicts with Spring and other popular frameworks. So, making that as part of the SDK contract would be going in the other direction.

Regarding gRPC managed channel, that could be interesting. Although I would probably create a new builder class to keep the existing one protocol neutral.

@subash89
Copy link
Author

subash89 commented Mar 3, 2022

So how daprclient going to support http ? I guess the interfaces have to change to accept a Callable or Function that will take care of http call ?
I agree with to keep the current one protocol neutral. Will create a new Builder and update the PR.

@mukundansundar mukundansundar added this to the v1.6 milestone Apr 8, 2022
@artursouza artursouza modified the milestones: v1.6, v1.7 Jun 23, 2022
@artursouza artursouza modified the milestones: v1.7, v1.8 Sep 30, 2022
@artursouza artursouza added the P2 label Nov 7, 2022
@artursouza
Copy link
Member

We plan to deprecate HTTP client in the next release and only offer gRPC.

@artursouza artursouza removed this from the v1.8 milestone Nov 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants