Skip to content

Merge Microsoft feature flags #551

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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

zhiyuanliang-ms
Copy link
Contributor

@zhiyuanliang-ms zhiyuanliang-ms commented Aug 6, 2025

Why this PR?

#507

There is a well-known pitfall when merging array value in configuration system in .NET.

We presented a solution in v4.2.1: concatenate feature flag arrays and let the last feature definition win.

However, a customer raised a concern that this behavior change will break their current usage. The customer’s team distributed a base appsettings file in the docker image. Developers just want to toggle the feature flag and what they do is to use another configuration source and update the “enabled” field for certain feature flags.

Base appsettings.json:

{
  "feature_management": {
    "feature_flags": [
      {
        "id": "FeatureA",
        "enabled": true,
        "conditions": {
          "client_filters": [ ... ]
        }
      },
      {
        "id": "FeatureB",
        "enabled": false,
        "variants": [ ... ],
        "allocation": { ... }
      }
    ]
  }
}

People may want to use the following appsetting.development.json file to toggle the “FeatureB” flag:

{
  "feature_management": {
    "feature_flags": [
      {
        "id": "FeatureA",
        "enabled": true
      },
      {
        "id": "FeatureB",
        "enabled": true // toggle "FeatureB"
      }
    ]
  }
}

However, in 4.2.1, our concat ff arrays behavior will produce the following configuration:

{
  "feature_management": {
    "feature_flags": [
      {
        "id": "FeatureA",
        "enabled": true,
        "conditions": {
          "client_filters": [ ... ]
        }
      },
      {
        "id": "FeatureB",
        "enabled": false,
        "variants": [ ... ],
        "allocation": { ... }
      },
      {
        "id": "FeatureA",
        "enabled": true
      },
      {
        "id": "FeatureB",
        "enabled": true
      }
    ]
  }
}

The merge result that customer wanted

{
  "feature_management": {
    "feature_flags": [
      {
        "id": "FeatureA",
        "enabled": true,
        "conditions": {
          "client_filters": [ ... ]
        }
      },
      {
        "id": "FeatureB",
        "enabled": true, // toggled
        "variants": [ ... ],
        "allocation": { ... }
      }
    ]
  }
}

In addition, some customers may have the following usage:
appsettings.json

{
    "feature_management": {
        "feature_flags": [
            {
                "id": "FeatureA",
                "enabled": true,
                "variants": [ ... ],
                "allocation": { ... }
            }
        ]
    }
}

appsettings.Production.json

{
    "feature_management": {
        "feature_flags": [
            {
                "id": "FeatureA",
                 // enabled/variant/allocation field is not specified
                "conditions": {  // add condition for production
                    "client_filters": [
                        {
                            "name": "FeatureFilterA"
                        }
                    ]
                }
            }
        ]
    }
}

In short, what people is a layering-manner merge behavior.

Visible Changes

  • This PR is built on Merge feature flags from different configuration source #536 and Bug fix - Respect root configuration fallback #547. The ConfigurationFeatureDefinitionProvider will not only read feature_flags array from different ConfigurationProvider, but also it will maintain a dictionary where key is feature name and value is List<IConfigurationSection> that contains feature definition from different configuration source. In this case, ConfigurationFeatureDefinitionProvider can merge them in an intuitive manner:

  • The later feature definition will override the previous feature defintion. The granularity of the override behavior is at the property level within the FeatureDefinition class. For example:

    "feature_flags": [
        {
            "id": "FeatureA",
            "enabled": false,
            "variants": [
                {
                    "name": "Variant1",
                    "configuration_value": "Value1"
                },
                {
                    "name": "Variant2",
                    "configuration_value": "Value2"
                }
            ]
        },
        {
            "id": "FeatureA",
            "enabled": true,
            "variants": [
                {
                    "name": "Variant2",
                    "configuration_value": "Value2-updated"
                },
                {
                    "name": "Variant3",
                    "configuration_value": "Value3"
                },
            ],
            "conditions": {
                "client_filters": [...]
            }
        }
    ]

    The above configuration will produce such a feature definition after merging:

    {
        "id": "FeatureA",
        "enabled": true,
        "variants": [
            {
                "name": "Variant1",
                "configuration_value": "Value1"
            },
            {
                "name": "Variant2",
                "configuration_value": "Value2-updated"
            },
            {
                "name": "Variant3",
                "configuration_value": "Value3"
            }
        ],
        "conditions": {
            "client_filters": [...]
        }
    }

    For more information, please go to testcases.

  • A lock is used in the EnsureInit method.

@zhiyuanliang-ms zhiyuanliang-ms changed the title Merge microsoft feature flag Merge Microsoft feature flags Aug 6, 2025
@juniwang
Copy link

juniwang commented Aug 6, 2025

If the scenarios described in the samples(add condition for production, new variant Variant3 added after merging) are reasonable, it also makes sense to remove condition/variant after merging, which is not supported by the new proposal, but supported by the current impl of overriding the whole FF.

@zhiyuanliang-ms
Copy link
Contributor Author

it also makes sense to remove condition/variant after merging

This is against the normal pattern of native .net configuration system. What our customers want is the native .net configuration behavior of layering configurations.

The solution we offered in 4.2.1 is more like that we proposed a new way to tackle the issue of merging array values. But before we present the solution, some customers they have their own solutions which utilized the native .net configuration layering behavior. So we want to improve our solution based on real customers' usage.

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

Successfully merging this pull request may close these issues.

2 participants