[WIP] add variant allocation context option

[CuddleBunny]

This is quick and dirty, but I needed it for a POC project and thought getting it some attention would be helpful. I still need to add unit tests. Resolves #460.

Usage:

{
  "id": "TestVariant",
  "enabled": true,
  "allocation": {
    "default_when_enabled": "Default",
    "client_filters": [
      {
        "name": "TestFilter",
        "variant": "V1",
        "parameters": {
          "SomeParam": "SomeValue"
        }
      },
      {
        "name": "TestFilter",
        "variant": "V2",
        "parameters": {
          "SomeParam": "SomeValue2"
        }
      }
    ]
  },
  "variants": [
    {
      "name": "Default",
      "configuration_value": "Default"
    },
    {
      "name": "V1",
      "configuration_value": "Variant 1"
    },
    {
      "name": "V2",
      "configuration_value": "Variant 2"
    }
  ]
}

var test = await variantFeatureManager.GetVariantAsync("TestVariant", new TestFilterContext { SomeParam = "SomeValue" }); 
// test.Configuration.Value == "Variant 1"

[rossgrambo]

Thank you for the PR!

We've looked into this a few times and haven't aligned on a solution here. I'll add some of my notes here. We considered exactly this design you have here. If I recall correctly, the biggest argument against it was the idea of "stability" for allocation. In short, the idea is:

A given TargetingContext should always allocate in the exact same way.

This is a new concept that filters don't do. Filters break this hard rule, because a filter is free to run any custom code it wants and might simply flip its results on each run. This stability is important- as it gives us new guarantees on users who are assigned variants (we know what they got and what they'll always get).

The design I liked a little more was keeping the filters outside, but allowing variant shortcutting. Having the logic be "filters" -> if no variant -> "allocation" Like:

{
  "id": "TestVariant",
  "enabled": true,
  "conditions": {
    "client_filters": [
      {
        "name": "TestFilter",
        "variant": "V1",
        "parameters": {
          "SomeParam": "SomeValue"
        }
      },
      {
        "name": "TestFilter",
        "variant": "V2",
        "parameters": {
          "SomeParam": "SomeValue2"
        }
      }
    ]
  },
  "variants": [
    {
      "name": "Default",
      "configuration_value": "Default"
    },
    {
      "name": "V1",
      "configuration_value": "Variant 1"
    },
    {
      "name": "V2",
      "configuration_value": "Variant 2"
    }
  ]
}

The pushback to this design is two main items:

1. Adding variants here affects the IsEnabled call that normally checks these filters. Allocation only runs if IsEnabled returns true (aka- a filter returns true or there are no filters). Adding variant override filters here makes running allocation complicated- the logic might look like: If a filter with no variant returns true or all filters have variants and are just overrides or no filters are defined.
2. Variants are no longer specific to "allocation" and no longer have the predictability guarantee.

[rossgrambo]

Either way- I think this PR is valuable. Both as a place to discuss and a resource for others looking to do the same thing.

[CuddleBunny]

@rossgrambo - I think I like the short circuit option more actually. It's similar to a few other systems I've used before. Some thoughts:

A given TargetingContext should always allocate in the exact same way.

Makes sense, in this PR if you provide a context that implements ITargetingContext it will not run the filters, though this is an unintentional side effect of the rushed code rather than something I did on purpose.

This stability is important- as it gives us new guarantees on users who are assigned variants (we know what they got and what they'll always get).

I think it's fair that if a developer writes custom filters, they're also on the hook for maintaining that consistency (or choosing not to). This could be particularly relevant for third-party IFeatureDefinitionProvider implementations that might save assigned variants to a database.

Adding variant override filters here makes running allocation complicated- the logic might look like: If a filter with no variant returns true or all filters have variants and are just overrides or no filters are defined.

I ran into this as well, while it pained me to duplicate much of the filter evaluation code, I couldn't think of a clean way to keep the Any vs All paradigm. It made the most sense that the first matching filter would win but this might not be desirable in all scenarios. I do believe that DefaultWhenEnabled|Disabled is flexible enough to handle the case where a filter does not assign a variant directly.

Variants are no longer specific to "allocation" and no longer have the predictability guarantee.

Just wild spit balling here, but what if there was no allocation section and we simply relied on Microsoft.Targeting filters instead? This would take care of the "no filters are defined" scenario because you'd always need at least one. Probably too much churn this late in the game though.

You do lose the ability to enable or disable variants with the filters with this approach, but unlike boolean flags it's more likely that the variant's filters would be more exhaustive. From a glance this also seems similar to how OpenFeature works, but I've never used that: https://openfeature.dev/specification/glossary#evaluating-flag-values