FF-2678 flag evaluation details #13

rasendubi · 2024-07-17T19:40:07Z

https://linear.app/eppo/issue/FF-2678/evaluation-reasons-in-rust

Examples:

Example 1

{
  "variation": true,
  "action": null,
  "evaluationDetails": {
    "flagKey": "a-boolean-flag",
    "subjectKey": "test-subject",
    "subjectAttributes": {
      "name": "oleksii"
    },
    "timestamp": "2024-07-22T18:02:20.122461Z",
    "configFetchedAt": "2024-07-22T18:02:20.119761Z",
    "configPublishedAt": "2024-07-22T17:41:28.164Z",
    "environmentName": "eppo.cloud",
    "flagEvaluationCode": "MATCH",
    "variationKey": "true",
    "variationValue": true,
    "banditKey": null,
    "banditAction": null,
    "allocations": [
      {
        "key": "allocation-4233",
        "orderPosition": 1,
        "allocationEvaluationCode": "FAILING_RULE",
        "evaluatedRules": [
          {
            "matched": false,
            "conditions": [
              {
                "condition": {
                  "operator": "MATCHES",
                  "attribute": "email",
                  "value": ".*@example\\.com"
                },
                "attributeValue": null,
                "matched": false
              }
            ]
          }
        ],
        "evaluatedSplits": []
      },
      {
        "key": "allocation-4246",
        "orderPosition": 2,
        "allocationEvaluationCode": "MATCHED",
        "evaluatedRules": [
          {
            "matched": true,
            "conditions": [
              {
                "condition": {
                  "operator": "ONE_OF",
                  "attribute": "name",
                  "value": [
                    "leo",
                    "oleksii",
                    "felipe"
                  ]
                },
                "attributeValue": "oleksii",
                "matched": true
              }
            ]
          }
        ],
        "evaluatedSplits": [
          {
            "variationKey": "true",
            "matched": true,
            "shards": [
              {
                "matched": true,
                "shard": {
                  "salt": "a-boolean-flag-4246-split",
                  "ranges": [
                    {
                      "start": 0,
                      "end": 10000
                    }
                  ]
                },
                "shardValue": 4610
              }
            ]
          }
        ]
      },
      {
        "key": "allocation-4446",
        "orderPosition": 3,
        "allocationEvaluationCode": "UNEVALUATED",
        "evaluatedRules": [],
        "evaluatedSplits": []
      },
      {
        "key": "allocation-4447",
        "orderPosition": 4,
        "allocationEvaluationCode": "UNEVALUATED",
        "evaluatedRules": [],
        "evaluatedSplits": []
      },
      {
        "key": "allocation-3207",
        "orderPosition": 5,
        "allocationEvaluationCode": "UNEVALUATED",
        "evaluatedRules": [],
        "evaluatedSplits": []
      }
    ]
  }
}

Example 2

{
  "variation": null,
  "action": null,
  "evaluationDetails": {
    "flagKey": "non-existing-flag",
    "subjectKey": "test-subject",
    "subjectAttributes": {
      "name": "oleksii"
    },
    "timestamp": "2024-07-22T18:01:37.953516Z",
    "configFetchedAt": "2024-07-22T18:01:37.953377Z",
    "configPublishedAt": "2024-07-22T17:41:28.164Z",
    "environmentName": "eppo.cloud",
    "flagEvaluationCode": "FLAG_UNRECOGNIZED_OR_DISABLED",
    "variationKey": null,
    "variationValue": null,
    "banditKey": null,
    "banditAction": null,
    "allocations": []
  }
}

Example 3

{
  "variation": null,
  "action": null,
  "evaluationDetails": {
    "flagKey": "a-boolean-flag",
    "subjectKey": "test-subject",
    "subjectAttributes": {
      "name": "oleksii"
    },
    "timestamp": "2024-07-22T18:03:13.600731Z",
    "configFetchedAt": null,
    "configPublishedAt": null,
    "environmentName": null,
    "flagEvaluationCode": "FLAG_UNRECOGNIZED_OR_DISABLED",
    "variationKey": null,
    "variationValue": null,
    "banditKey": null,
    "banditAction": null,
    "allocations": []
  }
}

Example 4

{
  "variation": null,
  "action": null,
  "evaluationDetails": {
    "flagKey": "a-numeric-flag",
    "subjectKey": "subject1",
    "subjectAttributes": {},
    "timestamp": "2024-07-22T18:06:09.001990Z",
    "configFetchedAt": "2024-07-22T18:06:09.001904Z",
    "configPublishedAt": "2024-07-22T17:41:28.164Z",
    "environmentName": "eppo.cloud",
    "flagEvaluationCode": "TYPE_MISMATCH",
    "variationKey": null,
    "variationValue": null,
    "banditKey": null,
    "banditAction": null,
    "allocations": [
      {
        "key": "allocation-7054",
        "orderPosition": 1,
        "allocationEvaluationCode": "UNEVALUATED",
        "evaluatedRules": [],
        "evaluatedSplits": []
      },
      {
        "key": "allocation-6824",
        "orderPosition": 2,
        "allocationEvaluationCode": "UNEVALUATED",
        "evaluatedRules": [],
        "evaluatedSplits": []
      },
      {
        "key": "allocation-6823",
        "orderPosition": 3,
        "allocationEvaluationCode": "UNEVALUATED",
        "evaluatedRules": [],
        "evaluatedSplits": []
      },
      {
        "key": "allocation-6821",
        "orderPosition": 4,
        "allocationEvaluationCode": "UNEVALUATED",
        "evaluatedRules": [],
        "evaluatedSplits": []
      },
      {
        "key": "allocation-6820",
        "orderPosition": 5,
        "allocationEvaluationCode": "UNEVALUATED",
        "evaluatedRules": [],
        "evaluatedSplits": []
      }
    ]
  }
}

Example 5

{
  "variation": null,
  "action": null,
  "evaluationDetails": {
    "flagKey": "a-numeric-flag",
    "subjectKey": "subject1",
    "subjectAttributes": {},
    "timestamp": "2024-07-22T18:06:42.931967Z",
    "configFetchedAt": "2024-07-22T18:06:42.931459Z",
    "configPublishedAt": "2024-07-22T17:41:28.164Z",
    "environmentName": "eppo.cloud",
    "flagEvaluationCode": "DEFAULT_ALLOCATION_NULL",
    "variationKey": null,
    "variationValue": null,
    "banditKey": null,
    "banditAction": null,
    "allocations": [
      {
        "key": "allocation-7054",
        "orderPosition": 1,
        "allocationEvaluationCode": "FAILING_RULE",
        "evaluatedRules": [
          {
            "matched": false,
            "conditions": [
              {
                "condition": {
                  "operator": "MATCHES",
                  "attribute": "regex_value",
                  "value": "0"
                },
                "attributeValue": null,
                "matched": false
              }
            ]
          }
        ],
        "evaluatedSplits": []
      },
      {
        "key": "allocation-6824",
        "orderPosition": 2,
        "allocationEvaluationCode": "FAILING_RULE",
        "evaluatedRules": [
          {
            "matched": false,
            "conditions": [
              {
                "condition": {
                  "operator": "ONE_OF",
                  "attribute": "forcedVariation",
                  "value": [
                    "1"
                  ]
                },
                "attributeValue": null,
                "matched": false
              }
            ]
          }
        ],
        "evaluatedSplits": []
      },
      {
        "key": "allocation-6823",
        "orderPosition": 3,
        "allocationEvaluationCode": "FAILING_RULE",
        "evaluatedRules": [
          {
            "matched": false,
            "conditions": [
              {
                "condition": {
                  "operator": "ONE_OF",
                  "attribute": "forceVariation",
                  "value": [
                    "control"
                  ]
                },
                "attributeValue": null,
                "matched": false
              }
            ]
          }
        ],
        "evaluatedSplits": []
      },
      {
        "key": "allocation-6821",
        "orderPosition": 4,
        "allocationEvaluationCode": "FAILING_RULE",
        "evaluatedRules": [
          {
            "matched": false,
            "conditions": [
              {
                "condition": {
                  "operator": "ONE_OF",
                  "attribute": "userId",
                  "value": [
                    "85",
                    "31",
                    "12"
                  ]
                },
                "attributeValue": null,
                "matched": false
              }
            ]
          },
          {
            "matched": false,
            "conditions": [
              {
                "condition": {
                  "operator": "MATCHES",
                  "attribute": "email",
                  "value": "geteppo.com$"
                },
                "attributeValue": null,
                "matched": false
              }
            ]
          }
        ],
        "evaluatedSplits": []
      },
      {
        "key": "allocation-6820",
        "orderPosition": 5,
        "allocationEvaluationCode": "TRAFFIC_EXPOSURE_MISS",
        "evaluatedRules": [],
        "evaluatedSplits": [
          {
            "variationKey": "1",
            "matched": false,
            "shards": [
              {
                "matched": false,
                "shard": {
                  "salt": "a-numeric-flag-6820-traffic",
                  "ranges": [
                    {
                      "start": 0,
                      "end": 0
                    }
                  ]
                },
                "shardValue": 2734
              }
            ]
          },
          {
            "variationKey": "10",
            "matched": false,
            "shards": [
              {
                "matched": false,
                "shard": {
                  "salt": "a-numeric-flag-6820-traffic",
                  "ranges": [
                    {
                      "start": 0,
                      "end": 0
                    }
                  ]
                },
                "shardValue": 2734
              }
            ]
          },
          {
            "variationKey": "100",
            "matched": false,
            "shards": [
              {
                "matched": false,
                "shard": {
                  "salt": "a-numeric-flag-6820-traffic",
                  "ranges": [
                    {
                      "start": 0,
                      "end": 0
                    }
                  ]
                },
                "shardValue": 2734
              }
            ]
          }
        ]
      }
    ]
  }
}

greghuels

So I love a lot of the changes you made here. I do have some concerns that the structure differs from the docs, but I also don't think it's that big of a deal so long as the team has some appetite for SDK differences.

Outside of these structural differences there are a few things missing. They don't necessarily have to be in this PR, but I think they should be in the release.

orderPosition is missing from each allocation. Since the allocation keys are just allocation-1234, it's a lot easier to identify the allocation by the order (1-indexed), since they're also numbered in the UI. This is called out in the docs quite a bit, so I think it'd be good to add.
evaluationDetails needs to be added to the AssignmentEvent for AssignmentLogger.log_assignment. More often than not, I'd imagine engineers are going to log evaluation details to a service like Datadog so that they can diagnose issues encountered by a user.
We'll want to leverage the tests from the sdk-test-data repo. Each of the test cases in ufc/tests/* has an evaluationDetails key for this purpose.
Nit: I don't think this is blocking, but I do think we should split allocations into matchedAllocation, unmatchedAllocations, and unevaluatedAllocations as the JS/Node SDK does. This would make the feature more consistent with docs, and it's also a better developer experience to immediately show which allocation was matched / missed at the top level of the object, rather than having them dig through the array of each allocation to find this. Perf is a non-issue since we're only talking about a handful of allocations per flag and it can be done in an O(n) time complexity. I can also disagree and commit on this if others think it's not necessary.

eppo_core/src/ufc/eval_details.rs

rasendubi · 2024-07-22T12:21:05Z

(rebased to resolve merge conflict)

aarsilv

halfway through reviewing

aarsilv · 2024-07-22T14:40:13Z

eppo_core/src/ufc/error.rs

+/// Enum representing possible errors that can occur during flag evaluation.
+#[derive(thiserror::Error, Debug, Clone, Copy, PartialEq, Serialize, Deserialize)]
+#[non_exhaustive]
+pub enum FlagEvaluationError {


These enums seem pretty different from the JavaScript ones (link). We should probably aim to keep these consistent and talk about which ones we want.

For example, the JavaScript has a specific NO_ACTIONS_SUPPLIED_FOR_BANDIT, and also BANDIT_ERROR if a variation was successfully assigned but an issue was encountered selecting an action.

I have updated this one:

Split it into EvaluationError (which are real error and are returned to the user) and an internal EvaluationFailure (which describes all reasons why we could fail to assign variation for a flag).

Brought it closer to JS.

aarsilv · 2024-07-22T14:42:50Z

eppo_core/src/ufc/eval.rs

+                       subject_key;
+                       "error occurred while evaluating a flag: {err}",
+            );
+            Err(err)


Does this make the error "fatal"?

We want all calls to be "safe" (if graceful mode--which we should support toggling--is on)

Parse errors and such can be thrown on initialization, as we encourage wrapping initialization in a try-catch

No. There are no fatal error in Rust

Parsing error happens outside of initialization as configuration is fetched in a separate background thread.

aarsilv · 2024-07-22T14:51:26Z

eppo_core/src/ufc/eval.rs

+    visitor.on_configuration(config);
+
+    config.flags.eval_flag(
+        visitor,


Interesting thought using the visitor pattern here. Do you think the speedup is worth the additional code complexity vs just always computing the details and then returning only the value for the non-details assignment methods?

Yes. Detailed evaluation is currently 80%–4x slower than detail-less version. See this comment for more benchmarking details.

aarsilv · 2024-07-22T14:53:11Z

eppo_core/src/ufc/eval.rs

-            None
+    ) -> Result<&Split, AllocationNonMatchReason> {
+        if self.start_at.is_some_and(|t| now < t) {
+            return Err(AllocationNonMatchReason::BeforeStartDate);


is this a normal pattern in rust to return errors in the course of "normal" operation?

Yes. Error<T, E> is just a normal rust type for when one branch is returning a value (Ok(value)) and another one fails to produce the value for any reason (Err(err)). Unless the reason is trivial, in which case one would use Option<T> with Some(value) and None. This is actually the reason why the function signature changed from Option to Result—because we want to know the reason

eppo_core/src/ufc/eval_details.rs

aarsilv

Rest of review

Also, of course, combine with today's live discussion notes (link).

aarsilv · 2024-07-22T17:32:40Z

rust-sdk/examples/assignment_details/main.rs

+    // Start a poller thread to fetch configuration from the server.
+    let poller = client.start_poller_thread()?;
+
+    // Block waiting for configuration. Until this call returns, the client will return None for all


minor, but I thinking "blocking wait" is the term we're looking for

I think "blocking wait" is a noun? I'm using the verb here to describe what's going on.

aarsilv · 2024-07-22T17:39:34Z

rust-sdk/src/client.rs

+    /// Get the assignment value for a given feature flag and subject, along with details of why
+    /// this value was selected.
+    ///
+    /// *NOTE:* It is a debug function and is slower due to the need to collect all the


Is it really that much slower? Our JS implementation always calls it, and we haven't heard any complaints (at least not yet).

I almost think in a different world, if we started from scratch, we'd only have this method.

Yes, see above comment

aarsilv · 2024-07-22T17:40:32Z

rust-sdk/src/client.rs

        let config = self.configuration_store.get_configuration();
-        let assignment =
-            config.get_assignment(flag_key, subject_key, subject_attributes, expected_type)?;
+        let assignment = get_assignment(


I don't think we want the "untyped" assignment methods in UFC, forcing folks to declare the type they want.

get_assignment here is an internal function.

We do have a top-level Client.get_assignment and Client.get_assignment_details though, that I'm OK to remove. (Though they are still typed in a sense that they require user to check the type before accessing the value.)

Previously, AssignmentValue was serialized as follows: { "BOOLEAN": true } After this change, it is serialized as: { "type": "BOOLEAN", "value": true }

rasendubi · 2024-08-02T20:25:43Z

eppo_core/src/eval/eval_details.rs

+    pub bandit_evaluation_code: Option<BanditEvaluationCode>,
+    pub flag_evaluation_code: Option<FlagEvaluationCode>,


One new deviation from JS SDK: bandit_evaluation_code and flag_evaluation_code are exposed separately. The reasoning is that overriding flag_evaluation_code with bandit evaluation result may hide some interesting details (i.e., why the given variation was selected)

rasendubi changed the title ~~WIP: feat: flag evaluation reasons~~ WIP: FF-2678 flag evaluation reasons Jul 17, 2024

rasendubi force-pushed the feat-evaluation-reasons branch 2 times, most recently from 9c5b0be to 890378a Compare July 18, 2024 13:43

rasendubi changed the title ~~WIP: FF-2678 flag evaluation reasons~~ FF-2678 flag evaluation details Jul 18, 2024

leoromanovsky requested review from leoromanovsky, aarsilv and greghuels July 18, 2024 20:17

rasendubi requested a review from sameerank July 18, 2024 20:19

greghuels requested changes Jul 19, 2024

View reviewed changes

rasendubi force-pushed the feat-evaluation-reasons branch 7 times, most recently from 2639301 to 3df8cdb Compare July 19, 2024 19:18

rasendubi commented Jul 19, 2024

View reviewed changes