diff --git a/aws_lambda_powertools/utilities/feature_flags/feature_flags.py b/aws_lambda_powertools/utilities/feature_flags/feature_flags.py index 3bc7d87b9e8..1244db34aa3 100644 --- a/aws_lambda_powertools/utilities/feature_flags/feature_flags.py +++ b/aws_lambda_powertools/utilities/feature_flags/feature_flags.py @@ -91,6 +91,10 @@ def _match_by_action( condition_value: Any, context_value: Any, context_key_present: bool = True, + *, + feature_name: str | None = None, + rule_name: str | None = None, + context_key: str | None = None, ) -> bool: try: func = RULE_ACTION_MAPPING.get(action, lambda a, b: False) @@ -107,6 +111,13 @@ def _match_by_action( return matched except Exception as exc: self.logger.debug(f"caught exception while matching action: action={action}, exception={str(exc)}") + if context_key_present: + # Missing keys are ordinary non-matches. For invalid operands, identify the condition + # without logging values or exception messages that might contain customer data. + self.logger.warning( + f"Failed to evaluate feature flag condition: feature={feature_name}, rule={rule_name}, " + f"key={context_key}, action={action}, exception_type={type(exc).__name__}", + ) handler = self._lookup_exception_handler(exc) if handler: @@ -155,6 +166,9 @@ def _evaluate_conditions( condition_value=cond_value, context_value=context_value, context_key_present=context_key_present, + feature_name=feature_name, + rule_name=rule_name, + context_key=cond_key, ): self.logger.debug( f"rule did not match action, rule_name={rule_name}, rule_value={rule_match_value}, " diff --git a/docs/utilities/feature_flags.md b/docs/utilities/feature_flags.md index 0ccf6e74a3e..ba5212f9d2f 100644 --- a/docs/utilities/feature_flags.md +++ b/docs/utilities/feature_flags.md @@ -420,6 +420,10 @@ The `conditions` block is a list of conditions that contain `action`, `key`, and --8<-- "examples/feature_flags/src/conditions.json" ``` +If a comparator raises an exception, such as comparing a string with a number, a warning identifies the feature, rule, key, action, and exception type. +The warning excludes operand values and exception messages. The condition still evaluates as a non-match unless a registered validation exception handler returns a different result. +Missing context keys and ordinary non-matches do not produce warnings. + The `action` configuration can have the following values, where the expressions **`a`** is the `key` and **`b`** is the `value` above: | Action | Equivalent expression | diff --git a/tests/functional/feature_flags/test_comparator_warnings.py b/tests/functional/feature_flags/test_comparator_warnings.py new file mode 100644 index 00000000000..27712c9cddb --- /dev/null +++ b/tests/functional/feature_flags/test_comparator_warnings.py @@ -0,0 +1,112 @@ +from __future__ import annotations + +import logging +from typing import Any + +import pytest + +from aws_lambda_powertools.utilities.feature_flags import FeatureFlags +from aws_lambda_powertools.utilities.feature_flags.base import StoreProvider +from aws_lambda_powertools.utilities.feature_flags.feature_flags import RULE_ACTION_MAPPING + + +class InMemoryStore(StoreProvider): + def __init__(self, action: str, value: Any): + self.configuration = { + "premium": { + "default": False, + "rules": { + "eligible_customer": { + "when_match": True, + "conditions": [{"key": "customer", "action": action, "value": value}], + }, + }, + }, + } + + @property + def get_raw_configuration(self) -> dict[str, Any]: + return self.configuration + + def get_configuration(self) -> dict[str, Any]: + return self.configuration + + +@pytest.mark.parametrize("all_features", [False, True]) +@pytest.mark.parametrize( + "action,context_value,condition_value,exception_type", + [ + ("STARTSWITH", 123, "sensitive-condition", "AttributeError"), + ("KEY_GREATER_THAN_VALUE", "sensitive-context", 123, "TypeError"), + ("ANY_IN_VALUE", "sensitive-context", ["sensitive-condition"], "ValueError"), + ], +) +def test_comparator_failure_warns_without_changing_result( + caplog, + all_features, + action, + context_value, + condition_value, + exception_type, +): + # GIVEN a valid rule whose operands are incompatible at evaluation time + flags = FeatureFlags(InMemoryStore(action, condition_value)) + + # WHEN either public evaluation API evaluates that rule at the default log level + with caplog.at_level(logging.WARNING): + if all_features: + assert flags.get_enabled_features(context={"customer": context_value}) == [] + else: + assert flags.evaluate(name="premium", context={"customer": context_value}, default=True) is False + + # THEN identify the failing condition without exposing operand values + assert len(caplog.records) == 1 + message = caplog.records[0].getMessage() + for value in ("premium", "eligible_customer", "customer", action, exception_type): + assert value in message + assert "sensitive-context" not in message + assert "sensitive-condition" not in message + assert caplog.records[0].levelno == logging.WARNING + + +@pytest.mark.parametrize("context", [{}, {"customer": "ordinary"}]) +def test_missing_context_and_valid_nonmatch_do_not_warn(caplog, context): + flags = FeatureFlags(InMemoryStore("STARTSWITH", "premium")) + + with caplog.at_level(logging.WARNING): + assert flags.evaluate(name="premium", context=context, default=True) is False + + assert not caplog.records + + +def test_comparator_warning_preserves_exception_handler(caplog): + flags = FeatureFlags(InMemoryStore("STARTSWITH", "premium")) + handled = [] + + @flags.validation_exception_handler(AttributeError) + def handle_error(exc): + handled.append(exc) + return True + + with caplog.at_level(logging.WARNING): + assert flags.evaluate(name="premium", context={"customer": 123}, default=False) is True + + assert len(handled) == 1 + assert isinstance(handled[0], AttributeError) + assert len(caplog.records) == 1 + + +def test_warning_excludes_exception_message(caplog, monkeypatch): + def fail_with_customer_data(context_value, condition_value): + raise ValueError(f"Cannot compare {context_value!r} and {condition_value!r}") + + monkeypatch.setitem(RULE_ACTION_MAPPING, "STARTSWITH", fail_with_customer_data) + flags = FeatureFlags(InMemoryStore("STARTSWITH", "sensitive-condition")) + + with caplog.at_level(logging.WARNING): + assert flags.evaluate(name="premium", context={"customer": "sensitive-context"}, default=True) is False + + assert len(caplog.records) == 1 + assert "ValueError" in caplog.text + assert "sensitive-context" not in caplog.text + assert "sensitive-condition" not in caplog.text