Allow selection and sentinel keys on feature flags

[RemyBoyer]

Hi everyone,

We are currently building a feature flag infrastructure for all our applications. We want to use AppConfiguration, AppConfiguration-DotnetProvider and FeatureManagement-Dotnet to store and check the feature flags on the applications. We'll build our own feature flag management app though, to provide advanced logic (custom filters, audit, authentication/authorization, more...) to our internal users.

Since we are in a micro-services environment, we would like to reuse a single Azure App Configuration to decrease the costs and, hopefully, the infrastructure complexity.

We would ideally like to use and watch feature flags the same way it's done with "classic" app configuration usage.

For example, with the SDK we can currently do this:

                options.Select("App:MyApp:*");

                options.Connect(new Uri(endpoint), credential).ConfigureRefresh(refreshOptions =>
                        {
                            refreshOptions.Register("App:MyAppSentinelKey", true);
                        });

Bu this can't be done use feature flags.

The main reasons we would like to do this would be:

• to reduce the number of feature flags that are scanned every 30s (or whatever the cache expiration is set to)

Indeed, putting aside the overload that it surely represents on the AppConfiguration instance, it would also require multiple calls for each multiple of 100 feature flags, because of paging. Since our instance will be used by many apps, we expect it to contains many feature flags. If we imagine the app containing 1000 feature flags, this would mean that each app has to make 10 calls every 30s just to check if something has changed. We would break the 20k max requests per hour with just 17 apps.
Of course, this could mitigated by increasing the cache expiration, but we'd loose reactivity, with doesn't seems to be a good tradeoff.

• to avoid downloading feature flags updated data that aren't used by the application.

Indeed, with the currently implemented feature flag configuration, the app watches all the feature flags. This means that when a feature flag used by app A is updated, it will be watched & downloaded by app B, even if it doesn't use it.

For these reasons, we'd like to be able to use explicit .Select and .Register, which will eliminate those two drawbacks by providing the ability to select only the feature flags that are useful to the app, and by using sentinel keys that will be automatically updated by our Feature Flags management app.

We actually managed to enable this by copying and adapting this method

AppConfiguration-DotnetProvider/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationOptions.cs

Line 138 in d19492d

public AzureAppConfigurationOptions UseFeatureFlags(Action<FeatureFlagOptions> configure = null)

into something like this:

    public static class AzureAppConfigurationOptionsExtensions
    {
        internal static readonly TimeSpan DefaultFeatureFlagsCacheExpiration = TimeSpan.FromSeconds(30);
        internal static readonly TimeSpan MinimumFeatureFlagsCacheExpiration = TimeSpan.FromMilliseconds(1000);

        public static AzureAppConfigurationOptions UseMyFeatureFlags(this AzureAppConfigurationOptions options, Action<MyFeatureFlagsOptions> configure)
        {
            // This code is identical
            var featureFlagOptions = new MyFeatureFlagsOptions();
            configure?.Invoke(featureFlagOptions);

            if (featureFlagOptions.CacheExpirationTime < MinimumFeatureFlagsCacheExpiration)
            {
                throw new ArgumentOutOfRangeException(nameof(featureFlagOptions.CacheExpirationTime), featureFlagOptions.CacheExpirationTime.TotalMilliseconds,
                    string.Format(ErrorMessages.CacheExpirationTimeTooShort, MinimumFeatureFlagsCacheExpiration.TotalMilliseconds));
            }

            // This is just a Reflection hack to add FeatureManagementKeyValueAdapter
            var adapters = typeof(AzureAppConfigurationOptions).GetField("_adapters", BindingFlags.NonPublic | BindingFlags.Instance).GetValue(options) as IList;

            var featureManagementKeyValueAdapter = Activator.CreateInstance(typeof(AzureAppConfigurationOptions).Assembly.GetType("Microsoft.Extensions.Configuration.AzureAppConfiguration.FeatureManagement.FeatureManagementKeyValueAdapter"), nonPublic: true);

            adapters.Add(featureManagementKeyValueAdapter);

            // This allows us to select only the flags that we use.
            foreach (var featureFlagKey in featureFlagOptions.FeatureFlags)
            {
                options.Select(FeatureManagementConstants.FeatureFlagMarker + featureFlagKey, featureFlagOptions.Label);
            }

            options.ConfigureRefresh(r =>
            {
                r.SetCacheExpiration(featureFlagOptions.CacheExpirationTime);
                // This allows us to use a sentinel key.
                r.Register(FeatureManagementConstants.FeatureFlagScopeKey + featureFlagOptions.Scope, featureFlagOptions.Label, true);
            });

            return options;
        }

    }

    internal class FeatureManagementConstants
    {
        public const string FeatureFlagScopeKey = ".appconfig.scope/";
        public const string FeatureFlagMarker = ".appconfig.featureflag.my/";
        public const string ContentType = "application/vnd.microsoft.appconfig.ff+json";
        public const string SectionName = "FeatureManagement";
        public const string EnabledFor = "EnabledFor";
    }

    public class MyFeatureFlagsOptions : FeatureFlagOptions
    {
        internal List<string> FeatureFlags { get; set; } = new List<string>();

        public string Scope { get; set; } = default!;

        // As you can see here, Feature Flags are opted-in
        public void Use(string feature)
        {
            FeatureFlags.Add(feature);
        }
    }

As you can see, this code isn't pretty, but quite simple.
Additionally to FeatureManagementKeyValueAdapter and _adapters which are respectively internal and private, there is one additional hack which consists of using a different prefix for the feature flags, that can be seen on FeatureManagementConstants.FeatureFlagMarker.
This is necessary because there is a check here that forces a full scanning during refresh if one of the watched key filter starts with .appconfig.featureflag/:

AppConfiguration-DotnetProvider/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs

Line 151 in d19492d

bool useDefaultQuery = !_options.KeyValueSelectors.Any(selector => !selector.KeyFilter.StartsWith(FeatureManagementConstants.FeatureFlagMarker));

By putting all of our feature flags under .appconfig.featureflag.my/, we workaround this check.

So my final questions would be:

• Would it be possible to implement AzureAppConfigurationOptions.Select and AzureAppConfigurationRefreshOptions.Register on feature flags usage? It could probably be additional properties/methods on FeatureFlagOptions, or perhaps another overload (or anything you could think of!).
• If not, could you please expose some private/internal dependencies, so we can implement it ourselves? This would include exposing some way of adding FeatureManagementKeyValueAdapter, and also, ideally, provide a way to disable the global scan on AzureAppConfigurationProvider.LoadAll.

[RemyBoyer]

Hi there.

Have you had any time to look into this request?

On further thoughts, we think that having access to the currently internal interface IKeyValueAdapter, plus a way to add a custom implementation and instance to AzureAppConfigurationOptions.Adapters could be just enough.

This way, we could even create a custom adapter (we have a few use cases for that) and add a new implementation without accessing any internal stuff.

If needed, we could provide a PR, even if this seems to be quite a minor code change, and is probably more of a design decision.

[abhilasharora]

@RemyBoyer I apologize this completely slipped through the cracks. Thanks for the detailed explanation of your scenario. I understand that applications may only want to retrieve the subset of feature flags that they need.

When using UseFeatureFlags, we currently configure all feature flags to be included in the initial configuration by calling Select and also register them for refresh. Here's one way to support this for a subset of feature flags, by adding an additional property FeatureFlagPrefix to FeatureFlagOptions. Would this meet the requirements for your feature flag management application?

var configuration = new ConfigurationBuilder()
    .AddAzureAppConfiguration(options =>
    {
        options.Connect(connectionString);

        options.UseFeatureFlags(featureFlagOptions =>
        {
            featureFlagOptions.FeatureFlagPrefix = "MyApp/*"; // Refers to ".appconfig.featureflag/MyApp/*"
            featureFlagOptions.Label = "MyLabel";
            featureFlagOptions.CacheExpirationInterval = cacheExpirationTimeSpan;
        });

        options.UseFeatureFlags(featureFlagOptions =>
        {
            featureFlagOptions.FeatureFlagPrefix = "Common/MyFeatureFlag"; // Refers to ".appconfig.featureflag/Common/MyFeatureFlag"
            featureFlagOptions.Label = "MyLabel";
            featureFlagOptions.CacheExpirationInterval = cacheExpirationTimeSpan;
        });
    })
    .Build();

[RemyBoyer]

@abhilasharora Thanks for your reply!

Unfortunately, I don't think it would be sufficient. Indeed, we have an N-N relationship between feature flags and what we call "scopes", which would in your case correspond to prefixes.

Here's an example:
We have two features called Feature1 and Feature2, and multiple applications that use scopes called ScopeA and ScopeB.

As long as ScopeA only contains Feature1 and ScopeB only containsFeature2, we could indeed use A and B as the prefixes. This would mean that we would have features called A/Feature1 and B/Feature2, and this way the selection of the feature flags would work (and I suppose the refresh would be segregated too, but I haven't tested it myself).

Unfortunately, as soon as we add the scope ScopeB to A/Feature1, it doesn't work anymore. To make this work would require that we duplicate A/Feature1 to B/Feature1, which we would like to avoid.

That's why in my proposed solution, scopes are not prefixes but rather refresh keys.
This decouples the application scope of a feature from the feature itself (either prefix or label). Since I know in my feature flag management app which scopes are applied to which feature flags, each time I change a feature flag, I increment the corresponding scope values. On the other side, for the running applications that use the feature flags, changing the scope value triggers the refresh for all registered feature flags, since it's the sentinel key. One app is meant to watch one and only one scope. But there are many apps and many scopes.

I hope this is clear 😅 Please let me know if you need additional information.

[RemyBoyer]

Hi @abhilasharora, have you had time to consider my response? After more than a year at building this as a side project when we had some free time at work, we are reaching the completion of our MVP, and need to know if this feature can make it to the App Configuration nuget. Having this feature would clearly simplify some of our code and configuration.

Thanks in advance!

[jimmyca15]

Hey @RemyBoyer I took a look at this and here's what I came up with. Mostly I'm close to what @abhilasharora suggested.

• to reduce the number of feature flags that are scanned every 30s (or whatever the cache expiration is set to)

I would recommend that you use the push model for refreshing configuration/feature flags. This is the best bet for scalability since no polling is needed; not even for sentinel keys. Documentation is in the works here: https://github.com/MicrosoftDocs/azure-docs-pr/pull/127339/files?short_path=a0a5347#diff-a0a534713c23d4e3ed6659ef5b36d60a79f74b15b072b482fadc19e60484c8a4

I took a look at your PR. Even if the polling model for refresh is used instead of push model, I don't think we need FeatureFlagOptions.RegisterRefreshKey. We can simply use the existing AzureAppConfigurationOptions.ConfigureRefresh to set up a sentinel key. When a sentinel key refreshes everything, that includes feature flags.

You mentioned the automatic paginated scanning of feature flags. We do need to provide a way to disable the automatic registration of feature flags for polling when UseFeatureFlags is called.

• to avoid downloading feature flags updated data that aren't used by the application.

When we use feature flags we pull all key-values with a given prefix ".appconfig.featureflag/". We also wire up a polling/scan refresh for these key-values. This paradigm of working with prefixes to use a set of feature flags has already been established. Adding a Select method which can pull an individual feature flag could add some confusion to how the selection of that single flag would work with the built in polling/scan refresh. If we stick with the idea of a prefix, all is well for selection and refresh.

---

Given the requests, having taken a look at your PR, and considering the current design, here is what I think we can do.

It is mostly what @abhilasharora suggested. Here though we register a few sentinel keys for your idea of "scopes". We also disable polling refresh for feature flags so that only the sentinel keys are checked.

var configuration = new ConfigurationBuilder()
    .AddAzureAppConfiguration(options =>
    {
        options.Connect(connectionString);

        options.ConfigureRefresh(refreshOptions =>
        {
            refreshOptions.Register("ScopeA", true); // A change in this will refresh everything, including feature flags.
            refreshOptions.Register("ScopeB", true);
        });

        options.UseFeatureFlags(featureFlagOptions =>
        {
            featureFlagOptions.Prefix = "MyApp/"; // NEW api // requests all feature flags that start with .appconfig.featureflag/MyApp/
            featureFlagOptions.Label = "MyLabel";
            featureFlagOptions.CacheExpirationInterval = cacheExpirationTimeSpan;
            featureFlagOptions.EnableRefresh = false; // NEW api // Disable the automatic behavior of checking all key-values starting with ".appconfig.featureflag/MyApp/" for changes when a refresh is performed.
        });

        options.UseFeatureFlags(featureFlagOptions =>
        {
            featureFlagOptions.Prefix = "Common/MyFeatureFlag"; // requests all feature flags that start with .appconfig.featureflag/Common/MyFeatureFlag as well as the single feature flag named .appconfig.featureflag/Common/MyFeatureFlag if it exists
            featureFlagOptions.Label = "MyLabel";
            featureFlagOptions.CacheExpirationInterval = cacheExpirationTimeSpan;
            featureFlagOptions.EnableRefresh = false;
        });

        //
        // Both feature flag registrations exist at the same time

    })
    .Build();

[RemyBoyer]

Hi @jimmyca15, thanks for your response.

I would recommend that you use the push model for refreshing configuration/feature flags. This is the best bet for scalability since no polling is needed; not even for sentinel keys. Documentation is in the works here: https://github.com/MicrosoftDocs/azure-docs-pr/pull/127339/files?short_path=a0a5347#diff-a0a534713c23d4e3ed6659ef5b36d60a79f74b15b072b482fadc19e60484c8a4

This is a very interesting capability. Unfortunately, I can't access this pull request. I'm a bit concerned by the engineering overhead that might be required for this push capability to work, though. If that overhead doesn't scale well (with the number of apps that will use the az app config), it might not be worth it. I guess it will be much clearer as soon as I have access to the documentation.

Regarding usage of the "prefix" that you suggest, I understand how it handles both the selection and the refresh. The issue we have is that it would not fit by default to our N-N relationship between the feature flags and the scopes (which are mostly groups of executable apps).
But, I suppose we could make it fit : if we use the scopes as prefixes, it will ease integration for the consuming apps. On the producer end, we would "just" have to copy the feature flags for every scope that it is applied to. It's not ideal, but it would indeed fix most of the concerns I had. We will still need to take care of what updates will imply as the multiple updates won't be ACID, but we can manage that.

With this denormalization, I think we could indeed use the code you've just showed! I'm updating the code to prepare for this usage.

I have a few questions:

• Are the Prefix and DisableRefresh already meant to work the way we described them? I suppose so, but I'm just wondering if they need adjustments.
• Do you know when they will both land on a (preview) nuget package?

[abhilasharora]

For the push model, the documentation can be found here. This is our recommended approach for scenarios where there are a large number of instances retrieving configuration from App Configuration, or if there are concerns about exceeding the rate limits.

For the poll model, the Prefix and EnableRefresh/DisableRefresh properties do not currently exist and are part of the suggested proposal for your use case. In case this is something that seems to be the best fit for your scenario, please let us know so we can include it in our plan for the next release. Based on that, we could assess when it would be available on NuGet.

[RemyBoyer]

@abhilasharora thanks for the documentation. We'll look into that

Regarding the Prefix and EnableRefresh, as said earlier, it fits our scenario with some changes on our side. If it fits the current designs and what you want to provide to others in the future, then I guess we have a good compromise.

I didn't understand that those proposals were "new", though. If you can add it to your roadmap, then please do so!

May we help on the implementation?

[jimmyca15]

@RemyBoyer we are looking into it and should be able to get to it ourselves.

I looked again at the proposal, and I don't think that DisableRefresh is needed at all. The scenario of preventing full-scan of feature flags should be covered by setting a prohibitively high cache expiration on the feature flag options. The feature flag cache expiration is separate from the sentinel key cache expiration, so there is no problem of "losing reactivity" as originally mentioned. This will save the addition of a new API, but the scenario should behave the same as if we had DisableRefresh.

[RemyBoyer]

@jimmyca15 I agree with you, having a very high expiration won't be that much different than having none at all.

The key part is the Prefix, where we can narrow the selection to specific "scopes". I imagine that adding the prefix will also trim the name? Like the TrimPrefix option does for non feature flags? This would keep the feature flag names as easy to use as possible.

[jimmyca15]

It wouldn't automatically, but adding a TrimPrefix option is possible, just like we have for normal settings.

One thing to consider when trimming prefixes is that if two feature flags have a matching name after trimming the prefix, their value can be indeterministic. Is this okay in your case?

[RemyBoyer]

Yes, it's ok in our case, since the feature flag names are guaranteed to be unique, as they are created with a specific app that enforces this. I guess it's how it works with the normal settings anyway?

[jimmyca15]

Yes.

[avanigupta]

@RemyBoyer, the feature has been released in package version 4.3.0-preview:

• Microsoft.Extensions.Configuration.AzureAppConfiguration
• Microsoft.Azure.AppConfiguration.AspNetCore

Please let us know if you have any feedback on the new functionality.

[RemyBoyer]

@avanigupta Great news, thank you! I'll check it out asap

[RemyBoyer]

@avanigupta, I've tested the changes I have a few feedbacks: one regarding performance and two other ones regarding usage.

For a bit of context, here's how I use the new and existing features to achieve the goal stated in the original request:

options.UseFeatureFlags(options =>
{
    options.Select($"{scope}/*");
    options.TrimFeatureFlagPrefix($".appconfig.featureflag/{scope}");

    options.CacheExpirationInterval = TimeSpan.FromDays(1); // something very long, as the sentinel does the job of invalidation
});

options.ConfigureRefresh(options =>
{
    options.Register($".appconfig.featureflag-scope/{scope}/.sentinel", true);
});

Loading issue

The issue with this is that despite selecting a specific prefix, the library still loads the entire AAC data.
The reason is that this code checks if the selector starts with the feature flag standard prefix:

https://github.com/Azure/AppConfiguration-DotnetProvider/blob/main/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs#L197

I suppose that this is not the intended behavior. As the original request was to avoid downloading extraneous feature flags, both for performance considerations, but also to "scope" feature flags, so that the consuming apps won't see flags that are not theirs.

I tried to fix this behavior and found two ways.
The first one is just to change the line above to an equality comparison with the default select:

bool useDefaultQuery = !_options.KeyValueSelectors.Any(selector => selector.KeyFilter != FeatureManagementConstants.FeatureFlagMarker + "*");

It's actually the proposal I made in my PR.

The second solution is to add a dummy Select(), so that useDefaultQuery resolves to false:

options.Select($"non-existing"); // options is AzureAppConfigurationOptions and not FeatureFlagOptions

Obviously, this trick doesn't seem ideal, but at least with this I can test the expected loading behavior, without having to change the library.

Select and TrimFeatureFlagPrefix feedback

As stated above, I use options.Select($"{scope}/*"); and options.TrimFeatureFlagPrefix($".appconfig.featureflag/{scope}"); to select and trim.

I think it's not ideal that one of them has the .appconfig.featureflag/ prefix and the other doesn't. It can add confusion. I can't add the prefix to Select because it gets added automatically. Maybe more logical approaches would be to add the prefix for Trim, or even add the prefixes on both only if they are not present?

Select and Label

I hit this exception because on our development env, we share a single AAC to simulate multiple application environments. The environments are represented as labels.

I had to switch to an environment prefix to bypass this exception.
On the long run, I though that the labels would be a good way to version the feature flags. This way, when a breaking change would appear, we could just copy all the existing feature flags to a new label that is identified by the version number. And then, the consuming apps could gradually migrate to this new label by updating our in-house nuget packages, that control the labels usage.

This limitation isn't a blocker, as we can fall back to more prefixes. But I wanted to share with you examples where, IMO, the usage of labels are preferable to prefixes. I don't know why this limitation exists, and maybe it's technical, but if it's to simplify "usage", I would rather prefer being able to do it.

That's it, thanks a lot for the work you've put into this !

[jimmyca15]

Thank you for the awesome feedback @RemyBoyer

Loading issue

The feedback is noted and we'll think about it a bit more but want to offer some details.

AAC provides settings and feature flags. By default, just calling AddAzureAppConfiguration pulls all settings. We didn't want the addition of UseFeatureFlags to automatically take away all the settings that were put into the application, that's why we still query all settings (implicit Select("*")) even if feature flags are being used.

In the past, we have suggested users who only want to query feature flags to have a structure like the following. It is pretty much the Select("non-existent") that you suggested.

builder.AddAzureAppConfiguration(o =>
{
    o.Select("_");

    o.UseFeatureFlags();
});

Select and TrimFeatureFlagPrefix feedback

This one is interesting. Are you creating your feature flags manually/programatically? Feature flag objects in AAC have an id property that is used as the name of the feature flag. When it is created through the AAC portal or CLI it doesn't start with ".appconfig.featureflag/". The library uses this id property as the name of the feature and the prefix is trimmed from it.

It seems that your feature flags have ids that start with ".appconfig.featureflag/" ?

Select and Label

I see. Thanks for letting us know you hit this.

A tricky part about querying across labels is that the same key can be retrieved from multiple labels. With a label filter of "*" the resulting key (in this case feature flag) that ends up in the applications configuration can be nondeterministic. This is not something that we want to introduce into the system. Our suggestion for composition of multiple labels is to be explicit through the use of multiple select invocations.

// Use 2 labels and label 2 always overrides label 1
o.Select("keyFilter*", "label1");
o.Select("keyFilter*", "label2");

[RemyBoyer]

Hi @jimmyca15, thank you for your response.

Loading issue

Ok, I understand your point, and wasn't aware of this intention.
I would just add that to me, Select() on FeatureFlagOptions should behave as much as possible like Select() on AzureAppConfigurationOptions.

But since I have a way of enforcing this, it's not a blocker.

Select and TrimFeatureFlagPrefix feedback

On this one, I misunderstood what TrimFeatureFlagPrefix does. I thought that it would trim from AAC key. It does not, it actually trims from the id.
Our feature flags are indeed created programmatically, but they are like the one you showed: the id doesn't contain any prefix. We didn't even add the scope prefix to it. This made me realize that right now, I don't even need to call TrimFeatureFlagPrefix. Calling it previously didn't trim anything.

So my new options are:

• don't change anything, don't call TrimFeatureFlagPrefix
• store the "path" to the feature flag, which includes our "scope", into the id

I don't think the second solution adds any value. Do you have an opinion on this?
If we keep the first option, I hope that TrimFeatureFlagPrefix will prove useful to anyone out there 😅

Select and Label

Oh, I didn't see that Select() accepts a label 😅
I've reverted to storing with labels, used this overload, and everything's working as well as before.

With these three updates, I now have something that's behaving as I intended it to! Thanks a lot for your detailed and swift response.

[jimmyca15]

don't change anything, don't call TrimFeatureFlagPrefix

If this works for you this is what I would opt for.

With these three updates, I now have something that's behaving as I intended it to! Thanks a lot for your detailed and swift response.

Thank you for the continuous, thorough feedback. We are happy to help. Please let us know if you find any further room for improvement.

[RemyBoyer]

Since you're asking @jimmyca15, there is another issue that's important to us: microsoft/FeatureManagement-Dotnet#28

The solution we have chosen right now to bypass it is to fork the FeatureManagement libs.
We would have prefered avoiding this. We will go back to the official libs if you provide this enhancement.

[jimmyca15]

@RemyBoyer
I see, for that enhancement it looks like we could not come to a solution that we agreed on at that point. We will need to revisit that.

Regarding this issue, since TrimFeatureFlagPrefix ended up not being necessary after being introduced in our preview API version 4.3.0-preview we will exclude it in our upcoming stable release.