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

Using Label prevents cache expiration #30

Closed
millicandavid opened this issue Dec 3, 2019 · 17 comments
Closed

Using Label prevents cache expiration #30

millicandavid opened this issue Dec 3, 2019 · 17 comments

Comments

@millicandavid
Copy link

I never see updates reflected if I try to apply a Label filter. Simply commenting out the setting of Label in the code below restores cache expiration and I see updates again but any time Label is set I only get the initial value.

var env = hostingContext.HostingEnvironment.EnvironmentName; config.AddAzureAppConfiguration(options => options .Connect(connectionString) .Select(KeyFilter.Any, LabelFilter.Null) .Select(KeyFilter.Any, env) .UseFeatureFlags(options => { options.CacheExpirationTime = TimeSpan.FromSeconds(5); options.Label = env; }) );

@millicandavid
Copy link
Author

Actually, this only works at all in build 1251. In build 1263 I never get updates regardless of setting options or not setting options at all. So today, I'm using 1251 and adding an environment string to different flag keys rather than applying Labels to existing flag keys the way I can for Azure App Configuration.

@jimmyca15
Copy link
Member

@millicandavid do you mean to say that you're app is unable to detect feature flag changes when using the Azure App Configuration configuration provider? Did you recently update the Microsoft.Extensions.Configuration.AzureAppConfiguration version that your application was referencing?

I'm unable to reproduce your issue. Can you give me some info to help out?

  • What runtime did you hit this issue with? (netcoreapp2.1, netcoreapp3.0)
  • What version of Microsoft.Extensions.Configuration.AzureAppConfiguration are you referencing?
  • What did you do to test this out?

@millicandavid
Copy link
Author

millicandavid commented Dec 3, 2019

Yes. Using Azure App Configuration with UseFeatureFlags as shown in the code above, I get no updates when using build 1263 regardless of supplying any options or not. Using build 1251, I could only get updates when I did NOT supply a value for options.Label. I got updates otherwise.

I have no reference to Microsoft.Extensions.Configuration.AzureAppConfiguration. I am using Microsoft.Azure.AppConfiguration.AspNetCore as was shown in the Quickstart I believe.

What runtime did you hit this issue with? netcoreapp3.0
What version of Microsoft.Extensions.Configuration.AzureAppConfiguration are you referencing?
Microsoft.Azure.AppConfiguration.AspNetCore (3.0.0-preview-010560002-1165)
What did you do to test this out?
I have a simple service endpoint that just accepts a feature name as a parameter in the route and calls _featureManager.IsEnabled(featureName) and returns the value. I started my service and made the call using "Beta" for instance. It was off so it returns false as expected. I change the value in the Azure Portal's App Config Feature Manager. Wait 7s(CacheExpirationTime was set to 5s), and make the call again. There was no change. When it is working, sometimes a second request is required to get the expected result but I can live with that. The problem is that if I want to use Label to distinguish values for Development, Staging, Production, the updates never occur no matter how long I wait or how many requests are made. However, I was NEVER getting hot updates using build 1263. I had to restart the service to see any changes.

I'm going to check out Microsoft.Extensions.Configuration.AzureAppConfiguration to see if that package resolves my issue. I'll update here if it does.

@millicandavid
Copy link
Author

Something else I observed was that I had a feature with a Label and no Label config in my app setup. But the value that was being returned was that of the Labeled feature and not the feature without the Label. Sorry if that's confusing. I have a Beta feature with no Label and value of false. Added a Label of Staging with value of true. I was getting true returned when I did not specify Label in UseFeatureFlag options.

@millicandavid
Copy link
Author

using Microsoft.Extensions.Configuration.AzureAppConfiguration did NOT seem to resolve my issue.

@jimmyca15
Copy link
Member

Okay it sounds like this issue is really two issues.

  1. Microsoft.FeatureManagement: 2.0.0-preview-010610001-1263 is not recognizing changes in the configuration system.
  2. When referencing Microsoft.FeatureManagement: 1.0.0-preview-009000001-1251 and Microsoft.Azure.AppConfiguration.AspNetCore and using label to pull settings from Azure App Configuration, feature flag udpates are not being respected.

For issue 1 this could be related to a caching strategy we added in the 2.0.0-preview package to improve performance. The feature management library relies on the root IConfiguration being present in the dependency injection container for the dynamic values to be updated. In most applications this is done completely unnoticed, however it could get messed up if IConfiguration is being added in an unexpected way. Are you handling a ConfigurationBuilder yourself and adding it into the service collection? Or perhaps calling services.Add(IConfiguration) somewhere?

The sample app here shows a working example of using dynamic feature flags with Azure App Configuration. Configuration is wired up in Program.cs like in the Azure App Configuration quick starts.

For issue 2 I will have to investigate that a bit further and get back to you.

@millicandavid
Copy link
Author

@jimmyca15 First, thanks for looking into this. I really do appreciate your effort here. And yes, I believe there are two different issues.

Below is the relevant portion of my initialization code. I'm not sure that I'd consider this unexpected but may not be as conventional. I'm not explicitly adding IConfiguration to the ServiceCollection. Just using WebHostBuilder.ConfigureAppConfiguration.

	public static IHostBuilder CreateHostBuilder(string[] args)
            => Host.CreateDefaultBuilder(args).ConfigureWebHostDefaults(webBuilder =>
            {
                webBuilder.ConfigureAppConfiguration((hostingContext, config) =>
                {
                    var connectionString = ....;

                    var env = hostingContext.HostingEnvironment.EnvironmentName;
                    config.AddAzureAppConfiguration(options => options
                        .Connect(connectionString)
                        .Select(KeyFilter.Any, LabelFilter.Null)
                        .Select(KeyFilter.Any, env)
                        .UseFeatureFlags(options =>
                        {
                            options.CacheExpirationTime = TimeSpan.FromSeconds(5);
                            //options.Label = env; //commented out as this seems to break updates in 1.0.0-preview-009000001-1251
                        })
                    ); 
                });
                webBuilder.ConfigureServices(services =>
                {
                    ...
                    services.AddFeatureManagement();
                    ...
                });
                webBuilder.Configure(app =>
                {
                    ...
                    app.UseAzureAppConfiguration();
                    ...
                });
                webBuilder.UseLogging();
            });

@jimmyca15
Copy link
Member

@millicandavid thanks for providing the code snippet. I still am unable to repro this issue. I put together a repro project which can be seen here: https://github.com/jimmyca15/FM-Repro. It uses the setup that you provided and everything is working.

In your setup is dynamic configuration working from Azure App Configuration? This means that you can modify settings in Azure App Configuration and then see the new values in IConfiguration. The feature management library relies on the underlying IConfiguration being updated to see feature changes.

@dnak
Copy link

dnak commented Dec 5, 2019

I'm seeing this same issue. If I filter with Labels, I have to restart my app to see configuration changes. I'm targeting netcoreapp2.2.

I'm referencing:

  • Microsoft.FeatureManagement 2.0.0-preview-010610001-1263
  • Microsoft.Azure.AppConfiguration.AspNetCore 2.1.0-preview-010380003-1338

Here's my configuration builder code:

configurationBuilder.AddAzureAppConfiguration(o =>
                                                    {
                                                      o.Connect(appConfigConnectionString)
                                                       .Use(KeyFilter.Any, options.EnvironmentName)
                                                       .UseFeatureFlags(flagOptions => { flagOptions.CacheExpirationTime = TimeSpan.FromSeconds(60); });
                                                    });

If I change my configuration code to not filter on label and then remove the labels from my flags, then I do receive hot updates.

Anyways, I was trying to troubleshoot a bit and was inspecting the FeatureManager instance at runtime to see if I could figure out what I was misconfiguring. I don't know much yet about how this library works, but it looked like there is a MultiKeyWatcher registered for flags without labels, but there isn't one that is registered that would see flags with labels. Does this have something to do with it?

image

@jimmyca15
Copy link
Member

@dnak thanks for the write-up. This looks like a bug in the Microsoft.Extensions.Configuration.AzureAppConfiguration package (referenced indirectly through Microsoft.Azure.AppConfiguration.AspNetCore). We are currently investigating to repro.

@jimmyca15
Copy link
Member

@dnak I just realized that you are not specifying the label for your feature flag options. If you want the feature flags to use a certain label you need to put that in the feature flag options. The configuration selector and feature flag selector are independent. They can be used separate or together.

so it would be like

configurationBuilder.AddAzureAppConfiguration(o => {
    o.Connect(appConfigConnectionString)
     .Use(KeyFilter.Any, options.EnvironmentName)
     .UseFeatureFlags(flagOptions => {
         flagOptions.Label = options.EnvironmentName;
         flagOptions.CacheExpirationTime = TimeSpan.FromSeconds(60);
     });
});

I misread at first and thought that you were experiencing the problem that millicandavid mentioned where even when he was specifying the label in the flag options that he was not getting updates. However, we cannot reproduce that issue.

Let me know if this helps. Thanks.

@millicandavid
Copy link
Author

Using both still resulted in no hot updates for me

@jimmyca15
Copy link
Member

@millicandavid

I am thinking that you are mismatching the label in your Azure App Configuration with the label you are specifying in your application. They must match exactly. If you are specifying a label in your feature flag options, your feature flags in Azure App Configuration must be created with that label. Conversely* if the feature flags in Azure App Configuration are created without a label, then specifying a label in the feature flag options of the application will cause them not to be observed.

@millicandavid
Copy link
Author

If you look at the code I listed previously, you can see that I'm using the same variable for both just as you are in your example above.

@jimmyca15
Copy link
Member

@millicandavid I mean the label must be the same as the label for the feature flag being stored in Azure App Configuration. That can't be verified by looking at the code.

@dnak
Copy link

dnak commented Dec 13, 2019

@jimmyca15 - Thanks! That did it. I've set the Label on the flagOptions and I've also ensured that all of my flags in the Azure App Configuration portal have labels attached. After doing both these steps, I'm receiving hot updates.

@jimmyca15
Copy link
Member

@millicandavid I was never able to repro this and it looks like the conversation dropped off. Please re-open if you are still experiencing an issue.

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

3 participants