stiano / unleash-client-dotnet

Unleash FeatureToggle Client for .Net
https://github.com/Unleash/unleash
Apache License 2.0
20 stars 8 forks source link

DI scoping issue when not using statics #17

Open scottt732 opened 5 years ago

scottt732 commented 5 years ago

I'm having trouble hooking Unleash up in IServiceCollection. We're trying to use the DI IDisposable BeginScope() to store an IUnleashContextProvider backed by a Dictionary<string, string> as opposed to relying on HttpContext.Current.Items as in the README. The reason is b/c we use the scope pattern outside of ASP.NET Core and would like to be able to use the same Unleash boilerplate code regardless of whether we're in an ASP.NET Core app, a generic host builder, or a simple console app (creating a scope per ASP.NET request, MQ message received, timer interval, etc.).

This almost works, except that the IUnleashContextProvider is a property on the UnleashSettings class. IUnleash is registered as a singleton (per the docs), IUnleashContextProvider is registered as Scoped (also recommended in the docs and critical to capture request/message/etc-specific scope). UnleashSettings is registered as a singleton.

When ASPNETCORE_ENVIRONMENT=development, IServiceCollection.BuildServiceProvider(true) is called by the framework, which ensures that we're not capturing scoped/transient dependencies in a singleton. As far as I can tell it's unavoidable given the current architecture... I tried (see here: https://github.com/scottt732/UnleashClientTests/tree/master/UnleashClientTests).

DefaultUnleash(singleton) -> UnleashSettings(singleton) -> UnleashContextProvider(necessarily scoped, effectively captured) = invalid (single state would be shared across requests/race conditions).

The example code in the README ~gets around this issue by effectively having the UnleashContextProvider as a singleton which refers to HttpContext.Current?.Items. It will happen to be there in an ASP.NET request because your BeginRequest handler puts it there, but this requires using a static for state and breaks the ability for the same configuration to work in different application types.

mmalyska commented 1 year ago

I raise the same issue. We are consuming rabbit messages so our context is not a HttpContext.