Skip to content

Commit

Permalink
feat: support nested props in fractional evaluator (#869)
Browse files Browse the repository at this point in the history 
<!-- Please use this template for your pull request. -->
<!-- Please use the sections that you need and delete other sections -->

## This PR
<!-- add the description of the PR here -->

This PR adds the ability to use nested properties for the bucket key in
fractional evaluation. It is a breaking change from the current
behaviour. If the bucket key is not provided it will default to `{"cat":
[{"var":"$flagd.flagKey"}, {"var":"targetingKey"}]}`.

@toddbaert I am not sure that my approach to the missing bucket key is
the best. I would have preferred to add `{"cat":
[{"var":"$flagd.flagKey"}, {"var":"targetingKey"}]}` to the targeting
object and let JsonLogic handle it, but it would be a bit clumsy and
involve string manipulation. I don't think there is a nice way to do it,
but I'll play around a bit more.

### Related Issues
<!-- add here the GitHub issue that this PR resolves if applicable -->

See #848.
Closes #843.

### Notes
<!-- any additional notes for this PR -->

### Follow-up Tasks
<!-- anything that is related to this PR but not done here should be
noted under this section -->
<!-- if there is a need for a new issue, please link it here -->

### How to test
<!-- if applicable, add testing instructions under this section -->

---------

Signed-off-by: Craig Pastro <[email protected]>
Co-authored-by: Todd Baert <[email protected]>
  • Loading branch information
@toddbaert
Craig Pastro and toddbaert authored Aug 28, 2023
1 parent e007fcc commit 50ff739
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 70 deletions.
43 changes: 19 additions & 24 deletions core/pkg/eval/fractional_evaluation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func NewFractionalEvaluator(logger *logger.Logger) *FractionalEvaluator {
return &FractionalEvaluator{Logger: logger}
}

func (fe *FractionalEvaluator) FractionalEvaluation(values, data interface{}) interface{} {
func (fe *FractionalEvaluator) FractionalEvaluation(values, data any) any {
valueToDistribute, feDistributions, err := parseFractionalEvaluationData(values, data)
if err != nil {
fe.Logger.Error(fmt.Sprintf("parse fractional evaluation data: %v", err))
Expand All @@ -32,54 +32,49 @@ func (fe *FractionalEvaluator) FractionalEvaluation(values, data interface{}) in
return distributeValue(valueToDistribute, feDistributions)
}

func parseFractionalEvaluationData(values, data interface{}) (string, []fractionalEvaluationDistribution, error) {
valuesArray, ok := values.([]interface{})
func parseFractionalEvaluationData(values, data any) (string, []fractionalEvaluationDistribution, error) {
valuesArray, ok := values.([]any)
if !ok {
return "", nil, errors.New("fractional evaluation data is not an array")
}
if len(valuesArray) < 2 {
return "", nil, errors.New("fractional evaluation data has length under 2")
}

bucketBy, ok := valuesArray[0].(string)
if !ok {
return "", nil, errors.New("first element of fractional evaluation data isn't of type string")
}

dataMap, ok := data.(map[string]interface{})
dataMap, ok := data.(map[string]any)
if !ok {
return "", nil, errors.New("data isn't of type map[string]interface{}")
return "", nil, errors.New("data isn't of type map[string]any")
}

// Ignore the ok as we can't really do anything if the properties are
// Ignore the error as we can't really do anything if the properties are
// somehow missing.
properties, _ := getFlagdProperties(dataMap)

v, ok := dataMap[bucketBy]
if !ok {
return "", nil, nil
}

valueToDistribute, ok := v.(string)
if !ok {
return "", nil, fmt.Errorf("var: %v isn't of type string", v)
bucketBy, ok := valuesArray[0].(string)
if ok {
valuesArray = valuesArray[1:]
} else {
bucketBy, ok = dataMap[targetingKeyKey].(string)
if !ok {
return "", nil, errors.New("bucketing value not supplied and no targetingKey in context")
}
}

feDistributions, err := parseFractionalEvaluationDistributions(valuesArray)
if err != nil {
return "", nil, err
}

return fmt.Sprintf("%s$%s", properties.FlagKey, valueToDistribute), feDistributions, nil
return fmt.Sprintf("%s%s", properties.FlagKey, bucketBy), feDistributions, nil
}

func parseFractionalEvaluationDistributions(values []interface{}) ([]fractionalEvaluationDistribution, error) {
func parseFractionalEvaluationDistributions(values []any) ([]fractionalEvaluationDistribution, error) {
sumOfPercentages := 0
var feDistributions []fractionalEvaluationDistribution
for i := 1; i < len(values); i++ {
distributionArray, ok := values[i].([]interface{})
for i := 0; i < len(values); i++ {
distributionArray, ok := values[i].([]any)
if !ok {
return nil, errors.New("distribution elements aren't of type []interface{}")
return nil, errors.New("distribution elements aren't of type []any")
}

if len(distributionArray) != 2 {
Expand Down
101 changes: 68 additions & 33 deletions core/pkg/eval/fractional_evaluation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,22 +29,22 @@ func TestFractionalEvaluation(t *testing.T) {
},
{
"fractionalEvaluation": [
"email",
{"var": "email"},
[
"red",
25
"red",
25
],
[
"blue",
25
"blue",
25
],
[
"green",
25
"green",
25
],
[
"yellow",
25
"yellow",
25
]
]
}, null
Expand All @@ -69,41 +69,41 @@ func TestFractionalEvaluation(t *testing.T) {
context: map[string]any{
"email": "[email protected]",
},
expectedVariant: "blue",
expectedValue: "#0000FF",
expectedVariant: "yellow",
expectedValue: "#FFFF00",
expectedReason: model.TargetingMatchReason,
},
"phoebe@faas.com": {
"monica@faas.com": {
flags: flags,
flagKey: "headerColor",
context: map[string]any{
"email": "phoebe@faas.com",
"email": "monica@faas.com",
},
expectedVariant: "yellow",
expectedValue: "#FFFF00",
expectedVariant: "green",
expectedValue: "#00FF00",
expectedReason: model.TargetingMatchReason,
},
"monica@faas.com": {
"joey@faas.com": {
flags: flags,
flagKey: "headerColor",
context: map[string]any{
"email": "monica@faas.com",
"email": "joey@faas.com",
},
expectedVariant: "red",
expectedValue: "#FF0000",
expectedVariant: "blue",
expectedValue: "#0000FF",
expectedReason: model.TargetingMatchReason,
},
"rossg@faas.com": {
"ross@faas.com": {
flags: flags,
flagKey: "headerColor",
context: map[string]any{
"email": "rossg@faas.com",
"email": "ross@faas.com",
},
expectedVariant: "green",
expectedValue: "#00FF00",
expectedVariant: "red",
expectedValue: "#FF0000",
expectedReason: model.TargetingMatchReason,
},
"rossg@faas.com with different flag key": {
"ross@faas.com with different flag key": {
flags: Flags{
Flags: map[string]model.Flag{
"footerColor": {
Expand All @@ -124,7 +124,7 @@ func TestFractionalEvaluation(t *testing.T) {
},
{
"fractionalEvaluation": [
"email",
{"var": "email"},
[
"red",
25
Expand All @@ -150,10 +150,10 @@ func TestFractionalEvaluation(t *testing.T) {
},
flagKey: "footerColor",
context: map[string]any{
"email": "rossg@faas.com",
"email": "ross@faas.com",
},
expectedVariant: "red",
expectedValue: "#FF0000",
expectedVariant: "blue",
expectedValue: "#0000FF",
expectedReason: model.TargetingMatchReason,
},
"non even split": {
Expand Down Expand Up @@ -201,8 +201,8 @@ func TestFractionalEvaluation(t *testing.T) {
context: map[string]any{
"email": "[email protected]",
},
expectedVariant: "red",
expectedValue: "#FF0000",
expectedVariant: "green",
expectedValue: "#00FF00",
expectedReason: model.TargetingMatchReason,
},
"fallback to default variant if no email provided": {
Expand All @@ -219,7 +219,7 @@ func TestFractionalEvaluation(t *testing.T) {
},
Targeting: []byte(`{
"fractionalEvaluation": [
"email",
{"var": "email"},
[
"red",
25
Expand Down Expand Up @@ -261,7 +261,7 @@ func TestFractionalEvaluation(t *testing.T) {
},
Targeting: []byte(`{
"fractionalEvaluation": [
"email",
{"var": "email"},
[
"black",
100
Expand Down Expand Up @@ -293,7 +293,7 @@ func TestFractionalEvaluation(t *testing.T) {
},
Targeting: []byte(`{
"fractionalEvaluation": [
"email",
{"var": "email"},
[
"red",
25
Expand All @@ -315,6 +315,41 @@ func TestFractionalEvaluation(t *testing.T) {
expectedValue: "#FF0000",
expectedReason: model.DefaultReason,
},
"default to targetingKey if no bucket key provided": {
flags: Flags{
Flags: map[string]model.Flag{
"headerColor": {
State: "ENABLED",
DefaultVariant: "red",
Variants: map[string]any{
"red": "#FF0000",
"blue": "#0000FF",
"green": "#00FF00",
"yellow": "#FFFF00",
},
Targeting: []byte(`{
"fractionalEvaluation": [
[
"blue",
50
],
[
"green",
50
]
]
}`),
},
},
},
flagKey: "headerColor",
context: map[string]any{
"targetingKey": "[email protected]",
},
expectedVariant: "green",
expectedValue: "#00FF00",
expectedReason: model.TargetingMatchReason,
},
}
const reqID = "default"
for name, tt := range tests {
Expand Down
5 changes: 5 additions & 0 deletions core/pkg/eval/json_evaluator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ const (
SelectorMetadataKey = "scope"

flagdPropertiesKey = "$flagd"

// targetingKeyKey is used to extract the targetingKey to bucket on in fractional
// evaluation if the user did not supply the optional bucketing property.
targetingKeyKey = "targetingKey"
)

var regBrace *regexp.Regexp
Expand Down Expand Up @@ -325,6 +329,7 @@ func (je *JSONEvaluator) evaluateVariant(reqID string, flagKey string, context m

return "", flag.Variants, model.ErrorReason, metadata, errors.New(model.ErrorReason)
}

var result bytes.Buffer
// evaluate json-logic rules to determine the variant
err = jsonlogic.Apply(bytes.NewReader(targetingBytes), bytes.NewReader(b), &result)
Expand Down
27 changes: 14 additions & 13 deletions docs/other_resources/in-process-providers/evaluators/fractional_evaluation.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ referenced evaluation context property, the [MurmurHash3](https://github.com/aap
hash function should be used. This is to ensure that flag resolution requests yield the same result,
regardless of which implementation of the in-process flagd provider is being used.

array containing at least two items, with the first item being an optional [json logic variable declaration](https://jsonlogic.com/operations.html#var)
specifying the target property to base the distribution of values on. If not supplied, a concatination of the
`flagKey` and `targetingKey` are used: `{"cat": [{"var":"$flagd.flag_key"}, {"var":"user.email"}]}`.
The supplied array must contain at least two items, with the first item being an optional [json logic variable declaration](https://jsonlogic.com/operations.html#var)
specifying the bucketing property to base the distribution of values on. If not supplied, a concatination of the
`flagKey` and `targetingKey` are used: `{"cat": [{"var":"$flagd.flagKey"}, {"var":"targetingKey"}]}`.
The remaining items are `arrays`, each with two values, with the first being `string` item representing the name of the variant, and the
second being a `float` item representing the percentage for that variant. The percentages of all items must add up to
100.0, otherwise unexpected behavior can occur during the evaluation. The `data` object can be an arbitrary
Expand Down Expand Up @@ -59,18 +59,19 @@ The following flow chart depicts the logic of this evaluator:

```mermaid
flowchart TD
A[Parse targetingRule] --> B{Is an array containing at least two items?};
A[Parse targetingRule] --> B{Is an array containing at least one item?};
B -- Yes --> C{Is targetingRule at index 0 a string?};
B -- No --> D[Return nil];
C -- Yes --> E[targetPropertyValue := targetingRule at index 0];
C -- No --> D;
E -- Yes --> F[Iterate through the remaining elements of the targetingRule array and parse the variants and their percentages];
F --> G{Parsing successful?};
G -- No --> D;
G -- Yes --> H{Does percentage of variants add up to 100?};
B -- No --> D[return nil]
C -- No --> E[bucketingPropertyValue := default to targetingKey];
C -- Yes --> F[bucketingPropertyValue := targetingRule at index 0];
E --> G[Iterate through the remaining elements of the targetingRule array and parse the variants and their percentages];
F --> G;
G --> H{Parsing successful?};
H -- No --> D;
H -- Yes --> I[hash := murmur3Hash of targetPropertyValue divided by Int64.MaxValue]
I --> L[Iterate through the variant and increment the threshold by the percentage of each variant. Return the first variant where the bucket is smaller than the threshold.]
H -- Yes --> I{Does percentage of variants add up to 100?};
I -- No --> D;
I -- Yes --> J[hash := murmur3Hash of bucketingPropertyValue divided by Int64.MaxValue]
J --> K[Iterate through the variant and increment the threshold by the percentage of each variant. Return the first variant where the bucket is smaller than the threshold.]
```

As a reference, below is a simplified version of the actual implementation of this evaluator in Go.
Expand Down

0 comments on commit 50ff739

Please sign in to comment.