-
Notifications
You must be signed in to change notification settings - Fork 119
Add support for dynamic features. #132
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
Add support for dynamic features. #132
Conversation
examples/CustomAssignmentConsoleApp/CustomAssignmentConsoleApp.csproj
Outdated
Show resolved
Hide resolved
src/Microsoft.FeatureManagement/Microsoft.FeatureManagement.csproj
Outdated
Show resolved
Hide resolved
src/Microsoft.FeatureManagement/Targeting/TargetingFeatureVariantAssigner.cs
Show resolved
Hide resolved
Refers to: src/Microsoft.FeatureManagement/Targeting/ContextualTargetingFilter.cs:35 in edb08e3. [](commit_id = edb08e3, deletion_comment = False) |
src/Microsoft.FeatureManagement/Targeting/ContextualTargetingFeatureVariantAssigner.cs
Show resolved
Hide resolved
src/Microsoft.FeatureManagement/IFeatureVariantOptionsResolver.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.FeatureManagement/ContextualFeatureVariantAssignerEvaluator.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.FeatureManagement/ContextualFeatureVariantAssignerEvaluator.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.FeatureManagement/ContextualFeatureVariantAssignerEvaluator.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.FeatureManagement/ContextualFeatureVariantAssignerEvaluator.cs
Show resolved
Hide resolved
src/Microsoft.FeatureManagement/ContextualFeatureVariantAssignerEvaluator.cs
Outdated
Show resolved
Hide resolved
6c01a45 to
57b06b3
Compare
| if (filter is IFeatureFilter featureFilter && await featureFilter | ||
| .EvaluateAsync(context, cancellationToken) | ||
| .ConfigureAwait(false)) | ||
| if (filter is IFeatureFilter featureFilter && await featureFilter.EvaluateAsync(context, cancellationToken).ConfigureAwait(false)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Want to change this to the else clause of the if(useAppContext) too for consistency? #Closed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would be a behavioral change. Right now, since multiple filters can be registered for a feature, we allow contextual and non-contextual feature filters to be evaluated for a single feature flag evaluation.
Right now for assignment, since there's only one assigner that can run, we require a contextual assigner if app context is passed in.
Not sure if we want to unify these behaviors. But, if we do, I would opt to change the assignment requirement to allow a non-contextual assigner to run even if a context is passed in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. Looks like we didn't fail when a contextual filter is listed but without a context being provided today. This is a pseudo code I have in mind. Do you feel it's easier to understand the logic you explained above?
if (it's contextual filter)
{
if (useAppContext)
{
do work for contextual filter
}
else
{
fail // we can skip this else block if we want to avoid the the breaking change.
}
}
else
{
do work for non-contextual filter
}
I agree we should do the same for the assignment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do think it makes it more clear that we expect a filter to be either contextual or noncontextual. Originally this was written with the design that a filter could be both, but we then added a check to ensure that a filter isn't used that implements both.
Given that we only expect a filter to be one of the two types, your layout is probably better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The latest commit contains a change that is somewhat between your suggestion and what already existed. It does clarify that at the root we distinguish non-contextual filter vs. contextual filter. However, we do check if app context is being used before trying to check if a filter is a contextual filter since the app context type is used in that process.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the change, the following conditions result in exceptions, whereas they did not before.
- if feature references a contextual filter and caller passes a different app context type then the contextual filter supports.
- if feature references a contextual filter and caller doesn't pass an app context
src/Microsoft.FeatureManagement/Targeting/ContextualTargetingFeatureVariantAssigner.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.FeatureManagement/Targeting/ContextualTargetingFeatureVariantAssigner.cs
Show resolved
Hide resolved
src/Microsoft.FeatureManagement/Targeting/ContextualTargetingFeatureVariantAssigner.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.FeatureManagement/Targeting/ContextualTargetingFeatureVariantAssigner.cs
Show resolved
Hide resolved
* Clarified error types. * Updated/added tests
Updated error codes for consistency.
…or impossible contextual evaluations.
src/Microsoft.FeatureManagement/Targeting/ContextualTargetingFeatureVariantAssigner.cs
Show resolved
Hide resolved
src/Microsoft.FeatureManagement/Targeting/TargetingEvaluator.cs
Outdated
Show resolved
Hide resolved
zhenlan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
![]()
Updated exception used for null child property.
zhenlan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's roll :)
Dynamic Features
This PR adds a new paradigm named dynamic features, which are features that may have different variants. Any modification or improvement to an application can be considered a feature. When new features are being added to applications there may be multiple different designs. The different options for the design of a feature can be referred to as variants of the feature, and the feature itself can be referred to as a dynamic feature. A common pattern when rolling out dynamic features is to surface the different variants of a feature to different segments of a user base and to see how each variant is perceived. The most well received variant could be the one that gets rolled out to the entire user base, or if necessary the feature could be scrapped. There could be other reasons to expose different variants of a feature, for example using a different version every day of the week. The goal of this method is to establish a model that can help solve these common patterns that occur when rolling out features that can have different variants.
Consumption
Feature variant's are accessible through the
IFeatureVariantManagerinterface.The variant manager performs a resolution process that takes the name of a feature and returns a strongly typed value to represent the variant's options.
The following steps are performed during the retrieval of a dynamic feature's variant
Usage Example
One possible example of when variants may be used is in a web application when there is a desire to test different visuals. In the following examples a mock of how one might assign different variants of a web page background to their users is shown.
Configuring a Dynamic Feature
Assignment
One question that arises when working with variants is how to know which variant should be used at any given time. The act of choosing which variant should be used is called assignment. In this PR a built-in method of assignment is provided through the already established targeting paradigm. The pre-existing targeting feature filter used in feature flags allows certain segments of a user base to see a feature as on. Now there is a targeting assigner that can help assign different variants of a feature to different segments of an application's user base.
Custom Assignment
There may come a time when custom criteria is needed to decide which variant of a feature should be assigned when a feature is referenced. This is made possible by an extensibility model that allows the act of assignment to be overriden. Every feature registered in the feature management system that uses feature variants specifies what assigner should be used to choose a variant.
Example implementation can be found in ~examples/CustomAssignmentConsoleApp/DayOfWeekAssigner.cs