Merge pull request #19 from ksumit-nitp/feature/add-guardrail-cutoff-validation

Add validation for guardrail cutoff signs

Author: MaloChevillotte

eppo_metrics_sync/eppo_metrics_sync.py

@@ -6,7 +6,8 @@
 from eppo_metrics_sync.validation import (
     unique_names,
     valid_fact_references,
-    metric_aggregation_is_valid
+    metric_aggregation_is_valid,
+    valid_guardrail_cutoff_signs
 )
 
 from eppo_metrics_sync.dbt_model_parser import DbtModelParser
@@ -110,6 +111,7 @@ def validate(self):
         unique_names(self)
         valid_fact_references(self)
         metric_aggregation_is_valid(self)
+        valid_guardrail_cutoff_signs(self)
 
         if self.validation_errors:
             error_count = len(self.validation_errors)

eppo_metrics_sync/validation.py

@@ -87,6 +87,33 @@ def metric_aggregation_is_valid(payload):
                 )
 
 
+def valid_guardrail_cutoff_signs(payload):
+    facts = dict()
+    for fact_source in payload.fact_sources:
+        for fact in fact_source['facts']:
+            facts[fact['name']] = fact
+
+    for m in payload.metrics:
+        numerator_fact_name = m['numerator']['fact_name']
+        if is_guardrail_cutoff_exist(m) and numerator_fact_name in facts and 'desired_change' in facts[numerator_fact_name]:
+            error = is_valid_guardrail_cutoff_sign(m, facts[numerator_fact_name])
+            if error:
+                payload.validation_errors.append(
+                    f"{m['name']} is having invalid guardrail_cutoff sign: {error}"
+                )
+
+
+def is_valid_guardrail_cutoff_sign(metric, numerator_fact):
+    if numerator_fact['desired_change'] == 'decrease' and metric['guardrail_cutoff'] < 0:
+        return f"guardrail_cutoff value should be positive"
+    elif numerator_fact['desired_change'] == 'increase' and metric['guardrail_cutoff'] > 0:
+        return f"guardrail_cutoff value should be negative"
+
+
+def is_guardrail_cutoff_exist(metric):
+    return 'is_guardrail' in metric and metric['is_guardrail'] and 'guardrail_cutoff' in metric
+
+
 def distinct_advanced_aggregation_parameter_set(
         aggregation,
         operation,

tests/test_validation.py

@@ -39,6 +39,16 @@ def test_unique_fact_names():
         eppo_metrics_sync.validate()
 
 
+def test_valid_guardrail_cutoff_signs():
+    eppo_metrics_sync = EppoMetricsSync(directory=None)
+    eppo_metrics_sync.load_eppo_yaml(
+        path=test_yaml_dir + "/invalid_guardrail_cutoff_sign.yaml")
+
+    with pytest.raises(ValueError, match="Total Upgrades to Paid Plan is having invalid guardrail_cutoff sign: "
+                                         "guardrail_cutoff value should be negative"):
+        eppo_metrics_sync.validate()
+
+
 """def test_unique_fact_property_names():
 
     eppo_metrics_sync = EppoMetricsSync(directory = None)

tests/yaml/invalid/invalid_guardrail_cutoff_sign.yaml

@@ -0,0 +1,18 @@
+fact_sources:
+  - name: upgrades_table
+    sql: select * from upgrades
+    timestamp_column: ts
+    entities:
+      - entity_name: user
+        column: user_id
+    facts:
+      - name: upgrades
+        desired_change: increase
+metrics:
+  - name: Total Upgrades to Paid Plan
+    entity: User
+    is_guardrail: true
+    guardrail_cutoff: 1
+    numerator:
+      fact_name: upgrades
+      operation: sum