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

Microsoft.FeatureManagement Feature-Based Injection #68

Closed
philcarbone opened this issue May 23, 2019 · 5 comments
Closed

Microsoft.FeatureManagement Feature-Based Injection #68

philcarbone opened this issue May 23, 2019 · 5 comments
Assignees

Comments

@philcarbone
Copy link

philcarbone commented May 23, 2019

Please consider adding the ability to inject based on a feature. For instance, If we had multiple versions of an algorithm and we wanted to toggle between versions 1, 2 and 3... We would prefer to implement this functionality in different classes, which implement the same interface. Then we would like to inject the proper implementation based on the feature.

Code for feature implementation

	interface ISpecialFunctionality {
		string GetSecretSauceAnswer(string data);
	}

	class SecretSauceV1 : ISpecialFunctionality {
		public string GetSecretSauceAnswer(string data) {
			// do things ...
		}
	}

	class SecretSauceV2 : ISpecialFunctionality {
		public string GetSecretSauceAnswer(string data) {
			// do things ...
		}
	}

	class SecretSauceV3 : ISpecialFunctionality {
		public string GetSecretSauceAnswer(string data) {
			// do things ...
		}
	}

Implementation

	public class HomeController : Controller {
		private readonly ISpecialFunctionality _secretSauce;

		public HomeController(ISpecialFunctionality secretSauce) {
			_secretSauce = secretSauce;
		}

		public IActionResult Index() {
			var answer = secretSauce.GetSecretSauceAnswer("blah");
			// do things...
		}
	}
@philcarbone philcarbone changed the title Microsoft.FeatureManagement Microsoft.FeatureManagement Feature-Based Injection May 23, 2019
@jimmyca15
Copy link
Member

Hey Phil, I'm wondering how this would work when choosing between implementations when the features that enable them are all met. So far feature management is about enabling functionality, we don't have much of the notion of choosing between things. For example, with the current design it is very reasonable to expect an IEnumerable<ISpecialFunctionality> who's items will be populated based on the state of a select list of features.

SecretSaucev1 requires Alpha feature
SecretSaucev2 requires Beta feature.

If alpha and beta are on, then IEnumerable<ISpecialFunctionality> can contain both implementations, where as if one or none of those feature's are on, then the enumerable would be empty. It could be said that SecretSaucev2 may somehow override SecretSaucev1 but the picture isn't so clear to me at this point of how one feature can override another and what it should be like to wire something like this up.

Perhaps something like

services.AddSingleton<IFunctionality, DefaultSauce>()
        .OverrideForFeature<IFunctionality, SecretSaucev1>("Alpha")
        .OverrideForFeature<IFunctionality, SecretSaucev2>("Beta")

But then the order becomes very important.

It's a good suggestion, we'll look into it. Thanks !

@philcarbone
Copy link
Author

Hi @jimmyca15

Thanks for the response! I think what you are talking about makes sense... Having a default where a feature can then override that functionality if turned on.

For the software that I work with, we are usually enhancing a feature and performing dark launches. When we enhance the feature, we toggle between old and new functionality.

We've seen, in some toggle frameworks (e.g. LaunchDarkly), you can provide a function to call for a feature toggle (enabled) and an alternative function to call for the toggle disabled. This works well in dynamic code like python.

Our team was trying to think of a way to do this in a more strongly & static typed way and the idea of using interfaces and injection felt like a good fit.

For us it's simply: Use A or B. Where A is the existing functionality and B is the new functionality (including backwards compatibility)

So, I could see us using a simplified version of what you posted:

services.AddSingleton<IFunctionality, SecretSaucev1>()  // v1 - this is the default version
        .OverrideForFeature<IFunctionality, SecretSaucev2>("NewEnhancedVersion") // v2 - this is the new version

Later down the road we could just make the following change (once a second round of enhancements come through):

services.AddSingleton<IFunctionality, SecretSaucev2>()  // v2
        .OverrideForFeature<IFunctionality, SecretSaucev3>("NewEnhancedVersion") // v3

then

services.AddSingleton<IFunctionality, SecretSaucev3>()  // v3
        .OverrideForFeature<IFunctionality, SecretSaucev4>("NewEnhancedVersion") // v4

and so on...

We may simply inherit the previous versions for shared functionality, or something similar (and refactor as obsoleted versions that are no longer necessary, over time).

In this simplified version, where we are still overriding the default implementation and we don't really need an IEnumerable. I can also see scenarios that fit better with your exact proposal. My team, specifically doesn't have that need at the moment, but maybe in the future.

Another option, if you don't end up implementing something: My team may try something and see if we can contribute the source back. We would just need to flesh out the detail.

@philcarbone
Copy link
Author

philcarbone commented May 24, 2019

Another thought:
My team is trying to avoid using the toggles in control flow code (e.g. if-statements). I would prefer that we inject or use a technique that allows the feature-implementation to be decoupled from feature-toggle-logic. While also supporting the old/new toggle requirement as described in my previous post.

@SacredSkull
Copy link

I recently went up against this when trying to programmatically change the service implementation based on a feature toggle:

if (featureManager.IsEnabled(flag)) {
    builder.Services.AddTransient<IEmailService, RealEmailer>();
} else {
    builder.Services.AddTransient<IEmailService, NullEmailer>();
}

The idea being that in production, we'd enable this feature, and otherwise it would just log what it would've sent instead of actually sending emails to test accounts that don't really exist.

I was hoping that the Feature Management would cater for this, but I tried serviceProvider.GetService<FeatureManager>() and it died trying to resolve some of the service hierarchy regarding logging. I like the idea of the service provider builder API having this integrated, it does make for better reading - BUT:

Another thought:
My team is trying to avoid using the toggles in control flow code (e.g. if-statements). I would prefer that we inject or use a technique that allows the feature-implementation to be decoupled from feature-toggle-logic. While also supporting the old/new toggle requirement as described in my previous post.

I don't think using the feature management stuff in Configure/Startup directly like that is a bad thing provided it only occurs in your composition layer - if statements or otherwise.

@jimmyca15
Copy link
Member

This issue has been moved to microsoft/FeatureManagement-Dotnet#39 so we can track it better and come to a solution.

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

No branches or pull requests

4 participants