flipt icon indicating copy to clipboard operation
flipt copied to clipboard

fix(eval): flag match

Open anthonpetrow opened this issue 1 year ago • 3 comments

if the index does not match the length of the distribution, then we will get a flag mismatch, although this should not have affected the flag match

anthonpetrow avatar Apr 12 '24 06:04 anthonpetrow

Thanks @anthonpetrow ! could you add a little more info about how this situation could occur? like what your setup/evaluation request is?

markphelps avatar Apr 14 '24 18:04 markphelps

@markphelps I can't reproduce such a case through a query, but the presence of the possibility to influence the distribution in the code on the segmentation of the flag doesn't sound very convincing

anthonpetrow avatar Apr 15 '24 01:04 anthonpetrow

This is a bit of a fiddly one. Definitely shines a light on a subtle bit of the evaluator that needs some thought. I don't think anything is necessarily broken here, it is perhaps just different expectations around the contract.

What you're saying @anthonpetrow with this change makes sense on paper. The constraints / rules has matched the context to a segment. It is just that we cannot identify an assocated variant on the flag for the matched segment, since none of them fall into any identified distribution.

The thing we have to decide is whether changing this creates an unexpected / undesirable break in behaviour for folks. Currently, (without this change) there is an implicit assumption you can make, which is that matched == true implies a variant key is present in .Value.

Can we safely break that assumption @markphelps ? I worry we might not.

Putting on the retrospective glasses, perhaps the concept of a default variant would've helped here. Or perhaps some validation that the entire 100% of buckets point to a valid variant in a distribution. However, this kind of change is hard to retrofit.

Or perhaps the existing behaviour is still reasonable and that perhaps it just needs to be clearer in the documentation. That being that: adding distributions to a rule implies a further condition that could cause the match to fail.

GeorgeMac avatar Apr 16 '24 09:04 GeorgeMac

Fixed by #3271

markphelps avatar Jul 23 '24 13:07 markphelps