Azure / AppConfiguration-DotnetProvider

The .NET Standard configuration provider for Azure App Configuration
https://github.com/Azure/AppConfiguration
MIT License
76 stars 32 forks source link

Should .NET provider detect duplicated feature flags? #542

Open zhiyuanliang-ms opened 2 months ago

zhiyuanliang-ms commented 2 months ago

Currently, our portal allows users to declare two feature flags with the same name. Note that the keys of two feature flags are different.

image

The provider will use the id property from the json value of the feature flag key-value as the path where to put the feature flag to the configuration. https://github.com/Azure/AppConfiguration-DotnetProvider/blob/main/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureManagementKeyValueAdapter.cs#L44

This will cause duplicated properties in the feature management configuration. These feature flags are different on server side, but on the client side, only the first one will be used by the feature management.

Now we have display_name and id in MicrosoftFeatureFlagSchema v2: https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/appconfiguration/Azure.Data.AppConfiguration/src/Models/FeatureFlagConfigurationSetting.cs#L36

The portal doesn't set the display_name for feature flag. In my mind, the id should match the key. And the display_name should be the name displayed on the portal. Portal should not allow ids of feature flags to be duplicated. If portal works as this, then our .NET provider would be fine.

I found that portal has updated the json schema: image So I guess this could be considered as a bug of portal.

I didn't find display_name here But I think it makes sense since we will only use id. Or should it also be put in the configuration?

zhenlan commented 2 months ago

First, let me explain why it is the way it is. Then we can discuss options.

The design of the key is to allow users to add namespaces for feature flags. For example, you may have two different apps and they both use a feature flag that happens to have the same name.

Name Key
Beta .appconfig.featureflag/App1/Beta
Beta .appconfig.featureflag/App2/Beta

Then in the code of your app, you can load feature flags that are relevant to your app.

.UseFeatureFlags(featureFlagOptions =>
           {
               featureFlagOptions.Select("App1/*");
           });

What you observed is expected. Our .NET FM library always takes the first one that matches the feature flag id.

As far as the naming goes, if we want the consistency, we can rename "Name" to "Id" in the Feature Manager portal. We also need to update the create/edit view and rename "Feature flag name" to "Feature flag Id". This is open for discussion.

Regarding the "display_name", it's an optional field and we don't use it. We could have dropped it if we started over, but it's already part of the SDK, so it's harder to drop it now.