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

(feature) Support for Custom Assignment/Targeting Keys #2135

Closed
thomaspoignant opened this issue Jul 24, 2024 · 6 comments · Fixed by #2218
Closed

(feature) Support for Custom Assignment/Targeting Keys #2135

thomaspoignant opened this issue Jul 24, 2024 · 6 comments · Fixed by #2218
Assignees
Labels
enhancement New feature or request p2 Medium priority

Comments

@thomaspoignant
Copy link
Owner

thomaspoignant commented Jul 24, 2024

Description

This feature was originally asked by Ricardo Tomasi in this slack conversation.

This feature request proposes to do the percentages rollout based on another field than the targeting key.
This would allow for greater flexibility in defining target audiences for feature flags, specifically facilitating scenarios where flags need to be assigned based on attributes other than the user ID.

Current Behavior:

Currently, GO Feature Flagassign variations are based on the targetingKey by default. This presents limitations when targeting needs to be based on other attributes, like team ID.

This can cause some limitations, especially when using a client-side SDK that uses a single evaluation context to evaluate all the flags and it is not possible to change it per flags.

Note

In a percentage split the way we affect users to the variation is something like

(targetingKey + flag name) = 100000

// depending on the percentage we have buckets like this
| variation A (0..5000) | variation B (5000...40000) | variation C (40000...100000) |

Desired Behavior:

Dynamic Targeting Key: Introduce a new field within the feature flag configuration to allow choosing which field from the evaluation context will be used to do the variation affectation in a percentage split of the traffic.

Proposed solution

To support this feature we can add a new field in the rules called bucketingKey that will allow to specify which key from the evaluation context will be used to split the traffic.

It will look like this:

my-flag:
  variation:
     enabled: true
     disabled: false
  defaultRule:
     percentage:
       enabled: 10
       disabled: 90
     bucketingKey: teamId

Now when evaluating a flag with an evaluation context looking like this:

{
  "targetingKey": "04b86a91-101d-4b16-87c7-ecb3af38a869",
  "email": "[email protected]"
  "teamId": 156
}

Instead of affecting like this :

(targetingKey + flag name) = 100000

We will do

(teamId + flag name) = 100000

⚠️ If for any reason the you bucketingKey is not available in the evaluation context, we will use the targetingKey as a fallback method to affect the right variation.

@thomaspoignant thomaspoignant added enhancement New feature or request needs-triage A priority should be added to the issue p2 Medium priority and removed needs-triage A priority should be added to the issue labels Jul 24, 2024
@ricardobeat
Copy link
Contributor

ricardobeat commented Aug 8, 2024

Hi @thomaspoignant! Thanks for taking this suggestion forward.

Would it make sense to put bucketingKey at the root, since it would apply to all targeting rules?

As for the fallback behaviour, it would be desirable that assignment fails / returns the default rule when the bucketingKey is not found. Falling back to targetingKey would potentially expose users to multiple variants and invalidate the test results.

@thomaspoignant
Copy link
Owner Author

thomaspoignant commented Aug 8, 2024

@ricardobeat Yes, it's a great idea to put the bucketingKey on top level.
The problem of failing to the default is that the default may need the bucketingKey too.

For example in this example:

my-flag:
  variation:
     enabled: true
     disabled: false
  defaultRule:
     percentage:
       enabled: 10
       disabled: 90
  bucketingKey: teamId

You can see that the defaultRule is percentage-based, and we need a key to do the split of people.
This is why I propose to failover the targetingKey.

@ricardobeat
Copy link
Contributor

ricardobeat commented Aug 9, 2024

You can see that the defaultRule is percentage-based, and we need a key to do the split of people.

I see your point, but in that case, the correct action would be to return the default (off) value for the flag. Assigning variants based on another key (targetingKey, most likely a user ID) would return unexpected results. We might need a separate defaultVariation as well to achieve this.

Say you want bucketing per team, with bucketingKey: teamId, because your feature needs to be consistent across people working on a shared project. Falling back to the base targetingKey will distribute the feature randomly across users instead, making it inconsistent within a team; now we have both a user experience problem, and an analytics one: if you are running an A/B test, your metrics for the experiment will be reliant on the bucketing happening based on that specific key (team-level metrics vs user metrics).

In fact, the targetingKey becomes redundant once a custom bucketing key is defined - or the bucketingKey value replaces it. Does that make sense?

@ricardobeat
Copy link
Contributor

In the same line of thought, when using bucketingKey I would assume key refers to it, and not the original targetingKey:

my-flag:
  bucketingKey: teamId
  variation:
     A: false
     B: true
  targeting:
    - name: 'Teams in beta program'
      query: key in [3338, 1337]
      variation: B
  defaultRule:
     variation: A

@ricardobeat
Copy link
Contributor

ricardobeat commented Aug 13, 2024

@thomaspoignant I've made a draft PR implementing this to see what it looks like, curious to hear your thoughts.

I'm not sure about the naming: something like targetingKeyFrom / customKey seems more descriptive since the usual targeting key is also a bucketing key?

@thomaspoignant
Copy link
Owner Author

thomaspoignant commented Aug 15, 2024

In the same line of thought, when using bucketingKey I would assume key refers to it, and not the original targetingKey:

my-flag:
  bucketingKey: teamId
  variation:
     A: false
     B: true
  targeting:
    - name: 'Teams in beta program'
      query: key in [3338, 1337]
      variation: B
  defaultRule:
     variation: A

I would avoid to use key for the new bucketing key.
It is more explicit to use in your targeting rules the name of the actual key than any alias.

I found it easier to understand to have something like this:

my-flag:
  bucketingKey: teamId
  variation:
     A: false
     B: true
  targeting:
    - name: 'Teams in beta program'
      query: teamId in [3338, 1337] # <-- more explicit to use the actual key from the context (and the split will still happen based on teamId)
      variation: B
  defaultRule:
     variation: A

I'm not sure about the naming: something like targetingKeyFrom / customKey seems more descriptive since the usual targeting key is also a bucketing key?

I kinda like bucketingKey because this is what it is.
In the documentation, we can explain it by saying something like

bucketingKey is the name of the evaluation context field used to split the traffic in your percentage-based rules.
If not set we are using by default the targetingKey.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request p2 Medium priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants