dotnet / extensions

This repository contains a suite of libraries that provide facilities commonly needed when creating production-ready applications.
MIT License
2.55k stars 732 forks source link

Improve discoverability of DI prerequisites in Http.Diagnostics package #4675

Open xakep139 opened 8 months ago

xakep139 commented 8 months ago

Currently, if I use Microsoft.Extensions.Http.Diagnostics package, it's assumed that I've already referenced Microsoft.Extensions.Compliance.Redaction package and registered AddRedaction() in the IServiceCollection.

For LatencyContext feature it's also required to call AddLatencyContext().

If any of these actions weren't done, then a user gets an exception:

Some services are not able to be constructed (Error while validating the service descriptor 'ServiceType:
Microsoft.Extensions.Http.Diagnostics.IHttpRouteParser Lifetime: Singleton ImplementationType: Microsoft.Extensions.Http.Diagnostics.HttpRouteParser':
Unable to resolve service for type 'Microsoft.Extensions.Compliance.Redaction.IRedactorProvider' while attempting to activate 'Microsoft.Extensions.Http.Diagnostics.HttpRouteParser'.)
...

For LatencyContext it's at the same level of being counter-intuitive on how to resolve the issue:

Some services are not able to be constructed (Error while validating the service descriptor 'ServiceType:
Microsoft.Extensions.Http.Logging.HttpClientLogger Lifetime: Singleton ServiceKey: MyNamedClient KeyedImplementationType: Microsoft.Extensions.Http.Logging.HttpClientLogger':
Unable to resolve service for type 'Microsoft.Extensions.Diagnostics.Latency.ILatencyContextTokenIssuer' while attempting to activate 'Microsoft.Extensions.Http.Latency.Internal.HttpClientLatencyLogEnricher'.)

We need to throw at least some human-understandable exception that explains what requirements/missing dependencies need to be registered.

This is slightly related to #4560

Tratcher commented 8 months ago

Registering any service should register all of its required dependencies. If some of those are conditionally required then there should be separate extensions for those scenarios where they are required.

xakep139 commented 8 months ago

Regarding redaction, my suggestion would be to register NullRedactorProvider if IRedactorProvider isn't registered in the DI yet. But we need to ensure that subsequent call to AddRedaction() replaces previous registration.