flagsmith
flagsmith copied to clipboard
Flagsmith
Segment percentage split evaluation is not consistent between Core API and local evaluation
Describe the bug
Identities are evaluated differently for segments containing a percentage split rule when evaluated against the Core API versus local evaluation mode in SDKs.
To Reproduce
Steps to reproduce the behavior:
- Run the Core API (e.g. self hosted)
- Create a segment with a percentage split rule of 50% and add a segment override to a basic feature in a given environment
- Instantiate two instances of a client (e.g. Node.js), one running in local evaluation mode and one in remote
- Evaluate the flag for multiple identities in both instances of the client and compare the results
- Notice that the state of the flag can differ for certain identities between local and remote evaluation
Expected behavior
The result is the same for both local and remote evaluation for all identities
Screenshots
N/a
How are you running Flagsmith?
- [x] Self Hosted with Docker
- [x] Self Hosted with Kubernetes
- [x] SaaS at flagsmith.com (only for Core projects)
- [ ] Some other way
Additional context
This will affect the following clients when running in local evaluation mode:
- [x] PHP (Matt)
- [x] Node.js (Matt)
- [x] Java (Kim)
- [x] Elixir (Matt)
- [x] .NET (Kim)
- [x] Go (Kim)
Note that python, ruby, rust are not affected as they will use the django id if it exists.
To fix this, we have a couple of options:
- Change the SDKs listed above to use the django id if present
- Change the API to use the composite key and change those that are 'not affected' to always use the composite key as well
Option (1) is by far the simplest option but is slightly less clean than option 2. It also somewhat diverges from the logic that was implemented for MV V2 evaluations (details of which can be found here)
I'm in favour of 2. FWIW it's an API change + 3 SDKs vs 6 SDKs so simplicity of Option (1) isn't that apparent for me?
The ability to major version the SDKs is much easier than the API, however and I would suggest that a change in behaviour like this would require a major version - or an approach similar to the one done for #1882 where users need to opt in to the consistent evaluation on the API.
If the some of the other languages are already doing option 1 I think lets stick with 1? This avoids API versioning and makes the SDKs consistent with each other?
Node.js PR: https://github.com/Flagsmith/flagsmith-nodejs-client/pull/119
I've just realised that using the DjangoId attribute is not a valid solution to this issue since the identity information from the Core API is, in fact, never passed to the SDKs, particularly in local evaluation mode.
More research needed into the scope of the issue and a new solution.
The only solution I can see here is to ensure that all points at which percentage split evaluations are carried out, we use the composite key since this is the only relevant data that all evaluation engines will always have.
To achieve this, we should also have a setting on the environment which changes to use the composite key across the board. Essentially replicating the logic we already have for use_mv_v2_evaluation.
To confirm, we only need to make this change in the Core API. All the clients are now consistent that they use the Django ID if it exists, otherwise, they use the composite key.
Latest update: percentage split is still prioritising the django_id in our Core API, even if using consistent hashing is enabled. Since v2.85.0 we are using the flagsmith engine for evaluating segments and hence will need to make the change in the engine to verify if the environment is configured to use consistent hashing.