Merge branch 'main' of github.com:Unleash/unleash-client-python

Author: ivanklee86

UnleashClient/__init__.py

@@ -15,6 +15,7 @@
 from UnleashClient.api import register_client
 from UnleashClient.constants import DISABLED_VARIATION, ETAG, METRIC_LAST_SENT_TIME
 from UnleashClient.events import UnleashEvent, UnleashEventType
+from UnleashClient.features import Feature
 from UnleashClient.loader import load_features
 from UnleashClient.periodic_tasks import (
     aggregate_and_send_metrics,
@@ -45,13 +46,14 @@ class UnleashClient:
 
     :param url: URL of the unleash server, required.
     :param app_name: Name of the application using the unleash client, required.
-    :param  environment: Name of the environment using the unleash client, optional & defaults to "default".
+    :param environment: Name of the environment using the unleash client, optional & defaults to "default".
     :param instance_id: Unique identifier for unleash client instance, optional & defaults to "unleash-client-python"
     :param refresh_interval: Provisioning refresh interval in seconds, optional & defaults to 15 seconds
     :param refresh_jitter: Provisioning refresh interval jitter in seconds, optional & defaults to None
     :param metrics_interval: Metrics refresh interval in seconds, optional & defaults to 60 seconds
     :param metrics_jitter: Metrics refresh interval jitter in seconds, optional & defaults to None
     :param disable_metrics: Disables sending metrics to unleash server, optional & defaults to false.
+    :param disable_registration: Disables registration with unleash server, optional & defaults to false.
     :param custom_headers: Default headers to send to unleash server, optional & defaults to empty.
     :param custom_options: Default requests parameters, optional & defaults to empty.  Can be used to skip SSL verification.
     :param custom_strategies: Dictionary of custom strategy names : custom strategy objects.
@@ -139,7 +141,7 @@ def __init__(
                 "If using a custom scheduler, you must specify a executor."
             )
         else:
-            if not scheduler:
+            if not scheduler and scheduler_executor:
                 LOGGER.warning(
                     "scheduler_executor should only be used with a custom scheduler."
                 )
@@ -287,7 +289,7 @@ def initialize_client(self, fetch_toggles: bool = True) -> None:
                 )
                 raise excep
             else:
-                # Set is_iniialized to true if no exception is encountered.
+                # Set is_initialized to true if no exception is encountered.
                 self.is_initialized = True
         else:
             warnings.warn(
@@ -348,6 +350,11 @@ def is_enabled(
                 feature = self.features[feature_name]
                 feature_check = feature.is_enabled(context)
 
+                if feature.only_for_metrics:
+                    return self._get_fallback_value(
+                        fallback_function, feature_name, context
+                    )
+
                 try:
                     if self.unleash_event_callback and feature.impression_data:
                         event = UnleashEvent(
@@ -379,9 +386,17 @@ def is_enabled(
                     "Error checking feature flag: %s",
                     excep,
                 )
+                # The feature doesn't exist, so create it to track metrics
+                new_feature = Feature.metrics_only_feature(feature_name)
+                self.features[feature_name] = new_feature
+
+                # Use the feature's is_enabled method to count the call
+                new_feature.is_enabled(context)
+
                 return self._get_fallback_value(
                     fallback_function, feature_name, context
                 )
+
         else:
             LOGGER.log(
                 self.unleash_verbose_log_level,
@@ -448,7 +463,13 @@ def get_variant(self, feature_name: str, context: Optional[dict] = None) -> dict
                     "Error checking feature flag variant: %s",
                     excep,
                 )
-                return DISABLED_VARIATION
+
+                # The feature doesn't exist, so create it to track metrics
+                new_feature = Feature.metrics_only_feature(feature_name)
+                self.features[feature_name] = new_feature
+
+                # Use the feature's get_variant method to count the call
+                return new_feature.get_variant(context)
         else:
             LOGGER.log(
                 self.unleash_verbose_log_level,

UnleashClient/cache.py

@@ -67,7 +67,7 @@ class FileCache(BaseCache):
         unleash_client = UnleashClient(
             "https://my.unleash.server.com",
             "HAMSTER_API",
-            cache=cache
+            cache=my_cache
         )
 
     :param name: Name of cache.

UnleashClient/features/Feature.py

@@ -1,5 +1,5 @@
 # pylint: disable=invalid-name
-from typing import Optional
+from typing import Dict, Optional, cast
 
 from UnleashClient.constants import DISABLED_VARIATION
 from UnleashClient.utils import LOGGER
@@ -36,6 +36,11 @@ def __init__(
         # Stats tracking
         self.yes_count = 0
         self.no_count = 0
+        ## { [ variant name ]: number }
+        self.variant_counts: Dict[str, int] = {}
+
+        # Whether the feature exists only for tracking metrics or not.
+        self.only_for_metrics = False
 
     def reset_stats(self) -> None:
         """
@@ -45,6 +50,7 @@ def reset_stats(self) -> None:
         """
         self.yes_count = 0
         self.no_count = 0
+        self.variant_counts = {}
 
     def increment_stats(self, result: bool) -> None:
         """
@@ -58,6 +64,15 @@ def increment_stats(self, result: bool) -> None:
         else:
             self.no_count += 1
 
+    def _count_variant(self, variant_name: str) -> None:
+        """
+        Count a specific variant.
+
+        :param variant_name: The name of the variant to count.
+        :return:
+        """
+        self.variant_counts[variant_name] = self.variant_counts.get(variant_name, 0) + 1
+
     def is_enabled(
         self, context: dict = None, default_value: bool = False
     ) -> bool:  # pylint: disable=unused-argument
@@ -105,4 +120,11 @@ def get_variant(self, context: dict = None) -> dict:
             except Exception as variant_exception:
                 LOGGER.warning("Error selecting variant: %s", variant_exception)
 
+        self._count_variant(cast(str, variant["name"]))
         return variant
+
+    @staticmethod
+    def metrics_only_feature(feature_name: str):
+        feature = Feature(feature_name, False, [])
+        feature.only_for_metrics = True
+        return feature

UnleashClient/loader.py

@@ -145,6 +145,11 @@ def load_features(
             "impressionData", False
         )
 
+        # If the feature had previously been added to the features list only for
+        # tracking, indicate that it is now a real feature that should be
+        # evaluated properly.
+        feature_for_update.only_for_metrics = False
+
     # Handle creation or deletions
     new_features = list(set(feature_names) - set(feature_toggles.keys()))
 

UnleashClient/periodic_tasks/send_metrics.py

@@ -26,6 +26,7 @@ def aggregate_and_send_metrics(
             features[feature_name].name: {
                 "yes": features[feature_name].yes_count,
                 "no": features[feature_name].no_count,
+                "variants": features[feature_name].variant_counts,
             }
         }
 

tests/unit_tests/api/test_feature.py

@@ -1,3 +1,6 @@
+from datetime import date
+
+import pytest
 import responses
 from pytest import mark, param
 
@@ -86,6 +89,10 @@ def test_get_feature_toggle_failed_etag():
     assert not etag
 
 
+@pytest.mark.skipif(
+    date.today() < date(2023, 7, 1),
+    reason="This is currently breaking due to a dependency or the test setup. Skipping this allows us to run tests in CI without this popping up as an error all the time.",
+)
 @responses.activate
 def test_get_feature_toggle_etag_present():
     responses.add(

tests/unit_tests/periodic/test_aggregate_and_send_metrics.py

@@ -3,6 +3,7 @@
 
 import responses
 
+from tests.utilities.mocks.mock_variants import VARIANTS
 from tests.utilities.testing_constants import (
     APP_NAME,
     CUSTOM_HEADERS,
@@ -16,6 +17,7 @@
 from UnleashClient.features import Feature
 from UnleashClient.periodic_tasks import aggregate_and_send_metrics
 from UnleashClient.strategies import Default, RemoteAddress
+from UnleashClient.variants import Variants
 
 FULL_METRICS_URL = URL + METRICS_URL
 print(FULL_METRICS_URL)
@@ -33,10 +35,19 @@ def test_aggregate_and_send_metrics():
     my_feature1.yes_count = 1
     my_feature1.no_count = 1
 
-    my_feature2 = Feature("My Feature2", True, strategies)
+    my_feature2 = Feature(
+        "My Feature2", True, strategies, variants=Variants(VARIANTS, "My Feature2")
+    )
     my_feature2.yes_count = 2
     my_feature2.no_count = 2
 
+    feature2_variant_counts = {
+        "VarA": 56,
+        "VarB": 0,
+        "VarC": 4,
+    }
+    my_feature2.variant_counts = feature2_variant_counts
+
     my_feature3 = Feature("My Feature3", True, strategies)
     my_feature3.yes_count = 0
     my_feature3.no_count = 0
@@ -53,6 +64,10 @@ def test_aggregate_and_send_metrics():
     assert len(request["bucket"]["toggles"].keys()) == 2
     assert request["bucket"]["toggles"]["My Feature1"]["yes"] == 1
     assert request["bucket"]["toggles"]["My Feature1"]["no"] == 1
+    assert (
+        request["bucket"]["toggles"]["My Feature2"]["variants"]
+        == feature2_variant_counts
+    )
     assert "My Feature3" not in request["bucket"]["toggles"].keys()
     assert cache.get(METRIC_LAST_SENT_TIME) > start_time
 

tests/unit_tests/test_client.py

@@ -410,47 +410,71 @@ def test_uc_metrics(unleash_client):
 
 
 @responses.activate
-def test_uc_disabled_registration(unleash_client_toggle_only):
-    unleash_client = unleash_client_toggle_only
-    # Set up APIs
-    responses.add(responses.POST, URL + REGISTER_URL, json={}, status=401)
+def test_uc_registers_metrics_for_nonexistent_features(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_RESPONSE, status=200
     )
-    responses.add(responses.POST, URL + METRICS_URL, json={}, status=401)
+    responses.add(responses.POST, URL + METRICS_URL, json={}, status=202)
 
+    # Create Unleash client and check initial load
     unleash_client.initialize_client()
-    unleash_client.is_enabled("testFlag")
-    time.sleep(20)
-    assert unleash_client.is_enabled("testFlag")
+    time.sleep(1)
 
-    for api_call in responses.calls:
-        assert "/api/client/features" in api_call.request.url
+    # Check a flag that doesn't exist
+    unleash_client.is_enabled("nonexistent-flag")
 
+    # Verify that the metrics are serialized
+    time.sleep(12)
+    request = json.loads(responses.calls[-1].request.body)
+    assert request["bucket"]["toggles"]["nonexistent-flag"]["no"] == 1
 
-@responses.activate
-def test_uc_server_error(unleash_client):
-    # Verify that Unleash Client will still fall back gracefully if SERVER ANGRY RAWR, and then recover gracefully.
 
-    unleash_client = unleash_client
-    # Set up APIs
-    responses.add(responses.POST, URL + REGISTER_URL, json={}, status=401)
-    responses.add(responses.GET, URL + FEATURES_URL, status=500)
-    responses.add(responses.POST, URL + METRICS_URL, json={}, status=401)
+@responses.activate
+def test_uc_registers_variant_metrics_for_nonexistent_features(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_RESPONSE, status=200
+    )
+    responses.add(responses.POST, URL + METRICS_URL, json={}, status=202)
 
+    # Create Unleash client and check initial load
     unleash_client.initialize_client()
-    assert not unleash_client.is_enabled("testFlag")
+    time.sleep(1)
 
-    responses.remove(responses.GET, URL + FEATURES_URL)
+    # Check a flag that doesn't exist
+    unleash_client.get_variant("nonexistent-flag")
+
+    # Verify that the metrics are serialized
+    time.sleep(12)
+    request = json.loads(responses.calls[-1].request.body)
+    assert request["bucket"]["toggles"]["nonexistent-flag"]["no"] == 1
+    assert request["bucket"]["toggles"]["nonexistent-flag"]["variants"]["disabled"] == 1
+
+
+@responses.activate
+def test_uc_disabled_registration(unleash_client_toggle_only):
+    unleash_client = unleash_client_toggle_only
+    # Set up APIs
+    responses.add(responses.POST, URL + REGISTER_URL, json={}, status=401)
     responses.add(
         responses.GET, URL + FEATURES_URL, json=MOCK_FEATURE_RESPONSE, status=200
     )
+    responses.add(responses.POST, URL + METRICS_URL, json={}, status=401)
+
+    unleash_client.initialize_client()
+    unleash_client.is_enabled("testFlag")
     time.sleep(20)
     assert unleash_client.is_enabled("testFlag")
 
+    for api_call in responses.calls:
+        assert "/api/client/features" in api_call.request.url
+
 
 @responses.activate
-def test_uc_server_error_recovery(unleash_client):
+def test_uc_server_error(unleash_client):
     # Verify that Unleash Client will still fall back gracefully if SERVER ANGRY RAWR, and then recover gracefully.
 
     unleash_client = unleash_client
@@ -682,7 +706,7 @@ def test_multiple_instances_no_warnings_or_errors_with_different_client_configs(
     UnleashClient(
         URL, "some-probably-unique-but-different-app-name", refresh_interval="60"
     )
-    assert not all(
+    assert not any(
         ["Multiple instances has been disabled" in r.msg for r in caplog.records]
     )
 
@@ -698,7 +722,7 @@ def test_multiple_instances_are_unique_on_api_key(caplog):
         "some-probably-unique-app-name",
         custom_headers={"Authorization": "hamsters"},
     )
-    assert not all(
+    assert not any(
         ["Multiple instances has been disabled" in r.msg for r in caplog.records]
     )
 

tests/unit_tests/test_features.py

@@ -76,3 +76,31 @@ def test_select_variation_variation(test_feature_variants):
     selected_variant = test_feature_variants.get_variant({"userId": "2"})
     assert selected_variant["enabled"]
     assert selected_variant["name"] == "VarB"
+
+
+def test_variant_metrics_are_reset(test_feature_variants):
+    test_feature_variants.get_variant({"userId": "2"})
+    assert test_feature_variants.variant_counts["VarB"] == 1
+
+    test_feature_variants.reset_stats()
+    assert not test_feature_variants.variant_counts
+
+
+def test_variant_metrics_with_existing_variant(test_feature_variants):
+    for iteration in range(1, 7):
+        test_feature_variants.get_variant({"userId": "2"})
+        assert test_feature_variants.variant_counts["VarB"] == iteration
+
+
+def test_variant_metrics_with_disabled_feature(test_feature_variants):
+    test_feature_variants.enabled = False
+    assert not test_feature_variants.is_enabled()
+    for iteration in range(1, 7):
+        test_feature_variants.get_variant({})
+        assert test_feature_variants.variant_counts["disabled"] == iteration
+
+
+def test_variant_metrics_feature_has_no_variants(test_feature):
+    for iteration in range(1, 7):
+        test_feature.get_variant({})
+        assert test_feature.variant_counts["disabled"] == iteration