Skip to content
Merged
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
149 changes: 148 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ The feature management library supports appsettings.json as a feature flag sourc
}
```

The `FeatureManagement` section of the json document is used by convention to load feature flag settings. In the section above, we see that we have provided three different features. Features define their feature filters using the `EnabledFor` property. In the feature filters for `FeatureT` we see `AlwaysOn`. This feature filter is built-in and if specified will always enable the feature. The `AlwaysOn` feature filter does not require any configuration so it only has the `Name` property. `FeatureU` has no filters in its `EnabledFor` property and thus will never be enabled. Any functionality that relies on this feature being enabled will not be accessible as long as the feature filters remain empty. However, as soon as a feature filter is added that enables the feature it can begin working. `FeatureV` specifies a feature filter named `TimeWindow`. This is an example of a configurable feature filter. We can see in the example that the filter has a `Parameters` property. This is used to configure the filter. In this case, the start and end times for the feature to be active are configured.
The `FeatureManagement` section of the json document is used by convention to load feature flag settings. In the section above, we see that we have provided three different features. Features define their feature filters using the `EnabledFor` property. In the feature filters for `FeatureT` we see `AlwaysOn`. This feature filter is built-in and if specified will always enable the feature. The `AlwaysOn` feature filter does not require any configuration so it only has the `Name` property. The alternative name for this filter is `On`. `FeatureU` has no filters in its `EnabledFor` property and thus will never be enabled. Any functionality that relies on this feature being enabled will not be accessible as long as the feature filters remain empty. However, as soon as a feature filter is added that enables the feature it can begin working. `FeatureV` specifies a feature filter named `TimeWindow`. This is an example of a configurable feature filter. We can see in the example that the filter has a `Parameters` property. This is used to configure the filter. In this case, the start and end times for the feature to be active are configured.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the On alias something new? What's the reason we want to advertise an alternative name?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't part of the original planning, but it was part of the variants documentation doc that I referenced for my PR. I assumed it was because seeing a flag called AlwaysOn when you can actually override it to be off with a variant's StatusOverride property would be confusing.

@jimmyca15 jimmyca15 Sep 27, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the presence of 'On' I would suggest we drop "AlwaysOn" from being mentioned here.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy with On as well. But someone looking at a config for the first time should be able to determine what AlwaysOn means. Perhaps we can leave a note at the bottom of the readme that AlwaysOn was the previous name for On?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will have to change the portal and CLI for file exporting to json and yaml. The file import then has to deal with both AlwaysOn and On. Is the renaming worth it?

The AlwaysOn is a half-internal workaround solution. I bet most AppConfig feature flag users never heard about it. I know its name is not ideal, but how bad it is, especially given the renaming cost?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also wanted to mention why this came up. Previously, there never was a reason for a user to see or use "AlwaysOn". A simple "on" feature could be represented as a boolean literal like my previous comment.

This changes with variants. Now to write a flag with variants in appsettings.json, users have to specify a filter. The idea was to make the declaration shorter than "AlwaysOn" which was never really used anyhow.

Under the hood, "On" and "AlwaysOn" both work. The suggestion wasn't a rename, rather just to support "On" as well.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand the motivation and I understand the AlwaysOn naming is not ideal.

I guess the file import part must understand AlwaysOn (and On in the future if we add it). I didn't search the CLI code, but looks like our sync library has it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, we do use it. I'm fine to avoid the change to introduce "On" then.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, so should I not mention On in this file at all in that case?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, and removal of On in code.


### On/Off Declaration

Expand Down Expand Up @@ -139,6 +139,23 @@ A `RequirementType` of `All` changes the traversal. First, if there are no filte

In the above example, `FeatureW` specifies a `RequirementType` of `All`, meaning all of it's filters must evaluate to true for the feature to be enabled. In this case, the feature will be enabled for 50% of users during the specified time window.

### Status

The `Status` property of a feature flag determines whether the remaining feature definition should be evaluated. `Status` is equal to `Conditional` by default, so the feature definition will determine whether the flag is enabled or which variant to return. If `Status` is equal to `Disabled`, then the feature flag is always considered disabled and will always return the `DefaultWhenDisabled` variant if specified.
Comment thread
amerjusupovic marked this conversation as resolved.
Outdated

```
"FeatureX": {
"Status": "Disabled",
"EnabledFor": [
{
"Name": "On"
}
]
}
```

In this example, even though the `On` filter would normally always make the feature enabled, the `Status` property is set to `Disabled`, so this feature will always be disabled.

### Referencing

To make it easier to reference these feature flags in code, we recommend to define feature flag variables like below.
Expand Down Expand Up @@ -636,6 +653,136 @@ When defining an Audience, users and groups can be excluded from the audience. T

In the above example, the feature will be enabled for users named `Jeff` and `Alicia`. It will also be enabled for users in the group named `Ring0`. However, if the user is named `Mark`, the feature will be disabled, regardless if they are in the group `Ring0` or not. Exclusions take priority over the rest of the targeting filter.

## Variants

Variants represent a configuration of a feature. A feature flag with variants contains a list of variants and assignment parameters dictating under what conditions each variant should be used. A variant's configuration can be a complex object, but it can also be as simple as a string or number.
Comment thread
amerjusupovic marked this conversation as resolved.
Outdated

``` C#
public class Variant
{
/// <summary>
/// The name of the variant.
/// </summary>
public string Name { get; set; }

/// <summary>
/// The configuration of the variant.
/// </summary>
public IConfigurationSection Configuration { get; set; }
}
```

### Getting a Feature's Variant

Variants expand on the basic flow of feature management and enable getting a feature's variant to later perform actions based on the name or configuration. This is done through the `IVariantFeatureManager`'s `GetVariantAsync` method.

``` C#
IVariantFeatureManager featureManager;
Variant variant = await featureManager.GetVariantAsync(nameof(MyFeatureFlags.FeatureU));
Comment thread
amerjusupovic marked this conversation as resolved.
Outdated

IConfigurationSection variantConfiguration = variant.Configuration;

// Do something with the resulting variant and its configuration
```

### Setting a Variant's Configuration

For each of the variants in the `Variants` property of a feature, there is a specified configuration. This can be set using either the `ConfigurationReference` or `ConfigurationValue` properties. `ConfigurationReference` is a string path that references a section of the current configuration that contains the feature flag declaration. `ConfigurationValue` is an inline configuration. If both are specified, `ConfigurationValue` is used.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can I omit both ConfigurationReference and ConfigurationValue?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the Variant.Configuration value will just be null

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, we can make it clear in the description.

This enables customers to have very concise variants like the one below.

"Variants": [
    { 
        "Name": "On"
    },
    { 
        "Name": "Off", 
        "StatusOverride": "Disabled"
    }    
]


```
"Variants": [
{
"Name": "Big",
"ConfigurationReference": "ShoppingCart:Big"
},
{
"Name": "Small",
"ConfigurationValue": {
"Size": 300
}
}
]
```

### Assigning a Variant
Comment thread
amerjusupovic marked this conversation as resolved.
Outdated

The process of deciding which variant to return for a feature is called assignment. Assignment is determined by the `Allocation` property of the feature. Allocation logic is similar to the [Microsoft.Targeting](./README.md#MicrosoftTargeting) feature filter, with some additional parameters to allow for more specific variant assignment.
Comment thread
amerjusupovic marked this conversation as resolved.
Outdated

```
"Allocation": {
"DefaultWhenEnabled": "Small",
"DefaultWhenDisabled": "Small",
"User": [
{
"Variant": "Big",
"Users": [
"Marsha"
]
}
],
"Group": [
{
"Variant": "Big",
"Groups": [
"Ring1"
]
}
],
"Percentile": [
{
"Variant": "Big",
"From": 0,
"To": 10
}
],
"Seed": "13973240"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if the Seed is not specified? Do we use a default value?

@amerjusupovic amerjusupovic Sep 27, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea I should mention this, we create a new string using the feature name and use that instead of the Seed value. I guess that would be the default value.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

featurename + "\nallocation"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would consider it appropriate for us to mention that if seed is omitted, then a default seed based off the flag name is used. But I would say away from mentioning our exact implementation.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it will be great to clarify.

},
"Variants": [
{
"Name": "Big",
"ConfigurationReference": "ShoppingCart:Big"
},
{
"Name": "Small",
"ConfigurationValue": {
"Size": 300

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is required to be a JSON? Can I have more than one key value?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I can explain that better, but it can be either a JSON or just a string after ConfigurationValue and it will return the configuration section starting with ConfigurationValue as the key. From there, you can index it like a normal configuration section. You can add any number of values or any structure of nested JSONs if you want.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can it be a number or a boolean?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, anything that could normally be in a configuration file like this, should I mention each of those?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think so. As an example, I think we should show a primitive value rather than a more complex JSON.

}
}
]
```

In the above example, if the feature is not enabled, `GetVariantAsync` would return the variant allocated by `DefaultWhenDisabled`, which is `Small` in this case.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be worth it to call out each property of Allocation individually and describe what it's for. E.g.

The Allocation setting of a feature flag has the following properties.

DefaultWhenDisabled: Specifies which variant should be used when a variant is requested while the flag is considered disabled.
DefaultWhenEnabled: ...
...Other properties...

And then go into the specific behavior of this example.


If the feature is enabled, the feature manager will check the `User`, `Group`, and `Percentile` allocations in that order to see if they match the targeting context or calculated percentile for that context. If the user is named `Marsha`, in the group named `Ring1`, or the user happens to fall between the 0 and 10th percentile calculated with the given `Seed`, then the specified variant is returned for that allocation. In this case, all of these would return the `Big` variant. If none of these allocations match the targeting context, the `DefaultWhenEnabled` variant is returned.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if they match the targeting context or calculated percentile for that context

I would leave out reference to targeting context. Instead mention the intent, like if the user being evaluated is specified in the users section, or if the user is in a group specified in the groups section, or if the user fits into the specified percentage allocation.

A separate section can be used, if necessary, to describe how that user/group info is determined (targeting context)


### StatusOverride
Comment thread
amerjusupovic marked this conversation as resolved.
Outdated

The `StatusOverride` property, if set for the assigned variant, overrides a feature's state. By default, this property is equal to `None`, which does not affect the feature state. If `StatusOverride` is equal to `Enabled` for the assigned variant, then the feature is enabled. If `StatusOverride` is equal to `Disabled` for the assigned variant, then the feature is disabled.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The `StatusOverride` property, if set for the assigned variant, overrides a feature's state. By default, this property is equal to `None`, which does not affect the feature state. If `StatusOverride` is equal to `Enabled` for the assigned variant, then the feature is enabled. If `StatusOverride` is equal to `Disabled` for the assigned variant, then the feature is disabled.
`StatusOverride` is an optional property of a variant that allows it to override whether a flag should be considered enabled or disabled. This gives variants an opportunity to extend the evaluation of a feature flag. If a caller is checking whether a flag that has variants is enabled, then variant allocation will be performed to see if an allocated variant is set up to override the result. By default, this property is set to `None`, which means the variant doesn't affect whether the flag is considered enabled or disabled. Setting `StatusOverride` to `Enabled` allows the variant, when chosen, to override a flag to be enabled. Setting `StatusOverride` to `Disabled` provides the opposite functionality, therefore disabling the flag when the variant is chosen.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On second thought. I might suggest rearranging this to first introduce the concept of a variants overriding the enabled state of a flag. Then go into the property that is used to do it.


```
"Allocation": {
"DefaultWhenEnabled": "OffVariant"
},
"Variants": [
{
"Name": "OffVariant",
"ConfigurationValue": {
"Size": 300
},
"StatusOverride": "Disabled"
}
],
"EnabledFor": [
{
"Name": "On"
}
]
```

In the above example, the feature is enabled by the `On` filter and assigns the variant set for `DefaultWhenEnabled`, which is the `OffVariant` variant. The `OffVariant` variant has the `StatusOverride` property set to `Disabled`, so calling `IsEnabledAsync` for this feature will return disabled, even though the feature would otherwise be enabled by its filters.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While nothing is wrong in this example, it's unclear what the StatusOverride is for. Can we have a more real-world example?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we do add an example for StatusOverride, we should make it very clear that it's a bandaid for a specific situation.

That situation is if you want to use allocation to control true and false without making code changes.

If you're writing a new app or adding new features, use variants with allocation. If you want simple true and false, you can achieve it with a targeting filter. If you want "variants" for your existing true and false, use the defaults.

Honestly, I prefer not having an example for StatusOverride as I don't want new customers grabbing the snippet.

@zhenlan zhenlan Sep 30, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There must be some misunderstanding. I can agree the StatusOverride is a bandaid fix (in terms of design) but it is NOT just for existing users who don't want to make code changes.

The allocation is more powerful than the targeting filter (see an example later). We will want our customers (existing or new) to adopt the allocation with variants. However, without StatusOverride, they will have to write everything like

if (await fm.GetVariantAsync("foo").Configuration["ConfigurationValue"] == "true") ... 
// or
if (await fm.GetVariantAsync("foo").Name == "On") ... 

instead of simply

if (await fm.IsEnabledAsync("foo"))...

Our whole MS.FM.AspNetCore package is essentially built on top of the IsEnabledAsync API. Without StatusOverride, that package can be thrown out of the window. We all know On/Off accounts for the majority of the real-world usage. This is why StatusOverride is important for both existing and new customers.

We should explain what the StatusOverride is for and why it's useful. Here is an example that you can't achieve with targeting, and is meaningful in the real world.

"Allocation": {
    "Percentile": [{
        "Variant": "On",
        "From": 10,
        "To": 20
    }],
    "DefaultWhenEnabled":  "Off",
    "Seed": "Black-Friday-Feature-Group"
},
"Variants": [
    { 
        "Name": "On", 
        "ConfigurationValue": true
    },
    { 
        "Name": "Off", 
        "ConfigurationValue": false,
        "StatusOverride": "Disabled"
    }    
],
"EnabledFor": [ 
    { 
        "Name": "AlwaysOn" 
    } 
] 

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've pretty much added this example to the StatusOverride section now


Comment thread
amerjusupovic marked this conversation as resolved.
## Caching

Feature state is provided by the IConfiguration system. Any caching and dynamic updating is expected to be handled by configuration providers. The feature manager asks IConfiguration for the latest value of a feature's state whenever a feature is checked to be enabled.
Expand Down