microsoft / FeatureManagement-Dotnet

Microsoft.FeatureManagement provides standardized APIs for enabling feature flags within applications. Utilize this library to secure a consistent experience when developing applications that use patterns such as beta access, rollout, dark deployments, and more.
MIT License
1.05k stars 115 forks source link

Difference in behavior between .NET and Microsoft schemas with configuration merging #507

Closed CuddleBunny closed 3 days ago

CuddleBunny commented 1 month ago

This isn't necessarily an issue, but it should probably be called out somewhere: when switching from the .NET schema to the Microsoft schema you lose the ability to spread definitions across multiple configuration files (ie. appsettings.json and appsettings.Prod.json) because feature_management:feature_flags is an array which causes additional configuration sources to overwrite the original. This isn't the case with the .NET schema because the collection is an object and merges fine.

CuddleBunny commented 4 days ago

This bit me again today, so I'll illustrate further:

appsettings.json

{
  "feature_management": {
    "feature_flags: [
      {
        "id": "SameForAllEnvironmentsThing",
        "enabled": true
      },
      {
        "id": "SameForAllEnvironmentsThing2",
        "enabled": true
      }
    ]
  }
}

appsettings.Prod.json

{
  "feature_management": {
    "feature_flags: [
      {
        "id": "ProdOnlyThing",
        "enabled": true
      }
    ]
  }
}

After these are loaded into configuration, the resulting feature_flags array contains only the two items after merging when all three are expected:

[ { id = "ProdOnlyThing" }, { id = "SameForAllEnvironmentsThing2" } ]
rossgrambo commented 4 days ago

Hey @CuddleBunny, thank you for calling this out and following up on it.   It's unfortunate that this change to unify our libraries caused this to work differently.

The issue with Configuration merging is discussed here. Sadly, it's upstream of FeatureManagement- as this library reads whatever is made available by IConfiguration.

Theres two workarounds I'm aware of:

Define your own IFeatureDefinitionProvider

I know this could solve the issue if you manually read the files and merge them how you see appropriate. Our default implementation reads from IConfiguration but you don't have to in your version.

It might be possible to have your implementation call each JsonConfigurationProvider within the Configuration separately so you can handle the merge yourself and don't have to re-read these files. If this is possible and not too heavy it could be something we consider adding to the default IFeatureDefintionProvider. But I think that's unlikely because there's not one way to merge that works for everyone.

:# syntax (Not recommended)

Using :# to override arrays appropriately. When defining an array in json to be used in IConfiguration, you can define it like:

  "feature_management": {
    "feature_flags:0": {
      "id": "OnTestFeature",
      "enabled": true
    }
  }

In your example, you could define your appsettings.prod.json as

{
  "feature_management": {
    "feature_flags:2": {
      "id": "ProdOnlyThing",
      "enabled": true
    }
  }
}

Unfortunately, this is finicky to line things up when you're trying to override correctly.

If your only goal is to merge I believe you start at a large number (like :1000) in all of your prod flags and they would always append to the end of the other flags. This is not an intended feature of our library so there's no guarantee that it'll continue to work but it seems to work for me at first glance.

CuddleBunny commented 3 days ago

I've considered the IFeatureDefinitionProvider route already and will likely take that path. We should close this issue, but it could probably be called out in the docs somewhere as a possible gotcha, especially because the default dotnet templates have configuration per environment built in so it's widely used.