Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Obey HTTP status codes from upstream. #299

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions UnleashClient/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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:
"""
Expand All @@ -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
Expand Down Expand Up @@ -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 {}
Expand All @@ -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 = (
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
49 changes: 49 additions & 0 deletions UnleashClient/api/backoff.py
Original file line number Diff line number Diff line change
@@ -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
11 changes: 11 additions & 0 deletions UnleashClient/api/features.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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.
Expand All @@ -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}
Expand Down Expand Up @@ -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(
Expand Down
3 changes: 3 additions & 0 deletions UnleashClient/api/metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand All @@ -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.")
Expand Down
3 changes: 3 additions & 0 deletions UnleashClient/periodic_tasks/fetch_and_load.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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,
Expand All @@ -30,6 +32,7 @@ def fetch_and_load_features(
request_retries,
project,
cache.get(ETAG),
backoff_strategy,
)

if feature_provisioning:
Expand Down
17 changes: 16 additions & 1 deletion UnleashClient/periodic_tasks/send_metrics.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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):
Expand Down Expand Up @@ -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.")
8 changes: 7 additions & 1 deletion tests/unit_tests/api/test_metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
Loading