diff --git a/UnleashClient/__init__.py b/UnleashClient/__init__.py index 89bf9fb2..1aac98d4 100644 --- a/UnleashClient/__init__.py +++ b/UnleashClient/__init__.py @@ -13,6 +13,7 @@ from apscheduler.triggers.interval import IntervalTrigger from UnleashClient.api import register_client +from UnleashClient.api.backoff import BackoffStrategy from UnleashClient.constants import ( DISABLED_VARIATION, ETAG, @@ -45,6 +46,14 @@ INSTANCES = InstanceCounter() +class BackoffStrategies: + def __init__( + self, refresh_strategy=BackoffStrategy(), metrics_strategy=BackoffStrategy() + ): + self.refresh_strategy = refresh_strategy + self.metrics_strategy = metrics_strategy + + # pylint: disable=dangerous-default-value class UnleashClient: """ @@ -55,6 +64,7 @@ class UnleashClient: :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 backoff_strategies: Encapsulates 2 backoff strategies for refresh and metrics, optional & defaults BackoffStrategy for both :params request_timeout: Timeout for requests to unleash server in seconds, optional & defaults to 30 seconds :params request_retries: Number of retries for requests to unleash server, optional & defaults to 3 :param refresh_jitter: Provisioning refresh interval jitter in seconds, optional & defaults to None @@ -99,6 +109,7 @@ def __init__( scheduler_executor: Optional[str] = None, multiple_instance_mode: InstanceAllowType = InstanceAllowType.WARN, event_callback: Optional[Callable[[UnleashEvent], None]] = None, + backoff_strategies: BackoffStrategies = BackoffStrategies(), ) -> None: custom_headers = custom_headers or {} custom_options = custom_options or {} @@ -110,6 +121,7 @@ def __init__( self.unleash_environment = environment self.unleash_instance_id = instance_id self.unleash_refresh_interval = refresh_interval + self.unleash_refresh_backoff = backoff_strategies.refresh_strategy self.unleash_request_timeout = request_timeout self.unleash_request_retries = request_retries self.unleash_refresh_jitter = ( @@ -119,6 +131,7 @@ def __init__( self.unleash_metrics_jitter = ( int(metrics_jitter) if metrics_jitter is not None else None ) + self.unleash_metrics_backoff = backoff_strategies.metrics_strategy self.unleash_disable_metrics = disable_metrics self.unleash_disable_registration = disable_registration self.unleash_custom_headers = custom_headers @@ -237,6 +250,7 @@ def initialize_client(self, fetch_toggles: bool = True) -> None: "features": self.features, "cache": self.cache, "request_timeout": self.unleash_request_timeout, + "backoff_strategy": self.unleash_metrics_backoff, } # Register app @@ -265,6 +279,7 @@ def initialize_client(self, fetch_toggles: bool = True) -> None: "request_timeout": self.unleash_request_timeout, "request_retries": self.unleash_request_retries, "project": self.unleash_project_name, + "backoff_strategy": self.unleash_refresh_backoff, } job_func: Callable = fetch_and_load_features else: diff --git a/UnleashClient/api/backoff.py b/UnleashClient/api/backoff.py new file mode 100644 index 00000000..2ccf7ce4 --- /dev/null +++ b/UnleashClient/api/backoff.py @@ -0,0 +1,49 @@ +from UnleashClient.utils import LOGGER + + +class BackoffStrategy: + def __init__(self): + self.failures = 0 + self.skips = 0 + self.max_skips = 10 + self.normal_interval_seconds = 5 + self.longest_acceptable_interval_seconds = 60 * 60 * 24 * 7 # 1 week + self.initialized = ( + False # Consider not initialized until we have a successful call to the API + ) + + def handle_response(self, url: str, status_code: int): + if self.initialized and status_code in [401, 403, 404]: + self.skips = self.max_skips + self.failures += 1 + LOGGER.error( + f"Server said that the endpoint at {url} does not exist. Backing off to {self.skips} times our poll interval (of {self.normal_interval_seconds} seconds) to avoid overloading server" + if status_code == 404 + else f"Client was not authorized to talk to the Unleash API at {url}. Backing off to {self.skips} times our poll interval (of {self.normal_interval_seconds} seconds) to avoid overloading server" + ) + + elif self.initialized and status_code == 429: + self.failures += 1 + self.skips = min(self.max_skips, self.failures) + LOGGER.info( + f"RATE LIMITED for the {self.failures} time. Further backing off. Current backoff at {self.skips} times our interval (of {self.normal_interval_seconds} seconds)" + ) + elif self.initialized and status_code > 500: + self.failures += 1 + self.skips = min(self.max_skips, self.failures) + LOGGER.info( + f"Server failed with a {status_code} status code. Backing off. Current backoff at {self.skips} times our poll interval (of {self.normal_interval_seconds} seconds)" + ) + + else: + # successful response + self.initialized = True + if self.failures > 0: + self.failures -= 1 + self.skips = max(0, self.failures) + + def performAction(self): + return self.skips <= 0 + + def skipped(self): + self.skips -= 1 diff --git a/UnleashClient/api/features.py b/UnleashClient/api/features.py index 210c2d6b..268e04c8 100644 --- a/UnleashClient/api/features.py +++ b/UnleashClient/api/features.py @@ -4,6 +4,7 @@ from requests.adapters import HTTPAdapter from urllib3 import Retry +from UnleashClient.api.backoff import BackoffStrategy from UnleashClient.constants import FEATURES_URL from UnleashClient.utils import LOGGER, log_resp_info @@ -19,6 +20,7 @@ def get_feature_toggles( request_retries: int, project: Optional[str] = None, cached_etag: str = "", + backoff_strategy: Optional[BackoffStrategy] = None, ) -> Tuple[dict, str]: """ Retrieves feature flags from unleash central server. @@ -36,9 +38,17 @@ def get_feature_toggles( :param request_retries: :param project: :param cached_etag: + :param backoff_strategy: :return: (Feature flags, etag) if successful, ({},'') if not """ try: + backoff_strategy = ( + backoff_strategy or BackoffStrategy() + ) # TODO creating it here doesn't make sense + if not backoff_strategy.performAction(): + backoff_strategy.skipped() + return {}, "" + LOGGER.info("Getting feature flag.") headers = {"UNLEASH-APPNAME": app_name, "UNLEASH-INSTANCEID": instance_id} @@ -66,6 +76,7 @@ def get_feature_toggles( **custom_options, ) + backoff_strategy.handle_response(base_url, resp.status_code) if resp.status_code not in [200, 304]: log_resp_info(resp) LOGGER.warning( diff --git a/UnleashClient/api/metrics.py b/UnleashClient/api/metrics.py index 0b35dcc9..4eda6c8f 100644 --- a/UnleashClient/api/metrics.py +++ b/UnleashClient/api/metrics.py @@ -2,6 +2,7 @@ import requests +from UnleashClient.api.backoff import BackoffStrategy from UnleashClient.constants import APPLICATION_HEADERS, METRICS_URL from UnleashClient.utils import LOGGER, log_resp_info @@ -13,6 +14,7 @@ def send_metrics( custom_headers: dict, custom_options: dict, request_timeout: int, + backoff_strategy: BackoffStrategy, ) -> bool: """ Attempts to send metrics to Unleash server @@ -39,6 +41,7 @@ def send_metrics( **custom_options, ) + backoff_strategy.handle_response(url + METRICS_URL, resp.status_code) if resp.status_code != 202: log_resp_info(resp) LOGGER.warning("Unleash CLient metrics submission failed.") diff --git a/UnleashClient/periodic_tasks/fetch_and_load.py b/UnleashClient/periodic_tasks/fetch_and_load.py index b9d9acfc..d8ef3888 100644 --- a/UnleashClient/periodic_tasks/fetch_and_load.py +++ b/UnleashClient/periodic_tasks/fetch_and_load.py @@ -1,6 +1,7 @@ from typing import Optional from UnleashClient.api import get_feature_toggles +from UnleashClient.api.backoff import BackoffStrategy from UnleashClient.cache import BaseCache from UnleashClient.constants import ETAG, FEATURES_URL from UnleashClient.loader import load_features @@ -19,6 +20,7 @@ def fetch_and_load_features( request_timeout: int, request_retries: int, project: Optional[str] = None, + backoff_strategy: Optional[BackoffStrategy] = BackoffStrategy(), ) -> None: (feature_provisioning, etag) = get_feature_toggles( url, @@ -30,6 +32,7 @@ def fetch_and_load_features( request_retries, project, cache.get(ETAG), + backoff_strategy, ) if feature_provisioning: diff --git a/UnleashClient/periodic_tasks/send_metrics.py b/UnleashClient/periodic_tasks/send_metrics.py index 3df7ec7d..bd1bbb7f 100644 --- a/UnleashClient/periodic_tasks/send_metrics.py +++ b/UnleashClient/periodic_tasks/send_metrics.py @@ -1,7 +1,9 @@ from collections import ChainMap from datetime import datetime, timezone +from typing import Optional from UnleashClient.api import send_metrics +from UnleashClient.api.backoff import BackoffStrategy from UnleashClient.cache import BaseCache from UnleashClient.constants import METRIC_LAST_SENT_TIME from UnleashClient.utils import LOGGER @@ -16,8 +18,15 @@ def aggregate_and_send_metrics( features: dict, cache: BaseCache, request_timeout: int, + backoff_strategy: Optional[BackoffStrategy] = None, ) -> None: feature_stats_list = [] + backoff_strategy = ( + backoff_strategy or BackoffStrategy() + ) # TODO creating it here doesn't make sense + if not backoff_strategy.performAction(): + backoff_strategy.skipped() + return {}, "" for feature_name in features.keys(): if not (features[feature_name].yes_count or features[feature_name].no_count): @@ -46,8 +55,14 @@ def aggregate_and_send_metrics( if feature_stats_list: send_metrics( - url, metrics_request, custom_headers, custom_options, request_timeout + url, + metrics_request, + custom_headers, + custom_options, + request_timeout, + backoff_strategy, ) + # TODO should we do if send_metrics then update cache? We're also updating in the case of an exception cache.set(METRIC_LAST_SENT_TIME, datetime.now(timezone.utc)) else: LOGGER.debug("No feature flags with metrics, skipping metrics submission.") diff --git a/tests/unit_tests/api/test_metrics.py b/tests/unit_tests/api/test_metrics.py index 2e3d3567..f368f911 100644 --- a/tests/unit_tests/api/test_metrics.py +++ b/tests/unit_tests/api/test_metrics.py @@ -10,6 +10,7 @@ URL, ) from UnleashClient.api import send_metrics +from UnleashClient.api.backoff import BackoffStrategy from UnleashClient.constants import METRICS_URL FULL_METRICS_URL = URL + METRICS_URL @@ -33,7 +34,12 @@ def test_send_metrics(payload, status, expected): responses.add(responses.POST, FULL_METRICS_URL, **payload, status=status) result = send_metrics( - URL, MOCK_METRICS_REQUEST, CUSTOM_HEADERS, CUSTOM_OPTIONS, REQUEST_TIMEOUT + URL, + MOCK_METRICS_REQUEST, + CUSTOM_HEADERS, + CUSTOM_OPTIONS, + REQUEST_TIMEOUT, + BackoffStrategy(), ) assert len(responses.calls) == 1