diff --git a/.sampo/changesets/unresolved-local-flags.md b/.sampo/changesets/unresolved-local-flags.md new file mode 100644 index 000000000..282fa87ab --- /dev/null +++ b/.sampo/changesets/unresolved-local-flags.md @@ -0,0 +1,5 @@ +--- +pypi/posthog: minor +--- + +Report flags that local evaluation could not resolve. `FeatureFlagEvaluations.unresolved_flags` maps each such flag to an `UnresolvedFlagReason`, and reading one reports the `local_evaluation_inconclusive` error instead of `flag_missing`. Log a warning once per definition when a loaded flag has experience continuity enabled, and evaluate an inactive flag with experience continuity to `false` locally. diff --git a/posthog/__init__.py b/posthog/__init__.py index 5d6ae1193..4f3b87c22 100644 --- a/posthog/__init__.py +++ b/posthog/__init__.py @@ -83,6 +83,7 @@ FeatureFlagEvaluationRuntime as FeatureFlagEvaluationRuntime, FlagValue as FlagValue, FlagsAndPayloads as FlagsAndPayloads, + UnresolvedFlagReason as UnresolvedFlagReason, ) from posthog.types import ( FeatureFlagResult as FeatureFlagResult, diff --git a/posthog/client.py b/posthog/client.py index 2bb16c492..ba6733eff 100644 --- a/posthog/client.py +++ b/posthog/client.py @@ -113,6 +113,7 @@ FlagsResponse, FlagValue, SendFeatureFlagsOptions, + UnresolvedFlagReason, normalize_flags_response, to_flags_and_payloads, to_payloads, @@ -924,6 +925,9 @@ def __init__( self._flag_definition_cache_generation = 0 self._flag_definition_publication_lock = threading.RLock() self._flag_definition_cache_write_lock = threading.RLock() + # Flag key -> definition of flags already warned about as never resolvable + # locally, so each warning fires once until the definition changes. + self._warned_unresolvable_flags: Dict[str, str] = {} self._flag_definition_cache_provider = flag_definition_cache_provider self._flag_definition_cache_provider_async_runner: Optional[ _BackgroundEventLoopRunner @@ -3174,6 +3178,7 @@ def _update_flag_state( self._minimal_flag_called_events = ( data.get("minimal_flag_called_events") is True ) + self._warn_unresolvable_flags(data["flags"]) if fingerprint != self._flag_definition_fingerprint: self._flag_definition_fingerprint = fingerprint @@ -3187,6 +3192,21 @@ def _update_flag_state( return old_fingerprint return None + def _warn_unresolvable_flags(self, flags: List[Dict[str, Any]]) -> None: + for flag in flags or []: + if not flag.get("active") or not flag.get("ensure_experience_continuity"): + continue + key = flag["key"] + definition = json.dumps(flag, sort_keys=True, default=str) + if self._warned_unresolvable_flags.get(key) == definition: + continue + self._warned_unresolvable_flags[key] = definition + self.log.warning( + f"[FEATURE FLAGS] Flag '{key}' can't be evaluated locally " + f"(reason: {UnresolvedFlagReason.EXPERIENCE_CONTINUITY.value}); " + "it is resolved only by remote evaluation" + ) + def _local_evaluation_snapshot(self) -> _LocalEvaluationSnapshot: # Capture references together. Publication replaces these collections, so # recursive and multi-flag evaluations can finish on their original rules. @@ -3491,12 +3511,15 @@ def _compute_flag_locally( # Create evaluation cache for flag dependencies evaluation_cache: dict[str, Optional[FlagValue]] = {} - if feature_flag.get("ensure_experience_continuity", False): - raise InconclusiveMatchError("Flag has experience continuity enabled") - if not feature_flag.get("active"): return False + if feature_flag.get("ensure_experience_continuity", False): + raise InconclusiveMatchError( + "Flag has experience continuity enabled", + reason=UnresolvedFlagReason.EXPERIENCE_CONTINUITY, + ) + flag_filters = feature_flag.get("filters") or {} aggregation_group_type_index = flag_filters.get("aggregation_group_type_index") group_type_mapping = snapshot["group_type_mapping"] @@ -3509,7 +3532,10 @@ def _compute_flag_locally( f"[FEATURE FLAGS] Unknown group type index {aggregation_group_type_index} for feature flag {feature_flag['key']}" ) # failover to `/flags` - raise InconclusiveMatchError("Flag has unknown group type index") + raise InconclusiveMatchError( + "Flag has unknown group type index", + reason=UnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ) if group_name not in groups: # Group flags are never enabled in `groups` aren't passed in @@ -3526,7 +3552,8 @@ def _compute_flag_locally( if group_name not in group_properties: raise InconclusiveMatchError( - f"Flag has no group properties for group '{group_name}'" + f"Flag has no group properties for group '{group_name}'", + reason=UnresolvedFlagReason.MISSING_CONTEXT, ) focused_group_properties = group_properties[group_name] group_key = groups[group_name] @@ -4534,6 +4561,7 @@ def evaluate_flags( errors_while_computing = False quota_limited = False locally_evaluated_keys: set[str] = set() + unresolved: Dict[str, UnresolvedFlagReason] = {} # Source the gate the same way as has_experiment below; see # _capture_feature_flag_called_if_needed for why. Defaults to the poller's # current state; a successful remote fallback overwrites it with that @@ -4551,6 +4579,7 @@ def evaluate_flags( group_properties=group_properties, flag_keys_to_evaluate=flag_keys, device_id=device_id, + unresolved=unresolved, ) feature_flags_by_key: Dict[str, Any] = self.feature_flags_by_key or {} @@ -4655,6 +4684,9 @@ def evaluate_flags( errors_while_computing=errors_while_computing, quota_limited=quota_limited, minimal_flag_called_events=minimal_flag_called_events, + unresolved_flags={ + key: reason for key, reason in unresolved.items() if key not in records + }, ) _feature_flag_evaluations_host_cache: Optional[_FeatureFlagEvaluationsHost] = None @@ -4677,6 +4709,7 @@ def _get_all_flags_and_payloads_locally( warn_on_unknown_groups=False, flag_keys_to_evaluate: Optional[list[str]] = None, device_id: Optional[str] = None, + unresolved: Optional[Dict[str, UnresolvedFlagReason]] = None, ) -> tuple[FlagsAndPayloads, bool]: person_properties = person_properties or {} group_properties = group_properties or {} @@ -4717,9 +4750,11 @@ def _get_all_flags_and_payloads_locally( ) if matched_payload is not None: payloads[flag["key"]] = matched_payload - except InconclusiveMatchError: + except (InconclusiveMatchError, RequiresServerEvaluation) as e: # No need to log this, since it's just telling us to fall back to `/flags` fallback_to_flags = True + if unresolved is not None: + unresolved[flag["key"]] = e.reason except Exception as e: self.log.exception( f"[FEATURE FLAGS] Error while computing variant and payload: {e}" diff --git a/posthog/feature_flag_evaluations.py b/posthog/feature_flag_evaluations.py index c260fbde7..5a8a185e9 100644 --- a/posthog/feature_flag_evaluations.py +++ b/posthog/feature_flag_evaluations.py @@ -6,9 +6,10 @@ """ from dataclasses import dataclass +from types import MappingProxyType from typing import Any, Callable, Dict, List, Mapping, Optional, Set, Union -from posthog.types import FlagValue +from posthog.types import FlagValue, UnresolvedFlagReason @dataclass(frozen=True) @@ -73,6 +74,7 @@ def __init__( quota_limited: bool = False, minimal_flag_called_events: bool = False, accessed: Optional[Set[str]] = None, + unresolved_flags: Optional[Mapping[str, UnresolvedFlagReason]] = None, ) -> None: """Internal — instances are created by the SDK via ``Client.evaluate_flags()``.""" self._host = host @@ -89,6 +91,9 @@ def __init__( # server response, not whatever the client-wide gate happens to be at send time. self._minimal_flag_called_events = minimal_flag_called_events self._accessed: Set[str] = set(accessed) if accessed is not None else set() + self._unresolved_flags: Dict[str, UnresolvedFlagReason] = dict( + unresolved_flags or {} + ) def is_enabled(self, key: str, default_value: bool = False) -> bool: """Return whether the flag is enabled. Fires ``$feature_flag_called`` on the @@ -165,6 +170,19 @@ def keys(self) -> List[str]: """Return the flag keys that are part of this evaluation.""" return list(self._flags.keys()) + @property + def unresolved_flags(self) -> Mapping[str, UnresolvedFlagReason]: + """Return a read-only mapping of flag key to the reason local evaluation + left it without a value. + + A flag is listed when it has a loaded local definition, was in the requested + scope, was inconclusive locally, and no remote value filled the gap. Listed + flags are not part of :attr:`keys`. Reading this does not count as an access + and does not fire any event. Snapshots returned by :meth:`only` and + :meth:`only_accessed` carry no entries. + """ + return MappingProxyType(self._unresolved_flags) + # --- Internal ------------------------------------------------------------- def _get_event_properties(self) -> Dict[str, Any]: @@ -263,7 +281,11 @@ def _record_access(self, key: str) -> None: if self._quota_limited: errors.append("quota_limited") if flag is None: - errors.append("flag_missing") + errors.append( + "local_evaluation_inconclusive" + if key in self._unresolved_flags + else "flag_missing" + ) if errors: properties["$feature_flag_error"] = ",".join(errors) diff --git a/posthog/feature_flags.py b/posthog/feature_flags.py index 87da025b5..085cbf278 100644 --- a/posthog/feature_flags.py +++ b/posthog/feature_flags.py @@ -11,7 +11,7 @@ from typing import Optional from posthog import utils as utils -from posthog.types import FlagValue +from posthog.types import FlagValue, UnresolvedFlagReason from posthog.utils import convert_to_datetime_aware, is_valid_regex __LONG_SCALE__ = float(0xFFFFFFFFFFFFFFF) @@ -94,7 +94,17 @@ class ConditionMatch(Enum): class InconclusiveMatchError(Exception): - pass + """ + Raised when local evaluation can't determine a flag's value. ``reason`` says + why, as reported by ``FeatureFlagEvaluations.unresolved_flags``. + """ + + reason: UnresolvedFlagReason = UnresolvedFlagReason.MISSING_CONTEXT + + def __init__(self, *args, reason: Optional[UnresolvedFlagReason] = None): + super().__init__(*args) + if reason is not None: + self.reason = reason class RequiresServerEvaluation(Exception): @@ -106,7 +116,12 @@ class RequiresServerEvaluation(Exception): InconclusiveMatchError which allows trying other conditions. """ - pass + reason: UnresolvedFlagReason = UnresolvedFlagReason.UNSUPPORTED_DEFINITION + + def __init__(self, *args, reason: Optional[UnresolvedFlagReason] = None): + super().__init__(*args) + if reason is not None: + self.reason = reason # This function takes a bucketing value and a feature flag key and returns a float between 0 and 1. @@ -220,7 +235,8 @@ def evaluate_flag_dependency( if flags_by_key is None or evaluation_cache is None: # Cannot evaluate flag dependencies without required context raise InconclusiveMatchError( - f"Cannot evaluate flag dependency on '{property.get('key', 'unknown')}' without flags_by_key and evaluation_cache" + f"Cannot evaluate flag dependency on '{property.get('key', 'unknown')}' without flags_by_key and evaluation_cache", + reason=UnresolvedFlagReason.UNSUPPORTED_DEFINITION, ) # Validate the condition shape before walking the chain. A malformed shape @@ -244,7 +260,8 @@ def evaluate_flag_dependency( if "dependency_chain" not in property: # Missing dependency_chain indicates malformed server data raise InconclusiveMatchError( - f"Flag dependency property for '{property.get('key', 'unknown')}' is missing required 'dependency_chain' field" + f"Flag dependency property for '{property.get('key', 'unknown')}' is missing required 'dependency_chain' field", + reason=UnresolvedFlagReason.UNSUPPORTED_DEFINITION, ) dependency_chain = property["dependency_chain"] @@ -253,7 +270,8 @@ def evaluate_flag_dependency( if len(dependency_chain) == 0: log.debug(f"Circular dependency detected for flag: {property.get('key')}") raise InconclusiveMatchError( - f"Circular dependency detected for flag '{property.get('key', 'unknown')}'" + f"Circular dependency detected for flag '{property.get('key', 'unknown')}'", + reason=UnresolvedFlagReason.UNSUPPORTED_DEFINITION, ) # Evaluate and cache each flag in the chain; members already cached are @@ -267,7 +285,8 @@ def evaluate_flag_dependency( # Missing flag dependency - cannot evaluate locally evaluation_cache[dep_flag_key] = None raise InconclusiveMatchError( - f"Cannot evaluate flag dependency '{dep_flag_key}' - flag not found in local flags" + f"Cannot evaluate flag dependency '{dep_flag_key}' - flag not found in local flags", + reason=UnresolvedFlagReason.UNSUPPORTED_DEFINITION, ) # Check if the flag is active (same check as in client._compute_flag_locally) @@ -305,7 +324,13 @@ def evaluate_flag_dependency( # If we can't evaluate a dependency, store None and propagate the error evaluation_cache[dep_flag_key] = None raise InconclusiveMatchError( - f"Cannot evaluate flag dependency '{dep_flag_key}': {e}" + f"Cannot evaluate flag dependency '{dep_flag_key}': {e}", + reason=UnresolvedFlagReason.UNRESOLVED_DEPENDENCY, + ) from e + except RequiresServerEvaluation as e: + raise RequiresServerEvaluation( + f"Cannot evaluate flag dependency '{dep_flag_key}': {e}", + reason=UnresolvedFlagReason.UNRESOLVED_DEPENDENCY, ) from e # The condition matches iff the referenced flag's value matches the expected @@ -313,8 +338,16 @@ def evaluate_flag_dependency( # definitive False, which must be allowed to match `expected_value=False`. actual_value = evaluation_cache.get(flag_key) if actual_value is None: + # A referenced flag that has no loaded definition or is missing from the + # chain is a definition problem, not an unresolved dependency. + reason = ( + UnresolvedFlagReason.UNRESOLVED_DEPENDENCY + if flag_key in flags_by_key and flag_key in evaluation_cache + else UnresolvedFlagReason.UNSUPPORTED_DEFINITION + ) raise InconclusiveMatchError( - f"Flag dependency '{flag_key}' was inconclusive or not evaluated" + f"Flag dependency '{flag_key}' was inconclusive or not evaluated", + reason=reason, ) return matches_dependency_value(expected_value, actual_value) @@ -369,7 +402,8 @@ def resolve_bucketing_value(flag, distinct_id, device_id=None): if bucketing_identifier == "device_id": if not device_id: raise InconclusiveMatchError( - "Flag requires device_id for bucketing but none was provided" + "Flag requires device_id for bucketing but none was provided", + reason=UnresolvedFlagReason.MISSING_CONTEXT, ) return device_id return distinct_id @@ -411,6 +445,7 @@ def match_feature_flag_properties( flag_aggregation = flag_filters.get("aggregation_group_type_index") early_exit_enabled = flag_filters.get("early_exit") is_inconclusive = False + inconclusive_reason: Optional[UnresolvedFlagReason] = None cohort_properties = cohort_properties or {} groups = groups or {} group_properties = group_properties or {} @@ -442,6 +477,9 @@ def match_feature_flag_properties( continue if group_name not in group_properties: is_inconclusive = True + inconclusive_reason = ( + inconclusive_reason or UnresolvedFlagReason.MISSING_CONTEXT + ) continue effective_properties = group_properties[group_name] effective_bucketing = groups[group_name] @@ -484,14 +522,16 @@ def match_feature_flag_properties( except RequiresServerEvaluation: # Static cohort or other missing server-side data - must fallback to API raise - except InconclusiveMatchError: + except InconclusiveMatchError as e: # Evaluation error (bad regex, invalid date, missing property, etc.) # Track that we had an inconclusive match, but try other conditions is_inconclusive = True + inconclusive_reason = inconclusive_reason or e.reason if is_inconclusive: raise InconclusiveMatchError( - "Can't determine if feature flag is enabled or not with given properties" + "Can't determine if feature flag is enabled or not with given properties", + reason=inconclusive_reason, ) # We can only return False when either all conditions are False, or @@ -568,7 +608,8 @@ def is_condition_match( def _format_json_float(value: float) -> str: if not math.isfinite(value): raise InconclusiveMatchError( - "Non-finite property values cannot be represented by the flags service" + "Non-finite property values cannot be represented by the flags service", + reason=UnresolvedFlagReason.MISSING_CONTEXT, ) representation = repr(value) @@ -600,7 +641,8 @@ def _json_value_to_string(value) -> str: if isinstance(value, dict): if not all(isinstance(key, str) for key in value): raise InconclusiveMatchError( - "Property object keys must be strings for local evaluation" + "Property object keys must be strings for local evaluation", + reason=UnresolvedFlagReason.MISSING_CONTEXT, ) items = ( f"{json.dumps(key, ensure_ascii=False)}:{_json_value_to_string(value[key])}" @@ -609,7 +651,8 @@ def _json_value_to_string(value) -> str: return "{" + ",".join(items) + "}" raise InconclusiveMatchError( - f"Property value of type {type(value).__name__} is not JSON-compatible" + f"Property value of type {type(value).__name__} is not JSON-compatible", + reason=UnresolvedFlagReason.MISSING_CONTEXT, ) @@ -652,11 +695,15 @@ def match_property( value = property.get("value") if operator not in PROPERTY_OPERATORS: - raise InconclusiveMatchError(f"Unknown operator {operator}") + raise InconclusiveMatchError( + f"Unknown operator {operator}", + reason=UnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ) if key not in property_values: raise InconclusiveMatchError( - "can't match properties without a given property value" + "can't match properties without a given property value", + reason=UnresolvedFlagReason.MISSING_CONTEXT, ) if operator in ("is_set", "is_not_set"): @@ -768,12 +815,14 @@ def compare(lhs, rhs, operator): parsed_date = convert_to_datetime_aware(parsed_date) except Exception as e: raise InconclusiveMatchError( - "The date set on the flag is not a valid format" + "The date set on the flag is not a valid format", + reason=UnresolvedFlagReason.UNSUPPORTED_DEFINITION, ) from e if not parsed_date: raise InconclusiveMatchError( - "The date set on the flag is not a valid format" + "The date set on the flag is not a valid format", + reason=UnresolvedFlagReason.UNSUPPORTED_DEFINITION, ) if isinstance(override_value, datetime.datetime): @@ -796,10 +845,14 @@ def compare(lhs, rhs, operator): else: return override_date > parsed_date except Exception: - raise InconclusiveMatchError("The date provided is not a valid format") + raise InconclusiveMatchError( + "The date provided is not a valid format", + reason=UnresolvedFlagReason.MISSING_CONTEXT, + ) else: raise InconclusiveMatchError( - "The date provided must be a string or date object" + "The date provided must be a string or date object", + reason=UnresolvedFlagReason.MISSING_CONTEXT, ) if operator in SEMVER_OPERATORS: @@ -807,7 +860,8 @@ def compare(lhs, rhs, operator): override_parsed = parse_semver(override_value) except (ValueError, TypeError): raise InconclusiveMatchError( - f"Person property value '{override_value}' is not a valid semver" + f"Person property value '{override_value}' is not a valid semver", + reason=UnresolvedFlagReason.MISSING_CONTEXT, ) if operator in SEMVER_COMPARISON_OPERATORS: @@ -815,7 +869,8 @@ def compare(lhs, rhs, operator): flag_parsed = parse_semver(value) except (ValueError, TypeError): raise InconclusiveMatchError( - f"Flag semver value '{value}' is not a valid semver" + f"Flag semver value '{value}' is not a valid semver", + reason=UnresolvedFlagReason.UNSUPPORTED_DEFINITION, ) if operator == "semver_eq": @@ -836,7 +891,8 @@ def compare(lhs, rhs, operator): lower, upper = _tilde_bounds(str(value)) except (ValueError, TypeError): raise InconclusiveMatchError( - f"Flag semver value '{value}' is not valid for tilde operator" + f"Flag semver value '{value}' is not valid for tilde operator", + reason=UnresolvedFlagReason.UNSUPPORTED_DEFINITION, ) return lower <= override_parsed < upper @@ -845,7 +901,8 @@ def compare(lhs, rhs, operator): lower, upper = _caret_bounds(str(value)) except (ValueError, TypeError): raise InconclusiveMatchError( - f"Flag semver value '{value}' is not valid for caret operator" + f"Flag semver value '{value}' is not valid for caret operator", + reason=UnresolvedFlagReason.UNSUPPORTED_DEFINITION, ) return lower <= override_parsed < upper @@ -854,13 +911,17 @@ def compare(lhs, rhs, operator): lower, upper = _wildcard_bounds(str(value)) except (ValueError, TypeError): raise InconclusiveMatchError( - f"Flag semver value '{value}' is not valid for wildcard operator" + f"Flag semver value '{value}' is not valid for wildcard operator", + reason=UnresolvedFlagReason.UNSUPPORTED_DEFINITION, ) return lower <= override_parsed < upper # Unreachable: all operators in PROPERTY_OPERATORS are handled above, # and unknown operators are rejected at the top of this function. - raise InconclusiveMatchError(f"Operator {operator} {_UNHANDLED_OPERATOR_MESSAGE}") + raise InconclusiveMatchError( + f"Operator {operator} {_UNHANDLED_OPERATOR_MESSAGE}", + reason=UnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ) def match_cohort( @@ -905,7 +966,10 @@ def match_cohort( return matches if operator == "not_in": return not matches - raise InconclusiveMatchError(f"Unsupported cohort operator: {operator}") + raise InconclusiveMatchError( + f"Unsupported cohort operator: {operator}", + reason=UnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ) def match_property_group( @@ -936,6 +1000,7 @@ def match_property_group( decisive_result = None error_matching_locally = False + inconclusive_reason: Optional[UnresolvedFlagReason] = None for prop in properties: try: @@ -998,12 +1063,14 @@ def match_property_group( except InconclusiveMatchError as e: log.debug(f"Failed to compute property {prop} locally: {e}") error_matching_locally = True + inconclusive_reason = inconclusive_reason or e.reason if decisive_result is not None: return decisive_result if error_matching_locally: raise InconclusiveMatchError( - "Can't match cohort without a given cohort property value" + "Can't match cohort without a given cohort property value", + reason=inconclusive_reason, ) # AND: every entry matched. OR: none matched. diff --git a/posthog/test/test_evaluate_flags.py b/posthog/test/test_evaluate_flags.py index 32a63ab27..682ed2ea1 100644 --- a/posthog/test/test_evaluate_flags.py +++ b/posthog/test/test_evaluate_flags.py @@ -6,7 +6,9 @@ from posthog.client import Client from posthog.feature_flag_evaluations import FeatureFlagEvaluations +from posthog.request import GetResponse from posthog.test.test_utils import FAKE_TEST_API_KEY +from posthog.types import UnresolvedFlagReason def _flags_response_fixture(): @@ -552,6 +554,326 @@ def test_context_device_id_is_used_for_local_evaluation( patch_flags.assert_not_called() +def _always_on_flag(key, **extra): + return { + "id": 1, + "key": key, + "active": True, + "filters": {"groups": [{"properties": [], "rollout_percentage": 100}]}, + **extra, + } + + +def _flag_called_properties(patch_capture): + return { + c[1]["properties"]["$feature_flag"]: c[1]["properties"] + for c in patch_capture.call_args_list + if c[0] and c[0][0] == "$feature_flag_called" + } + + +class TestEvaluateFlagsUnresolved(unittest.TestCase): + def setUp(self): + self.client = Client(FAKE_TEST_API_KEY, secret_key="test") + self.client.feature_flags = [ + _always_on_flag("beta-ui"), + _always_on_flag("checkout", ensure_experience_continuity=True), + ] + + def tearDown(self): + self.client.shutdown() + + @mock.patch("posthog.client.flags") + @mock.patch.object(Client, "capture") + def test_local_only_reports_experience_continuity_flag( + self, patch_capture, patch_flags + ): + flags = self.client.evaluate_flags("user-123", only_evaluate_locally=True) + + self.assertEqual(flags.keys, ["beta-ui"]) + self.assertEqual( + dict(flags.unresolved_flags), + {"checkout": UnresolvedFlagReason.EXPERIENCE_CONTINUITY}, + ) + self.assertEqual(flags.unresolved_flags["checkout"], "experience_continuity") + patch_flags.assert_not_called() + self.assertEqual(_flag_called_properties(patch_capture), {}) + self.assertEqual(flags.only_accessed().keys, []) + + @mock.patch("posthog.client.flags") + def test_unresolved_flags_is_read_only(self, _patch_flags): + flags = self.client.evaluate_flags("user-123", only_evaluate_locally=True) + + with self.assertRaises(TypeError): + flags.unresolved_flags["beta-ui"] = UnresolvedFlagReason.MISSING_CONTEXT # type: ignore[index] + + @mock.patch("posthog.client.flags") + def test_missing_person_property_is_reported_as_missing_context(self, _patch_flags): + checkout = _always_on_flag("checkout") + checkout["filters"]["groups"][0]["properties"] = [ + {"key": "plan", "operator": "exact", "value": "pro", "type": "person"} + ] + self.client.feature_flags = [checkout] + + flags = self.client.evaluate_flags("user-123", only_evaluate_locally=True) + + self.assertEqual(flags.keys, []) + self.assertEqual( + dict(flags.unresolved_flags), + {"checkout": UnresolvedFlagReason.MISSING_CONTEXT}, + ) + + @mock.patch("posthog.client.flags") + def test_inconclusive_dependency_is_reported_as_unresolved_dependency( + self, _patch_flags + ): + base = _always_on_flag("base") + base["filters"]["groups"][0]["properties"] = [ + {"key": "plan", "operator": "exact", "value": "pro", "type": "person"} + ] + dependent = _always_on_flag("dependent") + dependent["filters"]["groups"][0]["properties"] = [ + { + "key": "base", + "type": "flag", + "value": True, + "operator": "flag_evaluates_to", + "dependency_chain": ["base"], + } + ] + self.client.feature_flags = [base, dependent] + + flags = self.client.evaluate_flags("user-123", only_evaluate_locally=True) + + self.assertEqual( + dict(flags.unresolved_flags), + { + "base": UnresolvedFlagReason.MISSING_CONTEXT, + "dependent": UnresolvedFlagReason.UNRESOLVED_DEPENDENCY, + }, + ) + + scoped = self.client.evaluate_flags( + "user-123", flag_keys=["dependent"], only_evaluate_locally=True + ) + + self.assertEqual( + dict(scoped.unresolved_flags), + {"dependent": UnresolvedFlagReason.UNRESOLVED_DEPENDENCY}, + ) + + @mock.patch("posthog.client.flags") + def test_dependency_without_loaded_definition_is_unsupported_definition( + self, _patch_flags + ): + dependent = _always_on_flag("dependent") + dependent["filters"]["groups"][0]["properties"] = [ + { + "key": "unknown", + "type": "flag", + "value": True, + "operator": "flag_evaluates_to", + "dependency_chain": ["unknown"], + } + ] + self.client.feature_flags = [dependent] + + flags = self.client.evaluate_flags("user-123", only_evaluate_locally=True) + + self.assertEqual( + dict(flags.unresolved_flags), + {"dependent": UnresolvedFlagReason.UNSUPPORTED_DEFINITION}, + ) + + @mock.patch("posthog.client.flags") + def test_static_cohort_is_reported_as_unsupported_definition(self, _patch_flags): + checkout = _always_on_flag("checkout") + checkout["filters"]["groups"][0]["properties"] = [ + {"key": "id", "type": "cohort", "value": 42} + ] + self.client.feature_flags = [checkout] + + flags = self.client.evaluate_flags("user-123", only_evaluate_locally=True) + + self.assertEqual( + dict(flags.unresolved_flags), + {"checkout": UnresolvedFlagReason.UNSUPPORTED_DEFINITION}, + ) + + @mock.patch("posthog.client.flags") + def test_flag_resolved_by_remote_fallback_is_not_unresolved(self, patch_flags): + patch_flags.return_value = { + "flags": {"checkout": {"enabled": True, "variant": None}} + } + + flags = self.client.evaluate_flags("user-123") + + self.assertIs(flags.get_flag("checkout"), True) + self.assertEqual(sorted(flags.keys), ["beta-ui", "checkout"]) + self.assertEqual(dict(flags.unresolved_flags), {}) + patch_flags.assert_called_once() + + @parameterized.expand( + [ + ("request_fails", Exception("flags request failed")), + ("key_not_returned", None), + ] + ) + @mock.patch("posthog.client.flags") + def test_flag_left_without_remote_value_is_unresolved( + self, _name, side_effect, patch_flags + ): + patch_flags.side_effect = side_effect + patch_flags.return_value = {"flags": {}} + + flags = self.client.evaluate_flags("user-123") + + self.assertNotIn("checkout", flags.keys) + self.assertEqual( + dict(flags.unresolved_flags), + {"checkout": UnresolvedFlagReason.EXPERIENCE_CONTINUITY}, + ) + + @mock.patch("posthog.client.flags") + @mock.patch.object(Client, "capture") + def test_key_without_local_definition_is_missing_not_unresolved( + self, patch_capture, _patch_flags + ): + self.client.feature_flags = [_always_on_flag("beta-ui")] + + flags = self.client.evaluate_flags("user-123", only_evaluate_locally=True) + flags.is_enabled("typo-flag") + + self.assertEqual(dict(flags.unresolved_flags), {}) + properties = _flag_called_properties(patch_capture)["typo-flag"] + self.assertEqual(properties["$feature_flag_error"], "flag_missing") + + @mock.patch("posthog.client.flags") + def test_flag_outside_requested_keys_is_not_unresolved(self, patch_flags): + flags = self.client.evaluate_flags("user-123", flag_keys=["beta-ui"]) + + self.assertEqual(flags.keys, ["beta-ui"]) + self.assertTrue(flags.is_enabled("beta-ui")) + self.assertEqual(dict(flags.unresolved_flags), {}) + patch_flags.assert_not_called() + + @mock.patch("posthog.client.flags") + def test_inactive_flag_is_false_not_unresolved(self, patch_flags): + self.client.feature_flags = [ + _always_on_flag("checkout", active=False, ensure_experience_continuity=True) + ] + + flags = self.client.evaluate_flags("user-123", only_evaluate_locally=True) + + self.assertIs(flags.get_flag("checkout"), False) + self.assertEqual(dict(flags.unresolved_flags), {}) + patch_flags.assert_not_called() + + @mock.patch("posthog.client.flags") + @mock.patch.object(Client, "capture") + def test_reading_remote_resolved_flag_reports_no_error( + self, patch_capture, patch_flags + ): + patch_flags.return_value = { + "flags": {"checkout": {"enabled": True, "variant": None}} + } + + flags = self.client.evaluate_flags("user-123") + + self.assertTrue(flags.is_enabled("checkout")) + properties = _flag_called_properties(patch_capture)["checkout"] + self.assertNotIn("$feature_flag_error", properties) + + @mock.patch("posthog.client.flags") + @mock.patch.object(Client, "capture") + def test_reading_unresolved_flag_reports_local_evaluation_inconclusive( + self, patch_capture, _patch_flags + ): + flags = self.client.evaluate_flags("user-123", only_evaluate_locally=True) + + self.assertFalse(flags.is_enabled("checkout")) + properties = _flag_called_properties(patch_capture)["checkout"] + self.assertEqual( + properties["$feature_flag_error"], "local_evaluation_inconclusive" + ) + + @mock.patch("posthog.client.flags") + @mock.patch.object(Client, "capture") + def test_filtered_snapshots_carry_no_unresolved_flags( + self, patch_capture, _patch_flags + ): + flags = self.client.evaluate_flags("user-123", only_evaluate_locally=True) + flags.is_enabled("beta-ui") + + for filtered in (flags.only(["beta-ui"]), flags.only_accessed()): + self.assertEqual(dict(filtered.unresolved_flags), {}) + filtered.get_flag("checkout") + properties = _flag_called_properties(patch_capture)["checkout"] + self.assertEqual(properties["$feature_flag_error"], "flag_missing") + self.assertEqual( + dict(flags.unresolved_flags), + {"checkout": UnresolvedFlagReason.EXPERIENCE_CONTINUITY}, + ) + + +class TestUnresolvableFlagWarning(unittest.TestCase): + def setUp(self): + self.client = Client( + FAKE_TEST_API_KEY, + secret_key="test", + enable_local_evaluation=False, + send=False, + ) + + def tearDown(self): + self.client.shutdown() + + def _refresh(self, flags): + with mock.patch("posthog.client.get") as get: + get.return_value = GetResponse( + data={"flags": flags, "group_type_mapping": {}, "cohorts": {}}, + etag=None, + not_modified=False, + ) + self.client._load_feature_flags() + + def _warnings(self, log): + return [ + c.args[0] + for c in log.warning.call_args_list + if "experience_continuity" in c.args[0] + ] + + @mock.patch("posthog.client.flags") + def test_warns_once_per_definition(self, patch_flags): + checkout = _always_on_flag("checkout", ensure_experience_continuity=True) + + with mock.patch.object(self.client, "log") as log: + self._refresh([checkout, _always_on_flag("beta-ui")]) + self._refresh([checkout, _always_on_flag("beta-ui")]) + + warnings_logged = self._warnings(log) + self.assertEqual(len(warnings_logged), 1) + self.assertIn("checkout", warnings_logged[0]) + + self._refresh([{**checkout, "id": 2}]) + self.assertEqual(len(self._warnings(log)), 2) + + patch_flags.assert_not_called() + + def test_does_not_warn_for_inactive_flag(self): + with mock.patch.object(self.client, "log") as log: + self._refresh( + [ + _always_on_flag( + "checkout", active=False, ensure_experience_continuity=True + ) + ] + ) + + self.assertEqual(self._warnings(log), []) + + class TestEvaluateFlagsFiltering(unittest.TestCase): def setUp(self): self.client = Client(FAKE_TEST_API_KEY) diff --git a/posthog/test/test_feature_flags.py b/posthog/test/test_feature_flags.py index a126a0092..7df0621e7 100644 --- a/posthog/test/test_feature_flags.py +++ b/posthog/test/test_feature_flags.py @@ -25,7 +25,7 @@ relative_date_parse_for_feature_flag_matching, ) from posthog.request import APIError, GetResponse -from posthog.types import FeatureFlagEvaluationRuntime +from posthog.types import FeatureFlagEvaluationRuntime, UnresolvedFlagReason from posthog.test.test_utils import FAKE_TEST_API_KEY from posthog.utils import FlagCache @@ -5267,6 +5267,48 @@ def property(self, key, value, operator=None): return result + @parameterized.expand( + [ + ("unknown_operator", "x", "bogus", {"key": "x"}, "UNSUPPORTED_DEFINITION"), + ("missing_property", "x", "exact", {}, "MISSING_CONTEXT"), + ( + "invalid_flag_date", + "not-a-date", + "is_date_before", + {"key": "2024-01-01"}, + "UNSUPPORTED_DEFINITION", + ), + ( + "invalid_property_date", + "2024-01-01", + "is_date_before", + {"key": "not-a-date"}, + "MISSING_CONTEXT", + ), + ( + "invalid_flag_semver", + "x.y", + "semver_gt", + {"key": "1.2.3"}, + "UNSUPPORTED_DEFINITION", + ), + ( + "invalid_property_semver", + "1.2.3", + "semver_gt", + {"key": "x.y"}, + "MISSING_CONTEXT", + ), + ] + ) + def test_inconclusive_match_reports_reason( + self, _name, value, operator, properties, reason + ): + with self.assertRaises(InconclusiveMatchError) as ctx: + match_property(self.property("key", value, operator), properties) + + self.assertEqual(ctx.exception.reason, UnresolvedFlagReason[reason]) + def test_every_supported_operator_has_a_dispatch_branch(self): # PROPERTY_OPERATORS gates local evaluation, but match_property dispatches # on a hand-written if-chain. An operator listed in the tuple but missing a diff --git a/posthog/types.py b/posthog/types.py index 308339319..8bc0b40d6 100644 --- a/posthog/types.py +++ b/posthog/types.py @@ -99,6 +99,30 @@ def matches(self, other: "FeatureFlagEvaluationRuntime") -> bool: return self is other or FeatureFlagEvaluationRuntime.ALL in (self, other) +class UnresolvedFlagReason(str, Enum): + """Why local evaluation left a flag without a value. + + Reported by :attr:`FeatureFlagEvaluations.unresolved_flags`. Inheriting from + ``str`` keeps the members directly comparable to their string values. + + Values: + EXPERIENCE_CONTINUITY: The flag has experience continuity enabled, which + local evaluation never resolves. Change the flag to fix it. + UNSUPPORTED_DEFINITION: The flag uses something the local evaluator does not + support, such as a static cohort, an unknown operator, a malformed value + or a dependency with no loaded definition. + MISSING_CONTEXT: The call did not pass a property, group key, group + property or device id the flag needs, or passed it in an unusable form. + UNRESOLVED_DEPENDENCY: A flag this flag depends on has a loaded definition + but was unresolved. + """ + + EXPERIENCE_CONTINUITY = "experience_continuity" + UNSUPPORTED_DEFINITION = "unsupported_definition" + MISSING_CONTEXT = "missing_context" + UNRESOLVED_DEPENDENCY = "unresolved_dependency" + + @dataclass(frozen=True) class FlagReason: """Reason metadata returned by the feature flag API. @@ -422,6 +446,8 @@ class FeatureFlagError: Error values: ERRORS_WHILE_COMPUTING: Server returned errorsWhileComputingFlags=true FLAG_MISSING: Requested flag not in API response + LOCAL_EVALUATION_INCONCLUSIVE: Requested flag has a loaded definition but + local evaluation was inconclusive and no remote value filled the gap QUOTA_LIMITED: Rate/quota limit exceeded TIMEOUT: Request timed out CONNECTION_ERROR: Network connectivity issue @@ -433,6 +459,7 @@ class FeatureFlagError: ERRORS_WHILE_COMPUTING = "errors_while_computing_flags" FLAG_MISSING = "flag_missing" + LOCAL_EVALUATION_INCONCLUSIVE = "local_evaluation_inconclusive" QUOTA_LIMITED = "quota_limited" TIMEOUT = "timeout" CONNECTION_ERROR = "connection_error" diff --git a/references/public_api_snapshot.txt b/references/public_api_snapshot.txt index 51ee00d9d..451f4942a 100644 --- a/references/public_api_snapshot.txt +++ b/references/public_api_snapshot.txt @@ -30,6 +30,7 @@ alias posthog.OptionalSetArgs -> posthog.args.OptionalSetArgs alias posthog.RequiresServerEvaluation -> posthog.feature_flags.RequiresServerEvaluation alias posthog.SocketOptions -> posthog.request.SocketOptions alias posthog.Span -> posthog.tracing.span.Span +alias posthog.UnresolvedFlagReason -> posthog.types.UnresolvedFlagReason alias posthog.VERSION -> posthog.version.VERSION alias posthog.ai.PromptResult -> posthog.ai.prompts.PromptResult alias posthog.ai.PromptSource -> posthog.ai.prompts.PromptSource @@ -272,6 +273,7 @@ alias posthog.client.RequiresServerEvaluation -> posthog.feature_flags.RequiresS alias posthog.client.SendFeatureFlagsOptions -> posthog.types.SendFeatureFlagsOptions alias posthog.client.SizeLimitedDict -> posthog.utils.SizeLimitedDict alias posthog.client.Span -> posthog.tracing.span.Span +alias posthog.client.UnresolvedFlagReason -> posthog.types.UnresolvedFlagReason alias posthog.client.VERSION -> posthog.version.VERSION alias posthog.client.batch_post -> posthog.request.batch_post alias posthog.client.clean -> posthog.utils.clean @@ -318,7 +320,9 @@ alias posthog.exception_capture.Client -> posthog.client.Client alias posthog.exception_utils.ExcInfo -> posthog.args.ExcInfo alias posthog.exception_utils.ExceptionArg -> posthog.args.ExceptionArg alias posthog.feature_flag_evaluations.FlagValue -> posthog.types.FlagValue +alias posthog.feature_flag_evaluations.UnresolvedFlagReason -> posthog.types.UnresolvedFlagReason alias posthog.feature_flags.FlagValue -> posthog.types.FlagValue +alias posthog.feature_flags.UnresolvedFlagReason -> posthog.types.UnresolvedFlagReason alias posthog.feature_flags.convert_to_datetime_aware -> posthog.utils.convert_to_datetime_aware alias posthog.feature_flags.is_valid_regex -> posthog.utils.is_valid_regex alias posthog.feature_flags.utils -> posthog.utils @@ -838,14 +842,17 @@ attribute posthog.exception_utils.VariableSizeLimiter.current_size = 0 attribute posthog.exception_utils.VariableSizeLimiter.max_size = max_size attribute posthog.exception_utils.epoch = datetime(1970, 1, 1) attribute posthog.feature_flag_evaluations.FeatureFlagEvaluations.keys: List[str] +attribute posthog.feature_flag_evaluations.FeatureFlagEvaluations.unresolved_flags: Mapping[str, UnresolvedFlagReason] attribute posthog.feature_flags.ConditionMatch.MATCH = 'match' attribute posthog.feature_flags.ConditionMatch.NO_MATCH = 'no_match' attribute posthog.feature_flags.ConditionMatch.OUT_OF_ROLLOUT_BOUND = 'out_of_rollout_bound' attribute posthog.feature_flags.DATE_OPERATORS = ('is_date_before', 'is_date_after') attribute posthog.feature_flags.EQUALITY_OPERATORS = ('exact', 'is_not', 'is_set', 'is_not_set') +attribute posthog.feature_flags.InconclusiveMatchError.reason: UnresolvedFlagReason = UnresolvedFlagReason.MISSING_CONTEXT attribute posthog.feature_flags.NONE_VALUES_ALLOWED_OPERATORS = ['is_not'] attribute posthog.feature_flags.NUMERIC_OPERATORS = ('gt', 'gte', 'lt', 'lte') attribute posthog.feature_flags.PROPERTY_OPERATORS = EQUALITY_OPERATORS + STRING_OPERATORS + NUMERIC_OPERATORS + DATE_OPERATORS + SEMVER_OPERATORS +attribute posthog.feature_flags.RequiresServerEvaluation.reason: UnresolvedFlagReason = UnresolvedFlagReason.UNSUPPORTED_DEFINITION attribute posthog.feature_flags.SEMVER_COMPARISON_OPERATORS = ('semver_eq', 'semver_neq', 'semver_gt', 'semver_gte', 'semver_lt', 'semver_lte') attribute posthog.feature_flags.SEMVER_OPERATORS = SEMVER_COMPARISON_OPERATORS + SEMVER_RANGE_OPERATORS attribute posthog.feature_flags.SEMVER_RANGE_OPERATORS = ('semver_tilde', 'semver_caret', 'semver_wildcard') @@ -1040,6 +1047,7 @@ attribute posthog.types.FeatureFlag.variant: Optional[str] attribute posthog.types.FeatureFlagError.CONNECTION_ERROR = 'connection_error' attribute posthog.types.FeatureFlagError.ERRORS_WHILE_COMPUTING = 'errors_while_computing_flags' attribute posthog.types.FeatureFlagError.FLAG_MISSING = 'flag_missing' +attribute posthog.types.FeatureFlagError.LOCAL_EVALUATION_INCONCLUSIVE = 'local_evaluation_inconclusive' attribute posthog.types.FeatureFlagError.QUOTA_LIMITED = 'quota_limited' attribute posthog.types.FeatureFlagError.TIMEOUT = 'timeout' attribute posthog.types.FeatureFlagError.UNKNOWN_ERROR = 'unknown_error' @@ -1074,6 +1082,10 @@ attribute posthog.types.SendFeatureFlagsOptions.group_properties: Optional[dict[ attribute posthog.types.SendFeatureFlagsOptions.only_evaluate_locally: Optional[bool] attribute posthog.types.SendFeatureFlagsOptions.person_properties: Optional[dict[str, Any]] attribute posthog.types.SendFeatureFlagsOptions.should_send: bool +attribute posthog.types.UnresolvedFlagReason.EXPERIENCE_CONTINUITY = 'experience_continuity' +attribute posthog.types.UnresolvedFlagReason.MISSING_CONTEXT = 'missing_context' +attribute posthog.types.UnresolvedFlagReason.UNRESOLVED_DEPENDENCY = 'unresolved_dependency' +attribute posthog.types.UnresolvedFlagReason.UNSUPPORTED_DEFINITION = 'unsupported_definition' attribute posthog.utils.CACHE_KEY_PREFIX = 'posthog:flags:' attribute posthog.utils.CACHE_MAX_SIZE = 10000 attribute posthog.utils.CACHE_STALE_TTL = 3600 @@ -1191,10 +1203,10 @@ class posthog.contexts.ContextScope(parent=None, fresh: bool = False, capture_ex class posthog.exception_capture.ExceptionCapture(client: Client, rate_limiting_enabled=False, bucket_size=DEFAULT_BUCKET_SIZE, refill_rate=DEFAULT_REFILL_RATE, refill_interval_seconds=DEFAULT_REFILL_INTERVAL_SECONDS) class posthog.exception_utils.AnnotatedValue(value, metadata) class posthog.exception_utils.VariableSizeLimiter(max_size=DEFAULT_TOTAL_VARIABLES_SIZE_LIMIT) -class posthog.feature_flag_evaluations.FeatureFlagEvaluations(host: _FeatureFlagEvaluationsHost, distinct_id: str, flags: Dict[str, _EvaluatedFlagRecord], groups: Optional[Mapping[str, Union[str, int]]] = None, disable_geoip: Optional[bool] = None, request_id: Optional[str] = None, evaluated_at: Optional[int] = None, errors_while_computing: bool = False, quota_limited: bool = False, minimal_flag_called_events: bool = False, accessed: Optional[Set[str]] = None) +class posthog.feature_flag_evaluations.FeatureFlagEvaluations(host: _FeatureFlagEvaluationsHost, distinct_id: str, flags: Dict[str, _EvaluatedFlagRecord], groups: Optional[Mapping[str, Union[str, int]]] = None, disable_geoip: Optional[bool] = None, request_id: Optional[str] = None, evaluated_at: Optional[int] = None, errors_while_computing: bool = False, quota_limited: bool = False, minimal_flag_called_events: bool = False, accessed: Optional[Set[str]] = None, unresolved_flags: Optional[Mapping[str, UnresolvedFlagReason]] = None) class posthog.feature_flags.ConditionMatch -class posthog.feature_flags.InconclusiveMatchError -class posthog.feature_flags.RequiresServerEvaluation +class posthog.feature_flags.InconclusiveMatchError(*args, reason: Optional[UnresolvedFlagReason] = None) +class posthog.feature_flags.RequiresServerEvaluation(*args, reason: Optional[UnresolvedFlagReason] = None) class posthog.flag_definition_cache.FlagDefinitionCacheData class posthog.flag_definition_cache.FlagDefinitionCacheProvider class posthog.integrations.celery.PosthogCeleryIntegration(client: Optional[Client] = None, capture_exceptions: bool = True, capture_task_lifecycle_events: bool = True, propagate_context: bool = True, task_filter: Optional[Callable[[Optional[str], dict[str, Any]], bool]] = None) @@ -1232,6 +1244,7 @@ class posthog.types.FlagsAndPayloads class posthog.types.FlagsResponse class posthog.types.LegacyFlagMetadata(payload: Any) class posthog.types.SendFeatureFlagsOptions +class posthog.types.UnresolvedFlagReason class posthog.utils.FlagCache(max_size=CACHE_MAX_SIZE, default_ttl=CACHE_TTL) class posthog.utils.FlagCacheEntry(flag_result, flag_definition_version, timestamp=None) class posthog.utils.RedisFlagCache(redis_client, default_ttl=CACHE_TTL, stale_ttl=CACHE_STALE_TTL, key_prefix=CACHE_KEY_PREFIX)