-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Dependency Injection IServiceProvider Decoration #45497
Comments
Tagging subscribers to this area: @eerhardt, @maryamariyan Issue DetailsProposed APII want to propose one api addition to MS DI which would be useful for use-cases like service decoration, debug/logging service requests, cross-wiring, child containers and scope tracking. public class ServiceProviderOptions
{
...
+ /// <summary>
+ /// Enables any service requests or IServiceProvider arguments to be delegated to the returned IServiceProvider.
+ /// </summary>
+ public Func<IServiceProvider, IServiceProvider>? ServiceProviderFactory { get; set; }
} Naming is of course up for discussion, Adding this specific api at this specific spot I believe is the smallest change, has the best chance of being generally useful, and as an added bonus does not directly add to the complexity of the shared specification for all container implementations (though a cursory look through other container's implementations of IServiceProvider looks to be a regular service that can be decorated readily). Background and MotivationProper child containers is the main reason for us as a company to propose this api. For the issues around implementing child containers and more generally cross-wiring I would point to some links by @dazinator (Dotnettency author), and benfoster (Saaskit author), there is plenty more if you search google and github.
Needless to say building this right is a delicate job with any container, however MS DI with its immutable design could, with some tweaks, be the most intuitively understandable and stable implementation out there for building child containers. Having a mutable service graph adds a lot more cognitive complexity. Moving to scope tracking, this is important for orderly disposal of the container as these may not live for the duration of the app. Our software has tenancy support just like Orchard which we like to run on MS DI, the integration with the framework, stability, immutable service graph and performance are all very good. Today after we do that swap with the old container we are forced to choose between:
The issue is we have no way of tracking active scopes (and I would find proposing an api for it questionable). Having the ability to decorate A prototype implementation of this can be found in the tests: PRI created a 'prototype' branch at https://github.com/NinoFloris/dotnet-runtime/tree/feature/service-provider-decorator which is complete, tested, and already handles all the fun re-entrant and concurrent cases, this serves a. I'm not ready to open a PR just yet as I want to flesh out a reliable child container implementation on top of this new api first, which will be at https://github.com/NinoFloris/dotnet-runtime/tree/example/child-container Regardless of this proposal gaining any approval I will round off our internal child container implementation based on this work but I would definitely like to see these changes 'upstreamed' :)
|
I think avoiding service descriptor API changes is best here so this has that going for it. The other thing I would say is that it needs to be free when this setting is null. I also don't quite understand how this extensibility solves the things described in the issue so I'd need to see some more concrete ways this solves those problems. |
@NinoFloris You can find my (working) implementation here: I was planning on migrating that accross to Dazinator.Extensions.DependencyInjection and releasing it on nuget. Related issue I had with the framework - that I had to workaround in my experiment above - is this whole issue of "decorating" not really being possible Final note, if you just want to reload middleware pipeline based on a change token signalling (like options monitor or config) you might find this useful |
Thanks @davidfowl that's an encouraging reply! I'll go through it piece by piece.
I hope you didn't mind the length of the proposal too much ;) it was precisely because I know how hard it is (something something constrained open generics) to get any changes into this area, really glad to hear this!
Understood and I wouldn't want it any other way, if you take a look at ILEmit as an example: - // [return] ProviderScope
+ // [return] ProviderScope.ServiceProvider
argument.Generator.Emit(OpCodes.Ldarg_1);
+ argument.Generator.Emit(OpCodes.Callvirt, ServiceProviderGetter); NinoFloris@76fd089#diff-fbaf59e20d1ca7916fb25c4e1b7a4f272024bf38fcd18d407d91dfacd14cf6a8R196-R198 This is the case for all the others as well, there are no extra allocs and the additional logic to assign the
I will absolutely get to that as I agree it's critical for this proposal to go anywhere.
I will have an implementation you can play with and check out. I already know it would require |
@dazinator thanks for the comment :) I know you battled with many of the same issues, at Crowded we started using .net core 1.0 alpha, back then we used StructureMap like Saaskit and like them we had an endless list of issues, around 2.0 we switched fully to MS DI where we are today. Our tenancy support is an implementation in F# and one day when all of this works without hacks I'd love to open source it, it's 'just plumbing' but it's really good. We're all bumping into the same limitations, for instance your experiment doesn't (and can't of course) wrap the All these edge cases is exactly why I opened this proposal, it's written in such a way you can hermetically decorate anything. Just take a look at some of these tests
I wish life was that simple ;) |
Yep thats the exact hurdle I hit. I got as far as exploring the native service provider code - but props on actually working through a solution. I'd love to see this feature land. Decorating the service provider seems the only way to implement scope tracking hermetically. (P.S thank you for also introducing me to the word |
@NinoFloris - given the lack of movement on this, what are you thoughts on forking and publishing the modified package? |
@NinoFloris I've published this as a seperate nuget package so I could continue to use it and try it out in some other projects. Hope that's ok? It's here: https://www.nuget.org/packages/Dazinator.Extensions.DependencyInjection.Microsoft/1.1.0-PullRequest0022.76 |
Related to #36021 |
Proposed API
I want to propose one api addition to MS DI which would be useful for use-cases like service decoration, debug/logging service requests, cross-wiring, child containers and scope tracking.
Naming is of course up for discussion,
ServiceProviderFactory
is an overloaded term but I didn't directly find any precedent for calling anything aDecorator
in dotnet/runtime so I left it like this for now.Adding this specific api at this specific spot I believe is the smallest change, has the best chance of being generally useful, and as an added bonus does not directly add to the complexity of the shared specification for all container implementations (though a cursory look through other container's implementations of IServiceProvider looks to be a regular service that can be decorated readily).
Background and Motivation
Proper child containers is the main reason for us as a company to propose this api.
For the issues around implementing child containers and more generally cross-wiring I would point to some links by @dazinator (Dotnettency author), and benfoster (Saaskit author), there is plenty more if you search google and github.
Needless to say building this right is a delicate job with any container, however MS DI with its immutable design could, with some tweaks, be the most intuitively understandable and stable implementation out there for building child containers. Having a mutable service graph adds a lot more cognitive complexity.
Moving to scope tracking, this is important for orderly disposal of the container as these may not live for the duration of the app. Our software has tenancy support just like Orchard which we like to run on MS DI, the integration with the framework, stability, immutable service graph and performance are all very good.
Specifically the support of soft reload allows us to reload tenant configuration without rebooting the entire app, we keep serving requests with the old container until the soft reload is done, at which point we swap the containers.
Today after we do that swap with the old container we are forced to choose between:
ServiceProvider
(note, the actual type) breaks all scopes which is unacceptable to us, it would defeat the soft part in 'soft reload'.ServiceProvider
, meaning singletons may never be fully cleaned up, we're choosing to do this today.The issue is we have no way of tracking active scopes (and I would find proposing an api for it questionable).
Having the ability to decorate
IServiceProvider
we can replace any requests forIServiceProviderFactory
with a decorated version counting (increment) allCreateScope()
calls, giving out decoratedIServiceScope
instances counting (decrement) all scope disposals. Now when our old container should be disposed we can have the last scope to dispose do so for us!A prototype implementation of this can be found in the tests:
https://github.com/NinoFloris/dotnet-runtime/blob/76fd089360e2e4fa76f05cbfa8a7c08345fd1a40/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceProviderFactoryTests.cs#L64-L137
PR
I created a 'prototype' branch at https://github.com/NinoFloris/dotnet-runtime/tree/feature/service-provider-decorator which is complete, tested, and already handles all the fun re-entrant and concurrent cases.
I'm not ready to open a PR just yet as I want to flesh out a reliable child container implementation on top of this new api first, which will be at https://github.com/NinoFloris/dotnet-runtime/tree/example/child-container
The most straightforward way to do that — until we have the ServiceDescriptor api changes — would be to cross-wire parent and child at the
GetService
level, this has the small downside of having a split service graph between two containers, meaning you cannot add or override a parent service dependency in a child container, but it will correctly handle any open generics and (singleton) disposal behavior.Regardless of this proposal gaining any approval I will round off our internal child container implementation based on this work but I would definitely like to see these changes 'upstreamed' :)
The text was updated successfully, but these errors were encountered: