fix: correctly count metrics when a flag with dependencies has disabl…

…ed parents (#304)

# Description

This PR fixes a bug where we didn't count metrics correctly: if the dependencies were not satisfied during a `get_variant` call, we wouldn't track any metrics for the flag at all. During an `is_enabled` call, we'd track the result of the child flag's evaluation without taking into account the dependencies.

Note that the **behavior** was still correct. This is only about metrics.

The root cause is related to how we check dependencies and act on that result: the Unleash client checks dependencies as part of its `is_enabled` and `get_variant` calls, but the metrics are tracked in the individual `Feature` objects.

The current implementation was the quickest way I could fix the issue. I think we should restructure it (probably; either now or later) to avoid calling internal methods of the Feature object. However, it serves as a nice way to show what's wrong and how we can fix it. (Maybe just make that `_count_variant` method non-internal?)

Due to the way the code is organized already, it might make sense to move the dependencies check into the Feature class instead of performing it in the Unleash Client. However, that would require us to have some way to access those dependencies from the dependent feature. This probably means more restructuring. I'm not sure that's worth it. Maybe it's better to leave as is? I'll take any input on it 🙋🏼 

## Type of change

Please delete options that are not relevant.

- [x] Bug fix (non-breaking change which fixes an issue)

# How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

- [x] Unit tests

# Checklist:

- [x] My code follows the style guidelines of this project
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my feature works
- [ ] New and existing unit tests pass locally with my changes
- [x] Any dependent changes have been merged and published in downstream modules

UnleashClient/__init__.py

@@ -364,9 +364,15 @@ def is_enabled(
         if self.unleash_bootstrapped or self.is_initialized:
             try:
                 feature = self.features[feature_name]
-                feature_check = feature.is_enabled(
-                    context
-                ) and self._dependencies_are_satisfied(feature_name, context)
+                dependency_check = self._dependencies_are_satisfied(
+                    feature_name, context
+                )
+
+                if dependency_check:
+                    feature_check = feature.is_enabled(context)
+                else:
+                    feature.increment_stats(False)
+                    feature_check = False
 
                 if feature.only_for_metrics:
                     return self._get_fallback_value(
@@ -449,6 +455,8 @@ def get_variant(self, feature_name: str, context: Optional[dict] = None) -> dict
                 feature = self.features[feature_name]
 
                 if not self._dependencies_are_satisfied(feature_name, context):
+                    feature.increment_stats(False)
+                    feature._count_variant("disabled")
                     return DISABLED_VARIATION
 
                 variant_check = feature.get_variant(context)

tests/unit_tests/test_client.py

@@ -572,6 +572,7 @@ def test_uc_doesnt_count_metrics_for_dependency_parents(unleash_client):
     time.sleep(1)
 
     child = "ChildWithVariant"
+    parent = "Parent"
     # Check a flag that depends on a parent
     unleash_client.is_enabled(child)
     unleash_client.get_variant(child)
@@ -581,7 +582,37 @@ def test_uc_doesnt_count_metrics_for_dependency_parents(unleash_client):
     request = json.loads(responses.calls[-1].request.body)
     assert request["bucket"]["toggles"][child]["yes"] == 2
     assert request["bucket"]["toggles"][child]["variants"]["childVariant"] == 1
-    assert "Parent" not in request["bucket"]["toggles"]
+    assert parent not in request["bucket"]["toggles"]
+
+
+@responses.activate
+def test_uc_counts_metrics_for_child_even_if_parent_is_disabled(unleash_client):
+    # Set up API
+    responses.add(responses.POST, URL + REGISTER_URL, json={}, status=202)
+    responses.add(
+        responses.GET,
+        URL + FEATURES_URL,
+        json=MOCK_FEATURE_WITH_DEPENDENCIES_RESPONSE,
+        status=200,
+    )
+    responses.add(responses.POST, URL + METRICS_URL, json={}, status=202)
+
+    # Create Unleash client and check initial load
+    unleash_client.initialize_client()
+    time.sleep(1)
+
+    child = "WithDisabledDependency"
+    parent = "Disabled"
+    # Check a flag that depends on a disabled parent
+    unleash_client.is_enabled(child)
+    unleash_client.get_variant(child)
+
+    # Verify that the parent doesn't have any metrics registered
+    time.sleep(12)
+    request = json.loads(responses.calls[-1].request.body)
+    assert request["bucket"]["toggles"][child]["no"] == 2
+    assert request["bucket"]["toggles"][child]["variants"]["disabled"] == 1
+    assert parent not in request["bucket"]["toggles"]
 
 
 @responses.activate