Skip to content
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
5 changes: 5 additions & 0 deletions .sampo/changesets/unresolved-local-flags.md
Original file line number Diff line number Diff line change
@@ -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.
1 change: 1 addition & 0 deletions posthog/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@
FeatureFlagEvaluationRuntime as FeatureFlagEvaluationRuntime,
FlagValue as FlagValue,
FlagsAndPayloads as FlagsAndPayloads,
UnresolvedFlagReason as UnresolvedFlagReason,
)
from posthog.types import (
FeatureFlagResult as FeatureFlagResult,
Expand Down
47 changes: 41 additions & 6 deletions posthog/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@
FlagsResponse,
FlagValue,
SendFeatureFlagsOptions,
UnresolvedFlagReason,
normalize_flags_response,
to_flags_and_payloads,
to_payloads,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Removed flags stay in memory

_warned_unresolvable_flags keeps each flag’s full definition after that flag disappears from loaded definitions. Long-running clients accumulate deleted or archived flags as projects create and remove them. Remove entries for keys no longer loaded so warning history stays bounded by the current flag set.

Prompt To Fix With AI
This is a comment left during a code review.
Path: posthog/client.py
Line: 3203

Comment:
**Removed flags stay in memory**

`_warned_unresolvable_flags` keeps each flag’s full definition after that flag disappears from loaded definitions. Long-running clients accumulate deleted or archived flags as projects create and remove them. Remove entries for keys no longer loaded so warning history stays bounded by the current flag set.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

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.
Expand Down Expand Up @@ -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"]
Expand All @@ -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
Expand All @@ -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]
Expand Down Expand Up @@ -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
Expand All @@ -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 {}
Expand Down Expand Up @@ -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
Expand All @@ -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 {}
Expand Down Expand Up @@ -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}"
Expand Down
26 changes: 24 additions & 2 deletions posthog/feature_flag_evaluations.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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]:
Expand Down Expand Up @@ -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)

Expand Down
Loading
Loading