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

feat(feature-flags): Bring your own logger for debug #709

Merged
merged 17 commits into from
Oct 1, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
96cbdc1
fix(idempotency): sorting keys before hashing
heitorlessa Aug 22, 2021
b9fa07a
Merge branch 'fix/idempotency-hash-order' into develop
heitorlessa Aug 22, 2021
06c3250
Merge branch 'develop' of https://github.com/awslabs/aws-lambda-power…
heitorlessa Sep 28, 2021
573ef89
Merge branch 'develop' of https://github.com/awslabs/aws-lambda-power…
heitorlessa Sep 28, 2021
4d7cf6b
Add logger to class inits.
Sep 28, 2021
dd6f67f
Updated documentation
Sep 28, 2021
97d5f96
Update aws_lambda_powertools/utilities/feature_flags/appconfig.py
gwlester Sep 30, 2021
29633dc
Update aws_lambda_powertools/utilities/feature_flags/feature_flags.py
gwlester Sep 30, 2021
fd516d2
Merge branch 'develop' of https://github.com/awslabs/aws-lambda-power…
heitorlessa Oct 1, 2021
dc14b5e
Merge branch 'develop' of https://github.com/awslabs/aws-lambda-power…
heitorlessa Oct 1, 2021
1233966
Add missing period to logger.debug
Oct 1, 2021
3d4305b
feat: add get_raw_configuration property in store; expose store
heitorlessa Oct 1, 2021
3493789
Merge branch 'develop' of https://github.com/awslabs/aws-lambda-power…
heitorlessa Oct 1, 2021
d0bd984
Merge branch 'develop' of https://github.com/awslabs/aws-lambda-power…
heitorlessa Oct 1, 2021
5e9b208
Merge branch 'develop' into feature-702
heitorlessa Oct 1, 2021
93f8a5c
fix: type annotation, clarify logger in docs
heitorlessa Oct 1, 2021
370d9ca
fix: remove logger from staticmethod
heitorlessa Oct 1, 2021
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
9 changes: 7 additions & 2 deletions aws_lambda_powertools/utilities/feature_flags/appconfig.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@
from .base import StoreProvider
from .exceptions import ConfigurationStoreError, StoreClientError

logger = logging.getLogger(__name__)

TRANSFORM_TYPE = "json"


Expand All @@ -25,6 +23,7 @@ def __init__(
sdk_config: Optional[Config] = None,
envelope: Optional[str] = "",
jmespath_options: Optional[Dict] = None,
logger=None,
):
"""This class fetches JSON schemas from AWS AppConfig

Expand All @@ -44,8 +43,14 @@ def __init__(
JMESPath expression to pluck feature flags data from config
jmespath_options : Optional[Dict]
Alternative JMESPath options to be included when filtering expr
logger: A logging object
Used to log messages. If None is supplied, one will be created.
gwlester marked this conversation as resolved.
Show resolved Hide resolved
"""
super().__init__()
if logger == None:
self.logger = logging.getLogger(__name__)
else:
self.logger = logger
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since None is a falsy value, you can make this more compact by changing to:

self.logger = logger or logging.getLogger(__name__)

self.environment = environment
self.application = application
self.name = name
Expand Down
43 changes: 23 additions & 20 deletions aws_lambda_powertools/utilities/feature_flags/feature_flags.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,9 @@
from .base import StoreProvider
from .exceptions import ConfigurationStoreError

logger = logging.getLogger(__name__)


class FeatureFlags:
def __init__(self, store: StoreProvider):
def __init__(self, store: StoreProvider, logger=None):
"""Evaluates whether feature flags should be enabled based on a given context.

It uses the provided store to fetch feature flag rules before evaluating them.
Expand All @@ -35,11 +33,16 @@ def __init__(self, store: StoreProvider):
----------
store: StoreProvider
Store to use to fetch feature flag schema configuration.
logger: A logging object
Used to log messages. If None is supplied, one will be created.
gwlester marked this conversation as resolved.
Show resolved Hide resolved
"""
self._store = store
if logger == None:
self.logger = logging.getLogger(__name__)
else:
self.logger = logger

@staticmethod
def _match_by_action(action: str, condition_value: Any, context_value: Any) -> bool:
def _match_by_action(self, action: str, condition_value: Any, context_value: Any) -> bool:
if not context_value:
return False
mapping_by_action = {
Expand All @@ -54,7 +57,7 @@ def _match_by_action(action: str, condition_value: Any, context_value: Any) -> b
func = mapping_by_action.get(action, lambda a, b: False)
return func(context_value, condition_value)
except Exception as exc:
logger.debug(f"caught exception while matching action: action={action}, exception={str(exc)}")
self.loggerdebug(f"caught exception while matching action: action={action}, exception={str(exc)}")
gwlester marked this conversation as resolved.
Show resolved Hide resolved
return False

def _evaluate_conditions(
Expand All @@ -65,7 +68,7 @@ def _evaluate_conditions(
conditions = cast(List[Dict], rule.get(schema.CONDITIONS_KEY))

if not conditions:
logger.debug(
self.loggerdebug(
gwlester marked this conversation as resolved.
Show resolved Hide resolved
f"rule did not match, no conditions to match, rule_name={rule_name}, rule_value={rule_match_value}, "
f"name={feature_name} "
)
Expand All @@ -77,13 +80,13 @@ def _evaluate_conditions(
cond_value = condition.get(schema.CONDITION_VALUE)

if not self._match_by_action(action=cond_action, condition_value=cond_value, context_value=context_value):
logger.debug(
self.loggerdebug(
gwlester marked this conversation as resolved.
Show resolved Hide resolved
f"rule did not match action, rule_name={rule_name}, rule_value={rule_match_value}, "
f"name={feature_name}, context_value={str(context_value)} "
)
return False # context doesn't match condition

logger.debug(f"rule matched, rule_name={rule_name}, rule_value={rule_match_value}, name={feature_name}")
self.loggerdebug(f"rule matched, rule_name={rule_name}, rule_value={rule_match_value}, name={feature_name}")
gwlester marked this conversation as resolved.
Show resolved Hide resolved
return True

def _evaluate_rules(
Expand All @@ -94,12 +97,12 @@ def _evaluate_rules(
rule_match_value = rule.get(schema.RULE_MATCH_VALUE)

# Context might contain PII data; do not log its value
logger.debug(f"Evaluating rule matching, rule={rule_name}, feature={feature_name}, default={feat_default}")
self.loggerdebug(f"Evaluating rule matching, rule={rule_name}, feature={feature_name}, default={feat_default}")
if self._evaluate_conditions(rule_name=rule_name, feature_name=feature_name, rule=rule, context=context):
return bool(rule_match_value)

# no rule matched, return default value of feature
logger.debug(f"no rule matched, returning feature default, default={feat_default}, name={feature_name}")
self.loggerdebug(f"no rule matched, returning feature default, default={feat_default}, name={feature_name}")
return feat_default
return False

Expand Down Expand Up @@ -146,7 +149,7 @@ def get_configuration(self) -> Union[Dict[str, Dict], Dict]:
```
"""
# parse result conf as JSON, keep in cache for max age defined in store
logger.debug(f"Fetching schema from registered store, store={self._store}")
self.loggerdebug(f"Fetching schema from registered store, store={self._store}")
config = self._store.get_configuration()
validator = schema.SchemaValidator(schema=config)
validator.validate()
Expand Down Expand Up @@ -190,21 +193,21 @@ def evaluate(self, *, name: str, context: Optional[Dict[str, Any]] = None, defau
try:
features = self.get_configuration()
except ConfigurationStoreError as err:
logger.debug(f"Failed to fetch feature flags from store, returning default provided, reason={err}")
self.loggerdebug(f"Failed to fetch feature flags from store, returning default provided, reason={err}")
return default

feature = features.get(name)
if feature is None:
logger.debug(f"Feature not found; returning default provided, name={name}, default={default}")
self.loggerdebug(f"Feature not found; returning default provided, name={name}, default={default}")
return default

rules = feature.get(schema.RULES_KEY)
feat_default = feature.get(schema.FEATURE_DEFAULT_VAL_KEY)
if not rules:
logger.debug(f"no rules found, returning feature default, name={name}, default={feat_default}")
self.loggerdebug(f"no rules found, returning feature default, name={name}, default={feat_default}")
return bool(feat_default)

logger.debug(f"looking for rule match, name={name}, default={feat_default}")
self.loggerdebug(f"looking for rule match, name={name}, default={feat_default}")
return self._evaluate_rules(feature_name=name, context=context, feat_default=bool(feat_default), rules=rules)

def get_enabled_features(self, *, context: Optional[Dict[str, Any]] = None) -> List[str]:
Expand Down Expand Up @@ -241,20 +244,20 @@ def get_enabled_features(self, *, context: Optional[Dict[str, Any]] = None) -> L
try:
features: Dict[str, Any] = self.get_configuration()
except ConfigurationStoreError as err:
logger.debug(f"Failed to fetch feature flags from store, returning empty list, reason={err}")
self.loggerdebug(f"Failed to fetch feature flags from store, returning empty list, reason={err}")
return features_enabled

logger.debug("Evaluating all features")
self.loggerdebug("Evaluating all features")
for name, feature in features.items():
rules = feature.get(schema.RULES_KEY, {})
feature_default_value = feature.get(schema.FEATURE_DEFAULT_VAL_KEY)
if feature_default_value and not rules:
logger.debug(f"feature is enabled by default and has no defined rules, name={name}")
self.loggerdebug(f"feature is enabled by default and has no defined rules, name={name}")
features_enabled.append(name)
elif self._evaluate_rules(
feature_name=name, context=context, feat_default=feature_default_value, rules=rules
):
logger.debug(f"feature's calculated value is True, name={name}")
self.loggerdebug(f"feature's calculated value is True, name={name}")
features_enabled.append(name)

return features_enabled
21 changes: 11 additions & 10 deletions aws_lambda_powertools/utilities/feature_flags/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@
from .base import BaseValidator
from .exceptions import SchemaValidationError

logger = logging.getLogger(__name__)

RULES_KEY = "rules"
FEATURE_DEFAULT_VAL_KEY = "default"
CONDITIONS_KEY = "conditions"
Expand Down Expand Up @@ -105,11 +103,15 @@ class SchemaValidator(BaseValidator):
```
"""

def __init__(self, schema: Dict[str, Any]):
def __init__(self, schema: Dict[str, Any], logger=None):
self.schema = schema
if logger == None:
self.logger = logging.getLogger(__name__)
else:
self.logger = logger

def validate(self) -> None:
logger.debug("Validating schema")
self.loggerdebug("Validating schema")
if not isinstance(self.schema, dict):
raise SchemaValidationError(f"Features must be a dictionary, schema={str(self.schema)}")

Expand All @@ -125,7 +127,7 @@ def __init__(self, schema: Dict):

def validate(self):
for name, feature in self.schema.items():
logger.debug(f"Attempting to validate feature '{name}'")
self.loggerdebug(f"Attempting to validate feature '{name}'")
self.validate_feature(name, feature)
rules = RulesValidator(feature=feature)
rules.validate()
Expand All @@ -150,14 +152,14 @@ def __init__(self, feature: Dict[str, Any]):

def validate(self):
if not self.rules:
logger.debug("Rules are empty, ignoring validation")
self.loggerdebug("Rules are empty, ignoring validation")
return

if not isinstance(self.rules, dict):
raise SchemaValidationError(f"Feature rules must be a dictionary, feature={self.feature_name}")

for rule_name, rule in self.rules.items():
logger.debug(f"Attempting to validate rule '{rule_name}'")
self.loggerdebug(f"Attempting to validate rule '{rule_name}'")
self.validate_rule(rule=rule, rule_name=rule_name, feature_name=self.feature_name)
conditions = ConditionsValidator(rule=rule, rule_name=rule_name)
conditions.validate()
Expand Down Expand Up @@ -194,13 +196,12 @@ def validate(self):
for condition in self.conditions:
self.validate_condition(rule_name=self.rule_name, condition=condition)

@staticmethod
def validate_condition(rule_name: str, condition: Dict[str, str]) -> None:
def validate_condition(self,rule_name: str, condition: Dict[str, str]) -> None:
if not condition or not isinstance(condition, dict):
raise SchemaValidationError(f"Feature rule condition must be a dictionary, rule={rule_name}")

# Condition can contain PII data; do not log condition value
logger.debug(f"Attempting to validate condition for '{rule_name}'")
self.loggerdebug(f"Attempting to validate condition for '{rule_name}'")
ConditionsValidator.validate_condition_action(condition=condition, rule_name=rule_name)
ConditionsValidator.validate_condition_key(condition=condition, rule_name=rule_name)
ConditionsValidator.validate_condition_value(condition=condition, rule_name=rule_name)
Expand Down
1 change: 1 addition & 0 deletions docs/utilities/feature_flags.md
Original file line number Diff line number Diff line change
Expand Up @@ -548,6 +548,7 @@ Parameter | Default | Description
**max_age** | `5` | Number of seconds to cache feature flags configuration fetched from AWS AppConfig
**sdk_config** | `None` | [Botocore Config object](https://botocore.amazonaws.com/v1/documentation/api/latest/reference/config.html){target="_blank"}
**jmespath_options** | `None` | For advanced use cases when you want to bring your own [JMESPath functions](https://github.com/jmespath/jmespath.py#custom-functions){target="_blank"}
**logger** | `None` | Logger to use. If None is supplied, then a logger will be created.
heitorlessa marked this conversation as resolved.
Show resolved Hide resolved

=== "appconfig_store_example.py"

Expand Down