microsoft / PowerPlatform-DataverseServiceClient

Code Replica for Microsoft.PowerPlatform.Dataverse.Client and supporting nuget packages.
MIT License
282 stars 52 forks source link

[Feature]Expose the underlying http handler for consumers #92

Open onsvejda opened 3 years ago

onsvejda commented 3 years ago

It is impossible to specify http message handler neither for WCF client nor for ODATA client. Exposing this can give a 3pty advantage of implementing basic policies like retry / circuit breaker on http level - instead of doing them on "higher" levels. Attaching a sample patch file how this change might look like.

0001-dev.patch.txt

MattB-msft commented 3 years ago

Thanks for you feedback and example @onsvejda ,

We do not do this explicitly as we are changing the implementation of many of these areas actively. we also do not usually expose these points to protect users from Protocol or API changes that may occur in the server, which is what we doing right now by transition this API from SOAP to ODATA without impacting the API surface.

Insofar as telemetry support, the CdsServiceClient ( and the CrmServiceClient) are wired up to work with the telemetry systems provided by the Dataverse server system itself. this is also an area that has seen frequent changes in the last few years.

Insofar as the support we offer,
We support client identification ( both SDK Client version and host process ), Request tracking, and Session Tracking features. A developer has control over the Request ID's and Session ID's that the client will use for each request. SDK version and host process name are collected automatically.

You can see this information by enabling verbose mode logging in the CDS Service client, detailed logs will be emitted covering request performance, request ID's, and session ID's if present.

Is there a further level of telemetry you would like?

onsvejda commented 3 years ago

Hi @MattB-msft , I understand you currently use this extension just for telemetry - that's not what I'm after - I'm after two things:

MattB-msft commented 3 years ago

Thanks @onsvejda. As I said, we are very guarded about the wire protocol and features we allow access too.
By way of explanation, We had overexposed this in the past with our SDK's and created a substantial support issue for ourselves as both the technology and protocols evolved and were updated. While there is a population of users out there that will invest the time to understand the settings and behaviors of the transport, it is more often that exposing them leads to folks manipulating them and creating a bad state. Additionally it locks us into a given wrapper tech for the transport's that we need to map to future tech.

In this case we are challenged with supporting both full framework and .net core concurrently. The feature you are looking at here is only available in .net core. and only from 2.2 +.

With that said,

We will discus this ask and see if this is something we are willing to expose.

mohsinonxrm commented 3 years ago

@onsvejda , I agree with your feature request as well. Would be really nice to get access to the underlying HTTP client handler. I'm mainly looking for fault handling and logging.

@MattB-msft , I understand from your point of view as well and I get that it is an opinionated implementation, after all, it is a "client" library but perhaps there could be a way to customize or let us provide our own overrides?

chinhpham commented 3 years ago

I'd vote for this too. I'm implementing a batch processing job to create and update data but got different results on each test run. Retries couldn't work as expected for the same reason mentioned here.

MattB-msft commented 3 years ago

As an update to this thread. We are still discussing providing some configuration support for Socket Handler, however we have decided against exposing the http transport at this time. This is due to many of the concerns I mentioned above and some current planning we are doing for the future of this client.