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

Caching for a custom FeatureDefinitionProvider #367

Open
tahazayed opened this issue Feb 11, 2024 · 18 comments
Open

Caching for a custom FeatureDefinitionProvider #367

tahazayed opened this issue Feb 11, 2024 · 18 comments
Assignees

Comments

@tahazayed
Copy link

I am building a custom FeatureDefinitionProvider and I want to support caching for it, but this interface IFeatureDefinitionProviderCacheable is internal; would you please change it to be public?

@thompson-tomo
Copy link
Contributor

Why don't you implement a configuration provider to fetch you feature flag definitions, that way the config layer would act as the caching?

@tahazayed
Copy link
Author

tahazayed commented Feb 12, 2024

I am using the same way as tests/Tests.FeatureManagement/InMemoryFeatureDefinitionProvider.cs, in either way, I need IFeatureDefinitionProviderCacheable to be public to use the caching as ConfigurationFeatureDefinitionProvider depends on IFeatureDefinitionProviderCacheable to support caching

@tahazayed tahazayed changed the title Cahing for a custom FeatureDefinitionProvider Caching for a custom FeatureDefinitionProvider Feb 12, 2024
@thompson-tomo
Copy link
Contributor

You shouldn't need to add that interface to your provider given it is simply to alter handling for test cases and is not needed in actual use case. Please try it without and also what i would be doing is, implementing your provider as a configuration provider that way you definitions are immediately cached for the library to use.

@jimmyca15
Copy link
Member

@tahazayed Indeed this functionality should be available to you.

@rossgrambo this conversation mentions a plan to open up this functionality and to file an issue. Are we tracking it?

@rossgrambo
Copy link
Contributor

No, I don't believe it was ever tracked. I'll treat this as the tracking task and look into it.

@rossgrambo rossgrambo self-assigned this Feb 12, 2024
@tahazayed
Copy link
Author

@jimmyca15 , @rossgrambo Thank you. In my circumstances, the alternative SaaS dynamic feature management costs $20,000 per month.

@rossgrambo
Copy link
Contributor

rossgrambo commented Feb 13, 2024

Hey @tahazayed, thanks for letting us know. I want to align on our different caching to ensure you're getting the caching you want.

IFeatureDefinitionProviderCacheable is an interface that enables a specific cache. This cache is for the binding of IConfiguration to a specific class. For our TimeWindowFilter- this cache prevents IConfiguration.Get() from firing too much, as this type conversion is expensive a few ms, and can be reduced to roughly ~1/100th of the time with this cache.

Separately, there's the more common caching concern- Feature Flags from a server being supplied to the Feature Management library. FeatureManagement reads flags though the IFeatureDefinitionProvider injected dependency. FeatureManagement does not handle caching in this way, instead, it will always request feature definitions directly from the provider. The provider is in charge of caching or to pull from a cache. The provided ConfigurationFeatureDefinitionProvider pulls from IConfiguration, which is essentially our cache for the flag data. Our server provider (Azure App Configuration's .NET Provider), updates the Configuration in order to dynamically update flags. This is what @thompson-tomo was referring to when they said the "config layer would act as the caching".

Being described like this, @tahazayed, which caching were you hoping to take advantage of?

I'll work on making IFeatureDefinitionProviderCacheable public / opening up that performance gain caching either way, but I don't want you to wait on it if it's not the cache you care about.

@tahazayed
Copy link
Author

Thank you for letting me know. I think I want to use what @thompson-tomo is referring to. Do you have an example of how to inject a custom configuration definition provider?

@thompson-tomo
Copy link
Contributor

@tahazayed easiest way i have found is to follow the example from Microsoft: https://learn.microsoft.com/en-us/dotnet/core/extensions/custom-configuration-provider and look at some of the samples around.

@tahazayed
Copy link
Author

I wonder where the cache configuration is and when it became invalid, as it seems after implementation, the cache lasts forever

@thompson-tomo
Copy link
Contributor

configuration never expires, it is up to the configuration provider to implement an update/expiration mechanism

@tahazayed
Copy link
Author

tahazayed commented Feb 15, 2024

What is the value of using it (other than modularity)? I get the same result as using AppSettings.json

@thompson-tomo
Copy link
Contributor

Value of using a custom configuration provider? Benefit is you can source your feature flags from an external system with the config provider doing the transformation to a dictionary hence making it usable.

@tahazayed
Copy link
Author

I can achieve the same with a custom FeatureDefinitionProvider as I have to implement a cachig with expiration myself anyway,
Also, if I use a configuration provider, I have to hide a property called "Data" from the base class ConfigurationProvider to get the caching work

@thompson-tomo
Copy link
Contributor

would work it work for you to simply periodically reload your config after an interval has elapsed? From memory this is what Azure does and can all be achieved by your custom config provider.

@tahazayed
Copy link
Author

tahazayed commented Feb 15, 2024

It is more complex than FeatureDefinitionProvider as per the proposed solution I have to build a background job and keep it running with a timer to reload the configuration.
Thank you anyway

@thompson-tomo
Copy link
Contributor

I don't think it is as complex as you are thinking. Perhaps take a look at: https://learn.microsoft.com/en-us/azure/azure-app-configuration/enable-dynamic-configuration-dotnet-core?tabs=core6x

@tahazayed
Copy link
Author

I have tried it; I had to write three classes instead of one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants