Skip to content

Commit

Permalink
feat: rule-level evaluation details
Browse files Browse the repository at this point in the history
  • Loading branch information
rasendubi committed Jul 18, 2024
1 parent c546daf commit 0176d1d
Show file tree
Hide file tree
Showing 5 changed files with 152 additions and 31 deletions.
16 changes: 10 additions & 6 deletions eppo_core/src/ufc/eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::{sharder::get_md5_shard, Attributes, Configuration};

use super::{
eval_details::{EvalFlagDetails, EvalFlagDetailsBuilder},
eval_visitor::{EvalAllocationVisitor, EvalVisitor, NoopEvalVisitor},
eval_visitor::{EvalAllocationVisitor, EvalRuleVisitor, EvalVisitor, NoopEvalVisitor},
Allocation, Assignment, AssignmentEvent, Flag, FlagEvaluationError, Shard, Split, Timestamp,
TryParse, UniversalFlagConfig, VariationType,
};
Expand Down Expand Up @@ -214,6 +214,7 @@ impl Flag {
let Some((allocation, split)) = self.allocations.iter().find_map(|allocation| {
let mut visitor = visitor.visit_allocation(allocation);
let result = allocation.get_matching_split(
&mut visitor,
subject_key,
&subject_attributes_with_id,
self.total_shards,
Expand Down Expand Up @@ -280,8 +281,9 @@ pub(super) enum AllocationNonMatchReason {
}

impl Allocation {
fn get_matching_split(
fn get_matching_split<V: EvalAllocationVisitor>(
&self,
visitor: &mut V,
subject_key: &str,
subject_attributes_with_id: &Attributes,
total_shards: u64,
Expand All @@ -295,10 +297,12 @@ impl Allocation {
}

let is_allowed_by_rules = self.rules.is_empty()
|| self
.rules
.iter()
.any(|rule| rule.eval(subject_attributes_with_id));
|| self.rules.iter().any(|rule| {
let mut visitor = visitor.visit_rule(rule);
let result = rule.eval(&mut visitor, subject_attributes_with_id);
visitor.on_result(result);
result
});
if !is_allowed_by_rules {
return Err(AllocationNonMatchReason::FailingRules);
}
Expand Down
77 changes: 70 additions & 7 deletions eppo_core/src/ufc/eval_details.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@ use std::collections::HashMap;
use chrono::{DateTime, Utc};
use serde::{Deserialize, Serialize};

use crate::{Attributes, Configuration};
use crate::{AttributeValue, Attributes, Configuration};

use super::{
eval::AllocationNonMatchReason, eval_visitor::*, Assignment, FlagEvaluationError, Split, Value,
eval::AllocationNonMatchReason, eval_visitor::*, Assignment, Condition, FlagEvaluationError,
Rule, Split, Value,
};

#[derive(Debug, Clone, Serialize, Deserialize)]
Expand Down Expand Up @@ -43,6 +44,8 @@ pub struct ConfigurationDetails {
pub struct EvalAllocationDetails {
pub key: String,
pub result: EvalAllocationResult,
#[serde(skip_serializing_if = "Vec::is_empty")]
pub evaluated_rules: Vec<EvalRuleDetails>,
}

#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
Expand All @@ -56,6 +59,21 @@ pub enum EvalAllocationResult {
TrafficExposureMiss,
}

#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct EvalRuleDetails {
pub matched: bool,
pub conditions: Vec<EvalConditionDetails>,
}

#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct EvalConditionDetails {
pub matched: bool,
pub condition: Condition,
pub attribute_value: Option<AttributeValue>,
}

pub(crate) struct EvalFlagDetailsBuilder {
flag_key: String,
subject_key: String,
Expand All @@ -74,10 +92,14 @@ pub(crate) struct EvalFlagDetailsBuilder {
}

pub(crate) struct EvalAllocationDetailsBuilder<'a> {
result: &'a mut EvalAllocationDetails,
allocation_details: &'a mut EvalAllocationDetails,
variation_key: &'a mut Option<String>,
}

pub(crate) struct EvalRuleDetailsBuilder<'a> {
rule_details: &'a mut EvalRuleDetails,
}

impl EvalFlagDetailsBuilder {
pub fn new(
flag_key: String,
Expand Down Expand Up @@ -119,6 +141,7 @@ impl EvalFlagDetailsBuilder {
None => EvalAllocationDetails {
key,
result: EvalAllocationResult::Unevaluated,
evaluated_rules: Vec::new(),
},
})
.collect(),
Expand All @@ -139,9 +162,10 @@ impl EvalVisitor for EvalFlagDetailsBuilder {
.or_insert(EvalAllocationDetails {
key: allocation.key.clone(),
result: EvalAllocationResult::Unevaluated,
evaluated_rules: Vec::new(),
});
EvalAllocationDetailsBuilder {
result,
allocation_details: result,
variation_key: &mut self.variation_key,
}
}
Expand All @@ -164,16 +188,36 @@ impl EvalVisitor for EvalFlagDetailsBuilder {
self.variation_value = Some(variation.value.clone());
}

fn on_result(&mut self, result: &Result<super::Assignment, super::FlagEvaluationError>) {
fn on_result(&mut self, result: &Result<Assignment, FlagEvaluationError>) {
self.result = Some(result.clone());
}
}

impl<'a> EvalAllocationVisitor for EvalAllocationDetailsBuilder<'a> {
impl<'b> EvalAllocationVisitor for EvalAllocationDetailsBuilder<'b> {
type RuleVisitor<'a> = EvalRuleDetailsBuilder<'a>
where
Self: 'a;

fn visit_rule<'a>(&'a mut self, _rule: &Rule) -> EvalRuleDetailsBuilder<'a> {
self.allocation_details
.evaluated_rules
.push(EvalRuleDetails {
matched: false,
conditions: Vec::new(),
});
EvalRuleDetailsBuilder {
rule_details: self
.allocation_details
.evaluated_rules
.last_mut()
.expect("we just inserted an element, so there must be last"),
}
}

fn on_result(&mut self, result: Result<&Split, AllocationNonMatchReason>) {
*self.variation_key = result.ok().map(|split| split.variation_key.clone());

self.result.result = match result {
self.allocation_details.result = match result {
Ok(_) => EvalAllocationResult::Matched,
Err(AllocationNonMatchReason::BeforeStartDate) => EvalAllocationResult::BeforeStartDate,
Err(AllocationNonMatchReason::AfterEndDate) => EvalAllocationResult::AfterEndDate,
Expand All @@ -184,3 +228,22 @@ impl<'a> EvalAllocationVisitor for EvalAllocationDetailsBuilder<'a> {
};
}
}

impl<'a> EvalRuleVisitor for EvalRuleDetailsBuilder<'a> {
fn on_condition_eval(
&mut self,
condition: &Condition,
attribute_value: Option<&AttributeValue>,
result: bool,
) {
self.rule_details.conditions.push(EvalConditionDetails {
matched: result,
condition: condition.clone(),
attribute_value: attribute_value.cloned(),
});
}

fn on_result(&mut self, result: bool) {
self.rule_details.matched = result;
}
}
39 changes: 35 additions & 4 deletions eppo_core/src/ufc/eval_visitor.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use crate::Configuration;
use crate::{AttributeValue, Configuration};

use super::{
eval::AllocationNonMatchReason, Allocation, Assignment, Flag, FlagEvaluationError, Split,
Variation,
eval::AllocationNonMatchReason, Allocation, Assignment, Condition, Flag, FlagEvaluationError,
Rule, Split, Variation,
};

pub(super) trait EvalVisitor {
Expand Down Expand Up @@ -34,11 +34,33 @@ pub(super) trait EvalVisitor {
}

pub(super) trait EvalAllocationVisitor {
type RuleVisitor<'a>: EvalRuleVisitor + 'a
where
Self: 'a;

fn visit_rule<'a>(&'a mut self, rule: &Rule) -> Self::RuleVisitor<'a>;

#[allow(unused_variables)]
#[inline]
fn on_result(&mut self, result: Result<&Split, AllocationNonMatchReason>) {}
}

pub(super) trait EvalRuleVisitor {
#[allow(unused_variables)]
#[inline]
fn on_condition_eval(
&mut self,
condition: &Condition,
attribute_value: Option<&AttributeValue>,
result: bool,
) {
}

#[allow(unused_variables)]
#[inline]
fn on_result(&mut self, result: bool) {}
}

/// Dummy visitor that does nothing.
///
/// It is designed so that all calls to it are optimized away (zero-cost).
Expand All @@ -53,4 +75,13 @@ impl EvalVisitor for NoopEvalVisitor {
}
}

impl EvalAllocationVisitor for NoopEvalVisitor {}
impl EvalAllocationVisitor for NoopEvalVisitor {
type RuleVisitor<'a> = NoopEvalVisitor;

#[inline]
fn visit_rule<'a>(&'a mut self, _rule: &Rule) -> Self::RuleVisitor<'a> {
NoopEvalVisitor
}
}

impl EvalRuleVisitor for NoopEvalVisitor {}
4 changes: 2 additions & 2 deletions eppo_core/src/ufc/models.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ pub struct Rule {

/// `Condition` is a check that given user `attribute` matches the condition `value` under the given
/// `operator`.
#[derive(Debug, Serialize, Deserialize, Clone)]
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
#[allow(missing_docs)]
pub struct Condition {
Expand Down Expand Up @@ -243,7 +243,7 @@ pub enum ConditionOperator {
IsNull,
}

#[derive(Debug, Serialize, Deserialize, Clone)]
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
#[serde(untagged)]
#[allow(missing_docs)]
pub enum ConditionValue {
Expand Down
47 changes: 35 additions & 12 deletions eppo_core/src/ufc/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,26 @@ use crate::{
AttributeValue, Attributes,
};

use super::eval_visitor::EvalRuleVisitor;

impl Rule {
pub(crate) fn eval(&self, attributes: &Attributes) -> bool {
pub(super) fn eval<V: EvalRuleVisitor>(
&self,
visitor: &mut V,
attributes: &Attributes,
) -> bool {
self.conditions
.iter()
.all(|condition| condition.eval(attributes))
.all(|condition| condition.eval(visitor, attributes))
}
}

impl Condition {
fn eval(&self, attributes: &Attributes) -> bool {
self.operator
.eval(attributes.get(&self.attribute), &self.value)
fn eval<V: EvalRuleVisitor>(&self, visitor: &mut V, attributes: &Attributes) -> bool {
let attribute = attributes.get(&self.attribute);
let result = self.operator.eval(attribute, &self.value);
visitor.on_condition_eval(self, attribute, result);
result
}
}

Expand Down Expand Up @@ -135,7 +143,7 @@ impl ConditionOperator {
mod tests {
use std::collections::HashMap;

use crate::ufc::{Condition, ConditionOperator, Rule};
use crate::ufc::{eval_visitor::NoopEvalVisitor, Condition, ConditionOperator, Rule};

#[test]
fn matches_regex() {
Expand Down Expand Up @@ -287,7 +295,7 @@ mod tests {
#[test]
fn empty_rule() {
let rule = Rule { conditions: vec![] };
assert!(rule.eval(&HashMap::from([])));
assert!(rule.eval(&mut NoopEvalVisitor, &HashMap::from([])));
}

#[test]
Expand All @@ -299,7 +307,10 @@ mod tests {
value: 10.0.into(),
}],
};
assert!(rule.eval(&HashMap::from([("age".into(), 11.0.into())])));
assert!(rule.eval(
&mut NoopEvalVisitor,
&HashMap::from([("age".into(), 11.0.into())])
));
}

#[test]
Expand All @@ -318,9 +329,18 @@ mod tests {
},
],
};
assert!(rule.eval(&HashMap::from([("age".into(), 20.0.into())])));
assert!(!rule.eval(&HashMap::from([("age".into(), 17.0.into())])));
assert!(!rule.eval(&HashMap::from([("age".into(), 110.0.into())])));
assert!(rule.eval(
&mut NoopEvalVisitor,
&HashMap::from([("age".into(), 20.0.into())])
));
assert!(!rule.eval(
&mut NoopEvalVisitor,
&HashMap::from([("age".into(), 17.0.into())])
));
assert!(!rule.eval(
&mut NoopEvalVisitor,
&HashMap::from([("age".into(), 110.0.into())])
));
}

#[test]
Expand All @@ -332,6 +352,9 @@ mod tests {
value: 10.0.into(),
}],
};
assert!(!rule.eval(&HashMap::from([("name".into(), "alice".into())])));
assert!(!rule.eval(
&mut NoopEvalVisitor,
&HashMap::from([("name".into(), "alice".into())])
));
}
}

0 comments on commit 0176d1d

Please sign in to comment.