From 191d1e6bf6df1c26dbb86884ce40006ccfb7129a Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Tue, 25 Aug 2026 16:46:41 +0200 Subject: [PATCH 1/9] feat(flags): add local feature flag evaluation --- .sampo/changesets/venerable-baroness-erika.md | 5 + CHANGELOG.md | 6 + lib/posthog/api.ex | 13 + lib/posthog/api/client.ex | 4 +- lib/posthog/config.ex | 57 +- lib/posthog/feature_flags.ex | 347 +++++- .../feature_flags/definition_loader.ex | 561 +++++++++ lib/posthog/feature_flags/evaluations.ex | 40 +- .../flag_definition_cache_provider.ex | 41 + lib/posthog/feature_flags/local_evaluator.ex | 1027 +++++++++++++++++ lib/posthog/supervisor.ex | 20 +- mix.exs | 2 +- public_api.snapshot | 7 + test/posthog/config_test.exs | 29 + .../feature_flags/definition_loader_test.exs | 451 ++++++++ .../flag_definition_cache_provider_test.exs | 276 +++++ .../local_evaluation_integration_test.exs | 293 +++++ .../feature_flags/local_evaluator_test.exs | 535 +++++++++ .../missing_key_knowledge_test.exs | 329 ++++++ test/support/api/stub.ex | 9 + 20 files changed, 4002 insertions(+), 50 deletions(-) create mode 100644 .sampo/changesets/venerable-baroness-erika.md create mode 100644 lib/posthog/feature_flags/definition_loader.ex create mode 100644 lib/posthog/feature_flags/flag_definition_cache_provider.ex create mode 100644 lib/posthog/feature_flags/local_evaluator.ex create mode 100644 test/posthog/feature_flags/definition_loader_test.exs create mode 100644 test/posthog/feature_flags/flag_definition_cache_provider_test.exs create mode 100644 test/posthog/feature_flags/local_evaluation_integration_test.exs create mode 100644 test/posthog/feature_flags/local_evaluator_test.exs create mode 100644 test/posthog/feature_flags/missing_key_knowledge_test.exs diff --git a/.sampo/changesets/venerable-baroness-erika.md b/.sampo/changesets/venerable-baroness-erika.md new file mode 100644 index 0000000..3af50b2 --- /dev/null +++ b/.sampo/changesets/venerable-baroness-erika.md @@ -0,0 +1,5 @@ +--- +hex/posthog: minor +--- + +Add local feature flag definition polling and evaluation with safe remote fallback. diff --git a/CHANGELOG.md b/CHANGELOG.md index 2706e01..9246424 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # posthog +## Unreleased + +### Minor changes + +- Add privileged, polled local feature flag evaluation with safe remote fallback, ETag definition loading, person/group/cohort/dependency matching, deterministic variants and payloads, and an optional bounded shared definition cache provider. Configure it with `secret_key`; existing installations without a secret remain remote-only. + ## 2.14.2 — 2026-08-25 ### Patch changes diff --git a/lib/posthog/api.ex b/lib/posthog/api.ex index 535c2f0..a6d71b8 100644 --- a/lib/posthog/api.ex +++ b/lib/posthog/api.ex @@ -13,4 +13,17 @@ defmodule PostHog.API do max_retries: client.feature_flags_request_max_retries ) end + + def flag_definitions(%__MODULE__.Client{} = client, project_api_key, secret_key, etag) do + headers = + [{"authorization", "Bearer #{secret_key}"}] + |> then(fn headers -> + if is_binary(etag), do: [{"if-none-match", etag} | headers], else: headers + end) + + client.module.request(client.client, :get, "/flags/definitions", + params: %{token: project_api_key, send_cohorts: true}, + headers: headers + ) + end end diff --git a/lib/posthog/api/client.ex b/lib/posthog/api/client.ex index 70af977..7c589f0 100644 --- a/lib/posthog/api/client.ex +++ b/lib/posthog/api/client.ex @@ -121,8 +121,8 @@ defmodule PostHog.API.Client do Response tuple returned by `c:request/4`. Successful responses must expose at least a numeric `:status` and decoded - `:body`; errors should return the exception or error struct from the HTTP - client. + `:body`; they may also expose response `:headers`. Errors should return the + exception or error struct from the HTTP client. """ @type response() :: {:ok, %{status: non_neg_integer(), body: any()}} | {:error, Exception.t()} diff --git a/lib/posthog/config.ex b/lib/posthog/config.ex index 853ae01..010cfbb 100644 --- a/lib/posthog/config.ex +++ b/lib/posthog/config.ex @@ -38,6 +38,45 @@ defmodule PostHog.Config do doc: "Number of retries for /flags requests after network, transport, or timeout failures. Set to 0 to disable retries." ], + secret_key: [ + type: {:or, [:string, nil]}, + default: nil, + doc: """ + A privileged project secret (`phs_`) or appropriately scoped personal API key (`phx_`) + used only to load definitions for local feature flag evaluation. Local evaluation is inert + and starts no poller when this value is absent or blank. + """ + ], + enable_local_evaluation: [ + type: :boolean, + default: true, + doc: + "Enable local feature flag evaluation when a non-empty `secret_key` is configured." + ], + feature_flags_poll_interval_ms: [ + type: :pos_integer, + default: 30_000, + doc: + "Interval in milliseconds between local feature flag definition refreshes." + ], + flag_definition_cache_provider: [ + type: {:or, [{:tuple, [:atom, :any]}, nil]}, + default: nil, + doc: + "Optional `{module, state}` implementing `PostHog.FeatureFlags.FlagDefinitionCacheProvider`." + ], + flag_definition_cache_provider_timeout_ms: [ + type: :pos_integer, + default: 5_000, + doc: + "Maximum time in milliseconds allowed for each definition cache provider callback." + ], + flag_definition_request_timeout_ms: [ + type: :pos_integer, + default: 10_000, + doc: + "Maximum time in milliseconds allowed for a local feature flag definition request." + ], supervisor_name: [ type: :atom, default: PostHog, @@ -219,7 +258,7 @@ defmodule PostHog.Config do ## Remarks - String `:api_key` and `:api_host` values are trimmed before validation. A blank + String `:api_key`, `:secret_key`, and `:api_host` values are trimmed before validation. A blank `:api_host` falls back to the default PostHog US ingestion host. """ @spec validate(options()) :: @@ -274,6 +313,13 @@ defmodule PostHog.Config do normalized_options end end) + |> then(fn normalized_options -> + if Keyword.has_key?(normalized_options, :secret_key) do + Keyword.update!(normalized_options, :secret_key, &normalize_secret_key/1) + else + normalized_options + end + end) |> then(fn normalized_options -> if Keyword.has_key?(normalized_options, :api_host) do Keyword.update!(normalized_options, :api_host, &normalize_api_host/1) @@ -287,6 +333,15 @@ defmodule PostHog.Config do defp normalize_api_key(nil), do: "" defp normalize_api_key(api_key), do: api_key + defp normalize_secret_key(secret_key) when is_binary(secret_key) do + case String.trim(secret_key) do + "" -> nil + value -> value + end + end + + defp normalize_secret_key(secret_key), do: secret_key + defp normalize_api_host(api_host) when is_binary(api_host) do api_host |> String.trim() diff --git a/lib/posthog/feature_flags.ex b/lib/posthog/feature_flags.ex index be75751..e4e9ff3 100644 --- a/lib/posthog/feature_flags.ex +++ b/lib/posthog/feature_flags.ex @@ -90,7 +90,7 @@ defmodule PostHog.FeatureFlags do @spec flags_for(PostHog.supervisor_name(), PostHog.distinct_id() | map() | nil) :: {:ok, map()} | {:error, Exception.t()} def flags_for(name \\ PostHog, distinct_id_or_body \\ nil) do - with {:ok, body} <- body_for_flags(distinct_id_or_body), + with {:ok, body} <- body_for_flags(name, distinct_id_or_body), {:ok, %{body: %{"flags" => flags}}} <- flags(name, body) do {:ok, flags} end @@ -105,9 +105,13 @@ defmodule PostHog.FeatureFlags do @doc """ Evaluates feature flags for a `distinct_id` and returns a snapshot. - Returns `{:ok, %PostHog.FeatureFlags.Evaluations{}}` on success. The snapshot - represents a single `/flags` call and lets you branch on multiple flags and - enrich captured events from the same fetch — see + Returns `{:ok, %PostHog.FeatureFlags.Evaluations{}}` on success. Definitions + are evaluated locally first when privileged local evaluation is configured; + unresolved flags are filled by at most one `/flags` call. If that fallback + fails after some flags resolved locally, the successful local subset is + returned as a partial snapshot; if nothing resolved, a safe empty snapshot + is returned. The snapshot lets you branch on multiple flags and enrich + captured events — see `PostHog.FeatureFlags.Evaluations` for the full snapshot API and `set_in_context/2` for the recommended capture-enrichment flow. @@ -124,8 +128,12 @@ defmodule PostHog.FeatureFlags do - `:person_properties` - `:group_properties` - `:disable_geoip` + - `:device_id` - alternate bucketing identifier for device-bucketed flags - Plus one snapshot-specific option: + Plus these snapshot-specific options: + + - `:only_evaluate_locally` - when true, never calls `/flags`; unresolved + flags are omitted from the snapshot. - `:flag_keys` - list of flag keys. A non-empty list is forwarded to the request as `flag_keys_to_evaluate` so the server returns only those flags. @@ -133,7 +141,9 @@ defmodule PostHog.FeatureFlags do omitted or `nil` value evaluates all flags through the normal request path. This scopes the network response, distinct from `PostHog.FeatureFlags.Evaluations.only/2` which filters an already-fetched - snapshot in memory. + snapshot in memory. Clean remote omissions for explicitly requested keys + are retained in a bounded per-instance set until definitions refresh, so + repeated and concurrent missing-key probes avoid duplicate requests. ## Examples @@ -157,20 +167,12 @@ defmodule PostHog.FeatureFlags do @spec evaluate_flags(PostHog.supervisor_name(), PostHog.distinct_id() | map() | nil) :: {:ok, __MODULE__.Evaluations.t()} | {:error, Exception.t()} def evaluate_flags(name \\ PostHog, distinct_id_or_body \\ nil) do - case body_for_flags(distinct_id_or_body) do + case body_for_flags(name, distinct_id_or_body) do {:ok, %{distinct_id: distinct_id, flag_keys: []}} -> - {:ok, __MODULE__.Evaluations.new(name, distinct_id, %{"flags" => %{}})} + {:ok, __MODULE__.Evaluations.from_results(name, distinct_id, %{}, %{})} {:ok, %{distinct_id: distinct_id} = body} -> - body = translate_flag_keys(body) - - case flags(name, body) do - {:ok, %{body: response_body}} -> - {:ok, __MODULE__.Evaluations.new(name, distinct_id, response_body)} - - {:error, _} = error -> - error - end + evaluate_snapshot(name, distinct_id, body) {:error, _} -> # Standardize on returning an empty snapshot when distinct_id can't be @@ -181,6 +183,223 @@ defmodule PostHog.FeatureFlags do end end + defp evaluate_snapshot(name, distinct_id, body) do + only_local? = Map.get(body, :only_evaluate_locally, false) == true + requested_keys = Map.get(body, :flag_keys) + loader_state = local_evaluation_state(name, requested_keys || []) + + case loader_state.definitions do + nil -> + if only_local? do + snapshot_from_results(name, distinct_id, %{}) + else + remote_snapshot(name, distinct_id, body, %{}, requested_keys, nil, []) + end + + _definitions -> + evaluate_loaded_snapshot( + name, + distinct_id, + body, + requested_keys, + loader_state, + true + ) + end + end + + defp evaluate_loaded_snapshot( + name, + distinct_id, + body, + requested_keys, + loader_state, + coordinate_missing? + ) do + definitions = loader_state.definitions + keys = requested_keys || Map.keys(definitions.flags_by_key) + local = __MODULE__.LocalEvaluator.evaluate(definitions, body, keys) + known_missing = if is_list(requested_keys), do: loader_state.known_missing, else: MapSet.new() + unresolved = MapSet.difference(local.unresolved, known_missing) + only_local? = Map.get(body, :only_evaluate_locally, false) == true + + cond do + MapSet.size(unresolved) == 0 or only_local? -> + snapshot_from_results(name, distinct_id, local.results) + + coordinate_missing? and is_list(requested_keys) -> + coordinate_missing_probe( + name, + distinct_id, + body, + requested_keys, + loader_state, + local, + unresolved + ) + + true -> + remote_loaded_snapshot(name, distinct_id, body, requested_keys, loader_state, local) + end + end + + defp coordinate_missing_probe( + name, + distinct_id, + body, + requested_keys, + loader_state, + local, + unresolved + ) do + absent = absent_definition_keys(requested_keys, loader_state.definitions) + probe_keys = absent |> MapSet.intersection(unresolved) |> MapSet.to_list() + + if probe_keys == [] do + remote_snapshot( + name, + distinct_id, + body, + local.results, + requested_keys, + loader_state.generation, + [] + ) + else + with_missing_probe_locks(name, probe_keys, fn -> + evaluate_after_missing_probe_lock(name, distinct_id, body, requested_keys) + end) + end + end + + defp evaluate_after_missing_probe_lock(name, distinct_id, body, requested_keys) do + fresh_state = local_evaluation_state(name, requested_keys) + + if is_nil(fresh_state.definitions) do + remote_snapshot(name, distinct_id, body, %{}, requested_keys, nil, []) + else + evaluate_loaded_snapshot(name, distinct_id, body, requested_keys, fresh_state, false) + end + end + + defp remote_loaded_snapshot(name, distinct_id, body, requested_keys, loader_state, local) do + absent = + if is_list(requested_keys) do + requested_keys + |> absent_definition_keys(loader_state.definitions) + |> MapSet.to_list() + else + [] + end + + remote_snapshot( + name, + distinct_id, + body, + local.results, + requested_keys, + loader_state.generation, + absent + ) + end + + defp absent_definition_keys(requested_keys, definitions) do + requested_keys + |> MapSet.new() + |> MapSet.difference(MapSet.new(Map.keys(definitions.flags_by_key))) + end + + defp remote_snapshot( + name, + distinct_id, + body, + local_results, + requested_keys, + generation, + negative_candidates + ) do + remote_body = + body + |> Map.delete(:only_evaluate_locally) + |> translate_flag_keys() + + case flags(name, remote_body) do + {:ok, %{body: %{"flags" => remote_flags} = response_body}} when is_map(remote_flags) -> + scoped_remote_flags = maybe_scope_remote_results(remote_flags, requested_keys) + + update_negative_knowledge( + name, + generation, + negative_candidates, + Map.keys(scoped_remote_flags), + clean_remote_response?(response_body) + ) + + remote_results = + Map.new(scoped_remote_flags, fn {key, flag_data} -> + {key, build_result(key, flag_data, response_body)} + end) + + merged = Map.merge(remote_results, local_results) + {:ok, __MODULE__.Evaluations.from_results(name, distinct_id, merged, response_body)} + + {:error, _reason} -> + snapshot_from_results(name, distinct_id, local_results) + end + end + + defp snapshot_from_results(name, distinct_id, results), + do: {:ok, __MODULE__.Evaluations.from_results(name, distinct_id, results, %{})} + + defp clean_remote_response?(body) do + Map.get(body, "errorsWhileComputingFlags") != true and + Map.get(body, "quotaLimited") in [nil, false, []] + end + + defp update_negative_knowledge(_name, nil, _requested, _returned, _clean?), do: :ok + + defp update_negative_knowledge(name, generation, requested, returned, clean?) do + __MODULE__.DefinitionLoader.update_negative_knowledge( + name, + generation, + requested, + returned, + clean? + ) + catch + :exit, _reason -> :ok + end + + defp maybe_scope_remote_results(results, keys) when is_list(keys), do: Map.take(results, keys) + defp maybe_scope_remote_results(results, _keys), do: results + + defp local_evaluation_state(name, keys) do + registry = PostHog.Registry.registry_name(name) + + if Process.whereis(registry) && + GenServer.whereis(PostHog.Registry.via(name, __MODULE__.DefinitionLoader)) do + __MODULE__.DefinitionLoader.evaluation_state(name, keys) + else + %{definitions: nil, generation: nil, known_missing: MapSet.new()} + end + catch + :exit, _reason -> %{definitions: nil, generation: nil, known_missing: MapSet.new()} + end + + defp with_missing_probe_locks(name, keys, callback) do + keys + |> Enum.uniq() + |> Enum.sort() + |> acquire_missing_probe_locks(name, callback) + end + + defp acquire_missing_probe_locks([], _name, callback), do: callback.() + + defp acquire_missing_probe_locks([key | rest], name, callback) do + lock = {{__MODULE__, :missing_probe, name, key}, self()} + :global.trans(lock, fn -> acquire_missing_probe_locks(rest, name, callback) end) + end + @doc """ Copies a snapshot's `$feature/` and `$active_feature_flags` properties into the default per-process PostHog context. @@ -475,20 +694,58 @@ defmodule PostHog.FeatureFlags do defp evaluate_flag(name, flag_name, distinct_id_or_body, opts) do send_event = Keyword.get(opts, :send_event, true) - with {:ok, %{distinct_id: distinct_id} = body} <- body_for_flags(distinct_id_or_body), - {:ok, %{body: body}} <- flags(name, body) do - case body do - %{"flags" => %{^flag_name => flag_data}} -> - flag_result = build_result(flag_name, flag_data, body) - maybe_log_feature_flag_usage(send_event, name, distinct_id, flag_result) + case body_for_flags(name, distinct_id_or_body) do + {:ok, %{distinct_id: distinct_id} = body} -> + case evaluate_single_flag(name, flag_name, body) do + {:ok, %__MODULE__.Result{} = result, response_body} -> + maybe_log_feature_flag_usage(send_event, name, distinct_id, result) + {:ok, result, response_body} + + {:ok, nil, response_body} -> + {:ok, nil, response_body} + + {:error, reason} -> + {:error, reason, nil} + end - {:ok, flag_result, body} + {:error, reason} -> + {:error, reason, nil} + end + end - %{"flags" => _} -> - {:ok, nil, body} + defp evaluate_single_flag(name, flag_name, body) do + local = + case local_evaluation_state(name, [flag_name]).definitions do + %{flags_by_key: %{^flag_name => _flag}} = definitions -> + __MODULE__.LocalEvaluator.evaluate(definitions, body, [flag_name]) + + _definitions -> + %{results: %{}, unresolved: MapSet.new([flag_name])} end + + case Map.fetch(local.results, flag_name) do + {:ok, result} -> + {:ok, result, %{}} + + :error -> + evaluate_single_flag_remotely(name, flag_name, body) + end + end + + defp evaluate_single_flag_remotely(name, flag_name, body) do + if Map.get(body, :only_evaluate_locally, false) == true do + {:ok, nil, %{}} else - {:error, reason} -> {:error, reason, nil} + case flags(name, Map.delete(body, :only_evaluate_locally)) do + {:ok, %{body: %{"flags" => %{^flag_name => flag_data}} = response_body}} -> + {:ok, build_result(flag_name, flag_data, response_body), response_body} + + {:ok, %{body: %{"flags" => _} = response_body}} -> + {:ok, nil, response_body} + + {:error, _reason} = error -> + error + end end end @@ -718,26 +975,46 @@ defmodule PostHog.FeatureFlags do defp maybe_put(map, _key, nil), do: map defp maybe_put(map, key, value), do: Map.put(map, key, value) - defp body_for_flags(distinct_id_or_body) do + defp body_for_flags(name, distinct_id_or_body) do + context = PostHog.get_context(name) + case distinct_id_or_body do %{distinct_id: _distinct_id} = body -> {:ok, body} + %{} = body -> + case context do + %{distinct_id: distinct_id} -> {:ok, Map.put(body, :distinct_id, distinct_id)} + _context -> missing_distinct_id() + end + nil -> - case PostHog.get_context() do + case context do %{distinct_id: distinct_id} -> - {:ok, %{distinct_id: distinct_id}} + {:ok, + context + |> Map.take([ + :distinct_id, + :groups, + :person_properties, + :group_properties, + :device_id + ]) + |> Map.put(:distinct_id, distinct_id)} _context -> - {:error, - %PostHog.Error{ - message: - "distinct_id is required but wasn't explicitly provided or found in the context" - }} + missing_distinct_id() end distinct_id when is_binary(distinct_id) -> {:ok, %{distinct_id: distinct_id}} end end + + defp missing_distinct_id do + {:error, + %PostHog.Error{ + message: "distinct_id is required but wasn't explicitly provided or found in the context" + }} + end end diff --git a/lib/posthog/feature_flags/definition_loader.ex b/lib/posthog/feature_flags/definition_loader.ex new file mode 100644 index 0000000..b745f68 --- /dev/null +++ b/lib/posthog/feature_flags/definition_loader.ex @@ -0,0 +1,561 @@ +defmodule PostHog.FeatureFlags.DefinitionLoader do + @moduledoc false + use GenServer + + require Logger + + defmodule Config do + @moduledoc false + @enforce_keys [ + :api_client, + :api_key, + :secret_key, + :supervisor_name, + :feature_flags_poll_interval_ms, + :flag_definition_request_timeout_ms, + :flag_definition_cache_provider_timeout_ms + ] + defstruct @enforce_keys ++ [flag_definition_cache_provider: nil] + end + + defimpl Inspect, for: Config do + import Inspect.Algebra + + def inspect(_config, _opts), + do: concat(["#PostHog.FeatureFlags.DefinitionLoader.Config"]) + end + + @max_quota_backoff_ms 60_000 + @negative_knowledge_capacity 1_000 + + @type snapshot :: %{ + flags: [map()], + flags_by_key: %{String.t() => map()}, + group_type_mapping: map(), + cohorts: map(), + minimal_flag_called_events: boolean() + } + + def child_spec(opts) do + config = opts |> Keyword.fetch!(:config) |> private_config() + callers = Keyword.get(opts, :callers, []) + + %{ + id: __MODULE__, + start: {__MODULE__, :start_link, [config, callers]}, + shutdown: + config.flag_definition_request_timeout_ms + + 3 * config.flag_definition_cache_provider_timeout_ms + 1_000 + } + end + + def start_link(%Config{} = config, callers) do + GenServer.start_link(__MODULE__, {config, callers}, name: via(config.supervisor_name)) + end + + @spec definitions(PostHog.supervisor_name()) :: snapshot() | nil + def definitions(name \\ PostHog), do: GenServer.call(via(name), :definitions) + + @spec ready?(PostHog.supervisor_name()) :: boolean() + def ready?(name \\ PostHog), do: GenServer.call(via(name), :ready?) + + @spec refresh(PostHog.supervisor_name()) :: :ok + def refresh(name \\ PostHog), do: GenServer.call(via(name), :refresh, :infinity) + + @doc false + def evaluation_state(name, keys), + do: GenServer.call(via(name), {:evaluation_state, keys}) + + @doc false + def update_negative_knowledge(name, generation, requested_keys, returned_keys, clean?) do + GenServer.call( + via(name), + {:update_negative_knowledge, generation, requested_keys, returned_keys, clean?} + ) + end + + defp via(name), do: PostHog.Registry.via(name, __MODULE__) + + @impl GenServer + def init({%Config{} = config, callers}) do + Process.flag(:trap_exit, true) + Process.put(:"$callers", callers) + + state = %{ + definitions: nil, + definition_generation: {make_ref(), 0}, + negative_keys: MapSet.new(), + negative_order: [], + etag: nil, + loaded_at: nil, + timer_ref: nil, + timer_generation: 0, + quota_backoff_ms: nil, + config: config + } + + {:ok, state, {:continue, :initial_load}} + end + + @impl GenServer + def handle_continue(:initial_load, state), do: {:noreply, refresh_and_schedule(state)} + + @impl GenServer + def handle_call(:definitions, _from, state), do: {:reply, state.definitions, state} + def handle_call(:ready?, _from, state), do: {:reply, not is_nil(state.definitions), state} + def handle_call(:refresh, _from, state), do: {:reply, :ok, refresh_and_schedule(state)} + + def handle_call({:evaluation_state, keys}, _from, state) do + known_missing = + keys + |> MapSet.new() + |> MapSet.intersection(state.negative_keys) + + reply = %{ + definitions: state.definitions, + generation: state.definition_generation, + known_missing: known_missing + } + + {:reply, reply, state} + end + + def handle_call( + {:update_negative_knowledge, generation, requested, returned, clean?}, + _from, + %{definition_generation: generation} = state + ) do + state = update_retained_missing(state, requested, returned, clean?) + {:reply, :ok, state} + end + + def handle_call( + {:update_negative_knowledge, _generation, _requested, _returned, _clean?}, + _from, + state + ), + do: {:reply, :stale_generation, state} + + @impl GenServer + def handle_info({:refresh, generation}, %{timer_generation: generation} = state), + do: {:noreply, refresh_and_schedule(state)} + + def handle_info({:refresh, _stale_generation}, state), do: {:noreply, state} + def handle_info({:boundary_result, _ref, _pid, _result}, state), do: {:noreply, state} + def handle_info({:DOWN, _monitor, :process, _pid, _reason}, state), do: {:noreply, state} + def handle_info(_message, state), do: {:noreply, state} + + @impl GenServer + def terminate(_reason, state) do + cancel_timer(state.timer_ref) + + case state.config.flag_definition_cache_provider do + {module, provider_state} -> + case invoke_provider(state, module, :shutdown, [provider_state]) do + {:ok, :ok} -> :ok + result -> provider_warning(:shutdown, result) + end + + nil -> + :ok + end + end + + defp refresh_and_schedule(state) do + state = invalidate_timer(state) + + state = + try do + refresh_state(state) + rescue + _exception -> + warning("definition refresh failed unexpectedly; keeping stale definitions") + state + catch + _kind, _reason -> + warning("definition refresh failed unexpectedly; keeping stale definitions") + state + end + + schedule_refresh(state) + end + + defp invalidate_timer(state) do + cancel_timer(state.timer_ref) + %{state | timer_ref: nil, timer_generation: state.timer_generation + 1} + end + + defp schedule_refresh(state) do + delay = state.quota_backoff_ms || state.config.feature_flags_poll_interval_ms + generation = state.timer_generation + timer_ref = Process.send_after(self(), {:refresh, generation}, delay) + %{state | timer_ref: timer_ref} + end + + defp refresh_state(%{config: %{flag_definition_cache_provider: nil}} = state), + do: fetch_direct(state, false) + + defp refresh_state( + %{ + config: %{flag_definition_cache_provider: {module, provider_state}} + } = state + ) do + case invoke_provider(state, module, :should_fetch_flag_definitions, [provider_state]) do + {:ok, true} -> + fetch_direct(state, true) + + {:ok, false} -> + read_provider_without_fetch_ownership(state, module, provider_state) + + other -> + provider_warning(:should_fetch_flag_definitions, other) + fetch_direct(state, true) + end + end + + defp read_provider_without_fetch_ownership(state, module, provider_state) do + case invoke_provider(state, module, :get_flag_definitions, [provider_state]) do + {:ok, nil} -> + provider_cache_miss(state) + + {:ok, cached} -> + case normalize_envelope(cached) do + {:ok, snapshot, _complete} -> install(state, snapshot, nil) + :error -> provider_read_failure(state, :malformed) + end + + other -> + provider_read_failure(state, other) + end + end + + defp provider_cache_miss(%{definitions: nil} = state), do: fetch_direct(state, false) + defp provider_cache_miss(state), do: state + + defp provider_read_failure(state, reason) do + provider_warning(:get_flag_definitions, reason) + + if is_nil(state.definitions) do + fetch_direct(state, false) + else + state + end + end + + defp fetch_direct(state, store?) do + config = state.config + + result = + invoke_boundary( + config.flag_definition_request_timeout_ms, + fn -> + PostHog.API.flag_definitions( + config.api_client, + config.api_key, + config.secret_key, + state.etag + ) + end + ) + + response = + case result do + {:ok, api_result} -> api_result + {:error, boundary_reason} -> {:error, boundary_reason} + end + + handle_response(state, response, store?) + end + + defp handle_response(state, {:ok, %{status: 200, body: body} = response}, store?) do + case normalize_envelope(body) do + {:ok, snapshot, complete} -> + state = install(state, snapshot, response_etag(response)) + state = %{state | quota_backoff_ms: nil} + if store?, do: store_provider(state, complete), else: state + + :error -> + warning("definition refresh returned a malformed response; keeping stale definitions") + state + end + end + + defp handle_response(state, {:ok, %{status: 304} = response}, _store?) do + state + |> Map.put(:etag, response_etag(response) || state.etag) + |> successful_definition_refresh(state.definitions) + end + + defp handle_response(state, {:ok, %{status: status}}, _store?) when status in [401, 402, 403] do + warning("definition refresh was rejected with HTTP #{status}; clearing local definitions") + %{state | definitions: nil, etag: nil, loaded_at: nil, quota_backoff_ms: nil} + end + + defp handle_response(state, {:ok, %{status: 429}}, _store?) do + warning("definition refresh was quota limited with HTTP 429; keeping stale definitions") + %{state | quota_backoff_ms: next_quota_backoff(state)} + end + + defp handle_response(state, {:ok, %{status: status}}, _store?) do + warning("definition refresh failed with HTTP #{status}; keeping stale definitions") + state + end + + defp handle_response(state, {:error, :timeout}, _store?) do + warning("definition refresh timed out; keeping stale definitions") + state + end + + defp handle_response(state, {:error, _reason}, _store?) do + # Do not inspect custom-client failures: request terms may contain authorization headers. + warning("definition refresh request failed; keeping stale definitions") + state + end + + defp handle_response(state, _other, _store?) do + warning("definition refresh returned an unexpected response; keeping stale definitions") + state + end + + defp next_quota_backoff(state) do + baseline = state.config.feature_flags_poll_interval_ms + current = max(state.quota_backoff_ms || baseline, baseline) + cap = max(@max_quota_backoff_ms, baseline * 8) + current |> Kernel.*(2) |> max(baseline) |> min(cap) + end + + defp install(state, snapshot, etag) do + state + |> Map.put(:etag, etag) + |> successful_definition_refresh(snapshot) + end + + defp successful_definition_refresh(state, snapshot) do + %{ + state + | definitions: snapshot, + definition_generation: next_definition_generation(state.definition_generation), + negative_keys: MapSet.new(), + negative_order: [], + loaded_at: DateTime.utc_now(), + quota_backoff_ms: nil + } + end + + defp next_definition_generation({incarnation, revision}), + do: {incarnation, revision + 1} + + defp store_provider( + %{config: %{flag_definition_cache_provider: {module, provider_state}}} = state, + complete + ) do + case invoke_provider(state, module, :on_flag_definitions_received, [provider_state, complete]) do + {:ok, :ok} -> + state + + other -> + provider_warning(:on_flag_definitions_received, other) + state + end + end + + defp normalize_envelope(body) when is_map(body) do + flags = value(body, "flags") + mapping = value(body, "group_type_mapping") + cohorts = value(body, "cohorts") + + if is_list(flags) and is_map(mapping) and is_map(cohorts) do + minimal = value(body, "minimal_flag_called_events") == true + + snapshot = %{ + flags: flags, + flags_by_key: + Map.new(Enum.filter(flags, &(is_map(&1) and is_binary(value(&1, "key")))), fn flag -> + {value(flag, "key"), flag} + end), + group_type_mapping: mapping, + cohorts: cohorts, + minimal_flag_called_events: minimal + } + + complete = %{ + "flags" => flags, + "group_type_mapping" => mapping, + "cohorts" => cohorts, + "minimal_flag_called_events" => minimal + } + + {:ok, snapshot, complete} + else + :error + end + end + + defp normalize_envelope(_body), do: :error + + defp value(map, "flags"), do: Map.get(map, "flags", Map.get(map, :flags)) + + defp value(map, "group_type_mapping"), + do: Map.get(map, "group_type_mapping", Map.get(map, :group_type_mapping)) + + defp value(map, "cohorts"), do: Map.get(map, "cohorts", Map.get(map, :cohorts)) + + defp value(map, "minimal_flag_called_events"), + do: Map.get(map, "minimal_flag_called_events", Map.get(map, :minimal_flag_called_events)) + + defp value(map, "key"), do: Map.get(map, "key", Map.get(map, :key)) + + defp response_etag(%{headers: headers}), do: header_value(headers, "etag") + defp response_etag(_response), do: nil + + defp header_value(headers, key) when is_map(headers) do + Enum.find_value(headers, fn {name, header} -> + if String.downcase(to_string(name)) == key, do: first_header(header) + end) + end + + defp header_value(headers, key) when is_list(headers) do + Enum.find_value(headers, fn + {name, header} -> if String.downcase(to_string(name)) == key, do: first_header(header) + _ -> nil + end) + end + + defp header_value(_headers, _key), do: nil + defp first_header([header | _]), do: header + defp first_header(header) when is_binary(header), do: header + defp first_header(_header), do: nil + + defp invoke_provider(state, module, callback, args) do + invoke_boundary( + state.config.flag_definition_cache_provider_timeout_ms, + fn -> apply(module, callback, args) end + ) + end + + defp invoke_boundary(timeout, callback) do + parent = self() + callers = Process.get(:"$callers", []) + ref = make_ref() + + {pid, monitor} = + spawn_monitor(fn -> + Process.put(:"$callers", callers) + result = safely_invoke(callback) + send(parent, {:boundary_result, ref, self(), result}) + end) + + receive do + {:boundary_result, ^ref, ^pid, result} -> + Process.demonitor(monitor, [:flush]) + result + + {:DOWN, ^monitor, :process, ^pid, reason} -> + {:error, boundary_error(reason)} + after + timeout -> + Process.exit(pid, :kill) + await_down(monitor, pid) + drain_boundary_result(ref, pid) + {:error, :timeout} + end + end + + defp safely_invoke(callback) do + {:ok, callback.()} + rescue + _exception -> {:error, :exception} + catch + :exit, reason -> {:error, boundary_error(reason)} + _kind, _reason -> {:error, :throw} + end + + defp boundary_error(:normal), do: :normal + defp boundary_error(:killed), do: :timeout + defp boundary_error(_reason), do: :exit + + defp await_down(monitor, pid) do + receive do + {:DOWN, ^monitor, :process, ^pid, _reason} -> :ok + after + 100 -> Process.demonitor(monitor, [:flush]) + end + end + + defp drain_boundary_result(ref, pid) do + receive do + {:boundary_result, ^ref, ^pid, _result} -> :ok + after + 0 -> :ok + end + end + + defp cancel_timer(nil), do: :ok + defp cancel_timer(ref), do: Process.cancel_timer(ref) + + defp provider_warning(callback, result) do + reason = + case result do + {:error, reason} -> reason + {:ok, unexpected} -> {:unexpected_return, unexpected} + other -> other + end + + warning("definition cache provider #{callback} failed (#{provider_reason(reason)})") + end + + defp provider_reason(:timeout), do: "timeout" + defp provider_reason(:exception), do: "exception" + defp provider_reason(:exit), do: "exit" + defp provider_reason(:throw), do: "throw" + defp provider_reason(:malformed), do: "malformed data" + defp provider_reason({:unexpected_return, _value}), do: "unexpected return" + defp provider_reason(_reason), do: "error" + + defp update_retained_missing(state, requested, returned, clean?) do + returned = MapSet.new(returned) + + order = Enum.reject(state.negative_order, &MapSet.member?(returned, &1)) + keys = MapSet.difference(state.negative_keys, returned) + + {order, keys} = + if clean? do + omissions = + requested + |> MapSet.new() + |> MapSet.difference(returned) + |> MapSet.to_list() + |> Enum.sort() + + Enum.reduce(omissions, {order, keys}, fn key, {order, keys} -> + {Enum.reject(order, &(&1 == key)) ++ [key], MapSet.put(keys, key)} + end) + else + {order, keys} + end + + overflow = max(length(order) - @negative_knowledge_capacity, 0) + retained_order = Enum.drop(order, overflow) + + %{ + state + | negative_order: retained_order, + negative_keys: MapSet.intersection(keys, MapSet.new(retained_order)) + } + end + + defp private_config(config) do + struct!(Config, %{ + api_client: config.api_client, + api_key: config.api_key, + secret_key: config.secret_key, + supervisor_name: config.supervisor_name, + feature_flags_poll_interval_ms: config.feature_flags_poll_interval_ms, + flag_definition_request_timeout_ms: config.flag_definition_request_timeout_ms, + flag_definition_cache_provider_timeout_ms: config.flag_definition_cache_provider_timeout_ms, + flag_definition_cache_provider: config.flag_definition_cache_provider + }) + end + + defp warning(message), do: Logger.warning(message, posthog_skip_capture: true) +end diff --git a/lib/posthog/feature_flags/evaluations.ex b/lib/posthog/feature_flags/evaluations.ex index ae0bce4..114373e 100644 --- a/lib/posthog/feature_flags/evaluations.ex +++ b/lib/posthog/feature_flags/evaluations.ex @@ -2,10 +2,10 @@ defmodule PostHog.FeatureFlags.Evaluations do @moduledoc """ Snapshot of feature flag evaluations for a single `distinct_id`. - An `Evaluations` struct represents the result of a single `/flags` call. It is - built by `PostHog.FeatureFlags.evaluate_flags/2` and lets you branch on - multiple flags and enrich captured events from the same fetch — without - paying the cost of one round-trip per flag. + An `Evaluations` struct represents one point-in-time evaluation assembled + locally and, when needed, from at most one `/flags` call. It is built by + `PostHog.FeatureFlags.evaluate_flags/2` and lets you branch on multiple flags + and enrich captured events from the same frozen result set. Each snapshot owns a small `Agent` linked to the calling process that tracks which flags were accessed via `enabled?/2` and `get_flag/2`. The Agent exits @@ -55,12 +55,12 @@ defmodule PostHog.FeatureFlags.Evaluations do - `:supervisor_name` - PostHog instance the snapshot was produced from; used when `enabled?/2` and `get_flag/2` fire `$feature_flag_called` events. - - `:distinct_id` - resolved distinct ID the `/flags` request was made for. - `""` for the empty fallback returned when no `distinct_id` could be - resolved; events are short-circuited in that case. + - `:distinct_id` - resolved distinct ID the local and/or remote evaluation + was performed for. `""` for the empty fallback returned when no + `distinct_id` could be resolved; events are short-circuited in that case. - `:flags` - map of flag key to `t:PostHog.FeatureFlags.Result.t/0`. - - `:request_id` - request ID returned by `/flags`. - - `:evaluated_at` - server-side evaluation timestamp. + - `:request_id` - request ID returned when remote fallback was used. + - `:evaluated_at` - server-side evaluation timestamp when remote fallback was used. - `:errors_while_computing` - whether the response signaled `errorsWhileComputingFlags`. When `true`, every event fired from this snapshot includes `errors_while_computing_flags` in its @@ -108,6 +108,28 @@ defmodule PostHog.FeatureFlags.Evaluations do } end + @doc false + @spec from_results( + PostHog.supervisor_name(), + PostHog.distinct_id(), + %{String.t() => Result.t()}, + map() + ) :: t() + def from_results(supervisor_name, distinct_id, results, metadata) + when is_map(results) and is_map(metadata) do + %__MODULE__{ + supervisor_name: supervisor_name, + distinct_id: distinct_id, + flags: results, + request_id: Map.get(metadata, :request_id, Map.get(metadata, "requestId")), + evaluated_at: Map.get(metadata, :evaluated_at, Map.get(metadata, "evaluatedAt")), + errors_while_computing: + Map.get(metadata, :errors_while_computing, Map.get(metadata, "errorsWhileComputingFlags")) == + true, + accessed_pid: start_accessed_agent() + } + end + @doc false @spec empty(PostHog.supervisor_name()) :: t() def empty(supervisor_name) do diff --git a/lib/posthog/feature_flags/flag_definition_cache_provider.ex b/lib/posthog/feature_flags/flag_definition_cache_provider.ex new file mode 100644 index 0000000..20a9ba0 --- /dev/null +++ b/lib/posthog/feature_flags/flag_definition_cache_provider.ex @@ -0,0 +1,41 @@ +defmodule PostHog.FeatureFlags.FlagDefinitionCacheProvider do + @moduledoc """ + Optional shared cache contract for local feature flag definitions. + + Configure a provider as `{module, provider_state}` using + `:flag_definition_cache_provider`. The definition loader bounds and isolates + every callback. Cached values must be maps containing `flags`, + `group_type_mapping`, and `cohorts` (string or atom keys are accepted). + + A minimal provider can coordinate fetching and keep the complete envelope in + an application-owned cache: + + defmodule MyApp.PostHogDefinitionCache do + @behaviour PostHog.FeatureFlags.FlagDefinitionCacheProvider + + def should_fetch_flag_definitions(state), do: state.fetch_owner?.() + def get_flag_definitions(state), do: state.read.() + def on_flag_definitions_received(state, definitions), do: state.write.(definitions) + def shutdown(_state), do: :ok + end + + config :posthog, + secret_key: System.fetch_env!("POSTHOG_SECRET_KEY"), + flag_definition_cache_provider: {MyApp.PostHogDefinitionCache, provider_state} + """ + + @type provider_state :: any() + @type definitions :: map() + + @doc "Returns whether this SDK instance should fetch fresh definitions from PostHog." + @callback should_fetch_flag_definitions(provider_state()) :: boolean() + + @doc "Reads definitions from the shared cache, or returns nil when it is empty." + @callback get_flag_definitions(provider_state()) :: definitions() | nil + + @doc "Stores a complete definition envelope fetched from PostHog." + @callback on_flag_definitions_received(provider_state(), definitions()) :: :ok + + @doc "Releases resources owned by the provider." + @callback shutdown(provider_state()) :: :ok +end diff --git a/lib/posthog/feature_flags/local_evaluator.ex b/lib/posthog/feature_flags/local_evaluator.ex new file mode 100644 index 0000000..23195c2 --- /dev/null +++ b/lib/posthog/feature_flags/local_evaluator.ex @@ -0,0 +1,1027 @@ +# The evaluator mirrors a recursive, tri-state rules language. Keeping each rules +# branch explicit is safer and more discoverable than hiding it behind dynamic dispatch. +# credo:disable-for-this-file Credo.Check.Readability.WithSingleClause +# credo:disable-for-this-file Credo.Check.Refactor.CyclomaticComplexity +# credo:disable-for-this-file Credo.Check.Refactor.FunctionArity +# credo:disable-for-this-file Credo.Check.Refactor.Nesting +defmodule PostHog.FeatureFlags.LocalEvaluator do + @moduledoc false + + alias PostHog.FeatureFlags.Result + + @long_scale 0xFFFFFFFFFFFFFFF + @semver_operators ~w(semver_eq semver_neq semver_gt semver_gte semver_lt semver_lte semver_tilde semver_caret semver_wildcard) + + @type outcome :: {:ok, boolean() | String.t()} | :inconclusive | :requires_server + + @spec evaluate(map(), map(), [String.t()] | nil) :: %{ + results: %{String.t() => Result.t()}, + unresolved: MapSet.t(String.t()) + } + def evaluate(definitions, context, keys \\ nil) do + keys = keys || Map.keys(definitions.flags_by_key) + context = normalize_context(context) + + {results, unresolved, _cache} = + Enum.reduce(keys, {%{}, MapSet.new(), %{}}, fn key, {results, unresolved, cache} -> + case Map.fetch(definitions.flags_by_key, key) do + {:ok, flag} -> + {outcome, cache} = evaluate_key(key, flag, definitions, context, cache) + + case outcome do + {:ok, value} -> + {Map.put(results, key, build_result(key, flag, value, definitions)), unresolved, + cache} + + status when status in [:inconclusive, :requires_server] -> + {results, MapSet.put(unresolved, key), cache} + end + + :error -> + {results, MapSet.put(unresolved, key), cache} + end + end) + + %{results: results, unresolved: unresolved} + end + + @doc false + @spec hash(String.t(), String.t(), String.t()) :: float() + def hash(key, bucketing_value, salt \\ "") do + <> = + :crypto.hash(:sha, "#{key}.#{bucketing_value}#{salt}") |> Base.encode16(case: :lower) + + {integer, ""} = Integer.parse(prefix, 16) + integer / @long_scale + end + + defp normalize_context(context) do + %{ + distinct_id: context_value(context, :distinct_id), + groups: context_value(context, :groups) || %{}, + person_properties: context_value(context, :person_properties) || %{}, + group_properties: context_value(context, :group_properties) || %{}, + device_id: context_value(context, :device_id), + now: context_value(context, :now) || DateTime.utc_now() + } + end + + defp context_value(context, key), + do: Map.get(context, key, Map.get(context, Atom.to_string(key))) + + defp evaluate_key(key, flag, definitions, context, cache) do + case Map.get(cache, key) do + :resolving -> + {:inconclusive, cache} + + {:done, outcome} -> + {outcome, cache} + + nil -> + cache = Map.put(cache, key, :resolving) + {outcome, cache} = evaluate_flag(flag, definitions, context, cache) + {outcome, Map.put(cache, key, {:done, outcome})} + end + rescue + _exception -> {:inconclusive, Map.put(cache, key, {:done, :inconclusive})} + catch + _kind, _reason -> {:inconclusive, Map.put(cache, key, {:done, :inconclusive})} + end + + defp evaluate_flag(flag, definitions, context, cache) when is_map(flag) do + cond do + value(flag, "active") == false -> + {{:ok, false}, cache} + + value(flag, "active") != true -> + {:inconclusive, cache} + + value(flag, "ensure_experience_continuity") == true -> + {:requires_server, cache} + + not is_binary(value(flag, "key")) or not is_map(value(flag, "filters")) -> + {:inconclusive, cache} + + true -> + evaluate_active_flag(flag, definitions, context, cache) + end + end + + defp evaluate_flag(_flag, _definitions, _context, cache), do: {:inconclusive, cache} + + defp evaluate_active_flag(flag, definitions, context, cache) do + filters = value(flag, "filters") + aggregation = value(filters, "aggregation_group_type_index") + + with {:ok, properties, bucketing} <- + evaluation_target(aggregation, flag, definitions, context) do + conditions = value(filters, "groups") || [] + + if is_list(conditions) do + evaluate_conditions( + conditions, + flag, + aggregation, + properties, + bucketing, + definitions, + context, + cache, + false + ) + else + {:inconclusive, cache} + end + else + status when status in [:inconclusive, :requires_server] -> {status, cache} + end + end + + defp evaluation_target(nil, flag, _definitions, context) do + case bucketing_value(flag, context) do + {:ok, bucketing} -> {:ok, context.person_properties, bucketing} + status -> status + end + end + + defp evaluation_target(index, _flag, definitions, context) do + group_name = map_value(definitions.group_type_mapping, to_string(index)) + group_key = if is_binary(group_name), do: map_value(context.groups, group_name) + properties = if is_binary(group_name), do: map_value(context.group_properties, group_name) + + cond do + not is_binary(group_name) or is_nil(group_key) -> :inconclusive + not is_map(properties) -> :inconclusive + true -> {:ok, properties, to_string(group_key)} + end + end + + defp bucketing_value(flag, context) do + filters = value(flag, "filters") || %{} + identifier = value(flag, "bucketing_identifier") || value(filters, "bucketing_identifier") + + case identifier do + "device_id" when is_binary(context.device_id) and context.device_id != "" -> + {:ok, context.device_id} + + "device_id" -> + :inconclusive + + _ when is_binary(context.distinct_id) -> + {:ok, context.distinct_id} + + _ -> + :inconclusive + end + end + + defp evaluate_conditions( + [], + _flag, + _aggregation, + _properties, + _bucketing, + _definitions, + _context, + cache, + inconclusive? + ) do + {if(inconclusive?, do: :inconclusive, else: {:ok, false}), cache} + end + + defp evaluate_conditions( + [condition | rest], + flag, + aggregation, + properties, + bucketing, + definitions, + context, + cache, + inconclusive? + ) do + case condition_target( + condition, + aggregation, + properties, + bucketing, + flag, + definitions, + context + ) do + {:ok, effective_properties, effective_bucketing} -> + {match, cache} = + condition_match( + condition, + flag, + effective_properties, + effective_bucketing, + definitions, + context, + cache + ) + + case match do + :match -> + value = matching_value(flag, condition, effective_bucketing) + {{:ok, value}, cache} + + :out_of_rollout -> + if value(value(flag, "filters"), "early_exit") == true and not inconclusive? do + {{:ok, false}, cache} + else + evaluate_conditions( + rest, + flag, + aggregation, + properties, + bucketing, + definitions, + context, + cache, + inconclusive? + ) + end + + :no_match -> + evaluate_conditions( + rest, + flag, + aggregation, + properties, + bucketing, + definitions, + context, + cache, + inconclusive? + ) + + :inconclusive -> + evaluate_conditions( + rest, + flag, + aggregation, + properties, + bucketing, + definitions, + context, + cache, + true + ) + + :requires_server -> + {:requires_server, cache} + end + + :skip -> + evaluate_conditions( + rest, + flag, + aggregation, + properties, + bucketing, + definitions, + context, + cache, + inconclusive? + ) + + :inconclusive -> + evaluate_conditions( + rest, + flag, + aggregation, + properties, + bucketing, + definitions, + context, + cache, + true + ) + end + end + + defp condition_target(condition, aggregation, properties, bucketing, flag, definitions, context) + when is_map(condition) do + condition_aggregation = + if has_key?(condition, "aggregation_group_type_index") do + value(condition, "aggregation_group_type_index") + else + aggregation + end + + if condition_aggregation == aggregation do + {:ok, properties, bucketing} + else + case condition_aggregation do + nil -> + case bucketing_value(flag, context) do + {:ok, person_bucketing} -> {:ok, context.person_properties, person_bucketing} + _ -> :inconclusive + end + + index -> + group_name = map_value(definitions.group_type_mapping, to_string(index)) + group_key = if is_binary(group_name), do: map_value(context.groups, group_name) + + group_props = + if is_binary(group_name), do: map_value(context.group_properties, group_name) + + cond do + is_nil(group_key) -> :skip + not is_map(group_props) -> :inconclusive + true -> {:ok, group_props, to_string(group_key)} + end + end + end + end + + defp condition_target( + _condition, + _aggregation, + _properties, + _bucketing, + _flag, + _definitions, + _context + ), + do: :inconclusive + + defp condition_match(condition, flag, properties, bucketing, definitions, context, cache) do + property_filters = value(condition, "properties") || [] + + if is_list(property_filters) do + {property_result, cache} = + match_all_properties(property_filters, properties, definitions, context, cache) + + case property_result do + :match -> rollout_match(condition, flag, bucketing, cache) + other -> {other, cache} + end + else + {:inconclusive, cache} + end + end + + defp rollout_match(condition, flag, bucketing, cache) do + case value(condition, "rollout_percentage") do + nil -> + {:match, cache} + + percentage when is_number(percentage) and percentage >= 100 -> + {:match, cache} + + percentage when is_number(percentage) and percentage <= 0 -> + {:out_of_rollout, cache} + + percentage when is_number(percentage) -> + result = + if hash(value(flag, "key"), bucketing) < percentage / 100, + do: :match, + else: :out_of_rollout + + {result, cache} + + _ -> + {:inconclusive, cache} + end + end + + defp match_all_properties(properties, property_values, definitions, context, cache) do + Enum.reduce_while(properties, {:match, cache}, fn property, {aggregate, cache} -> + {result, cache} = + match_typed_property(property, property_values, definitions, context, cache) + + case result do + :no_match -> {:halt, {:no_match, cache}} + :requires_server -> {:halt, {:requires_server, cache}} + :inconclusive -> {:cont, {:inconclusive, cache}} + :match -> {:cont, {aggregate, cache}} + end + end) + end + + defp match_typed_property(property, property_values, definitions, context, cache) + when is_map(property) do + {result, cache} = + case value(property, "type") do + "cohort" -> {match_cohort(property, property_values, definitions, context, cache), cache} + "flag" -> match_dependency(property, definitions, context, cache) + _ -> {match_property(property, property_values, context.now), cache} + end + + {apply_negation(result, value(property, "negation") == true), cache} + end + + defp match_typed_property(_property, _values, _definitions, _context, cache), + do: {:inconclusive, cache} + + defp apply_negation(:match, true), do: :no_match + defp apply_negation(:no_match, true), do: :match + defp apply_negation(result, _negation), do: result + + defp match_dependency(property, definitions, context, cache) do + key = value(property, "key") + expected = value(property, "value") + + cond do + value(property, "operator") != "flag_evaluates_to" -> + {:no_match, cache} + + not is_binary(key) or is_nil(expected) -> + {:no_match, cache} + + not has_key?(property, "dependency_chain") or value(property, "dependency_chain") == [] -> + {:inconclusive, cache} + + not is_list(value(property, "dependency_chain")) or + not Enum.all?(value(property, "dependency_chain"), &is_binary/1) -> + {:inconclusive, cache} + + true -> + case evaluate_dependency_chain( + value(property, "dependency_chain"), + definitions, + context, + cache + ) do + {:ok, cache} -> dependency_result(key, expected, cache) + {:error, cache} -> {:inconclusive, cache} + end + end + end + + defp evaluate_dependency_chain([], _definitions, _context, cache), do: {:ok, cache} + + defp evaluate_dependency_chain([key | rest], definitions, context, cache) do + case Map.get(cache, key) do + {:done, {:ok, _value}} -> + evaluate_dependency_chain(rest, definitions, context, cache) + + {:done, _outcome} -> + {:error, cache} + + :resolving -> + {:error, cache} + + nil -> + case Map.fetch(definitions.flags_by_key, key) do + :error -> + {:error, Map.put(cache, key, {:done, :inconclusive})} + + {:ok, dependency} -> + {outcome, cache} = evaluate_key(key, dependency, definitions, context, cache) + + case outcome do + {:ok, _value} -> evaluate_dependency_chain(rest, definitions, context, cache) + _other -> {:error, cache} + end + end + end + end + + defp dependency_result(key, expected, cache) do + case Map.get(cache, key) do + {:done, {:ok, actual}} -> + {if(dependency_matches?(expected, actual), do: :match, else: :no_match), cache} + + _other -> + {:inconclusive, cache} + end + end + + defp dependency_matches?(expected, actual) when is_binary(actual) and actual != "" do + (is_boolean(expected) and expected) or (is_binary(expected) and expected == actual) + end + + defp dependency_matches?(expected, actual) when is_boolean(expected) and is_boolean(actual), + do: expected == actual + + defp dependency_matches?(_expected, _actual), do: false + + defp match_cohort(property, property_values, definitions, context, cache) do + cohort_id = to_string(value(property, "value")) + + case map_fetch(definitions.cohorts, cohort_id) do + :error -> + :requires_server + + {:ok, group} -> + result = match_property_group(group, property_values, definitions, context, cache) + + case value(property, "operator") || "exact" do + operator when operator in ["exact", "in"] -> result + "not_in" -> apply_negation(result, true) + _ -> :inconclusive + end + end + end + + defp match_property_group(group, _property_values, _definitions, _context, _cache) + when group == %{}, + do: :match + + defp match_property_group(group, property_values, definitions, context, cache) + when is_map(group) do + type = value(group, "type") + values = value(group, "values") + + if type in ["AND", "OR"] and is_list(values) do + match_group_values(type, values, property_values, definitions, context, cache) + else + :requires_server + end + end + + defp match_property_group(_group, _property_values, _definitions, _context, _cache), + do: :requires_server + + defp match_group_values(_type, [], _properties, _definitions, _context, _cache), do: :match + + defp match_group_values(type, values, properties, definitions, context, cache) do + results = + Enum.map(values, fn entry -> + if is_map(entry) and (entry == %{} or has_key?(entry, "values")) do + entry + |> match_property_group(properties, definitions, context, cache) + |> apply_negation(value(entry, "negation") == true) + else + {result, _cache} = match_typed_property(entry, properties, definitions, context, cache) + result + end + end) + + combine_group_results(type, results) + end + + defp combine_group_results("AND", results) do + cond do + :requires_server in results -> :requires_server + :no_match in results -> :no_match + :inconclusive in results -> :inconclusive + true -> :match + end + end + + defp combine_group_results("OR", results) do + cond do + :requires_server in results -> :requires_server + :match in results -> :match + :inconclusive in results -> :inconclusive + true -> :no_match + end + end + + defp match_property(property, property_values, now) do + key = value(property, "key") + operator = value(property, "operator") || "exact" + filter_value = value(property, "value") + + case map_fetch(property_values, key) do + :error -> + :inconclusive + + {:ok, property_value} -> + apply_operator(operator, property_value, filter_value, now) + end + end + + defp apply_operator("is_set", _property, _filter, _now), do: :match + defp apply_operator("is_not_set", _property, _filter, _now), do: :no_match + + defp apply_operator("is_not", nil, filter, _now) do + match = if is_list(filter), do: nil in filter, else: is_nil(filter) + boolean_result(not match) + end + + defp apply_operator(_operator, nil, _filter, _now), do: :no_match + + defp apply_operator(operator, property, filter, _now) when operator in ["exact", "is_not"] do + match = + if is_list(filter) do + Enum.any?(filter, &case_insensitive_equal?(property, &1)) + else + case_insensitive_equal?(property, filter) + end + + boolean_result(if(operator == "exact", do: match, else: not match)) + end + + defp apply_operator(operator, property, filter, _now) + when operator in [ + "icontains", + "not_icontains", + "starts_with", + "not_starts_with", + "ends_with", + "not_ends_with" + ] do + property = ascii_downcase(to_string(property)) + filter = ascii_downcase(to_string(filter)) + + positive = + case operator do + op when op in ["icontains", "not_icontains"] -> String.contains?(property, filter) + op when op in ["starts_with", "not_starts_with"] -> String.starts_with?(property, filter) + _ -> String.ends_with?(property, filter) + end + + negative? = operator in ["not_icontains", "not_starts_with", "not_ends_with"] + boolean_result(if(negative?, do: not positive, else: positive)) + end + + defp apply_operator(operator, property, filter, _now) when operator in ["regex", "not_regex"] do + case Regex.compile(to_string(filter)) do + {:ok, regex} -> + matched = Regex.match?(regex, to_string(property)) + boolean_result(if(operator == "regex", do: matched, else: not matched)) + + {:error, _reason} -> + :inconclusive + end + end + + defp apply_operator(operator, property, filter, _now) + when operator in ["gt", "gte", "lt", "lte"] do + with {:ok, left} <- number(property), {:ok, right} <- number(filter) do + compare(operator, left, right) + else + _ -> :inconclusive + end + end + + defp apply_operator(operator, property, filter, now) + when operator in ["is_date_before", "is_date_after"] do + with {:ok, left} <- parse_datetime(property, now), + {:ok, right} <- parse_datetime(filter, now) do + comparison = DateTime.compare(left, right) + + boolean_result( + if(operator == "is_date_before", do: comparison == :lt, else: comparison == :gt) + ) + else + _ -> :inconclusive + end + end + + defp apply_operator(operator, property, filter, _now) when operator in @semver_operators do + semver_match(operator, property, filter) + end + + defp apply_operator(_operator, _property, _filter, _now), do: :inconclusive + + defp matching_value(flag, condition, bucketing) do + variants = variants(flag) + override = value(condition, "variant") + + cond do + is_binary(override) and Enum.any?(variants, &(value(&1, "key") == override)) -> override + variants == [] -> true + true -> matching_variant(flag, variants, bucketing) || true + end + end + + defp matching_variant(flag, variants, bucketing) do + target = hash(value(flag, "key"), bucketing, "variant") + + Enum.reduce_while(variants, 0.0, fn variant, lower -> + percentage = value(variant, "rollout_percentage") + + if is_number(percentage) do + upper = lower + percentage / 100 + + if target >= lower and target < upper, + do: {:halt, value(variant, "key")}, + else: {:cont, upper} + else + {:halt, nil} + end + end) + end + + defp variants(flag) do + with filters when is_map(filters) <- value(flag, "filters"), + multivariate when is_map(multivariate) <- value(filters, "multivariate"), + variants when is_list(variants) <- value(multivariate, "variants") do + variants + else + _ -> [] + end + end + + defp build_result(key, flag, value, definitions) do + enabled = value != false + variant = if is_binary(value), do: value + payload_key = if is_binary(value), do: value, else: to_string(value) + payloads = value(value(flag, "filters") || %{}, "payloads") || %{} + payload = payloads |> map_value(payload_key) |> decode_payload() + + %Result{ + key: key, + enabled: enabled, + variant: variant, + payload: payload, + id: value(flag, "id"), + version: value(flag, "version"), + has_experiment: boolean_or_nil(value(flag, "has_experiment")), + minimal_flag_called_events: definitions.minimal_flag_called_events + } + end + + defp decode_payload(nil), do: nil + + defp decode_payload(payload) when is_binary(payload) do + case Jason.decode(payload) do + {:ok, decoded} -> decoded + {:error, _reason} -> payload + end + end + + defp decode_payload(payload), do: payload + + defp boolean_or_nil(value) when is_boolean(value), do: value + defp boolean_or_nil(_value), do: nil + defp boolean_result(true), do: :match + defp boolean_result(false), do: :no_match + + defp case_insensitive_equal?(left, right), + do: ascii_downcase(to_string(left)) == ascii_downcase(to_string(right)) + + defp ascii_downcase(value) do + for <>, into: "" do + if character in ?A..?Z, do: <>, else: <> + end + end + + defp number(value) when is_number(value), do: {:ok, value} + + defp number(value) when is_binary(value) do + case Float.parse(value) do + {number, ""} -> {:ok, number} + _ -> :error + end + end + + defp number(_value), do: :error + + defp compare("gt", left, right), do: boolean_result(left > right) + defp compare("gte", left, right), do: boolean_result(left >= right) + defp compare("lt", left, right), do: boolean_result(left < right) + defp compare("lte", left, right), do: boolean_result(left <= right) + + defp parse_datetime(%DateTime{} = datetime, _now), do: {:ok, datetime} + + defp parse_datetime(%NaiveDateTime{} = datetime, _now), + do: DateTime.from_naive(datetime, "Etc/UTC") + + defp parse_datetime(%Date{} = date, _now) do + date |> NaiveDateTime.new!(~T[00:00:00]) |> DateTime.from_naive("Etc/UTC") + end + + defp parse_datetime(value, now) when is_binary(value) do + case Regex.run(~r/^-?(\d+)([hdwmy])$/, value) do + [_, amount, unit] -> + shift_relative_datetime(now, String.to_integer(amount), unit) + + nil -> + parse_iso_datetime(value) + end + end + + defp parse_datetime(_value, _now), do: :error + + defp shift_relative_datetime(_now, amount, _unit) when amount >= 10_000, do: :error + + defp shift_relative_datetime(now, amount, unit) when unit in ["h", "d", "w"] do + seconds = amount * Map.fetch!(%{"h" => 3_600, "d" => 86_400, "w" => 604_800}, unit) + {:ok, DateTime.add(now, -seconds, :second)} + end + + defp shift_relative_datetime(now, amount, "m"), + do: {:ok, DateTime.shift(now, month: -amount)} + + defp shift_relative_datetime(now, amount, "y"), + do: {:ok, DateTime.shift(now, year: -amount)} + + defp parse_iso_datetime(value) do + case DateTime.from_iso8601(value) do + {:ok, datetime, _offset} -> + {:ok, datetime} + + {:error, _reason} -> + parse_timezone_less_datetime(value) + end + end + + defp parse_timezone_less_datetime(value) do + normalized = String.replace(value, " ", "T", global: false) + + case NaiveDateTime.from_iso8601(normalized) do + {:ok, datetime} -> + DateTime.from_naive(datetime, "Etc/UTC") + + {:error, _reason} -> + case Date.from_iso8601(value) do + {:ok, date} -> parse_datetime(date, nil) + {:error, _reason} -> :error + end + end + end + + defp semver_match(operator, property, filter) do + with {:ok, actual} <- parse_semver(property) do + case operator do + "semver_wildcard" -> + semver_range(actual, wildcard_bounds(filter)) + + "semver_tilde" -> + semver_range(actual, tilde_bounds(filter)) + + "semver_caret" -> + semver_range(actual, caret_bounds(filter)) + + _ -> + with {:ok, expected} <- parse_semver(filter) do + ordering = compare_semver(actual, expected) + + boolean_result( + case operator do + "semver_eq" -> ordering == :eq + "semver_neq" -> ordering != :eq + "semver_gt" -> ordering == :gt + "semver_gte" -> ordering in [:gt, :eq] + "semver_lt" -> ordering == :lt + "semver_lte" -> ordering in [:lt, :eq] + end + ) + else + _ -> :inconclusive + end + end + else + _ -> :inconclusive + end + end + + defp semver_range(_actual, :error), do: :inconclusive + + defp semver_range(actual, {:ok, lower, upper}) do + boolean_result( + compare_semver(actual, lower) in [:eq, :gt] and compare_semver(actual, upper) == :lt + ) + end + + defp parse_semver(value) when is_binary(value) do + parts = + value + |> normalize_semver_text() + |> String.split(".") + + if parts == [] do + :error + else + with {:ok, numbers} <- parts |> Enum.take(3) |> parse_semver_numbers() do + {:ok, numbers |> Kernel.++([0, 0]) |> Enum.take(3) |> List.to_tuple()} + end + end + end + + defp parse_semver(_value), do: :error + + defp normalize_semver_text(value) do + value + |> String.trim() + |> String.replace_prefix("v", "") + |> String.replace_prefix("V", "") + |> String.split(~r/[-+]/, parts: 2) + |> hd() + end + + defp parse_semver_numbers(parts) do + Enum.reduce_while(parts, {:ok, []}, fn part, {:ok, numbers} -> + case parse_semver_number(part) do + {:ok, number} -> {:cont, {:ok, numbers ++ [number]}} + :error -> {:halt, :error} + end + end) + end + + defp compare_semver(left, right), do: compare_tuple(left, right) + defp compare_tuple(left, right) when left < right, do: :lt + defp compare_tuple(left, right) when left > right, do: :gt + defp compare_tuple(_left, _right), do: :eq + + defp tilde_bounds(value) do + with {:ok, {major, minor, patch}} <- parse_semver(value) do + {:ok, {major, minor, patch}, {major, minor + 1, 0}} + end + end + + defp caret_bounds(value) do + with {:ok, {major, minor, patch}} <- parse_semver(value) do + upper = + cond do + major > 0 -> {major + 1, 0, 0} + minor > 0 -> {0, minor + 1, 0} + true -> {0, 0, patch + 1} + end + + {:ok, {major, minor, patch}, upper} + end + end + + defp wildcard_bounds(value) when is_binary(value) do + normalized = + value + |> String.trim() + |> String.replace_prefix("v", "") + |> String.replace_prefix("V", "") + + case String.split(normalized, ".") do + parts when length(parts) in 2..4 -> + {numbers, [wildcard]} = Enum.split(parts, -1) + + with true <- wildcard in ["*", "x", "X"], + {:ok, parsed} <- parse_semver_numbers(numbers) do + wildcard_range(parsed) + else + _ -> :error + end + + _parts -> + :error + end + end + + defp wildcard_bounds(_value), do: :error + + defp wildcard_range([major]), do: {:ok, {major, 0, 0}, {major + 1, 0, 0}} + + defp wildcard_range([major, minor]), + do: {:ok, {major, minor, 0}, {major, minor + 1, 0}} + + defp wildcard_range([major, minor, patch]), + do: {:ok, {major, minor, patch}, {major, minor, patch + 1}} + + defp parse_semver_number(value) do + if Regex.match?(~r/^(0|[1-9]\d*)$/, value), + do: {:ok, String.to_integer(value)}, + else: :error + end + + defp has_key?(map, key), do: Map.has_key?(map, key) or Map.has_key?(map, known_atom(key)) + + defp value(map, key) when is_map(map), do: Map.get(map, key, Map.get(map, known_atom(key))) + defp value(_map, _key), do: nil + + defp known_atom("active"), do: :active + defp known_atom("aggregation_group_type_index"), do: :aggregation_group_type_index + defp known_atom("bucketing_identifier"), do: :bucketing_identifier + defp known_atom("dependency_chain"), do: :dependency_chain + defp known_atom("ensure_experience_continuity"), do: :ensure_experience_continuity + defp known_atom("early_exit"), do: :early_exit + defp known_atom("filters"), do: :filters + defp known_atom("groups"), do: :groups + defp known_atom("has_experiment"), do: :has_experiment + defp known_atom("id"), do: :id + defp known_atom("key"), do: :key + defp known_atom("multivariate"), do: :multivariate + defp known_atom("negation"), do: :negation + defp known_atom("operator"), do: :operator + defp known_atom("payloads"), do: :payloads + defp known_atom("properties"), do: :properties + defp known_atom("rollout_percentage"), do: :rollout_percentage + defp known_atom("type"), do: :type + defp known_atom("value"), do: :value + defp known_atom("values"), do: :values + defp known_atom("variant"), do: :variant + defp known_atom("variants"), do: :variants + defp known_atom("version"), do: :version + + defp map_value(map, key) when is_map(map) do + case map_fetch(map, key) do + {:ok, value} -> value + :error -> nil + end + end + + defp map_value(_map, _key), do: nil + + defp map_fetch(map, key) when is_map(map) do + case Map.fetch(map, key) do + {:ok, value} -> + {:ok, value} + + :error when is_binary(key) -> + Enum.find_value(map, :error, fn {candidate, value} -> + if (is_atom(candidate) or is_integer(candidate)) and to_string(candidate) == key, + do: {:ok, value} + end) + + :error -> + :error + end + end + + defp map_fetch(_map, _key), do: :error +end diff --git a/lib/posthog/supervisor.ex b/lib/posthog/supervisor.ex index ac0e6ec..b1dafe3 100644 --- a/lib/posthog/supervisor.ex +++ b/lib/posthog/supervisor.ex @@ -52,20 +52,36 @@ defmodule PostHog.Supervisor do @impl Supervisor def init({config, callers}) do + public_config = Map.drop(config, [:secret_key, :flag_definition_cache_provider]) + children = [ {Registry, keys: :unique, name: PostHog.Registry.registry_name(config.supervisor_name), - meta: [config: config]}, + meta: [config: public_config]}, {PostHog.FeatureFlags.CalledCache, supervisor_name: config.supervisor_name} - ] ++ senders(config) ++ sources(config) + ] ++ definition_loader(config, callers) ++ senders(config) ++ sources(config) Process.put(:"$callers", callers) Supervisor.init(children, strategy: :one_for_one) end + defp definition_loader( + %{ + enabled: true, + enable_local_evaluation: true, + secret_key: secret_key + } = config, + callers + ) + when is_binary(secret_key) and secret_key != "" do + [{PostHog.FeatureFlags.DefinitionLoader, config: config, callers: callers}] + end + + defp definition_loader(_config, _callers), do: [] + defp sources(%{enabled: false}), do: [] defp sources(config) do diff --git a/mix.exs b/mix.exs index 599aa5a..b256f1a 100644 --- a/mix.exs +++ b/mix.exs @@ -20,7 +20,7 @@ defmodule PostHog.MixProject do def application do [ mod: {PostHog.Application, []}, - extra_applications: [:logger] + extra_applications: [:logger, :crypto] ] end diff --git a/public_api.snapshot b/public_api.snapshot index 5f0b79f..4243197 100644 --- a/public_api.snapshot +++ b/public_api.snapshot @@ -148,6 +148,13 @@ PostHog.FeatureFlags.Evaluations macros: (none) +PostHog.FeatureFlags.FlagDefinitionCacheProvider + module_doc: documented + functions: + (none) + macros: + (none) + PostHog.FeatureFlags.Result module_doc: documented functions: diff --git a/test/posthog/config_test.exs b/test/posthog/config_test.exs index 37402a5..3d72e7e 100644 --- a/test/posthog/config_test.exs +++ b/test/posthog/config_test.exs @@ -25,6 +25,35 @@ defmodule PostHog.ConfigTest do assert config.api_host == "https://eu.i.posthog.com" end + test "validate normalizes local evaluation configuration" do + expect(PostHog.API.Mock, :client, 2, fn _api_key, _api_host -> + %PostHog.API.Client{client: :stub_client, module: PostHog.API.Mock} + end) + + assert {:ok, config} = + PostHog.Config.validate( + api_key: "project_api_key", + secret_key: " secret-key ", + api_client_module: PostHog.API.Mock + ) + + assert config.secret_key == "secret-key" + assert config.enable_local_evaluation + assert config.feature_flags_poll_interval_ms == 30_000 + assert config.flag_definition_cache_provider == nil + assert config.flag_definition_cache_provider_timeout_ms == 5_000 + assert config.flag_definition_request_timeout_ms == 10_000 + + assert {:ok, blank} = + PostHog.Config.validate( + api_key: "project_api_key", + secret_key: " ", + api_client_module: PostHog.API.Mock + ) + + assert blank.secret_key == nil + end + test "validate defaults a missing api_host" do expect(PostHog.API.Mock, :client, fn api_key, api_host -> assert api_key == "project_api_key" diff --git a/test/posthog/feature_flags/definition_loader_test.exs b/test/posthog/feature_flags/definition_loader_test.exs new file mode 100644 index 0000000..938dc68 --- /dev/null +++ b/test/posthog/feature_flags/definition_loader_test.exs @@ -0,0 +1,451 @@ +defmodule PostHog.FeatureFlags.DefinitionLoaderTest do + use ExUnit.Case, async: false + import ExUnit.CaptureLog + import Mox + + alias PostHog.FeatureFlags.DefinitionLoader + + defmodule ShutdownProvider do + @behaviour PostHog.FeatureFlags.FlagDefinitionCacheProvider + + def should_fetch_flag_definitions(_state), do: true + def get_flag_definitions(_state), do: nil + def on_flag_definitions_received(_state, _definitions), do: :ok + + def shutdown(owner) do + send(owner, :provider_shutdown) + :ok + end + end + + defmodule RedactedProvider do + @behaviour PostHog.FeatureFlags.FlagDefinitionCacheProvider + + def should_fetch_flag_definitions(_state), do: false + def get_flag_definitions(state), do: state.cached + def on_flag_definitions_received(_state, _definitions), do: :ok + + def shutdown(state) do + send(state.owner, :redacted_shutdown) + :ok + end + end + + setup :set_mox_from_context + setup :verify_on_exit! + + defp config(name, overrides \\ []) do + [ + api_key: "project-token", + secret_key: "secret-value", + api_client_module: PostHog.API.Mock, + supervisor_name: name, + test_mode: true, + feature_flags_poll_interval_ms: 60_000 + ] + |> Keyword.merge(overrides) + |> PostHog.Config.validate!() + |> Map.put(:sender_pool_size, 1) + end + + defp envelope(key \\ "flag") do + %{ + "flags" => [%{"key" => key, "active" => true, "filters" => %{"groups" => []}}], + "group_type_mapping" => %{"0" => "organization"}, + "cohorts" => %{"1" => %{}} + } + end + + test "wire envelope includes bearer auth, query, and ETag; 304 preserves definitions" do + test = self() + + PostHog.API.Mock + |> stub_with(PostHog.API.Stub) + |> expect(:request, fn :stub_client, :get, "/flags/definitions", opts -> + send(test, {:request, opts}) + {:ok, %{status: 200, headers: %{"etag" => ["v1"]}, body: envelope()}} + end) + |> expect(:request, fn :stub_client, :get, "/flags/definitions", opts -> + send(test, {:request, opts}) + {:ok, %{status: 304, headers: [{"ETag", "v2"}], body: nil}} + end) + |> expect(:request, fn :stub_client, :get, "/flags/definitions", opts -> + send(test, {:request, opts}) + {:ok, %{status: 304, headers: %{}, body: nil}} + end) + + cfg = config(__MODULE__.Wire) + start_supervised!({PostHog.Supervisor, cfg}) + + assert DefinitionLoader.ready?(__MODULE__.Wire) + assert DefinitionLoader.definitions(__MODULE__.Wire).flags_by_key["flag"] + assert_receive {:request, first} + assert first[:params] == %{token: "project-token", send_cohorts: true} + assert {"authorization", "Bearer secret-value"} in first[:headers] + refute Keyword.has_key?(first, :json) + + loader = GenServer.whereis(PostHog.Registry.via(__MODULE__.Wire, DefinitionLoader)) + loaded_at = :sys.get_state(loader).loaded_at + Process.sleep(1) + assert :ok = DefinitionLoader.refresh(__MODULE__.Wire) + assert_receive {:request, second} + assert {"if-none-match", "v1"} in second[:headers] + assert DateTime.compare(:sys.get_state(loader).loaded_at, loaded_at) == :gt + assert DefinitionLoader.definitions(__MODULE__.Wire).flags_by_key["flag"] + + assert :ok = DefinitionLoader.refresh(__MODULE__.Wire) + assert_receive {:request, third} + assert {"if-none-match", "v2"} in third[:headers] + refute Map.has_key?(PostHog.config(__MODULE__.Wire), :secret_key) + end + + test "valid empty definitions are ready and transient or malformed refreshes preserve stale data" do + stub_with(PostHog.API.Mock, PostHog.API.Stub) + + {:ok, counter} = Agent.start_link(fn -> 0 end) + + expect(PostHog.API.Mock, :request, 3, fn :stub_client, :get, "/flags/definitions", _opts -> + case Agent.get_and_update(counter, &{&1, &1 + 1}) do + 0 -> {:ok, %{status: 200, body: envelope("stale"), headers: %{}}} + 1 -> {:ok, %{status: 503, body: %{}, headers: %{}}} + 2 -> {:ok, %{status: 200, body: %{"flags" => []}, headers: %{}}} + end + end) + + start_supervised!({PostHog.Supervisor, config(__MODULE__.Stale)}) + assert DefinitionLoader.definitions(__MODULE__.Stale).flags_by_key["stale"] + + log = capture_log(fn -> DefinitionLoader.refresh(__MODULE__.Stale) end) + assert log =~ "HTTP 503" + assert DefinitionLoader.definitions(__MODULE__.Stale).flags_by_key["stale"] + + log = capture_log(fn -> DefinitionLoader.refresh(__MODULE__.Stale) end) + assert log =~ "malformed" + assert DefinitionLoader.definitions(__MODULE__.Stale).flags_by_key["stale"] + end + + test "auth and quota responses clear definitions while empty complete definitions remain ready" do + stub_with(PostHog.API.Mock, PostHog.API.Stub) + + {:ok, counter} = Agent.start_link(fn -> 0 end) + + expect(PostHog.API.Mock, :request, 3, fn :stub_client, :get, "/flags/definitions", _opts -> + case Agent.get_and_update(counter, &{&1, &1 + 1}) do + 0 -> + {:ok, %{status: 200, body: envelope(), headers: %{}}} + + 1 -> + {:ok, %{status: 401, body: %{}, headers: %{}}} + + 2 -> + {:ok, + %{ + status: 200, + body: %{"flags" => [], "group_type_mapping" => %{}, "cohorts" => %{}}, + headers: %{} + }} + end + end) + + start_supervised!({PostHog.Supervisor, config(__MODULE__.Clear)}) + assert DefinitionLoader.ready?(__MODULE__.Clear) + capture_log(fn -> DefinitionLoader.refresh(__MODULE__.Clear) end) + refute DefinitionLoader.ready?(__MODULE__.Clear) + DefinitionLoader.refresh(__MODULE__.Clear) + assert DefinitionLoader.ready?(__MODULE__.Clear) + assert DefinitionLoader.definitions(__MODULE__.Clear).flags == [] + end + + test "a successful 200 without ETag clears the prior API validator" do + test = self() + + PostHog.API.Mock + |> stub_with(PostHog.API.Stub) + |> expect(:request, fn :stub_client, :get, "/flags/definitions", opts -> + send(test, {:etag_request, opts}) + {:ok, %{status: 200, body: envelope(), headers: %{"etag" => "v1"}}} + end) + |> expect(:request, fn :stub_client, :get, "/flags/definitions", opts -> + send(test, {:etag_request, opts}) + {:ok, %{status: 200, body: envelope(), headers: %{}}} + end) + |> expect(:request, fn :stub_client, :get, "/flags/definitions", opts -> + send(test, {:etag_request, opts}) + {:ok, %{status: 304, body: nil, headers: %{}}} + end) + + start_supervised!({PostHog.Supervisor, config(__MODULE__.EtagClear)}) + assert_receive {:etag_request, first} + refute Enum.any?(first[:headers], &(elem(&1, 0) == "if-none-match")) + + DefinitionLoader.refresh(__MODULE__.EtagClear) + assert_receive {:etag_request, second} + assert {"if-none-match", "v1"} in second[:headers] + + DefinitionLoader.refresh(__MODULE__.EtagClear) + assert_receive {:etag_request, third} + refute Enum.any?(third[:headers], &(elem(&1, 0) == "if-none-match")) + end + + test "transport failures preserve stale definitions and never log bearer secrets" do + stub_with(PostHog.API.Mock, PostHog.API.Stub) + secret = "secret-value" + {:ok, counter} = Agent.start_link(fn -> 0 end) + + expect(PostHog.API.Mock, :request, 2, fn :stub_client, :get, "/flags/definitions", _opts -> + case Agent.get_and_update(counter, &{&1, &1 + 1}) do + 0 -> {:ok, %{status: 200, body: envelope("stale"), headers: %{}}} + 1 -> {:error, %RuntimeError{message: "authorization Bearer #{secret}"}} + end + end) + + start_supervised!({PostHog.Supervisor, config(__MODULE__.Transport)}) + + log = capture_log(fn -> DefinitionLoader.refresh(__MODULE__.Transport) end) + assert log =~ "request failed" + refute log =~ secret + assert DefinitionLoader.definitions(__MODULE__.Transport).flags_by_key["stale"] + end + + test "401, 402, and 403 clear definitions while 429 preserves stale state and backs off" do + stub_with(PostHog.API.Mock, PostHog.API.Stub) + statuses = [401, 402, 403, 429] + responses = Enum.flat_map(statuses, fn status -> [:ok, status] end) + {:ok, queue} = Agent.start_link(fn -> responses end) + owner = self() + + expect(PostHog.API.Mock, :request, 8, fn :stub_client, :get, "/flags/definitions", _opts -> + case Agent.get_and_update(queue, fn [next | rest] -> {next, rest} end) do + :ok -> + {:ok, %{status: 200, body: envelope(), headers: %{}}} + + status -> + send(owner, {:status_request, status}) + {:ok, %{status: status, body: %{}, headers: %{}}} + end + end) + + for status <- statuses do + name = Module.concat(__MODULE__, "Status#{status}") + start_supervised!({PostHog.Supervisor, config(name, feature_flags_poll_interval_ms: 20)}) + assert DefinitionLoader.ready?(name) + capture_log(fn -> DefinitionLoader.refresh(name) end) + assert_receive {:status_request, ^status} + + if status == 429 do + assert DefinitionLoader.ready?(name) + refute_receive {:status_request, 429}, 100 + else + refute DefinitionLoader.ready?(name) + end + + stop_supervised(name) + end + end + + test "named instances keep independent immutable definition state" do + stub_with(PostHog.API.Mock, PostHog.API.Stub) + + expect(PostHog.API.Mock, :request, fn :stub_client, :get, "/flags/definitions", _opts -> + {:ok, %{status: 200, body: envelope("first"), headers: %{}}} + end) + + start_supervised!({PostHog.Supervisor, config(__MODULE__.First)}) + assert DefinitionLoader.definitions(__MODULE__.First).flags_by_key["first"] + + expect(PostHog.API.Mock, :request, fn :stub_client, :get, "/flags/definitions", _opts -> + {:ok, %{status: 200, body: envelope("second"), headers: %{}}} + end) + + start_supervised!({PostHog.Supervisor, config(__MODULE__.Second)}) + assert DefinitionLoader.definitions(__MODULE__.First).flags_by_key["first"] + refute DefinitionLoader.definitions(__MODULE__.First).flags_by_key["second"] + assert DefinitionLoader.definitions(__MODULE__.Second).flags_by_key["second"] + end + + test "registry config redacts privileged key and provider state" do + stub_with(PostHog.API.Mock, PostHog.API.Stub) + sentinel = {:do_not_expose, make_ref()} + + cfg = + config(__MODULE__.Redacted, + enable_local_evaluation: false, + flag_definition_cache_provider: {ShutdownProvider, sentinel} + ) + + start_supervised!({PostHog.Supervisor, cfg}) + public = PostHog.config(__MODULE__.Redacted) + refute Map.has_key?(public, :secret_key) + refute Map.has_key?(public, :flag_definition_cache_provider) + refute inspect(public) =~ inspect(sentinel) + end + + test "concurrent manual refreshes are serialized without overlapping API loads" do + stub_with(PostHog.API.Mock, PostHog.API.Stub) + {:ok, activity} = Agent.start_link(fn -> %{active: 0, max: 0} end) + + expect(PostHog.API.Mock, :request, 6, fn :stub_client, :get, "/flags/definitions", _opts -> + Agent.update(activity, fn state -> + active = state.active + 1 + %{state | active: active, max: max(state.max, active)} + end) + + Process.sleep(5) + Agent.update(activity, &%{&1 | active: &1.active - 1}) + {:ok, %{status: 200, body: envelope(), headers: %{}}} + end) + + start_supervised!({PostHog.Supervisor, config(__MODULE__.Serialized)}) + assert DefinitionLoader.ready?(__MODULE__.Serialized) + + 1..5 + |> Enum.map(fn _index -> + Task.async(fn -> DefinitionLoader.refresh(__MODULE__.Serialized) end) + end) + |> Task.await_many() + + assert Agent.get(activity, & &1.max) == 1 + end + + test "blocked definition requests are canceled before bounded shutdown and provider cleanup" do + owner = self() + stub_with(PostHog.API.Mock, PostHog.API.Stub) + + expect(PostHog.API.Mock, :request, fn :stub_client, :get, "/flags/definitions", _opts -> + send(owner, :definition_request_started) + receive do: (:never -> :ok) + end) + + cfg = + config(__MODULE__.Blocked, + flag_definition_request_timeout_ms: 40, + flag_definition_cache_provider_timeout_ms: 40, + flag_definition_cache_provider: {ShutdownProvider, owner} + ) + + start_supervised!({PostHog.Supervisor, cfg}) + assert_receive :definition_request_started + started = System.monotonic_time(:millisecond) + assert :ok = stop_supervised(__MODULE__.Blocked) + assert System.monotonic_time(:millisecond) - started < 500 + assert_receive :provider_shutdown + end + + test "loader state and child start MFA redact secret and provider sentinels" do + stub_with(PostHog.API.Mock, PostHog.API.Stub) + secret = "secret-sentinel-#{System.unique_integer()}" + provider_sentinel = "provider-sentinel-#{System.unique_integer()}" + + provider = %{ + owner: self(), + sentinel: provider_sentinel, + cached: envelope("redacted") + } + + cfg = + config(__MODULE__.Inspect, + secret_key: secret, + flag_definition_cache_provider: {RedactedProvider, provider} + ) + + spec = DefinitionLoader.child_spec(config: cfg, callers: [self()]) + refute inspect(spec) =~ secret + refute inspect(spec) =~ provider_sentinel + + start_supervised!({PostHog.Supervisor, cfg}) + assert DefinitionLoader.ready?(__MODULE__.Inspect) + loader = GenServer.whereis(PostHog.Registry.via(__MODULE__.Inspect, DefinitionLoader)) + refute inspect(:sys.get_state(loader)) =~ secret + refute inspect(:sys.get_state(loader)) =~ provider_sentinel + stop_supervised(__MODULE__.Inspect) + assert_receive :redacted_shutdown + end + + test "child shutdown budget covers request and three serial provider boundaries" do + stub_with(PostHog.API.Mock, PostHog.API.Stub) + + cfg = + config(__MODULE__.Budget, + flag_definition_request_timeout_ms: 11, + flag_definition_cache_provider_timeout_ms: 13 + ) + + spec = DefinitionLoader.child_spec(config: cfg, callers: []) + assert spec.shutdown == 11 + 3 * 13 + 1_000 + end + + test "429 backoff never schedules sooner than a long configured baseline" do + stub_with(PostHog.API.Mock, PostHog.API.Stub) + {:ok, counter} = Agent.start_link(fn -> 0 end) + + expect(PostHog.API.Mock, :request, 2, fn :stub_client, :get, "/flags/definitions", _opts -> + case Agent.get_and_update(counter, &{&1, &1 + 1}) do + 0 -> {:ok, %{status: 200, body: envelope(), headers: %{}}} + 1 -> {:ok, %{status: 429, body: %{}, headers: %{}}} + end + end) + + baseline = 120_000 + + start_supervised!( + {PostHog.Supervisor, + config(__MODULE__.LongBackoff, feature_flags_poll_interval_ms: baseline)} + ) + + capture_log(fn -> DefinitionLoader.refresh(__MODULE__.LongBackoff) end) + loader = GenServer.whereis(PostHog.Registry.via(__MODULE__.LongBackoff, DefinitionLoader)) + state = :sys.get_state(loader) + assert state.quota_backoff_ms >= baseline + assert Process.read_timer(state.timer_ref) >= baseline - 100 + end + + test "stale timers, late boundary messages, DOWN signals, and stray messages are ignored" do + stub(PostHog.API.Mock, :client, fn _api_key, _host -> + %PostHog.API.Client{client: :stub_client, module: PostHog.API.Mock} + end) + + expect(PostHog.API.Mock, :request, fn :stub_client, :get, "/flags/definitions", _opts -> + {:ok, %{status: 200, body: envelope(), headers: %{}}} + end) + + start_supervised!({PostHog.Supervisor, config(__MODULE__.LateMessages)}) + assert DefinitionLoader.ready?(__MODULE__.LateMessages) + loader = GenServer.whereis(PostHog.Registry.via(__MODULE__.LateMessages, DefinitionLoader)) + state = :sys.get_state(loader) + send(loader, {:refresh, state.timer_generation - 1}) + send(loader, {:boundary_result, make_ref(), self(), {:ok, :late}}) + send(loader, {:DOWN, make_ref(), :process, self(), :normal}) + send(loader, {:unexpected, :message}) + Process.sleep(10) + assert Process.alive?(loader) + assert DefinitionLoader.ready?(__MODULE__.LateMessages) + end + + test "poll timer refreshes and disabled, no-secret, and local-disabled instances start no loader" do + test = self() + stub_with(PostHog.API.Mock, PostHog.API.Stub) + + expect(PostHog.API.Mock, :request, 2, fn :stub_client, :get, "/flags/definitions", _opts -> + send(test, :loaded) + {:ok, %{status: 200, body: envelope(), headers: %{}}} + end) + + start_supervised!( + {PostHog.Supervisor, config(__MODULE__.Timer, feature_flags_poll_interval_ms: 20)} + ) + + assert_receive :loaded + assert_receive :loaded, 200 + + for {name, overrides} <- [ + {__MODULE__.NoSecret, [secret_key: nil]}, + {__MODULE__.LocalDisabled, [enable_local_evaluation: false]}, + {__MODULE__.Disabled, [api_key: ""]} + ] do + cfg = config(name, overrides) + start_supervised!({PostHog.Supervisor, cfg}) + assert Process.whereis(PostHog.Registry.registry_name(name)) + assert GenServer.whereis(PostHog.Registry.via(name, DefinitionLoader)) == nil + end + end +end diff --git a/test/posthog/feature_flags/flag_definition_cache_provider_test.exs b/test/posthog/feature_flags/flag_definition_cache_provider_test.exs new file mode 100644 index 0000000..f37e6cd --- /dev/null +++ b/test/posthog/feature_flags/flag_definition_cache_provider_test.exs @@ -0,0 +1,276 @@ +defmodule PostHog.FeatureFlags.FlagDefinitionCacheProviderTest do + use ExUnit.Case, async: false + import ExUnit.CaptureLog + import Mox + + alias PostHog.FeatureFlags.DefinitionLoader + + defmodule Provider do + @behaviour PostHog.FeatureFlags.FlagDefinitionCacheProvider + + @impl true + def should_fetch_flag_definitions(agent), do: run(agent, :decision) + @impl true + def get_flag_definitions(agent), do: run(agent, :read) + + @impl true + def on_flag_definitions_received(agent, definitions) do + send(Agent.get(agent, & &1.owner), {:stored, definitions}) + run(agent, :store) + end + + @impl true + def shutdown(agent) do + send(Agent.get(agent, & &1.owner), :shutdown) + run(agent, :shutdown) + end + + defp run(agent, key) do + case Agent.get(agent, &Map.fetch!(&1, key)) do + {:sleep, milliseconds, result} -> + Process.sleep(milliseconds) + result + + {:raise, message} -> + raise message + + value -> + value + end + end + end + + setup :set_mox_from_context + setup :verify_on_exit! + + defp envelope(key) do + %{ + "flags" => [%{"key" => key, "active" => true, "filters" => %{"groups" => []}}], + "group_type_mapping" => %{"0" => "organization"}, + "cohorts" => %{}, + "minimal_flag_called_events" => true + } + end + + defp start_instance(name, provider, overrides \\ []) do + cfg = + [ + api_key: "token", + secret_key: "secret", + supervisor_name: name, + api_client_module: PostHog.API.Mock, + test_mode: true, + flag_definition_cache_provider: {Provider, provider}, + feature_flags_poll_interval_ms: 60_000, + flag_definition_cache_provider_timeout_ms: 20 + ] + |> Keyword.merge(overrides) + |> PostHog.Config.validate!() + |> Map.put(:sender_pool_size, 1) + + start_supervised!({PostHog.Supervisor, cfg}) + end + + test "negative decision reads complete cached definitions without an API request" do + stub_with(PostHog.API.Mock, PostHog.API.Stub) + + owner = self() + + {:ok, provider} = + Agent.start_link(fn -> + %{owner: owner, decision: false, read: envelope("cached"), store: :ok, shutdown: :ok} + end) + + start_instance(__MODULE__.Cached, provider) + + definitions = DefinitionLoader.definitions(__MODULE__.Cached) + assert definitions.flags_by_key["cached"] + assert definitions.group_type_mapping == %{"0" => "organization"} + assert definitions.minimal_flag_called_events + assert :ok = stop_supervised(__MODULE__.Cached) + assert_receive :shutdown + end + + test "positive decision fetches, publishes, and stores the complete envelope" do + stub_with(PostHog.API.Mock, PostHog.API.Stub) + + expect(PostHog.API.Mock, :request, fn :stub_client, :get, "/flags/definitions", _opts -> + {:ok, %{status: 200, body: envelope("fresh"), headers: %{}}} + end) + + owner = self() + + {:ok, provider} = + Agent.start_link(fn -> + %{owner: owner, decision: true, read: nil, store: :ok, shutdown: :ok} + end) + + start_instance(__MODULE__.Fresh, provider) + + assert DefinitionLoader.definitions(__MODULE__.Fresh).flags_by_key["fresh"] + assert_receive {:stored, stored} + assert stored["flags"] != [] + assert stored["group_type_mapping"] == %{"0" => "organization"} + assert stored["cohorts"] == %{} + assert stored["minimal_flag_called_events"] + assert :ok = stop_supervised(__MODULE__.Fresh) + assert_receive :shutdown + end + + test "negative decision preserves stale memory on empty, failed, timed out, or malformed reads" do + owner = self() + stub_with(PostHog.API.Mock, PostHog.API.Stub) + + expect(PostHog.API.Mock, :request, fn :stub_client, :get, "/flags/definitions", _opts -> + {:ok, %{status: 200, body: envelope("stale"), headers: %{}}} + end) + + stub(PostHog.API.Mock, :request, fn :stub_client, :get, "/flags/definitions", _opts -> + send(owner, :unexpected_api_fetch) + {:ok, %{status: 200, body: envelope("unexpected"), headers: %{}}} + end) + + {:ok, provider} = + Agent.start_link(fn -> + %{owner: owner, decision: true, read: nil, store: :ok, shutdown: :ok} + end) + + start_instance(__MODULE__.StaleReads, provider) + assert DefinitionLoader.definitions(__MODULE__.StaleReads).flags_by_key["stale"] + assert_receive {:stored, _definitions} + + for read <- [nil, {:raise, "read"}, {:sleep, 100, nil}, %{"flags" => []}] do + Agent.update(provider, &%{&1 | decision: false, read: read}) + capture_log(fn -> DefinitionLoader.refresh(__MODULE__.StaleReads) end) + assert DefinitionLoader.definitions(__MODULE__.StaleReads).flags_by_key["stale"] + refute_receive :unexpected_api_fetch + end + + stop_supervised(__MODULE__.StaleReads) + assert_receive :shutdown + end + + test "empty cache without memory performs an emergency fetch but does not store without ownership" do + owner = self() + stub_with(PostHog.API.Mock, PostHog.API.Stub) + + expect(PostHog.API.Mock, :request, fn :stub_client, :get, "/flags/definitions", _opts -> + {:ok, %{status: 200, body: envelope("emergency"), headers: %{}}} + end) + + {:ok, provider} = + Agent.start_link(fn -> + %{owner: owner, decision: false, read: nil, store: :ok, shutdown: :ok} + end) + + start_instance(__MODULE__.Emergency, provider) + assert DefinitionLoader.definitions(__MODULE__.Emergency).flags_by_key["emergency"] + refute_receive {:stored, _definitions} + stop_supervised(__MODULE__.Emergency) + assert_receive :shutdown + end + + test "provider-installed data clears the API ETag before a later owned fetch" do + owner = self() + stub_with(PostHog.API.Mock, PostHog.API.Stub) + + expect(PostHog.API.Mock, :request, fn :stub_client, :get, "/flags/definitions", opts -> + refute {"if-none-match", "api-v1"} in opts[:headers] + {:ok, %{status: 200, body: envelope("api"), headers: %{"etag" => "api-v1"}}} + end) + + {:ok, provider} = + Agent.start_link(fn -> + %{owner: owner, decision: true, read: envelope("cached"), store: :ok, shutdown: :ok} + end) + + start_instance(__MODULE__.ProviderEtag, provider) + assert_receive {:stored, _definitions} + + Agent.update(provider, &%{&1 | decision: false}) + DefinitionLoader.refresh(__MODULE__.ProviderEtag) + assert DefinitionLoader.definitions(__MODULE__.ProviderEtag).flags_by_key["cached"] + + expect(PostHog.API.Mock, :request, fn :stub_client, :get, "/flags/definitions", opts -> + refute Enum.any?(opts[:headers], &(elem(&1, 0) == "if-none-match")) + {:ok, %{status: 200, body: envelope("api-again"), headers: %{}}} + end) + + Agent.update(provider, &%{&1 | decision: true}) + DefinitionLoader.refresh(__MODULE__.ProviderEtag) + assert DefinitionLoader.definitions(__MODULE__.ProviderEtag).flags_by_key["api-again"] + stop_supervised(__MODULE__.ProviderEtag) + assert_receive :shutdown + end + + test "store timeout leaves fresh memory usable and callbacks remain exactly once" do + owner = self() + stub_with(PostHog.API.Mock, PostHog.API.Stub) + + expect(PostHog.API.Mock, :request, fn :stub_client, :get, "/flags/definitions", _opts -> + {:ok, %{status: 200, body: envelope("fresh-after-timeout"), headers: %{}}} + end) + + {:ok, provider} = + Agent.start_link(fn -> + %{ + owner: owner, + decision: true, + read: nil, + store: {:sleep, 100, :ok}, + shutdown: :ok + } + end) + + log = + capture_log(fn -> + start_instance(__MODULE__.StoreTimeout, provider) + + assert DefinitionLoader.definitions(__MODULE__.StoreTimeout).flags_by_key[ + "fresh-after-timeout" + ] + end) + + assert log =~ "on_flag_definitions_received failed (timeout)" + assert_receive {:stored, _definitions} + refute_receive {:stored, _definitions} + assert :ok = stop_supervised(__MODULE__.StoreTimeout) + assert_receive :shutdown + refute_receive :shutdown + end + + test "decision timeout falls back directly, store failure keeps memory, and shutdown is contained" do + stub_with(PostHog.API.Mock, PostHog.API.Stub) + + expect(PostHog.API.Mock, :request, fn :stub_client, :get, "/flags/definitions", _opts -> + {:ok, %{status: 200, body: envelope("fallback"), headers: %{}}} + end) + + owner = self() + + {:ok, provider} = + Agent.start_link(fn -> + %{ + owner: owner, + decision: {:sleep, 100, false}, + read: nil, + store: {:raise, "store"}, + shutdown: {:raise, "shutdown"} + } + end) + + log = + capture_log(fn -> + start_instance(__MODULE__.Failures, provider) + assert DefinitionLoader.definitions(__MODULE__.Failures).flags_by_key["fallback"] + assert :ok = Supervisor.terminate_child(__MODULE__.Failures, DefinitionLoader) + assert :ok = stop_supervised(__MODULE__.Failures) + end) + + assert log =~ "should_fetch_flag_definitions failed" + assert log =~ "on_flag_definitions_received failed" + assert log =~ "shutdown failed" + assert_receive :shutdown + refute_receive :shutdown + end +end diff --git a/test/posthog/feature_flags/local_evaluation_integration_test.exs b/test/posthog/feature_flags/local_evaluation_integration_test.exs new file mode 100644 index 0000000..3c4845c --- /dev/null +++ b/test/posthog/feature_flags/local_evaluation_integration_test.exs @@ -0,0 +1,293 @@ +defmodule PostHog.FeatureFlags.LocalEvaluationIntegrationTest do + use ExUnit.Case, async: false + import Mox + + alias PostHog.FeatureFlags + alias PostHog.FeatureFlags.Evaluations + + setup :set_mox_from_context + setup :verify_on_exit! + + setup do + stub(PostHog.API.Mock, :client, fn _api_key, _host -> + %PostHog.API.Client{client: :stub_client, module: PostHog.API.Mock} + end) + + :ok + end + + defp flag(key, properties \\ [], extra \\ %{}) do + Map.merge( + %{ + "id" => 11, + "version" => 3, + "key" => key, + "active" => true, + "filters" => %{ + "groups" => [%{"properties" => properties, "rollout_percentage" => 100}] + } + }, + extra + ) + end + + defp envelope(flags) do + %{"flags" => flags, "group_type_mapping" => %{"0" => "organization"}, "cohorts" => %{}} + end + + defp start_instance(name) do + config = + PostHog.Config.validate!( + api_key: "token", + secret_key: "secret", + api_client_module: PostHog.API.Mock, + supervisor_name: name, + test_mode: true, + feature_flags_poll_interval_ms: 60_000 + ) + |> Map.put(:sender_pool_size, 1) + + start_supervised!({PostHog.Supervisor, config}) + end + + test "matching local boolean and variant payload produce a frozen snapshot without /flags" do + variant = + flag("variant", [], %{ + "filters" => %{ + "groups" => [%{"properties" => [], "rollout_percentage" => 100, "variant" => "test"}], + "multivariate" => %{"variants" => [%{"key" => "test", "rollout_percentage" => 100}]}, + "payloads" => %{"test" => ~s({"answer":42})} + } + }) + + expect(PostHog.API.Mock, :request, fn :stub_client, :get, "/flags/definitions", _opts -> + {:ok, %{status: 200, body: envelope([flag("boolean"), variant]), headers: %{}}} + end) + + start_instance(__MODULE__.Local) + assert {:ok, snapshot} = FeatureFlags.evaluate_flags(__MODULE__.Local, "user") + assert PostHog.Test.all_captured(__MODULE__.Local) == [] + assert Evaluations.keys(snapshot) == ["boolean", "variant"] + assert snapshot.flags["boolean"].enabled + assert snapshot.flags["variant"].variant == "test" + assert Evaluations.get_flag_payload(snapshot, "variant") == %{"answer" => 42} + + assert Evaluations.get_flag(snapshot, "boolean") + assert PostHog.get_context(__MODULE__.Local)["$feature/boolean"] == true + assert [event] = PostHog.Test.all_captured(__MODULE__.Local) + assert event.event == "$feature_flag_called" + assert event.properties[:"$feature_flag_id"] == 11 + assert event.properties[:"$feature_flag_version"] == 3 + end + + test "unknown and missing properties fall back once and local results win merge conflicts" do + unknown = flag("unknown", [%{"key" => "x", "operator" => "future", "value" => 1}]) + + expect(PostHog.API.Mock, :request, 2, fn + :stub_client, :get, "/flags/definitions", _opts -> + {:ok, %{status: 200, body: envelope([flag("local"), unknown]), headers: %{}}} + + :stub_client, :post, "/flags", opts -> + send(self(), {:remote_body, opts[:json]}) + + {:ok, + %{ + status: 200, + body: %{ + "flags" => %{ + "local" => %{"enabled" => false}, + "unknown" => %{"enabled" => true}, + "extra" => %{"enabled" => true} + } + } + }} + end) + + start_instance(__MODULE__.Merge) + + assert {:ok, snapshot} = + FeatureFlags.evaluate_flags(__MODULE__.Merge, %{ + distinct_id: "user", + person_properties: %{x: 1} + }) + + assert snapshot.flags["local"].enabled + assert snapshot.flags["unknown"].enabled + assert snapshot.flags["extra"].enabled + end + + test "explicit scope is preserved and local-only returns partial results with no request" do + missing_property = flag("needs-server", [%{"key" => "plan", "value" => "pro"}]) + + expect(PostHog.API.Mock, :request, fn :stub_client, :get, "/flags/definitions", _opts -> + {:ok, %{status: 200, body: envelope([flag("local"), missing_property]), headers: %{}}} + end) + + start_instance(__MODULE__.LocalOnly) + + assert {:ok, snapshot} = + FeatureFlags.evaluate_flags(__MODULE__.LocalOnly, %{ + distinct_id: "user", + flag_keys: ["local", "needs-server", "missing"], + only_evaluate_locally: true + }) + + assert Evaluations.keys(snapshot) == ["local"] + end + + test "remote failure preserves a partial local snapshot" do + unknown = flag("unknown", [%{"key" => "x", "operator" => "future", "value" => 1}]) + + expect(PostHog.API.Mock, :request, 2, fn + :stub_client, :get, "/flags/definitions", _opts -> + {:ok, %{status: 200, body: envelope([flag("local"), unknown]), headers: %{}}} + + :stub_client, :post, "/flags", _opts -> + {:error, %RuntimeError{message: "offline"}} + end) + + start_instance(__MODULE__.Partial) + assert {:ok, snapshot} = FeatureFlags.evaluate_flags(__MODULE__.Partial, "user") + assert Evaluations.keys(snapshot) == ["local"] + end + + test "group absence makes only that flag fall back and requested missing keys are remotely scoped" do + group = put_in(flag("group"), ["filters", "aggregation_group_type_index"], 0) + caller = self() + + expect(PostHog.API.Mock, :request, 2, fn + :stub_client, :get, "/flags/definitions", _opts -> + {:ok, %{status: 200, body: envelope([group]), headers: %{}}} + + :stub_client, :post, "/flags", opts -> + send(caller, {:body, opts[:json]}) + + {:ok, + %{ + status: 200, + body: %{ + "flags" => %{ + "group" => %{"enabled" => false}, + "unrequested" => %{"enabled" => true} + } + } + }} + end) + + start_instance(__MODULE__.Group) + + assert {:ok, snapshot} = + FeatureFlags.evaluate_flags(__MODULE__.Group, %{ + distinct_id: "user", + flag_keys: ["group", "missing"] + }) + + assert_receive {:body, %{flag_keys_to_evaluate: ["group", "missing"]}} + refute snapshot.flags["group"].enabled + refute snapshot.flags["unrequested"] + end + + test "a requested key missing only from local definitions falls back exactly once" do + owner = self() + + expect(PostHog.API.Mock, :request, 2, fn + :stub_client, :get, "/flags/definitions", _opts -> + {:ok, %{status: 200, body: envelope([flag("local")]), headers: %{}}} + + :stub_client, :post, "/flags", opts -> + send(owner, {:missing_fallback, opts[:json]}) + {:ok, %{status: 200, body: %{"flags" => %{"missing" => %{"enabled" => true}}}}} + end) + + start_instance(__MODULE__.Missing) + + assert {:ok, snapshot} = + FeatureFlags.evaluate_flags(__MODULE__.Missing, %{ + distinct_id: "user", + flag_keys: ["missing"] + }) + + assert_receive {:missing_fallback, %{flag_keys_to_evaluate: ["missing"]}} + assert snapshot.flags["missing"].enabled + end + + test "deprecated getters evaluate dependencies locally and preserve local missing/send_event contracts" do + base = flag("base") + + dependency = %{ + "type" => "flag", + "key" => "base", + "operator" => "flag_evaluates_to", + "value" => true, + "dependency_chain" => ["base"] + } + + expect(PostHog.API.Mock, :request, fn :stub_client, :get, "/flags/definitions", _opts -> + {:ok, %{status: 200, body: envelope([base, flag("dependent", [dependency])]), headers: %{}}} + end) + + start_instance(__MODULE__.Dependencies) + assert {:ok, true} = FeatureFlags.check(__MODULE__.Dependencies, "dependent", "user") + + before_context = PostHog.get_context(__MODULE__.Dependencies) + captured_count = length(PostHog.Test.all_captured(__MODULE__.Dependencies)) + + assert {:ok, %{enabled: true}} = + FeatureFlags.get_feature_flag_result( + __MODULE__.Dependencies, + "dependent", + "user", + send_event: false + ) + + assert length(PostHog.Test.all_captured(__MODULE__.Dependencies)) == captured_count + assert PostHog.get_context(__MODULE__.Dependencies) == before_context + + assert {:ok, nil} = + FeatureFlags.get_feature_flag_result( + __MODULE__.Dependencies, + "missing", + %{distinct_id: "user", only_evaluate_locally: true}, + send_event: false + ) + + assert {:error, %PostHog.UnexpectedResponseError{}} = + FeatureFlags.check( + __MODULE__.Dependencies, + "missing", + %{distinct_id: "user", only_evaluate_locally: true} + ) + end + + test "snapshots stay frozen across refresh and named context plus deprecated API use local results" do + {:ok, counter} = Agent.start_link(fn -> 0 end) + + expect(PostHog.API.Mock, :request, 2, fn :stub_client, :get, "/flags/definitions", _opts -> + case Agent.get_and_update(counter, &{&1, &1 + 1}) do + 0 -> + {:ok, %{status: 200, body: envelope([flag("named")]), headers: %{}}} + + 1 -> + {:ok, + %{status: 200, body: envelope([%{flag("named") | "active" => false}]), headers: %{}}} + end + end) + + start_instance(__MODULE__.Named) + PostHog.set_context(__MODULE__.Named, %{distinct_id: "named-user"}) + + assert {:ok, old} = FeatureFlags.evaluate_flags(__MODULE__.Named, nil) + assert old.flags["named"].enabled + assert {:ok, true} = FeatureFlags.check(__MODULE__.Named, "named", nil) + + assert {:ok, %{enabled: true}} = + FeatureFlags.get_feature_flag_result(__MODULE__.Named, "named", nil, + send_event: false + ) + + FeatureFlags.DefinitionLoader.refresh(__MODULE__.Named) + assert old.flags["named"].enabled + assert {:ok, new} = FeatureFlags.evaluate_flags(__MODULE__.Named, nil) + refute new.flags["named"].enabled + end +end diff --git a/test/posthog/feature_flags/local_evaluator_test.exs b/test/posthog/feature_flags/local_evaluator_test.exs new file mode 100644 index 0000000..66528ce --- /dev/null +++ b/test/posthog/feature_flags/local_evaluator_test.exs @@ -0,0 +1,535 @@ +defmodule PostHog.FeatureFlags.LocalEvaluatorTest do + use ExUnit.Case, async: true + + alias PostHog.FeatureFlags.LocalEvaluator + alias PostHog.FeatureFlags.Result + + defp snapshot(flags, extras \\ %{}) do + Map.merge( + %{ + flags: flags, + flags_by_key: Map.new(flags, &{&1["key"], &1}), + group_type_mapping: %{}, + cohorts: %{}, + minimal_flag_called_events: false + }, + extras + ) + end + + defp flag(key, properties \\ [], rollout \\ 100) do + %{ + "id" => 1, + "version" => 2, + "key" => key, + "active" => true, + "filters" => %{ + "groups" => [%{"properties" => properties, "rollout_percentage" => rollout}] + } + } + end + + defp evaluate(flag, context \\ %{}) do + context = Map.merge(%{distinct_id: "user", person_properties: %{}}, context) + LocalEvaluator.evaluate(snapshot([flag]), context, [flag["key"]]) + end + + test "canonical hash vectors and fractional rollout boundaries" do + assert_in_delta LocalEvaluator.hash("flag", "user"), 0.4357368498163313, 1.0e-15 + assert_in_delta LocalEvaluator.hash("flag", "user", "variant"), 0.4727021985667222, 1.0e-15 + + assert %{"zero" => %Result{enabled: false}} = evaluate(flag("zero", [], 0)).results + assert %{"all" => %Result{enabled: true}} = evaluate(flag("all", [], 100)).results + + fractional = flag("fractional", [], 0.1) + + assert evaluate(fractional, %{distinct_id: "user-471"}).results["fractional"].enabled + refute evaluate(fractional, %{distinct_id: "user-0"}).results["fractional"].enabled + end + + test "inactive, experience continuity, and malformed flags are isolated" do + good = flag("good") + inactive = %{flag("inactive") | "active" => false} + continuity = Map.put(flag("continuity"), "ensure_experience_continuity", true) + malformed = %{"key" => "bad", "active" => true} + definitions = snapshot([good, inactive, continuity, malformed]) + + result = LocalEvaluator.evaluate(definitions, %{distinct_id: "user"}) + assert result.results["good"].enabled + refute result.results["inactive"].enabled + assert MapSet.equal?(result.unresolved, MapSet.new(["continuity", "bad"])) + end + + test "multivariate variants, overrides, and payload decoding" do + flag = + flag("variant") + |> put_in(["filters", "multivariate"], %{ + "variants" => [ + %{"key" => "control", "rollout_percentage" => 50}, + %{"key" => "test", "rollout_percentage" => 50} + ] + }) + |> put_in(["filters", "payloads"], %{"control" => ~s({"color":"blue"}), "test" => "false"}) + + result = evaluate(flag).results["variant"] + assert result.variant in ["control", "test"] + assert result.payload in [%{"color" => "blue"}, false] + + override = put_in(flag, ["filters", "groups", Access.at(0), "variant"], "test") + assert %Result{variant: "test", payload: false} = evaluate(override).results["variant"] + + invalid = put_in(flag, ["filters", "groups", Access.at(0), "variant"], "missing") + assert evaluate(invalid).results["variant"].variant == result.variant + + boolean = put_in(flag("boolean-payload"), ["filters", "payloads"], %{"true" => "true"}) + assert %Result{payload: true} = evaluate(boolean).results["boolean-payload"] + end + + test "canonical property operators have positive and negative cases" do + cases = [ + {"exact", "HELLO", "hello", true}, + {"is_not", "hello", "world", true}, + {"is_set", 1, nil, true}, + {"is_not_set", 1, nil, false}, + {"icontains", "Hello World", "WORLD", true}, + {"not_icontains", "Hello", "world", true}, + {"starts_with", 12_345, 123, true}, + {"not_starts_with", "hello", "x", true}, + {"ends_with", "HELLO", "llo", true}, + {"not_ends_with", "hello", "x", true}, + {"regex", "hello123", "[0-9]+$", true}, + {"not_regex", "hello", "[0-9]+$", true}, + {"gt", 3, 2, true}, + {"gte", 3, 3, true}, + {"lt", 2, 3, true}, + {"lte", 3, 3, true}, + {"semver_eq", "1.2.3", "1.2.3", true}, + {"semver_neq", "1.2.4", "1.2.3", true}, + {"semver_gt", "2.0.0", "1.2.3", true}, + {"semver_gte", "1.2.3", "1.2.3", true}, + {"semver_lt", "1.2.2", "1.2.3", true}, + {"semver_lte", "1.2.3", "1.2.3", true}, + {"semver_tilde", "1.2.9", "1.2.3", true}, + {"semver_caret", "1.9.0", "1.2.3", true}, + {"semver_wildcard", "1.2.9", "1.2.*", true} + ] + + for {operator, actual, expected, enabled} <- cases do + property = %{"key" => "prop", "operator" => operator, "value" => expected} + result = evaluate(flag(operator, [property]), %{person_properties: %{prop: actual}}) + assert result.results[operator].enabled == enabled, operator + end + end + + test "canonical property operators return definitive false for non-matches" do + cases = [ + {"exact", "goodbye", "hello"}, + {"is_not", "hello", "hello"}, + {"icontains", "hello", "world"}, + {"not_icontains", "hello world", "world"}, + {"starts_with", "hello", "world"}, + {"not_starts_with", "hello", "hell"}, + {"ends_with", "hello", "world"}, + {"not_ends_with", "hello", "ello"}, + {"regex", "hello", "[0-9]+$"}, + {"not_regex", "hello123", "[0-9]+$"}, + {"gt", 1, 2}, + {"gte", 1, 2}, + {"lt", 2, 1}, + {"lte", 2, 1}, + {"is_date_before", "2025-01-02", "2025-01-01"}, + {"is_date_after", "2025-01-01", "2025-01-02"}, + {"semver_eq", "1.2.4", "1.2.3"}, + {"semver_neq", "1.2.3", "1.2.3"}, + {"semver_gt", "1.2.2", "1.2.3"}, + {"semver_gte", "1.2.2", "1.2.3"}, + {"semver_lt", "1.2.4", "1.2.3"}, + {"semver_lte", "1.2.4", "1.2.3"}, + {"semver_tilde", "1.3.0", "1.2.3"}, + {"semver_caret", "2.0.0", "1.2.3"}, + {"semver_wildcard", "1.3.0", "1.2.*"} + ] + + for {operator, actual, expected} <- cases do + property = %{"key" => "prop", "operator" => operator, "value" => expected} + result = evaluate(flag(operator, [property]), %{person_properties: %{prop: actual}}) + refute result.results[operator].enabled, operator + end + end + + test "semver normalization matches maintained server SDKs" do + parity = [ + {" V1.2.3-beta+build ", "1.2.3", true}, + {"v1.2", "1.2.0", true}, + {"1", "1.0.0", true}, + {"1.2.4-alpha", "1.2.3+build", false} + ] + + for {actual, expected, equal?} <- parity do + property = %{"key" => "version", "operator" => "semver_eq", "value" => expected} + result = evaluate(flag("semver", [property]), %{person_properties: %{version: actual}}) + assert result.results["semver"].enabled == equal? + end + + for invalid <- ["01", "1.02", "1.2.03"] do + property = %{"key" => "version", "operator" => "semver_eq", "value" => "1.2.3"} + result = evaluate(flag("invalid", [property]), %{person_properties: %{version: invalid}}) + assert MapSet.member?(result.unresolved, "invalid") + end + + ignored_extra = %{"key" => "version", "operator" => "semver_eq", "value" => "1.2.3"} + + assert evaluate(flag("extra", [ignored_extra]), %{ + person_properties: %{version: "1.2.3.999.unused"} + }).results["extra"].enabled + + wildcard = %{"key" => "version", "operator" => "semver_wildcard", "value" => " V1.2.* "} + + assert evaluate(flag("wildcard", [wildcard]), %{person_properties: %{version: "1.2.9-rc"}}).results[ + "wildcard" + ].enabled + + malformed = %{wildcard | "value" => "1x.*"} + + assert MapSet.member?( + evaluate(flag("malformed", [malformed]), %{ + person_properties: %{version: "1.2.3"} + }).unresolved, + "malformed" + ) + end + + test "date operators use the injected clock and timezone-less ISO values as UTC" do + now = ~U[2025-01-10 00:00:00Z] + before = %{"key" => "created", "operator" => "is_date_before", "value" => "1d"} + after_property = %{"key" => "created", "operator" => "is_date_after", "value" => "2025-01-01"} + + assert evaluate(flag("before", [before]), %{ + person_properties: %{created: "2025-01-01"}, + now: now + }).results["before"].enabled + + assert evaluate(flag("after", [after_property]), %{ + person_properties: %{created: "2025-01-02"}, + now: now + }).results["after"].enabled + + for datetime <- ["2025-01-02T03:04:05", "2025-01-02 03:04:05"] do + property = %{ + "key" => "created", + "operator" => "is_date_after", + "value" => "2025-01-02T03:04:04Z" + } + + assert evaluate(flag("timezone-less", [property]), %{ + person_properties: %{created: datetime}, + now: now + }).results["timezone-less"].enabled + end + + for property <- [ + %{"key" => "x", "operator" => "unknown", "value" => 1}, + %{"key" => "x", "operator" => "regex", "value" => "["}, + %{"key" => "x", "operator" => "is_date_before", "value" => "never"}, + %{"key" => "x", "operator" => "semver_eq", "value" => "01.2.3"} + ] do + result = evaluate(flag("bad", [property]), %{person_properties: %{x: "bad"}}) + assert MapSet.member?(result.unresolved, "bad") + end + end + + test "explicit nil values only allow maintained is_not semantics" do + no_match_operators = [ + "exact", + "icontains", + "not_icontains", + "starts_with", + "not_starts_with", + "ends_with", + "not_ends_with", + "regex", + "not_regex", + "gt", + "gte", + "lt", + "lte", + "is_date_before", + "is_date_after", + "semver_eq", + "semver_neq", + "semver_gt", + "semver_gte", + "semver_lt", + "semver_lte", + "semver_tilde", + "semver_caret", + "semver_wildcard" + ] + + for operator <- no_match_operators do + property = %{"key" => "prop", "operator" => operator, "value" => "value"} + result = evaluate(flag(operator, [property]), %{person_properties: %{prop: nil}}) + assert %Result{enabled: false} = result.results[operator], operator + end + + is_not = %{"key" => "prop", "operator" => "is_not", "value" => "value"} + + assert evaluate(flag("is-not", [is_not]), %{person_properties: %{prop: nil}}).results[ + "is-not" + ].enabled + + absent = %{"key" => "prop", "operator" => "is_not_set", "value" => nil} + assert MapSet.member?(evaluate(flag("absent", [absent])).unresolved, "absent") + + refute evaluate(flag("present", [absent]), %{person_properties: %{prop: nil}}).results[ + "present" + ].enabled + end + + test "early exit preserves prior inconclusive state and later conditions can recover" do + missing = %{"properties" => [%{"key" => "missing", "value" => true}]} + out_of_rollout = %{"properties" => [], "rollout_percentage" => 0} + + early = + flag("early") + |> put_in(["filters", "early_exit"], true) + |> put_in(["filters", "groups"], [missing, out_of_rollout]) + + assert MapSet.member?(evaluate(early).unresolved, "early") + + recovered = + put_in(early, ["filters", "groups"], [ + missing, + %{"properties" => [], "rollout_percentage" => 100} + ]) + + assert evaluate(recovered).results["early"].enabled + end + + test "missing properties are inconclusive without contaminating another flag" do + definitions = + snapshot([ + flag("missing", [%{"key" => "plan", "operator" => "exact", "value" => "pro"}]), + flag("good") + ]) + + result = LocalEvaluator.evaluate(definitions, %{distinct_id: "user", person_properties: %{}}) + + assert result.results["good"].enabled + assert MapSet.member?(result.unresolved, "missing") + end + + test "group and mixed targeting use group properties and group keys" do + group = put_in(flag("group"), ["filters", "aggregation_group_type_index"], 0) + + group = + put_in(group, ["filters", "groups", Access.at(0), "properties"], [ + %{"key" => "tier", "value" => "pro", "operator" => "exact"} + ]) + + definitions = snapshot([group], %{group_type_mapping: %{"0" => "organization"}}) + + context = %{ + distinct_id: "person", + groups: %{organization: "org-1"}, + group_properties: %{organization: %{tier: "pro"}} + } + + assert LocalEvaluator.evaluate(definitions, context).results["group"].enabled + + missing_group_properties = %{distinct_id: "person", groups: %{organization: "org-1"}} + + assert MapSet.member?( + LocalEvaluator.evaluate(definitions, missing_group_properties).unresolved, + "group" + ) + + bucketed_group = put_in(group, ["filters", "groups", Access.at(0), "rollout_percentage"], 50) + + bucketed_definitions = + snapshot([bucketed_group], %{group_type_mapping: %{"0" => "organization"}}) + + inside_group = put_in(context, [:groups, :organization], "id-3") + outside_group = put_in(context, [:groups, :organization], "id-0") + assert LocalEvaluator.evaluate(bucketed_definitions, inside_group).results["group"].enabled + refute LocalEvaluator.evaluate(bucketed_definitions, outside_group).results["group"].enabled + + assert MapSet.member?( + LocalEvaluator.evaluate(definitions, %{distinct_id: "person"}).unresolved, + "group" + ) + + mixed = flag("mixed") + + condition = %{ + "aggregation_group_type_index" => 0, + "properties" => [%{"key" => "tier", "value" => "pro"}], + "rollout_percentage" => 100 + } + + mixed = put_in(mixed, ["filters", "groups"], [condition]) + definitions = snapshot([mixed], %{group_type_mapping: %{"0" => "organization"}}) + assert LocalEvaluator.evaluate(definitions, context).results["mixed"].enabled + end + + test "device bucketing requires a device id and uses it for rollout" do + device_flag = Map.put(flag("device", [], 50), "bucketing_identifier", "device_id") + assert MapSet.member?(evaluate(device_flag).unresolved, "device") + assert evaluate(device_flag, %{device_id: "id-0"}).results["device"].enabled + refute evaluate(device_flag, %{device_id: "id-2"}).results["device"].enabled + end + + test "nested cohorts and negation, missing cohorts, dependencies, and cycles" do + cohort_property = %{"type" => "cohort", "value" => 1, "operator" => "exact"} + cohort_flag = flag("cohort", [cohort_property]) + + cohort = %{ + "type" => "OR", + "values" => [ + %{"key" => "region", "value" => "UK"}, + %{"key" => "plan", "value" => "pro", "negation" => true} + ] + } + + definitions = snapshot([cohort_flag], %{cohorts: %{"1" => cohort}}) + + assert LocalEvaluator.evaluate(definitions, %{ + distinct_id: "u", + person_properties: %{region: "UK"} + }).results["cohort"].enabled + + missing = + LocalEvaluator.evaluate(snapshot([cohort_flag]), %{ + distinct_id: "u", + person_properties: %{region: "UK"} + }) + + assert MapSet.member?(missing.unresolved, "cohort") + + base = flag("base") + + dependency = %{ + "type" => "flag", + "key" => "base", + "operator" => "flag_evaluates_to", + "value" => true, + "dependency_chain" => ["base"] + } + + dependent = flag("dependent", [dependency]) + result = LocalEvaluator.evaluate(snapshot([base, dependent]), %{distinct_id: "u"}) + assert result.results["dependent"].enabled + + a_dep = %{dependency | "key" => "b", "dependency_chain" => ["b"]} + b_dep = %{dependency | "key" => "a", "dependency_chain" => ["a"]} + cycle = snapshot([flag("a", [a_dep]), flag("b", [b_dep])]) + result = LocalEvaluator.evaluate(cycle, %{distinct_id: "u"}) + assert MapSet.equal?(result.unresolved, MapSet.new(["a", "b"])) + end + + test "cohort requires-server precedence, nested empty groups, and nested negation" do + cohort_property = %{"type" => "cohort", "value" => "outer"} + cohort_flag = flag("cohort", [cohort_property]) + + missing_static = %{"type" => "cohort", "value" => "static"} + matching = %{"key" => "region", "value" => "UK"} + + for type <- ["AND", "OR"] do + cohort = %{"type" => type, "values" => [matching, missing_static]} + definitions = snapshot([cohort_flag], %{cohorts: %{"outer" => cohort}}) + + result = + LocalEvaluator.evaluate(definitions, %{ + distinct_id: "u", + person_properties: %{region: "UK"} + }) + + assert MapSet.member?(result.unresolved, "cohort") + end + + nested = %{ + "type" => "AND", + "values" => [ + %{}, + %{ + "type" => "OR", + "values" => [%{"key" => "region", "value" => "US"}], + "negation" => true + } + ] + } + + definitions = snapshot([cohort_flag], %{cohorts: %{"outer" => nested}}) + + assert LocalEvaluator.evaluate(definitions, %{ + distinct_id: "u", + person_properties: %{region: "UK"} + }).results["cohort"].enabled + + malformed = snapshot([cohort_flag], %{cohorts: %{"outer" => %{"values" => []}}}) + + assert MapSet.member?( + LocalEvaluator.evaluate(malformed, %{distinct_id: "u"}).unresolved, + "cohort" + ) + end + + test "dependency chains validate every member and compare false and variant results" do + false_flag = %{flag("false-flag") | "active" => false} + + variant = + flag("variant") + |> put_in(["filters", "multivariate"], %{ + "variants" => [%{"key" => "control", "rollout_percentage" => 100}] + }) + + false_dependency = %{ + "type" => "flag", + "key" => "false-flag", + "operator" => "flag_evaluates_to", + "value" => false, + "dependency_chain" => ["false-flag"] + } + + variant_dependency = %{ + "type" => "flag", + "key" => "variant", + "operator" => "flag_evaluates_to", + "value" => "control", + "dependency_chain" => ["variant"] + } + + repeated = flag("repeated", [variant_dependency, variant_dependency]) + + definitions = + snapshot([false_flag, variant, flag("false-parent", [false_dependency]), repeated]) + + result = LocalEvaluator.evaluate(definitions, %{distinct_id: "u"}) + assert result.results["false-parent"].enabled + assert result.results["repeated"].enabled + + missing_member = %{variant_dependency | "dependency_chain" => ["missing", "variant"]} + definitions = snapshot([variant, flag("missing-parent", [missing_member])]) + result = LocalEvaluator.evaluate(definitions, %{distinct_id: "u"}) + assert MapSet.member?(result.unresolved, "missing-parent") + end + + test "fixed context and clock are deterministic and unknown operators stay flag scoped" do + unknown = flag("unknown", [%{"key" => "x", "operator" => "future", "value" => 1}]) + dated = flag("dated", [%{"key" => "date", "operator" => "is_date_after", "value" => "1d"}]) + definitions = snapshot([unknown, dated, flag("good")]) + + context = %{ + distinct_id: "u", + person_properties: %{x: 1, date: "2025-01-11"}, + now: ~U[2025-01-11 12:00:00Z] + } + + first = LocalEvaluator.evaluate(definitions, context) + second = LocalEvaluator.evaluate(definitions, context) + assert first.results == second.results + assert first.results["good"].enabled + assert first.results["dated"].enabled + assert MapSet.equal?(first.unresolved, MapSet.new(["unknown"])) + end +end diff --git a/test/posthog/feature_flags/missing_key_knowledge_test.exs b/test/posthog/feature_flags/missing_key_knowledge_test.exs new file mode 100644 index 0000000..15a31b9 --- /dev/null +++ b/test/posthog/feature_flags/missing_key_knowledge_test.exs @@ -0,0 +1,329 @@ +defmodule PostHog.FeatureFlags.MissingKeyKnowledgeTest do + use ExUnit.Case, async: false + import ExUnit.CaptureLog + import Mox + + alias PostHog.FeatureFlags + alias PostHog.FeatureFlags.DefinitionLoader + alias PostHog.FeatureFlags.Evaluations + + setup :set_mox_from_context + setup :verify_on_exit! + + setup do + stub(PostHog.API.Mock, :client, fn _api_key, _host -> + %PostHog.API.Client{client: :stub_client, module: PostHog.API.Mock} + end) + + :ok + end + + defp envelope(flags \\ []) do + %{"flags" => flags, "group_type_mapping" => %{}, "cohorts" => %{}} + end + + defp start_instance(name, overrides \\ []) do + config = + [ + api_key: "token", + secret_key: "secret", + api_client_module: PostHog.API.Mock, + supervisor_name: name, + test_mode: true, + feature_flags_poll_interval_ms: 60_000 + ] + |> Keyword.merge(overrides) + |> PostHog.Config.validate!() + |> Map.put(:sender_pool_size, 1) + + start_supervised!({PostHog.Supervisor, config}) + end + + defp scoped(name, keys, distinct_id \\ "user") do + FeatureFlags.evaluate_flags(name, %{distinct_id: distinct_id, flag_keys: keys}) + end + + test "clean omission is retained sequentially and successful refresh invalidates it" do + owner = self() + {:ok, count} = Agent.start_link(fn -> 0 end) + + expect(PostHog.API.Mock, :request, 5, fn :stub_client, method, path, _opts -> + index = Agent.get_and_update(count, &{&1, &1 + 1}) + + case {index, method, path} do + {0, :get, "/flags/definitions"} -> + {:ok, %{status: 200, body: envelope(), headers: %{}}} + + {1, :post, "/flags"} -> + send(owner, :remote_probe) + {:ok, %{status: 200, body: %{"flags" => %{}}}} + + {2, :get, "/flags/definitions"} -> + {:ok, %{status: 503, body: %{}, headers: %{}}} + + {3, :get, "/flags/definitions"} -> + {:ok, %{status: 304, body: nil, headers: %{}}} + + {4, :post, "/flags"} -> + send(owner, :remote_probe) + {:ok, %{status: 200, body: %{"flags" => %{}}}} + end + end) + + start_instance(__MODULE__.Sequential) + assert {:ok, first} = scoped(__MODULE__.Sequential, ["missing"]) + assert Evaluations.keys(first) == [] + assert_receive :remote_probe + + assert {:ok, second} = scoped(__MODULE__.Sequential, ["missing"]) + assert Evaluations.keys(second) == [] + refute_receive :remote_probe + + capture_log(fn -> DefinitionLoader.refresh(__MODULE__.Sequential) end) + assert {:ok, still_retained} = scoped(__MODULE__.Sequential, ["missing"]) + assert Evaluations.keys(still_retained) == [] + refute_receive :remote_probe + + DefinitionLoader.refresh(__MODULE__.Sequential) + assert {:ok, _third} = scoped(__MODULE__.Sequential, ["missing"]) + assert_receive :remote_probe + end + + test "remote-only and locally unresolved total failures return safe empty snapshots without accessor retries" do + expect(PostHog.API.Mock, :request, fn :stub_client, :post, "/flags", _opts -> + {:error, %RuntimeError{message: "offline"}} + end) + + start_instance(__MODULE__.RemoteOnly, secret_key: nil) + assert {:ok, remote_only} = scoped(__MODULE__.RemoteOnly, ["missing"]) + refute Evaluations.enabled?(remote_only, "missing") + assert Evaluations.get_flag(remote_only, "missing") == nil + + {:ok, counter} = Agent.start_link(fn -> 0 end) + + expect(PostHog.API.Mock, :request, 3, fn :stub_client, method, path, _opts -> + case {Agent.get_and_update(counter, &{&1, &1 + 1}), method, path} do + {0, :get, "/flags/definitions"} -> {:ok, %{status: 200, body: envelope(), headers: %{}}} + {_index, :post, "/flags"} -> {:error, %RuntimeError{message: "offline"}} + end + end) + + start_instance(__MODULE__.Failure) + assert {:ok, failed} = scoped(__MODULE__.Failure, ["missing"]) + assert Evaluations.keys(failed) == [] + refute Evaluations.enabled?(failed, "missing") + assert {:ok, _retry} = scoped(__MODULE__.Failure, ["missing"]) + end + + test "dirty omissions do not suppress retries and returned keys remain context specific" do + {:ok, counter} = Agent.start_link(fn -> 0 end) + + expect(PostHog.API.Mock, :request, 5, fn :stub_client, method, path, _opts -> + case {Agent.get_and_update(counter, &{&1, &1 + 1}), method, path} do + {0, :get, "/flags/definitions"} -> + {:ok, %{status: 200, body: envelope(), headers: %{}}} + + {1, :post, "/flags"} -> + {:ok, %{status: 200, body: %{"flags" => %{}, "errorsWhileComputingFlags" => true}}} + + {2, :post, "/flags"} -> + {:ok, %{status: 200, body: %{"flags" => %{}, "quotaLimited" => ["feature_flags"]}}} + + {index, :post, "/flags"} when index in [3, 4] -> + {:ok, %{status: 200, body: %{"flags" => %{"missing" => %{"enabled" => true}}}}} + end + end) + + start_instance(__MODULE__.Dirty) + assert {:ok, _dirty} = scoped(__MODULE__.Dirty, ["missing"]) + assert {:ok, _quota} = scoped(__MODULE__.Dirty, ["missing"]) + assert {:ok, returned} = scoped(__MODULE__.Dirty, ["missing"], "one") + assert returned.flags["missing"].enabled + assert {:ok, returned_again} = scoped(__MODULE__.Dirty, ["missing"], "two") + assert returned_again.flags["missing"].enabled + end + + test "overlapping clean probes coalesce while a returned value makes the waiter probe itself" do + owner = self() + {:ok, calls} = Agent.start_link(fn -> 0 end) + + expect(PostHog.API.Mock, :request, 2, fn :stub_client, method, path, _opts -> + case {method, path} do + {:get, "/flags/definitions"} -> + {:ok, %{status: 200, body: envelope(), headers: %{}}} + + {:post, "/flags"} -> + call = Agent.get_and_update(calls, &{&1, &1 + 1}) + send(owner, {:overlap_probe, call, self()}) + receive do: (:release_overlap -> :ok) + {:ok, %{status: 200, body: %{"flags" => %{}}}} + end + end) + + start_instance(__MODULE__.Overlap) + first = Task.async(fn -> scoped(__MODULE__.Overlap, ["missing"], "one") end) + assert_receive {:overlap_probe, 0, worker} + second = Task.async(fn -> scoped(__MODULE__.Overlap, ["missing"], "two") end) + refute_receive {:overlap_probe, 1, _worker}, 50 + send(worker, :release_overlap) + assert {:ok, _snapshot} = Task.await(first) + assert {:ok, _snapshot} = Task.await(second) + assert Agent.get(calls, & &1) == 1 + + # A refreshed generation starts a new probe. Returning a key does not share + # its identity-specific value with the overlapping waiter. + expect(PostHog.API.Mock, :request, fn :stub_client, :get, "/flags/definitions", _opts -> + {:ok, %{status: 304, body: nil, headers: %{}}} + end) + + DefinitionLoader.refresh(__MODULE__.Overlap) + + expect(PostHog.API.Mock, :request, 2, fn :stub_client, :post, "/flags", _opts -> + call = Agent.get_and_update(calls, &{&1, &1 + 1}) + send(owner, {:returned_probe, call, self()}) + if call == 1, do: receive(do: (:release_returned -> :ok)) + {:ok, %{status: 200, body: %{"flags" => %{"missing" => %{"enabled" => true}}}}} + end) + + returned_first = Task.async(fn -> scoped(__MODULE__.Overlap, ["missing"], "three") end) + assert_receive {:returned_probe, 1, returned_worker} + returned_waiter = Task.async(fn -> scoped(__MODULE__.Overlap, ["missing"], "four") end) + refute_receive {:returned_probe, 2, _worker}, 50 + send(returned_worker, :release_returned) + assert_receive {:returned_probe, 2, _worker} + assert {:ok, _snapshot} = Task.await(returned_first) + assert {:ok, _snapshot} = Task.await(returned_waiter) + end + + test "disjoint probes proceed concurrently and mixed overlapping sets recheck before one original-scope request" do + owner = self() + + expect(PostHog.API.Mock, :request, fn :stub_client, :get, "/flags/definitions", _opts -> + {:ok, %{status: 200, body: envelope(), headers: %{}}} + end) + + expect(PostHog.API.Mock, :request, 2, fn :stub_client, :post, "/flags", opts -> + [key] = opts[:json].flag_keys_to_evaluate + send(owner, {:disjoint_started, key, self()}) + receive do: (:release_disjoint -> :ok) + {:ok, %{status: 200, body: %{"flags" => %{}}}} + end) + + start_instance(__MODULE__.Sets) + a = Task.async(fn -> scoped(__MODULE__.Sets, ["a"]) end) + b = Task.async(fn -> scoped(__MODULE__.Sets, ["b"]) end) + assert_receive {:disjoint_started, "a", worker_a} + assert_receive {:disjoint_started, "b", worker_b} + send(worker_a, :release_disjoint) + send(worker_b, :release_disjoint) + Task.await(a) + Task.await(b) + + expect(PostHog.API.Mock, :request, fn :stub_client, :get, "/flags/definitions", _opts -> + {:ok, %{status: 304, body: nil, headers: %{}}} + end) + + DefinitionLoader.refresh(__MODULE__.Sets) + + expect(PostHog.API.Mock, :request, 2, fn :stub_client, :post, "/flags", opts -> + keys = opts[:json].flag_keys_to_evaluate + send(owner, {:mixed_probe, keys, self()}) + if keys == ["a"], do: receive(do: (:release_mixed -> :ok)) + {:ok, %{status: 200, body: %{"flags" => %{}}}} + end) + + overlap = Task.async(fn -> scoped(__MODULE__.Sets, ["a"]) end) + assert_receive {:mixed_probe, ["a"], overlap_worker} + mixed = Task.async(fn -> scoped(__MODULE__.Sets, ["a", "b"]) end) + refute_receive {:mixed_probe, ["a", "b"], _worker}, 50 + send(overlap_worker, :release_mixed) + assert_receive {:mixed_probe, ["a", "b"], _worker} + Task.await(overlap) + Task.await(mixed) + end + + test "loader restarts use a new generation incarnation" do + expect(PostHog.API.Mock, :request, 2, fn :stub_client, :get, "/flags/definitions", _opts -> + {:ok, %{status: 200, body: envelope(), headers: %{}}} + end) + + start_instance(__MODULE__.Restart) + old_generation = DefinitionLoader.evaluation_state(__MODULE__.Restart, []).generation + assert :ok = stop_supervised(__MODULE__.Restart) + + start_instance(__MODULE__.Restart) + new_generation = DefinitionLoader.evaluation_state(__MODULE__.Restart, []).generation + + refute new_generation == old_generation + + assert :stale_generation = + DefinitionLoader.update_negative_knowledge( + __MODULE__.Restart, + old_generation, + ["missing"], + [], + true + ) + + assert MapSet.new() == + DefinitionLoader.evaluation_state(__MODULE__.Restart, ["missing"]).known_missing + end + + test "negative knowledge is bounded, returned keys are removed, and stale generations are discarded" do + expect(PostHog.API.Mock, :request, fn :stub_client, :get, "/flags/definitions", _opts -> + {:ok, %{status: 200, body: envelope(), headers: %{}}} + end) + + start_instance(__MODULE__.Capacity) + initial = DefinitionLoader.evaluation_state(__MODULE__.Capacity, []) + keys = for index <- 0..1_000, do: "key-#{String.pad_leading(to_string(index), 4, "0")}" + + assert :ok = + DefinitionLoader.update_negative_knowledge( + __MODULE__.Capacity, + initial.generation, + keys, + [], + true + ) + + retained = DefinitionLoader.evaluation_state(__MODULE__.Capacity, keys).known_missing + assert MapSet.size(retained) == 1_000 + refute MapSet.member?(retained, hd(keys)) + assert MapSet.member?(retained, List.last(keys)) + + returned = List.last(keys) + + assert :ok = + DefinitionLoader.update_negative_knowledge( + __MODULE__.Capacity, + initial.generation, + [], + [returned], + false + ) + + refute MapSet.member?( + DefinitionLoader.evaluation_state(__MODULE__.Capacity, [returned]).known_missing, + returned + ) + + expect(PostHog.API.Mock, :request, fn :stub_client, :get, "/flags/definitions", _opts -> + {:ok, %{status: 304, body: nil, headers: %{}}} + end) + + DefinitionLoader.refresh(__MODULE__.Capacity) + + assert :stale_generation = + DefinitionLoader.update_negative_knowledge( + __MODULE__.Capacity, + initial.generation, + ["race"], + [], + true + ) + + assert MapSet.size(DefinitionLoader.evaluation_state(__MODULE__.Capacity, keys).known_missing) == + 0 + end +end diff --git a/test/support/api/stub.ex b/test/support/api/stub.ex index 7b19120..5935d01 100644 --- a/test/support/api/stub.ex +++ b/test/support/api/stub.ex @@ -8,6 +8,15 @@ defmodule PostHog.API.Stub do end @impl PostHog.API.Client + def request(_client, :get, "/flags/definitions", _opts) do + {:ok, + %{ + status: 200, + headers: %{}, + body: %{"flags" => [], "group_type_mapping" => %{}, "cohorts" => %{}} + }} + end + def request(_client, :post, "/batch", _opts) do {:ok, %{status: 200, body: %{"status" => "Ok"}}} end From 3cbfb2881d4484a45c1ac4a619feefb2f6681a3b Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Tue, 25 Aug 2026 18:33:26 +0200 Subject: [PATCH 2/9] chore: remove unreleased changelog entry --- CHANGELOG.md | 6 ------ 1 file changed, 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9246424..2706e01 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,11 +1,5 @@ # posthog -## Unreleased - -### Minor changes - -- Add privileged, polled local feature flag evaluation with safe remote fallback, ETag definition loading, person/group/cohort/dependency matching, deterministic variants and payloads, and an optional bounded shared definition cache provider. Configure it with `secret_key`; existing installations without a secret remain remote-only. - ## 2.14.2 — 2026-08-25 ### Patch changes From 31377582dd8fffb22e214db5abc25263b4a37afe Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Tue, 25 Aug 2026 18:46:45 +0200 Subject: [PATCH 3/9] fix(flags): address local evaluation review feedback --- lib/posthog/feature_flags.ex | 24 +++- .../feature_flags/definition_loader.ex | 104 ++++++++++++++---- lib/posthog/feature_flags/local_evaluator.ex | 37 ++++++- .../feature_flags/definition_loader_test.exs | 33 ++++++ .../local_evaluation_integration_test.exs | 33 ++++++ .../feature_flags/local_evaluator_test.exs | 42 ++++++- .../missing_key_knowledge_test.exs | 14 +-- 7 files changed, 247 insertions(+), 40 deletions(-) diff --git a/lib/posthog/feature_flags.ex b/lib/posthog/feature_flags.ex index e4e9ff3..b282e0f 100644 --- a/lib/posthog/feature_flags.ex +++ b/lib/posthog/feature_flags.ex @@ -109,8 +109,9 @@ defmodule PostHog.FeatureFlags do are evaluated locally first when privileged local evaluation is configured; unresolved flags are filled by at most one `/flags` call. If that fallback fails after some flags resolved locally, the successful local subset is - returned as a partial snapshot; if nothing resolved, a safe empty snapshot - is returned. The snapshot lets you branch on multiple flags and enrich + returned as a partial snapshot. If nothing resolved, the remote error is + returned to preserve the existing `{:error, reason}` contract. The snapshot + lets you branch on multiple flags and enrich captured events — see `PostHog.FeatureFlags.Evaluations` for the full snapshot API and `set_in_context/2` for the recommended capture-enrichment flow. @@ -343,11 +344,26 @@ defmodule PostHog.FeatureFlags do merged = Map.merge(remote_results, local_results) {:ok, __MODULE__.Evaluations.from_results(name, distinct_id, merged, response_body)} - {:error, _reason} -> - snapshot_from_results(name, distinct_id, local_results) + {:ok, _malformed_response} -> + remote_failure_result( + name, + distinct_id, + local_results, + {:error, %RuntimeError{message: "invalid response from PostHog /flags endpoint"}} + ) + + {:error, _reason} = error -> + remote_failure_result(name, distinct_id, local_results, error) end end + defp remote_failure_result(_name, _distinct_id, local_results, error) + when map_size(local_results) == 0, + do: error + + defp remote_failure_result(name, distinct_id, local_results, _error), + do: snapshot_from_results(name, distinct_id, local_results) + defp snapshot_from_results(name, distinct_id, results), do: {:ok, __MODULE__.Evaluations.from_results(name, distinct_id, results, %{})} diff --git a/lib/posthog/feature_flags/definition_loader.ex b/lib/posthog/feature_flags/definition_loader.ex index b745f68..375ec4e 100644 --- a/lib/posthog/feature_flags/definition_loader.ex +++ b/lib/posthog/feature_flags/definition_loader.ex @@ -54,17 +54,24 @@ defmodule PostHog.FeatureFlags.DefinitionLoader do end @spec definitions(PostHog.supervisor_name()) :: snapshot() | nil - def definitions(name \\ PostHog), do: GenServer.call(via(name), :definitions) + def definitions(name \\ PostHog), do: readable_evaluation_state(name).definitions @spec ready?(PostHog.supervisor_name()) :: boolean() - def ready?(name \\ PostHog), do: GenServer.call(via(name), :ready?) + def ready?(name \\ PostHog), do: not is_nil(definitions(name)) @spec refresh(PostHog.supervisor_name()) :: :ok def refresh(name \\ PostHog), do: GenServer.call(via(name), :refresh, :infinity) @doc false - def evaluation_state(name, keys), - do: GenServer.call(via(name), {:evaluation_state, keys}) + def evaluation_state(name, keys) do + state = readable_evaluation_state(name) + + %{ + definitions: state.definitions, + generation: state.generation, + known_missing: keys |> MapSet.new() |> MapSet.intersection(state.negative_keys) + } + end @doc false def update_negative_knowledge(name, generation, requested_keys, returned_keys, clean?) do @@ -74,6 +81,43 @@ defmodule PostHog.FeatureFlags.DefinitionLoader do ) end + defp readable_evaluation_state(name) do + state = cached_evaluation_state(name) + + if state.initial_load_complete? do + state + else + try do + GenServer.call(via(name), :cached_evaluation_state, :infinity) + catch + :exit, _reason -> state + end + end + end + + defp cached_evaluation_state(name) do + registry = PostHog.Registry.registry_name(name) + + case Registry.lookup(registry, __MODULE__) do + [{_pid, %{definitions: _definitions} = state}] -> + state + + _other -> + empty_evaluation_state() + end + rescue + ArgumentError -> empty_evaluation_state() + end + + defp empty_evaluation_state do + %{ + definitions: nil, + generation: nil, + negative_keys: MapSet.new(), + initial_load_complete?: true + } + end + defp via(name), do: PostHog.Registry.via(name, __MODULE__) @impl GenServer @@ -91,41 +135,35 @@ defmodule PostHog.FeatureFlags.DefinitionLoader do timer_ref: nil, timer_generation: 0, quota_backoff_ms: nil, + initial_load_complete?: false, config: config } - {:ok, state, {:continue, :initial_load}} + {:ok, publish_evaluation_state(state), {:continue, :initial_load}} end @impl GenServer - def handle_continue(:initial_load, state), do: {:noreply, refresh_and_schedule(state)} + def handle_continue(:initial_load, state) do + state = state |> refresh_and_schedule() |> Map.put(:initial_load_complete?, true) + {:noreply, publish_evaluation_state(state)} + end @impl GenServer + def handle_call(:cached_evaluation_state, _from, state), + do: {:reply, evaluation_state_from(state), state} + def handle_call(:definitions, _from, state), do: {:reply, state.definitions, state} def handle_call(:ready?, _from, state), do: {:reply, not is_nil(state.definitions), state} def handle_call(:refresh, _from, state), do: {:reply, :ok, refresh_and_schedule(state)} - def handle_call({:evaluation_state, keys}, _from, state) do - known_missing = - keys - |> MapSet.new() - |> MapSet.intersection(state.negative_keys) - - reply = %{ - definitions: state.definitions, - generation: state.definition_generation, - known_missing: known_missing - } - - {:reply, reply, state} - end - def handle_call( {:update_negative_knowledge, generation, requested, returned, clean?}, _from, %{definition_generation: generation} = state ) do - state = update_retained_missing(state, requested, returned, clean?) + state = + state |> update_retained_missing(requested, returned, clean?) |> publish_evaluation_state() + {:reply, :ok, state} end @@ -177,7 +215,27 @@ defmodule PostHog.FeatureFlags.DefinitionLoader do state end - schedule_refresh(state) + state + |> schedule_refresh() + |> publish_evaluation_state() + end + + defp publish_evaluation_state(state) do + registry = PostHog.Registry.registry_name(state.config.supervisor_name) + + cached = evaluation_state_from(state) + + Registry.update_value(registry, __MODULE__, fn _old -> cached end) + state + end + + defp evaluation_state_from(state) do + %{ + definitions: state.definitions, + generation: state.definition_generation, + negative_keys: state.negative_keys, + initial_load_complete?: state.initial_load_complete? + } end defp invalidate_timer(state) do diff --git a/lib/posthog/feature_flags/local_evaluator.ex b/lib/posthog/feature_flags/local_evaluator.ex index 23195c2..0f7f56a 100644 --- a/lib/posthog/feature_flags/local_evaluator.ex +++ b/lib/posthog/feature_flags/local_evaluator.ex @@ -56,16 +56,45 @@ defmodule PostHog.FeatureFlags.LocalEvaluator do end defp normalize_context(context) do + distinct_id = context_value(context, :distinct_id) + groups = context_value(context, :groups) || %{} + + person_properties = + context + |> context_value(:person_properties) + |> Kernel.||(%{}) + |> put_default_property("distinct_id", distinct_id) + + group_properties = + Enum.reduce(groups, context_value(context, :group_properties) || %{}, fn + {group_type, group_key}, properties when is_map(properties) -> + group_type = to_string(group_type) + focused = map_value(properties, group_type) || %{} + Map.put(properties, group_type, put_default_property(focused, "$group_key", group_key)) + + _group, properties -> + properties + end) + %{ - distinct_id: context_value(context, :distinct_id), - groups: context_value(context, :groups) || %{}, - person_properties: context_value(context, :person_properties) || %{}, - group_properties: context_value(context, :group_properties) || %{}, + distinct_id: distinct_id, + groups: groups, + person_properties: person_properties, + group_properties: group_properties, device_id: context_value(context, :device_id), now: context_value(context, :now) || DateTime.utc_now() } end + defp put_default_property(properties, key, value) when is_map(properties) do + case map_fetch(properties, key) do + :error -> Map.put(properties, key, value) + {:ok, _existing} -> properties + end + end + + defp put_default_property(_properties, key, value), do: %{key => value} + defp context_value(context, key), do: Map.get(context, key, Map.get(context, Atom.to_string(key))) diff --git a/test/posthog/feature_flags/definition_loader_test.exs b/test/posthog/feature_flags/definition_loader_test.exs index 938dc68..b77a04b 100644 --- a/test/posthog/feature_flags/definition_loader_test.exs +++ b/test/posthog/feature_flags/definition_loader_test.exs @@ -99,6 +99,39 @@ defmodule PostHog.FeatureFlags.DefinitionLoaderTest do refute Map.has_key?(PostHog.config(__MODULE__.Wire), :secret_key) end + test "cached definitions remain readable while a refresh is in flight" do + owner = self() + + PostHog.API.Mock + |> stub_with(PostHog.API.Stub) + |> expect(:request, fn :stub_client, :get, "/flags/definitions", _opts -> + {:ok, %{status: 200, body: envelope("stale"), headers: %{}}} + end) + |> expect(:request, fn :stub_client, :get, "/flags/definitions", _opts -> + send(owner, {:refresh_started, self()}) + receive(do: (:release_refresh -> :ok)) + {:ok, %{status: 503, body: %{}, headers: %{}}} + end) + + start_supervised!({PostHog.Supervisor, config(__MODULE__.ConcurrentRead)}) + refresh = Task.async(fn -> DefinitionLoader.refresh(__MODULE__.ConcurrentRead) end) + assert_receive {:refresh_started, refresh_worker} + + read = Task.async(fn -> DefinitionLoader.evaluation_state(__MODULE__.ConcurrentRead, []) end) + + case Task.yield(read, 100) do + {:ok, state} -> + assert state.definitions.flags_by_key["stale"] + + nil -> + send(refresh_worker, :release_refresh) + flunk("cached definitions were blocked by the in-flight refresh") + end + + send(refresh_worker, :release_refresh) + assert :ok = Task.await(refresh) + end + test "valid empty definitions are ready and transient or malformed refreshes preserve stale data" do stub_with(PostHog.API.Mock, PostHog.API.Stub) diff --git a/test/posthog/feature_flags/local_evaluation_integration_test.exs b/test/posthog/feature_flags/local_evaluation_integration_test.exs index 3c4845c..a06c0e4 100644 --- a/test/posthog/feature_flags/local_evaluation_integration_test.exs +++ b/test/posthog/feature_flags/local_evaluation_integration_test.exs @@ -151,6 +151,39 @@ defmodule PostHog.FeatureFlags.LocalEvaluationIntegrationTest do assert Evaluations.keys(snapshot) == ["local"] end + test "malformed remote fallback preserves a partial local snapshot" do + unknown = flag("unknown", [%{"key" => "x", "operator" => "future", "value" => 1}]) + + expect(PostHog.API.Mock, :request, 2, fn + :stub_client, :get, "/flags/definitions", _opts -> + {:ok, %{status: 200, body: envelope([flag("local"), unknown]), headers: %{}}} + + :stub_client, :post, "/flags", _opts -> + {:ok, %{status: 200, body: %{"flags" => ["malformed"]}}} + end) + + start_instance(__MODULE__.MalformedFallback) + assert {:ok, snapshot} = FeatureFlags.evaluate_flags(__MODULE__.MalformedFallback, "user") + assert Evaluations.keys(snapshot) == ["local"] + end + + test "malformed remote fallback returns an error when nothing resolved locally" do + unknown = flag("unknown", [%{"key" => "x", "operator" => "future", "value" => 1}]) + + expect(PostHog.API.Mock, :request, 2, fn + :stub_client, :get, "/flags/definitions", _opts -> + {:ok, %{status: 200, body: envelope([unknown]), headers: %{}}} + + :stub_client, :post, "/flags", _opts -> + {:ok, %{status: 200, body: %{"flags" => ["malformed"]}}} + end) + + start_instance(__MODULE__.MalformedOnly) + + assert {:error, %RuntimeError{message: "invalid response from PostHog /flags endpoint"}} = + FeatureFlags.evaluate_flags(__MODULE__.MalformedOnly, "user") + end + test "group absence makes only that flag fall back and requested missing keys are remotely scoped" do group = put_in(flag("group"), ["filters", "aggregation_group_type_index"], 0) caller = self() diff --git a/test/posthog/feature_flags/local_evaluator_test.exs b/test/posthog/feature_flags/local_evaluator_test.exs index 66528ce..3fa1c39 100644 --- a/test/posthog/feature_flags/local_evaluator_test.exs +++ b/test/posthog/feature_flags/local_evaluator_test.exs @@ -238,7 +238,7 @@ defmodule PostHog.FeatureFlags.LocalEvaluatorTest do end end - test "explicit nil values only allow maintained is_not semantics" do + test "explicit nil values preserve is_set and is_not semantics" do no_match_operators = [ "exact", "icontains", @@ -278,6 +278,12 @@ defmodule PostHog.FeatureFlags.LocalEvaluatorTest do "is-not" ].enabled + is_set = %{"key" => "prop", "operator" => "is_set", "value" => nil} + + assert evaluate(flag("is-set", [is_set]), %{person_properties: %{prop: nil}}).results[ + "is-set" + ].enabled + absent = %{"key" => "prop", "operator" => "is_not_set", "value" => nil} assert MapSet.member?(evaluate(flag("absent", [absent])).unresolved, "absent") @@ -319,6 +325,40 @@ defmodule PostHog.FeatureFlags.LocalEvaluatorTest do assert MapSet.member?(result.unresolved, "missing") end + test "built-in identity properties are available without caller property maps" do + person_property = %{ + "key" => "distinct_id", + "operator" => "exact", + "value" => "person-1" + } + + assert evaluate(flag("person", [person_property]), %{distinct_id: "person-1"}).results[ + "person" + ].enabled + + refute evaluate(flag("person", [person_property]), %{ + distinct_id: "person-1", + person_properties: %{distinct_id: "override"} + }).results["person"].enabled + + group = + flag("group", [%{"key" => "$group_key", "operator" => "exact", "value" => "org-1"}]) + |> put_in(["filters", "aggregation_group_type_index"], 0) + + definitions = snapshot([group], %{group_type_mapping: %{"0" => "organization"}}) + + assert LocalEvaluator.evaluate(definitions, %{ + distinct_id: "person-1", + groups: %{organization: "org-1"} + }).results["group"].enabled + + refute LocalEvaluator.evaluate(definitions, %{ + distinct_id: "person-1", + groups: %{organization: "org-1"}, + group_properties: %{organization: %{"$group_key" => "override"}} + }).results["group"].enabled + end + test "group and mixed targeting use group properties and group keys" do group = put_in(flag("group"), ["filters", "aggregation_group_type_index"], 0) diff --git a/test/posthog/feature_flags/missing_key_knowledge_test.exs b/test/posthog/feature_flags/missing_key_knowledge_test.exs index 15a31b9..fc4fb8d 100644 --- a/test/posthog/feature_flags/missing_key_knowledge_test.exs +++ b/test/posthog/feature_flags/missing_key_knowledge_test.exs @@ -89,15 +89,15 @@ defmodule PostHog.FeatureFlags.MissingKeyKnowledgeTest do assert_receive :remote_probe end - test "remote-only and locally unresolved total failures return safe empty snapshots without accessor retries" do + test "remote-only and locally unresolved total failures preserve the error contract" do expect(PostHog.API.Mock, :request, fn :stub_client, :post, "/flags", _opts -> {:error, %RuntimeError{message: "offline"}} end) start_instance(__MODULE__.RemoteOnly, secret_key: nil) - assert {:ok, remote_only} = scoped(__MODULE__.RemoteOnly, ["missing"]) - refute Evaluations.enabled?(remote_only, "missing") - assert Evaluations.get_flag(remote_only, "missing") == nil + + assert {:error, %RuntimeError{message: "offline"}} = + scoped(__MODULE__.RemoteOnly, ["missing"]) {:ok, counter} = Agent.start_link(fn -> 0 end) @@ -109,10 +109,8 @@ defmodule PostHog.FeatureFlags.MissingKeyKnowledgeTest do end) start_instance(__MODULE__.Failure) - assert {:ok, failed} = scoped(__MODULE__.Failure, ["missing"]) - assert Evaluations.keys(failed) == [] - refute Evaluations.enabled?(failed, "missing") - assert {:ok, _retry} = scoped(__MODULE__.Failure, ["missing"]) + assert {:error, %RuntimeError{message: "offline"}} = scoped(__MODULE__.Failure, ["missing"]) + assert {:error, %RuntimeError{message: "offline"}} = scoped(__MODULE__.Failure, ["missing"]) end test "dirty omissions do not suppress retries and returned keys remain context specific" do From a68e720daf2f0b005944652900f95453af4673b9 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Tue, 25 Aug 2026 20:07:18 +0200 Subject: [PATCH 4/9] fix(flags): treat nil properties as not set --- lib/posthog/feature_flags/local_evaluator.ex | 1 + test/posthog/feature_flags/local_evaluator_test.exs | 10 ++++++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/posthog/feature_flags/local_evaluator.ex b/lib/posthog/feature_flags/local_evaluator.ex index 0f7f56a..d97995c 100644 --- a/lib/posthog/feature_flags/local_evaluator.ex +++ b/lib/posthog/feature_flags/local_evaluator.ex @@ -615,6 +615,7 @@ defmodule PostHog.FeatureFlags.LocalEvaluator do end end + defp apply_operator("is_set", nil, _filter, _now), do: :no_match defp apply_operator("is_set", _property, _filter, _now), do: :match defp apply_operator("is_not_set", _property, _filter, _now), do: :no_match diff --git a/test/posthog/feature_flags/local_evaluator_test.exs b/test/posthog/feature_flags/local_evaluator_test.exs index 3fa1c39..bb6ded3 100644 --- a/test/posthog/feature_flags/local_evaluator_test.exs +++ b/test/posthog/feature_flags/local_evaluator_test.exs @@ -238,7 +238,7 @@ defmodule PostHog.FeatureFlags.LocalEvaluatorTest do end end - test "explicit nil values preserve is_set and is_not semantics" do + test "explicit nil values follow operator-specific semantics" do no_match_operators = [ "exact", "icontains", @@ -280,10 +280,16 @@ defmodule PostHog.FeatureFlags.LocalEvaluatorTest do is_set = %{"key" => "prop", "operator" => "is_set", "value" => nil} - assert evaluate(flag("is-set", [is_set]), %{person_properties: %{prop: nil}}).results[ + refute evaluate(flag("is-set", [is_set]), %{person_properties: %{prop: nil}}).results[ "is-set" ].enabled + for value <- [false, 0, "", [], %{}] do + assert evaluate(flag("is-set", [is_set]), %{person_properties: %{prop: value}}).results[ + "is-set" + ].enabled + end + absent = %{"key" => "prop", "operator" => "is_not_set", "value" => nil} assert MapSet.member?(evaluate(flag("absent", [absent])).unresolved, "absent") From 95b7f02835e8a2ef7c456910e09eb51c6eab750c Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto <5731772+marandaneto@users.noreply.github.com> Date: Wed, 26 Aug 2026 02:47:22 +0700 Subject: [PATCH 5/9] fix(flags): address local evaluation review feedback --- lib/posthog/feature_flags.ex | 218 ++++++++++++++---- lib/posthog/feature_flags/called_cache.ex | 16 +- lib/posthog/feature_flags/evaluations.ex | 18 +- lib/posthog/feature_flags/local_evaluator.ex | 4 +- lib/posthog/feature_flags/result.ex | 8 +- .../feature_flags/called_cache_test.exs | 2 +- .../feature_flags/evaluations_test.exs | 38 +++ .../local_evaluation_integration_test.exs | 51 +++- .../feature_flags/local_evaluator_test.exs | 4 +- .../missing_key_knowledge_test.exs | 53 +++++ test/posthog/feature_flags_test.exs | 2 + 11 files changed, 358 insertions(+), 56 deletions(-) diff --git a/lib/posthog/feature_flags.ex b/lib/posthog/feature_flags.ex index b282e0f..f6c53ef 100644 --- a/lib/posthog/feature_flags.ex +++ b/lib/posthog/feature_flags.ex @@ -30,7 +30,7 @@ defmodule PostHog.FeatureFlags do config = PostHog.config(name) if Map.get(config, :enabled, true) do - request_flags(config.api_client, body) + request_flags(config.api_client, translate_disable_geoip(body)) else empty_flags_response() end @@ -128,9 +128,11 @@ defmodule PostHog.FeatureFlags do - `:groups` - `:person_properties` - `:group_properties` - - `:disable_geoip` - `:device_id` - alternate bucketing identifier for device-bucketed flags + The public `:disable_geoip` option is sent under the `/flags` wire key + `geoip_disable`. + Plus these snapshot-specific options: - `:only_evaluate_locally` - when true, never calls `/flags`; unresolved @@ -169,8 +171,9 @@ defmodule PostHog.FeatureFlags do {:ok, __MODULE__.Evaluations.t()} | {:error, Exception.t()} def evaluate_flags(name \\ PostHog, distinct_id_or_body \\ nil) do case body_for_flags(name, distinct_id_or_body) do - {:ok, %{distinct_id: distinct_id, flag_keys: []}} -> - {:ok, __MODULE__.Evaluations.from_results(name, distinct_id, %{}, %{})} + {:ok, %{distinct_id: distinct_id, flag_keys: []} = body} -> + metadata = %{groups: evaluation_groups(body)} + {:ok, __MODULE__.Evaluations.from_results(name, distinct_id, %{}, metadata)} {:ok, %{distinct_id: distinct_id} = body} -> evaluate_snapshot(name, distinct_id, body) @@ -192,7 +195,7 @@ defmodule PostHog.FeatureFlags do case loader_state.definitions do nil -> if only_local? do - snapshot_from_results(name, distinct_id, %{}) + snapshot_from_results(name, distinct_id, %{}, body) else remote_snapshot(name, distinct_id, body, %{}, requested_keys, nil, []) end @@ -204,7 +207,8 @@ defmodule PostHog.FeatureFlags do body, requested_keys, loader_state, - true + true, + nil ) end end @@ -215,7 +219,43 @@ defmodule PostHog.FeatureFlags do body, requested_keys, loader_state, - coordinate_missing? + coordinate_missing?, + owned_probe_keys + ) do + definitions = loader_state.definitions + only_local? = Map.get(body, :only_evaluate_locally, false) == true + + if is_nil(requested_keys) and map_size(definitions.flags_by_key) == 0 and not only_local? do + remote_loaded_snapshot( + name, + distinct_id, + body, + requested_keys, + loader_state, + %{results: %{}}, + owned_probe_keys + ) + else + evaluate_nonempty_loaded_snapshot( + name, + distinct_id, + body, + requested_keys, + loader_state, + coordinate_missing?, + owned_probe_keys + ) + end + end + + defp evaluate_nonempty_loaded_snapshot( + name, + distinct_id, + body, + requested_keys, + loader_state, + coordinate_missing?, + owned_probe_keys ) do definitions = loader_state.definitions keys = requested_keys || Map.keys(definitions.flags_by_key) @@ -225,8 +265,8 @@ defmodule PostHog.FeatureFlags do only_local? = Map.get(body, :only_evaluate_locally, false) == true cond do - MapSet.size(unresolved) == 0 or only_local? -> - snapshot_from_results(name, distinct_id, local.results) + only_local? or MapSet.size(unresolved) == 0 -> + snapshot_from_results(name, distinct_id, local.results, body) coordinate_missing? and is_list(requested_keys) -> coordinate_missing_probe( @@ -240,7 +280,15 @@ defmodule PostHog.FeatureFlags do ) true -> - remote_loaded_snapshot(name, distinct_id, body, requested_keys, loader_state, local) + remote_loaded_snapshot( + name, + distinct_id, + body, + requested_keys, + loader_state, + local, + owned_probe_keys + ) end end @@ -268,29 +316,50 @@ defmodule PostHog.FeatureFlags do ) else with_missing_probe_locks(name, probe_keys, fn -> - evaluate_after_missing_probe_lock(name, distinct_id, body, requested_keys) + evaluate_after_missing_probe_lock(name, distinct_id, body, requested_keys, probe_keys) end) end end - defp evaluate_after_missing_probe_lock(name, distinct_id, body, requested_keys) do + defp evaluate_after_missing_probe_lock(name, distinct_id, body, requested_keys, probe_keys) do fresh_state = local_evaluation_state(name, requested_keys) if is_nil(fresh_state.definitions) do remote_snapshot(name, distinct_id, body, %{}, requested_keys, nil, []) else - evaluate_loaded_snapshot(name, distinct_id, body, requested_keys, fresh_state, false) + evaluate_loaded_snapshot( + name, + distinct_id, + body, + requested_keys, + fresh_state, + false, + probe_keys + ) end end - defp remote_loaded_snapshot(name, distinct_id, body, requested_keys, loader_state, local) do + defp remote_loaded_snapshot( + name, + distinct_id, + body, + requested_keys, + loader_state, + local, + owned_probe_keys + ) do absent = - if is_list(requested_keys) do - requested_keys - |> absent_definition_keys(loader_state.definitions) - |> MapSet.to_list() - else - [] + cond do + is_list(owned_probe_keys) -> + owned_probe_keys + + is_list(requested_keys) -> + requested_keys + |> absent_definition_keys(loader_state.definitions) + |> MapSet.to_list() + + true -> + [] end remote_snapshot( @@ -327,45 +396,66 @@ defmodule PostHog.FeatureFlags do case flags(name, remote_body) do {:ok, %{body: %{"flags" => remote_flags} = response_body}} when is_map(remote_flags) -> scoped_remote_flags = maybe_scope_remote_results(remote_flags, requested_keys) + {remote_results, malformed?} = build_remote_results(scoped_remote_flags, response_body) update_negative_knowledge( name, generation, negative_candidates, Map.keys(scoped_remote_flags), - clean_remote_response?(response_body) + clean_remote_response?(response_body) and not malformed? ) - remote_results = - Map.new(scoped_remote_flags, fn {key, flag_data} -> - {key, build_result(key, flag_data, response_body)} - end) - merged = Map.merge(remote_results, local_results) - {:ok, __MODULE__.Evaluations.from_results(name, distinct_id, merged, response_body)} + + if malformed? and map_size(merged) == 0 do + remote_failure_result( + name, + distinct_id, + local_results, + body, + invalid_flags_response() + ) + else + metadata = Map.put(response_body, :groups, evaluation_groups(body)) + {:ok, __MODULE__.Evaluations.from_results(name, distinct_id, merged, metadata)} + end {:ok, _malformed_response} -> - remote_failure_result( - name, - distinct_id, - local_results, - {:error, %RuntimeError{message: "invalid response from PostHog /flags endpoint"}} - ) + remote_failure_result(name, distinct_id, local_results, body, invalid_flags_response()) {:error, _reason} = error -> - remote_failure_result(name, distinct_id, local_results, error) + remote_failure_result(name, distinct_id, local_results, body, error) end end - defp remote_failure_result(_name, _distinct_id, local_results, error) + defp remote_failure_result(_name, _distinct_id, local_results, _body, error) when map_size(local_results) == 0, do: error - defp remote_failure_result(name, distinct_id, local_results, _error), - do: snapshot_from_results(name, distinct_id, local_results) + defp remote_failure_result(name, distinct_id, local_results, body, _error), + do: snapshot_from_results(name, distinct_id, local_results, body) + + defp snapshot_from_results(name, distinct_id, results, body) do + metadata = %{groups: evaluation_groups(body)} + {:ok, __MODULE__.Evaluations.from_results(name, distinct_id, results, metadata)} + end - defp snapshot_from_results(name, distinct_id, results), - do: {:ok, __MODULE__.Evaluations.from_results(name, distinct_id, results, %{})} + defp build_remote_results(flags, response_body) do + Enum.reduce(flags, {%{}, false}, fn + {key, flag_data}, {results, malformed?} when is_binary(key) and is_map(flag_data) -> + {Map.put(results, key, build_result(key, flag_data, response_body)), malformed?} + + _entry, {results, _malformed?} -> + {results, true} + end) + end + + defp invalid_flags_response, + do: {:error, %RuntimeError{message: "invalid response from PostHog /flags endpoint"}} + + defp evaluation_groups(body) when is_map(body), + do: Map.get(body, :groups, Map.get(body, "groups", %{})) || %{} defp clean_remote_response?(body) do Map.get(body, "errorsWhileComputingFlags") != true and @@ -485,6 +575,14 @@ defmodule PostHog.FeatureFlags do defp translate_flag_keys(body), do: body + defp translate_disable_geoip(%{disable_geoip: disable_geoip} = body) do + body + |> Map.delete(:disable_geoip) + |> Map.put(:geoip_disable, disable_geoip) + end + + defp translate_disable_geoip(body), do: body + @deprecated "Use PostHog.FeatureFlags.evaluate_flags/2 with PostHog.FeatureFlags.Evaluations.enabled?/2 or get_flag/2" @doc false def check(flag_name, distinct_id_or_body) when not is_atom(flag_name), @@ -714,7 +812,14 @@ defmodule PostHog.FeatureFlags do {:ok, %{distinct_id: distinct_id} = body} -> case evaluate_single_flag(name, flag_name, body) do {:ok, %__MODULE__.Result{} = result, response_body} -> - maybe_log_feature_flag_usage(send_event, name, distinct_id, result) + maybe_log_feature_flag_usage( + send_event, + name, + distinct_id, + result, + evaluation_groups(body) + ) + {:ok, result, response_body} {:ok, nil, response_body} -> @@ -753,7 +858,8 @@ defmodule PostHog.FeatureFlags do {:ok, nil, %{}} else case flags(name, Map.delete(body, :only_evaluate_locally)) do - {:ok, %{body: %{"flags" => %{^flag_name => flag_data}} = response_body}} -> + {:ok, %{body: %{"flags" => %{^flag_name => flag_data}} = response_body}} + when is_map(flag_data) -> {:ok, build_result(flag_name, flag_data, response_body), response_body} {:ok, %{body: %{"flags" => _} = response_body}} -> @@ -765,9 +871,9 @@ defmodule PostHog.FeatureFlags do end end - defp maybe_log_feature_flag_usage(send_event, name, distinct_id, flag_result) do + defp maybe_log_feature_flag_usage(send_event, name, distinct_id, flag_result, groups) do if send_event do - log_feature_flag_usage(name, distinct_id, flag_result) + log_feature_flag_usage(name, distinct_id, flag_result, [], groups) end end @@ -884,7 +990,7 @@ defmodule PostHog.FeatureFlags do ) :: :ok | {:error, :missing_distinct_id} def log_feature_flag_usage(name, distinct_id, %__MODULE__.Result{} = result) do - log_feature_flag_usage(name, distinct_id, result, []) + log_feature_flag_usage(name, distinct_id, result, [], %{}) end @doc false @@ -897,6 +1003,25 @@ defmodule PostHog.FeatureFlags do :ok | {:error, :missing_distinct_id} def log_feature_flag_usage(name, distinct_id, %__MODULE__.Result{} = result, extra_errors) when is_list(extra_errors) do + log_feature_flag_usage(name, distinct_id, result, extra_errors, %{}) + end + + @doc false + @spec log_feature_flag_usage( + PostHog.supervisor_name(), + PostHog.distinct_id(), + __MODULE__.Result.t(), + [String.t()], + map() + ) :: :ok | {:error, :missing_distinct_id} + def log_feature_flag_usage( + name, + distinct_id, + %__MODULE__.Result{} = result, + extra_errors, + groups + ) + when is_list(extra_errors) and is_map(groups) do flag_missing? = "flag_missing" in extra_errors value = if flag_missing?, do: nil, else: __MODULE__.Result.value(result) errors = build_error_codes(result, extra_errors) @@ -916,8 +1041,10 @@ defmodule PostHog.FeatureFlags do |> maybe_put(:"$feature_flag_payload", result.payload) |> maybe_put(:"$feature_flag_has_experiment", result.has_experiment) |> maybe_put(:"$feature_flag_error", errors) + |> maybe_put(:"$groups", if(map_size(groups) == 0, do: nil, else: groups)) + |> Map.put(:locally_evaluated, result.locally_evaluated) - if PostHog.FeatureFlags.CalledCache.first_seen?(name, distinct_id, result.key, value) do + if PostHog.FeatureFlags.CalledCache.first_seen?(name, distinct_id, result.key, value, groups) do capture_called_event(name, distinct_id, result, properties) end @@ -941,6 +1068,7 @@ defmodule PostHog.FeatureFlags do :"$feature_flag_request_id", :"$feature_flag_evaluated_at", :"$feature_flag_error", + :locally_evaluated, :"$groups", :"$process_person_profile", :"$session_id", diff --git a/lib/posthog/feature_flags/called_cache.ex b/lib/posthog/feature_flags/called_cache.ex index 0ba41c6..a65587e 100644 --- a/lib/posthog/feature_flags/called_cache.ex +++ b/lib/posthog/feature_flags/called_cache.ex @@ -33,7 +33,13 @@ defmodule PostHog.FeatureFlags.CalledCache do @spec first_seen?(PostHog.supervisor_name(), PostHog.distinct_id(), String.t(), any()) :: boolean() def first_seen?(supervisor_name, distinct_id, flag_key, value) do - key = {to_string(distinct_id), flag_key, value} + first_seen?(supervisor_name, distinct_id, flag_key, value, %{}) + end + + @spec first_seen?(PostHog.supervisor_name(), PostHog.distinct_id(), String.t(), any(), map()) :: + boolean() + def first_seen?(supervisor_name, distinct_id, flag_key, value, groups) do + key = {to_string(distinct_id), flag_key, value, normalize_groups(groups)} table = table_name(supervisor_name) case :ets.insert_new(table, {key}) do @@ -76,5 +82,13 @@ defmodule PostHog.FeatureFlags.CalledCache do end end + defp normalize_groups(groups) when is_map(groups) do + groups + |> Enum.map(fn {group_type, group_key} -> {to_string(group_type), to_string(group_key)} end) + |> Enum.sort() + end + + defp normalize_groups(_groups), do: [] + defp table_name(supervisor_name), do: Module.concat(supervisor_name, CalledCacheTable) end diff --git a/lib/posthog/feature_flags/evaluations.ex b/lib/posthog/feature_flags/evaluations.ex index 114373e..0669ea6 100644 --- a/lib/posthog/feature_flags/evaluations.ex +++ b/lib/posthog/feature_flags/evaluations.ex @@ -59,6 +59,8 @@ defmodule PostHog.FeatureFlags.Evaluations do was performed for. `""` for the empty fallback returned when no `distinct_id` could be resolved; events are short-circuited in that case. - `:flags` - map of flag key to `t:PostHog.FeatureFlags.Result.t/0`. + - `:groups` - normalized group type/key context used for evaluation and + `$feature_flag_called` deduplication. - `:request_id` - request ID returned when remote fallback was used. - `:evaluated_at` - server-side evaluation timestamp when remote fallback was used. - `:errors_while_computing` - whether the response signaled @@ -71,6 +73,7 @@ defmodule PostHog.FeatureFlags.Evaluations do supervisor_name: PostHog.supervisor_name(), distinct_id: PostHog.distinct_id(), flags: %{String.t() => Result.t()}, + groups: %{String.t() => String.t()}, request_id: String.t() | nil, evaluated_at: integer() | nil, errors_while_computing: boolean(), @@ -85,6 +88,7 @@ defmodule PostHog.FeatureFlags.Evaluations do :accessed_pid, :request_id, :evaluated_at, + groups: %{}, errors_while_computing: false ] @@ -104,6 +108,7 @@ defmodule PostHog.FeatureFlags.Evaluations do request_id: Map.get(body, "requestId"), evaluated_at: Map.get(body, "evaluatedAt"), errors_while_computing: Map.get(body, "errorsWhileComputingFlags") == true, + groups: normalize_groups(Map.get(body, :groups, Map.get(body, "groups", %{}))), accessed_pid: start_accessed_agent() } end @@ -126,6 +131,7 @@ defmodule PostHog.FeatureFlags.Evaluations do errors_while_computing: Map.get(metadata, :errors_while_computing, Map.get(metadata, "errorsWhileComputingFlags")) == true, + groups: normalize_groups(Map.get(metadata, :groups, Map.get(metadata, "groups", %{}))), accessed_pid: start_accessed_agent() } end @@ -299,10 +305,18 @@ defmodule PostHog.FeatureFlags.Evaluations do defp log(%__MODULE__{distinct_id: ""}, _result, _extra_errors), do: :ok defp log( - %__MODULE__{supervisor_name: name, distinct_id: distinct_id}, + %__MODULE__{supervisor_name: name, distinct_id: distinct_id, groups: groups}, %Result{} = result, extra_errors ) do - PostHog.FeatureFlags.log_feature_flag_usage(name, distinct_id, result, extra_errors) + PostHog.FeatureFlags.log_feature_flag_usage(name, distinct_id, result, extra_errors, groups) end + + defp normalize_groups(groups) when is_map(groups) do + Map.new(groups, fn {group_type, group_key} -> + {to_string(group_type), to_string(group_key)} + end) + end + + defp normalize_groups(_groups), do: %{} end diff --git a/lib/posthog/feature_flags/local_evaluator.ex b/lib/posthog/feature_flags/local_evaluator.ex index d97995c..3e42aba 100644 --- a/lib/posthog/feature_flags/local_evaluator.ex +++ b/lib/posthog/feature_flags/local_evaluator.ex @@ -615,7 +615,6 @@ defmodule PostHog.FeatureFlags.LocalEvaluator do end end - defp apply_operator("is_set", nil, _filter, _now), do: :no_match defp apply_operator("is_set", _property, _filter, _now), do: :match defp apply_operator("is_not_set", _property, _filter, _now), do: :no_match @@ -754,7 +753,8 @@ defmodule PostHog.FeatureFlags.LocalEvaluator do id: value(flag, "id"), version: value(flag, "version"), has_experiment: boolean_or_nil(value(flag, "has_experiment")), - minimal_flag_called_events: definitions.minimal_flag_called_events + minimal_flag_called_events: definitions.minimal_flag_called_events, + locally_evaluated: true } end diff --git a/lib/posthog/feature_flags/result.ex b/lib/posthog/feature_flags/result.ex index cc67f47..86f7a08 100644 --- a/lib/posthog/feature_flags/result.ex +++ b/lib/posthog/feature_flags/result.ex @@ -26,6 +26,8 @@ defmodule PostHog.FeatureFlags.Result do explicitly `false`, `$feature_flag_called` events for this flag are sent with a minimal, allowlisted property shape. `false` whenever the server did not report the gate. + - `locally_evaluated` - Whether this result was resolved from local flag + definitions rather than the remote `/flags` response. The metadata fields are populated when the `/flags` response includes them and are forwarded as `$feature_flag_id`, `$feature_flag_version`, `$feature_flag_reason`, @@ -77,7 +79,8 @@ defmodule PostHog.FeatureFlags.Result do evaluated_at: integer() | nil, has_experiment: boolean() | nil, errors_while_computing: boolean(), - minimal_flag_called_events: boolean() + minimal_flag_called_events: boolean(), + locally_evaluated: boolean() } @enforce_keys [:key, :enabled] @@ -93,7 +96,8 @@ defmodule PostHog.FeatureFlags.Result do :evaluated_at, :has_experiment, errors_while_computing: false, - minimal_flag_called_events: false + minimal_flag_called_events: false, + locally_evaluated: false ] @doc """ diff --git a/test/posthog/feature_flags/called_cache_test.exs b/test/posthog/feature_flags/called_cache_test.exs index 1a4ba13..08b3ecc 100644 --- a/test/posthog/feature_flags/called_cache_test.exs +++ b/test/posthog/feature_flags/called_cache_test.exs @@ -17,7 +17,7 @@ defmodule PostHog.FeatureFlags.CalledCacheTest do test "flushes the cache when it reaches the maximum size", %{config: config} do supervisor_name = config.supervisor_name - seed_key = {"seed-user", "flag", true} + seed_key = {"seed-user", "flag", true, []} table = table(supervisor_name) full_cache = diff --git a/test/posthog/feature_flags/evaluations_test.exs b/test/posthog/feature_flags/evaluations_test.exs index 9337fac..05473be 100644 --- a/test/posthog/feature_flags/evaluations_test.exs +++ b/test/posthog/feature_flags/evaluations_test.exs @@ -121,6 +121,16 @@ defmodule PostHog.FeatureFlags.EvaluationsTest do }) end + test "translates disable_geoip to the flags wire key" do + expect(API.Mock, :request, fn _client, :post, "/flags", opts -> + assert opts[:json] == %{distinct_id: "foo", geoip_disable: true} + {:ok, stub_flags_response()} + end) + + assert {:ok, _} = + FeatureFlags.evaluate_flags(%{distinct_id: "foo", disable_geoip: true}) + end + test "forwards person_properties unchanged in the request body" do expect(API.Mock, :request, fn _client, :post, "/flags", opts -> assert opts[:json] == %{distinct_id: "foo", person_properties: %{plan: "enterprise"}} @@ -248,6 +258,7 @@ defmodule PostHog.FeatureFlags.EvaluationsTest do assert properties[:"$feature_flag_evaluated_at"] == 1_700_000_000 assert properties[:"$feature_flag_payload"] == %{"copy" => "hi"} assert properties[:"$feature_flag_has_experiment"] == true + assert properties[:locally_evaluated] == false assert properties["$feature/variant-flag"] == "control" end @@ -275,6 +286,32 @@ defmodule PostHog.FeatureFlags.EvaluationsTest do assert [%{event: "$feature_flag_called"}] = all_captured() end + test "dedupes normalized group context and emits again when the group changes" do + expect(API.Mock, :request, 3, fn _client, :post, "/flags", _opts -> + {:ok, stub_flags_response()} + end) + + contexts = [ + %{company: "company-1", team: 7}, + %{"team" => "7", "company" => "company-1"}, + %{company: "company-2", team: 7} + ] + + for groups <- contexts do + assert {:ok, snapshot} = + FeatureFlags.evaluate_flags(%{distinct_id: "group-user", groups: groups}) + + assert Evaluations.enabled?(snapshot, "boolean-flag") + end + + groups = Enum.map(all_captured(), & &1.properties[:"$groups"]) + + assert groups == [ + %{"company" => "company-2", "team" => "7"}, + %{"company" => "company-1", "team" => "7"} + ] + end + test "fires again when the same flag returns a different value" do true_result = %FeatureFlags.Result{key: "changing-flag", enabled: true} false_result = %FeatureFlags.Result{key: "changing-flag", enabled: false} @@ -678,6 +715,7 @@ defmodule PostHog.FeatureFlags.EvaluationsTest do Enum.find(events, &(&1.properties[:"$feature_flag"] == "experiment-flag")) assert minimal == %{ + locally_evaluated: false, "$feature_flag": "plain-flag", "$feature_flag_response": true, "$feature_flag_has_experiment": false, diff --git a/test/posthog/feature_flags/local_evaluation_integration_test.exs b/test/posthog/feature_flags/local_evaluation_integration_test.exs index a06c0e4..334e465 100644 --- a/test/posthog/feature_flags/local_evaluation_integration_test.exs +++ b/test/posthog/feature_flags/local_evaluation_integration_test.exs @@ -78,6 +78,30 @@ defmodule PostHog.FeatureFlags.LocalEvaluationIntegrationTest do assert event.event == "$feature_flag_called" assert event.properties[:"$feature_flag_id"] == 11 assert event.properties[:"$feature_flag_version"] == 3 + assert event.properties[:locally_evaluated] == true + end + + test "empty unscoped definitions fall back remotely unless local-only was requested" do + expect(PostHog.API.Mock, :request, 2, fn + :stub_client, :get, "/flags/definitions", _opts -> + {:ok, %{status: 200, body: envelope([]), headers: %{}}} + + :stub_client, :post, "/flags", _opts -> + {:ok, %{status: 200, body: %{"flags" => %{"remote" => %{"enabled" => true}}}}} + end) + + start_instance(__MODULE__.EmptyDefinitions) + + assert {:ok, remote} = FeatureFlags.evaluate_flags(__MODULE__.EmptyDefinitions, "user") + assert remote.flags["remote"].enabled + + assert {:ok, local_only} = + FeatureFlags.evaluate_flags(__MODULE__.EmptyDefinitions, %{ + distinct_id: "user", + only_evaluate_locally: true + }) + + assert Evaluations.keys(local_only) == [] end test "unknown and missing properties fall back once and local results win merge conflicts" do @@ -167,6 +191,31 @@ defmodule PostHog.FeatureFlags.LocalEvaluationIntegrationTest do assert Evaluations.keys(snapshot) == ["local"] end + test "malformed remote entries are skipped while valid local and remote results survive" do + unknown = flag("unknown", [%{"key" => "x", "operator" => "future", "value" => 1}]) + + expect(PostHog.API.Mock, :request, 2, fn + :stub_client, :get, "/flags/definitions", _opts -> + {:ok, %{status: 200, body: envelope([flag("local"), unknown]), headers: %{}}} + + :stub_client, :post, "/flags", _opts -> + {:ok, + %{ + status: 200, + body: %{ + "flags" => %{ + "remote" => %{"enabled" => true}, + "broken" => nil + } + } + }} + end) + + start_instance(__MODULE__.MalformedEntryPartial) + assert {:ok, snapshot} = FeatureFlags.evaluate_flags(__MODULE__.MalformedEntryPartial, "user") + assert Evaluations.keys(snapshot) == ["local", "remote"] + end + test "malformed remote fallback returns an error when nothing resolved locally" do unknown = flag("unknown", [%{"key" => "x", "operator" => "future", "value" => 1}]) @@ -175,7 +224,7 @@ defmodule PostHog.FeatureFlags.LocalEvaluationIntegrationTest do {:ok, %{status: 200, body: envelope([unknown]), headers: %{}}} :stub_client, :post, "/flags", _opts -> - {:ok, %{status: 200, body: %{"flags" => ["malformed"]}}} + {:ok, %{status: 200, body: %{"flags" => %{"unknown" => nil}}}} end) start_instance(__MODULE__.MalformedOnly) diff --git a/test/posthog/feature_flags/local_evaluator_test.exs b/test/posthog/feature_flags/local_evaluator_test.exs index bb6ded3..ad12d05 100644 --- a/test/posthog/feature_flags/local_evaluator_test.exs +++ b/test/posthog/feature_flags/local_evaluator_test.exs @@ -238,7 +238,7 @@ defmodule PostHog.FeatureFlags.LocalEvaluatorTest do end end - test "explicit nil values follow operator-specific semantics" do + test "is_set uses property presence while other operators handle nil values" do no_match_operators = [ "exact", "icontains", @@ -280,7 +280,7 @@ defmodule PostHog.FeatureFlags.LocalEvaluatorTest do is_set = %{"key" => "prop", "operator" => "is_set", "value" => nil} - refute evaluate(flag("is-set", [is_set]), %{person_properties: %{prop: nil}}).results[ + assert evaluate(flag("is-set", [is_set]), %{person_properties: %{prop: nil}}).results[ "is-set" ].enabled diff --git a/test/posthog/feature_flags/missing_key_knowledge_test.exs b/test/posthog/feature_flags/missing_key_knowledge_test.exs index fc4fb8d..5422be3 100644 --- a/test/posthog/feature_flags/missing_key_knowledge_test.exs +++ b/test/posthog/feature_flags/missing_key_knowledge_test.exs @@ -240,6 +240,59 @@ defmodule PostHog.FeatureFlags.MissingKeyKnowledgeTest do Task.await(mixed) end + test "a delayed omission cannot reinstall knowledge cleared by newer positive evidence" do + owner = self() + {:ok, a_requests} = Agent.start_link(fn -> 0 end) + + expect(PostHog.API.Mock, :request, 5, fn :stub_client, method, path, opts -> + case {method, path} do + {:get, "/flags/definitions"} -> + {:ok, %{status: 200, body: envelope(), headers: %{}}} + + {:post, "/flags"} -> + case opts[:json].flag_keys_to_evaluate do + ["a"] -> + Agent.update(a_requests, &(&1 + 1)) + {:ok, %{status: 200, body: %{"flags" => %{}}}} + + ["a", "b"] -> + send(owner, {:delayed_omission, self()}) + receive do: (:release_delayed_omission -> :ok) + {:ok, %{status: 200, body: %{"flags" => %{}}}} + + ["a", "c"] -> + {:ok, %{status: 200, body: %{"flags" => %{"a" => %{"enabled" => true}}}}} + end + end + end) + + start_instance(__MODULE__.Ordering) + + assert {:ok, _snapshot} = scoped(__MODULE__.Ordering, ["a"]) + + assert MapSet.member?( + DefinitionLoader.evaluation_state(__MODULE__.Ordering, ["a"]).known_missing, + "a" + ) + + delayed = Task.async(fn -> scoped(__MODULE__.Ordering, ["a", "b"], "delayed") end) + assert_receive {:delayed_omission, delayed_worker} + + assert {:ok, positive} = scoped(__MODULE__.Ordering, ["a", "c"], "positive") + assert positive.flags["a"].enabled + + send(delayed_worker, :release_delayed_omission) + assert {:ok, _snapshot} = Task.await(delayed) + + refute MapSet.member?( + DefinitionLoader.evaluation_state(__MODULE__.Ordering, ["a"]).known_missing, + "a" + ) + + assert {:ok, _snapshot} = scoped(__MODULE__.Ordering, ["a"], "after") + assert Agent.get(a_requests, & &1) == 2 + end + test "loader restarts use a new generation incarnation" do expect(PostHog.API.Mock, :request, 2, fn :stub_client, :get, "/flags/definitions", _opts -> {:ok, %{status: 200, body: envelope(), headers: %{}}} diff --git a/test/posthog/feature_flags_test.exs b/test/posthog/feature_flags_test.exs index 6cdce95..d562a85 100644 --- a/test/posthog/feature_flags_test.exs +++ b/test/posthog/feature_flags_test.exs @@ -525,6 +525,7 @@ defmodule PostHog.FeatureFlagsTest do "$feature_flag_request_id": "req-xyz", "$feature_flag_evaluated_at": 1_700_000_000, "$feature_flag_error": "errors_while_computing_flags", + locally_evaluated: false, "$groups": %{company: "acme"}, "$process_person_profile": false, "$lib": "posthog-elixir", @@ -558,6 +559,7 @@ defmodule PostHog.FeatureFlagsTest do "$feature_flag_reason": %{"code" => "condition_match"}, "$feature_flag_request_id": "req-xyz", "$feature_flag_evaluated_at": 1_700_000_000, + locally_evaluated: false, "$session_id": "session-123", "$lib": "posthog-elixir", "$lib_version": PostHog.Lib.version(), From 089ae9e44f814f7fc5a6038e9555d2a7d8b03884 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto <5731772+marandaneto@users.noreply.github.com> Date: Wed, 26 Aug 2026 03:37:05 +0700 Subject: [PATCH 6/9] fix(flags): bound missing-key probe work Preserve exact remote flag scopes while bypassing per-key coordination and negative caching for oversized missing-key requests. Keep retained negative knowledge updates bounded and linear. --- lib/posthog/feature_flags.ex | 4 ++- .../feature_flags/definition_loader.ex | 7 +++-- .../missing_key_knowledge_test.exs | 29 +++++++++++++++++++ 3 files changed, 36 insertions(+), 4 deletions(-) diff --git a/lib/posthog/feature_flags.ex b/lib/posthog/feature_flags.ex index f6c53ef..94e1292 100644 --- a/lib/posthog/feature_flags.ex +++ b/lib/posthog/feature_flags.ex @@ -3,6 +3,8 @@ defmodule PostHog.FeatureFlags do Convenience functions to work with Feature Flags API """ + @missing_probe_coordination_limit 1_000 + @doc """ Make request to [`/flags`](https://posthog.com/docs/api/flags) API. @@ -304,7 +306,7 @@ defmodule PostHog.FeatureFlags do absent = absent_definition_keys(requested_keys, loader_state.definitions) probe_keys = absent |> MapSet.intersection(unresolved) |> MapSet.to_list() - if probe_keys == [] do + if probe_keys == [] or length(probe_keys) > @missing_probe_coordination_limit do remote_snapshot( name, distinct_id, diff --git a/lib/posthog/feature_flags/definition_loader.ex b/lib/posthog/feature_flags/definition_loader.ex index 375ec4e..0a48c75 100644 --- a/lib/posthog/feature_flags/definition_loader.ex +++ b/lib/posthog/feature_flags/definition_loader.ex @@ -584,10 +584,11 @@ defmodule PostHog.FeatureFlags.DefinitionLoader do |> MapSet.difference(returned) |> MapSet.to_list() |> Enum.sort() + |> Enum.take(-@negative_knowledge_capacity) - Enum.reduce(omissions, {order, keys}, fn key, {order, keys} -> - {Enum.reject(order, &(&1 == key)) ++ [key], MapSet.put(keys, key)} - end) + omission_keys = MapSet.new(omissions) + order = Enum.reject(order, &MapSet.member?(omission_keys, &1)) ++ omissions + {order, MapSet.union(keys, omission_keys)} else {order, keys} end diff --git a/test/posthog/feature_flags/missing_key_knowledge_test.exs b/test/posthog/feature_flags/missing_key_knowledge_test.exs index 5422be3..32f5413 100644 --- a/test/posthog/feature_flags/missing_key_knowledge_test.exs +++ b/test/posthog/feature_flags/missing_key_knowledge_test.exs @@ -320,6 +320,35 @@ defmodule PostHog.FeatureFlags.MissingKeyKnowledgeTest do DefinitionLoader.evaluation_state(__MODULE__.Restart, ["missing"]).known_missing end + test "oversized missing-key scopes preserve the remote scope without retaining omissions" do + owner = self() + keys = for index <- 0..1_000, do: "key-#{index}" + + expect(PostHog.API.Mock, :request, 3, fn :stub_client, method, path, opts -> + case {method, path} do + {:get, "/flags/definitions"} -> + {:ok, %{status: 200, body: envelope(), headers: %{}}} + + {:post, "/flags"} -> + send(owner, {:oversized_scope, opts[:json].flag_keys_to_evaluate}) + {:ok, %{status: 200, body: %{"flags" => %{}}}} + end + end) + + start_instance(__MODULE__.Oversized) + + assert {:ok, first} = scoped(__MODULE__.Oversized, keys) + assert Evaluations.keys(first) == [] + assert_receive {:oversized_scope, ^keys} + + assert DefinitionLoader.evaluation_state(__MODULE__.Oversized, keys).known_missing == + MapSet.new() + + assert {:ok, second} = scoped(__MODULE__.Oversized, keys) + assert Evaluations.keys(second) == [] + assert_receive {:oversized_scope, ^keys} + end + test "negative knowledge is bounded, returned keys are removed, and stale generations are discarded" do expect(PostHog.API.Mock, :request, fn :stub_client, :get, "/flags/definitions", _opts -> {:ok, %{status: 200, body: envelope(), headers: %{}}} From 8c2aebce0a7014e379ac4a1ace17a1c708e1fc83 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Wed, 26 Aug 2026 08:48:32 +0200 Subject: [PATCH 7/9] fix(flags): align local evaluation with service semantics --- lib/posthog/feature_flags.ex | 30 ++++++++-- .../feature_flags/definition_loader.ex | 13 ++--- lib/posthog/feature_flags/evaluations.ex | 12 +++- lib/posthog/feature_flags/local_evaluator.ex | 2 +- .../local_evaluation_integration_test.exs | 29 +++++++++- .../feature_flags/local_evaluator_test.exs | 20 +++++++ .../missing_key_knowledge_test.exs | 57 ++++++++++++++++++- test/posthog/feature_flags_test.exs | 16 ++++++ 8 files changed, 162 insertions(+), 17 deletions(-) diff --git a/lib/posthog/feature_flags.ex b/lib/posthog/feature_flags.ex index 94e1292..0fa7d34 100644 --- a/lib/posthog/feature_flags.ex +++ b/lib/posthog/feature_flags.ex @@ -445,14 +445,28 @@ defmodule PostHog.FeatureFlags do defp build_remote_results(flags, response_body) do Enum.reduce(flags, {%{}, false}, fn - {key, flag_data}, {results, malformed?} when is_binary(key) and is_map(flag_data) -> - {Map.put(results, key, build_result(key, flag_data, response_body)), malformed?} + {key, flag_data}, {results, malformed?} -> + case build_result_safely(key, flag_data, response_body) do + {:ok, result} -> {Map.put(results, key, result), malformed?} + :error -> {results, true} + end _entry, {results, _malformed?} -> {results, true} end) end + defp build_result_safely(key, flag_data, response_body) + when is_binary(key) and is_map(flag_data) do + {:ok, build_result(key, flag_data, response_body)} + rescue + _exception -> :error + catch + _kind, _reason -> :error + end + + defp build_result_safely(_key, _flag_data, _response_body), do: :error + defp invalid_flags_response, do: {:error, %RuntimeError{message: "invalid response from PostHog /flags endpoint"}} @@ -860,9 +874,8 @@ defmodule PostHog.FeatureFlags do {:ok, nil, %{}} else case flags(name, Map.delete(body, :only_evaluate_locally)) do - {:ok, %{body: %{"flags" => %{^flag_name => flag_data}} = response_body}} - when is_map(flag_data) -> - {:ok, build_result(flag_name, flag_data, response_body), response_body} + {:ok, %{body: %{"flags" => %{^flag_name => flag_data}} = response_body}} -> + single_remote_result(flag_name, flag_data, response_body) {:ok, %{body: %{"flags" => _} = response_body}} -> {:ok, nil, response_body} @@ -873,6 +886,13 @@ defmodule PostHog.FeatureFlags do end end + defp single_remote_result(flag_name, flag_data, response_body) do + case build_result_safely(flag_name, flag_data, response_body) do + {:ok, result} -> {:ok, result, response_body} + :error -> {:ok, nil, response_body} + end + end + defp maybe_log_feature_flag_usage(send_event, name, distinct_id, flag_result, groups) do if send_event do log_feature_flag_usage(name, distinct_id, flag_result, [], groups) diff --git a/lib/posthog/feature_flags/definition_loader.ex b/lib/posthog/feature_flags/definition_loader.ex index 0a48c75..3a1d6a9 100644 --- a/lib/posthog/feature_flags/definition_loader.ex +++ b/lib/posthog/feature_flags/definition_loader.ex @@ -75,7 +75,7 @@ defmodule PostHog.FeatureFlags.DefinitionLoader do @doc false def update_negative_knowledge(name, generation, requested_keys, returned_keys, clean?) do - GenServer.call( + GenServer.cast( via(name), {:update_negative_knowledge, generation, requested_keys, returned_keys, clean?} ) @@ -156,23 +156,22 @@ defmodule PostHog.FeatureFlags.DefinitionLoader do def handle_call(:ready?, _from, state), do: {:reply, not is_nil(state.definitions), state} def handle_call(:refresh, _from, state), do: {:reply, :ok, refresh_and_schedule(state)} - def handle_call( + @impl GenServer + def handle_cast( {:update_negative_knowledge, generation, requested, returned, clean?}, - _from, %{definition_generation: generation} = state ) do state = state |> update_retained_missing(requested, returned, clean?) |> publish_evaluation_state() - {:reply, :ok, state} + {:noreply, state} end - def handle_call( + def handle_cast( {:update_negative_knowledge, _generation, _requested, _returned, _clean?}, - _from, state ), - do: {:reply, :stale_generation, state} + do: {:noreply, state} @impl GenServer def handle_info({:refresh, generation}, %{timer_generation: generation} = state), diff --git a/lib/posthog/feature_flags/evaluations.ex b/lib/posthog/feature_flags/evaluations.ex index 0669ea6..5510e9f 100644 --- a/lib/posthog/feature_flags/evaluations.ex +++ b/lib/posthog/feature_flags/evaluations.ex @@ -305,10 +305,20 @@ defmodule PostHog.FeatureFlags.Evaluations do defp log(%__MODULE__{distinct_id: ""}, _result, _extra_errors), do: :ok defp log( - %__MODULE__{supervisor_name: name, distinct_id: distinct_id, groups: groups}, + %__MODULE__{ + supervisor_name: name, + distinct_id: distinct_id, + groups: groups, + errors_while_computing: snapshot_error? + }, %Result{} = result, extra_errors ) do + result = %{ + result + | errors_while_computing: result.errors_while_computing or snapshot_error? + } + PostHog.FeatureFlags.log_feature_flag_usage(name, distinct_id, result, extra_errors, groups) end diff --git a/lib/posthog/feature_flags/local_evaluator.ex b/lib/posthog/feature_flags/local_evaluator.ex index 3e42aba..715020e 100644 --- a/lib/posthog/feature_flags/local_evaluator.ex +++ b/lib/posthog/feature_flags/local_evaluator.ex @@ -775,7 +775,7 @@ defmodule PostHog.FeatureFlags.LocalEvaluator do defp boolean_result(false), do: :no_match defp case_insensitive_equal?(left, right), - do: ascii_downcase(to_string(left)) == ascii_downcase(to_string(right)) + do: String.downcase(to_string(left)) == String.downcase(to_string(right)) defp ascii_downcase(value) do for <>, into: "" do diff --git a/test/posthog/feature_flags/local_evaluation_integration_test.exs b/test/posthog/feature_flags/local_evaluation_integration_test.exs index 334e465..b349f2c 100644 --- a/test/posthog/feature_flags/local_evaluation_integration_test.exs +++ b/test/posthog/feature_flags/local_evaluation_integration_test.exs @@ -140,6 +140,33 @@ defmodule PostHog.FeatureFlags.LocalEvaluationIntegrationTest do assert snapshot.flags["extra"].enabled end + test "snapshot-level remote errors are logged for locally resolved flags" do + unknown = flag("unknown", [%{"key" => "x", "operator" => "future", "value" => 1}]) + + expect(PostHog.API.Mock, :request, 2, fn + :stub_client, :get, "/flags/definitions", _opts -> + {:ok, %{status: 200, body: envelope([flag("local"), unknown]), headers: %{}}} + + :stub_client, :post, "/flags", _opts -> + {:ok, + %{ + status: 200, + body: %{ + "flags" => %{"unknown" => %{"enabled" => true}}, + "errorsWhileComputingFlags" => true + } + }} + end) + + start_instance(__MODULE__.SnapshotErrors) + assert {:ok, snapshot} = FeatureFlags.evaluate_flags(__MODULE__.SnapshotErrors, "user") + assert snapshot.errors_while_computing + assert Evaluations.enabled?(snapshot, "local") + + assert [%{properties: properties}] = PostHog.Test.all_captured(__MODULE__.SnapshotErrors) + assert properties[:"$feature_flag_error"] == "errors_while_computing_flags" + end + test "explicit scope is preserved and local-only returns partial results with no request" do missing_property = flag("needs-server", [%{"key" => "plan", "value" => "pro"}]) @@ -205,7 +232,7 @@ defmodule PostHog.FeatureFlags.LocalEvaluationIntegrationTest do body: %{ "flags" => %{ "remote" => %{"enabled" => true}, - "broken" => nil + "broken" => %{"enabled" => true, "metadata" => "bad"} } } }} diff --git a/test/posthog/feature_flags/local_evaluator_test.exs b/test/posthog/feature_flags/local_evaluator_test.exs index ad12d05..cc1f488 100644 --- a/test/posthog/feature_flags/local_evaluator_test.exs +++ b/test/posthog/feature_flags/local_evaluator_test.exs @@ -121,6 +121,15 @@ defmodule PostHog.FeatureFlags.LocalEvaluatorTest do end end + test "exact operators use Unicode lowercasing like the flags service" do + exact = %{"key" => "tier", "operator" => "exact", "value" => "ÉLITE"} + is_not = %{exact | "operator" => "is_not"} + context = %{person_properties: %{tier: "élite"}} + + assert evaluate(flag("exact", [exact]), context).results["exact"].enabled + refute evaluate(flag("is-not", [is_not]), context).results["is-not"].enabled + end + test "canonical property operators return definitive false for non-matches" do cases = [ {"exact", "goodbye", "hello"}, @@ -157,6 +166,17 @@ defmodule PostHog.FeatureFlags.LocalEvaluatorTest do end end + test "numeric operators leave semver-shaped strings inconclusive" do + property = %{"key" => "version", "operator" => "gt", "value" => "2.0.0"} + + result = + evaluate(flag("numeric-semver", [property]), %{ + person_properties: %{version: "2.5.0"} + }) + + assert MapSet.member?(result.unresolved, "numeric-semver") + end + test "semver normalization matches maintained server SDKs" do parity = [ {" V1.2.3-beta+build ", "1.2.3", true}, diff --git a/test/posthog/feature_flags/missing_key_knowledge_test.exs b/test/posthog/feature_flags/missing_key_knowledge_test.exs index 32f5413..000508e 100644 --- a/test/posthog/feature_flags/missing_key_knowledge_test.exs +++ b/test/posthog/feature_flags/missing_key_knowledge_test.exs @@ -43,6 +43,11 @@ defmodule PostHog.FeatureFlags.MissingKeyKnowledgeTest do FeatureFlags.evaluate_flags(name, %{distinct_id: distinct_id, flag_keys: keys}) end + defp sync_loader(name) do + :sys.get_state(PostHog.Registry.via(name, DefinitionLoader)) + :ok + end + test "clean omission is retained sequentially and successful refresh invalidates it" do owner = self() {:ok, count} = Agent.start_link(fn -> 0 end) @@ -74,6 +79,7 @@ defmodule PostHog.FeatureFlags.MissingKeyKnowledgeTest do assert {:ok, first} = scoped(__MODULE__.Sequential, ["missing"]) assert Evaluations.keys(first) == [] assert_receive :remote_probe + sync_loader(__MODULE__.Sequential) assert {:ok, second} = scoped(__MODULE__.Sequential, ["missing"]) assert Evaluations.keys(second) == [] @@ -113,6 +119,43 @@ defmodule PostHog.FeatureFlags.MissingKeyKnowledgeTest do assert {:error, %RuntimeError{message: "offline"}} = scoped(__MODULE__.Failure, ["missing"]) end + test "fallback does not wait for negative bookkeeping behind a stalled refresh" do + owner = self() + {:ok, definition_calls} = Agent.start_link(fn -> 0 end) + + expect(PostHog.API.Mock, :request, 3, fn :stub_client, method, path, _opts -> + case {method, path} do + {:get, "/flags/definitions"} -> + case Agent.get_and_update(definition_calls, &{&1, &1 + 1}) do + 0 -> + {:ok, %{status: 200, body: envelope(), headers: %{}}} + + 1 -> + send(owner, {:refresh_started, self()}) + receive do: (:release_refresh -> :ok) + {:ok, %{status: 304, body: nil, headers: %{}}} + end + + {:post, "/flags"} -> + {:ok, %{status: 200, body: %{"flags" => %{}}}} + end + end) + + start_instance(__MODULE__.NonBlocking) + refresh = Task.async(fn -> DefinitionLoader.refresh(__MODULE__.NonBlocking) end) + assert_receive {:refresh_started, refresh_worker} + + fallback = Task.async(fn -> scoped(__MODULE__.NonBlocking, ["missing"]) end) + fallback_result = Task.yield(fallback, 500) + + send(refresh_worker, :release_refresh) + assert :ok = Task.await(refresh) + + if is_nil(fallback_result), do: Task.await(fallback) + assert {:ok, {:ok, snapshot}} = fallback_result + assert Evaluations.keys(snapshot) == [] + end + test "dirty omissions do not suppress retries and returned keys remain context specific" do {:ok, counter} = Agent.start_link(fn -> 0 end) @@ -269,6 +312,7 @@ defmodule PostHog.FeatureFlags.MissingKeyKnowledgeTest do start_instance(__MODULE__.Ordering) assert {:ok, _snapshot} = scoped(__MODULE__.Ordering, ["a"]) + sync_loader(__MODULE__.Ordering) assert MapSet.member?( DefinitionLoader.evaluation_state(__MODULE__.Ordering, ["a"]).known_missing, @@ -280,9 +324,11 @@ defmodule PostHog.FeatureFlags.MissingKeyKnowledgeTest do assert {:ok, positive} = scoped(__MODULE__.Ordering, ["a", "c"], "positive") assert positive.flags["a"].enabled + sync_loader(__MODULE__.Ordering) send(delayed_worker, :release_delayed_omission) assert {:ok, _snapshot} = Task.await(delayed) + sync_loader(__MODULE__.Ordering) refute MapSet.member?( DefinitionLoader.evaluation_state(__MODULE__.Ordering, ["a"]).known_missing, @@ -307,7 +353,7 @@ defmodule PostHog.FeatureFlags.MissingKeyKnowledgeTest do refute new_generation == old_generation - assert :stale_generation = + assert :ok = DefinitionLoader.update_negative_knowledge( __MODULE__.Restart, old_generation, @@ -316,6 +362,8 @@ defmodule PostHog.FeatureFlags.MissingKeyKnowledgeTest do true ) + sync_loader(__MODULE__.Restart) + assert MapSet.new() == DefinitionLoader.evaluation_state(__MODULE__.Restart, ["missing"]).known_missing end @@ -367,6 +415,7 @@ defmodule PostHog.FeatureFlags.MissingKeyKnowledgeTest do true ) + sync_loader(__MODULE__.Capacity) retained = DefinitionLoader.evaluation_state(__MODULE__.Capacity, keys).known_missing assert MapSet.size(retained) == 1_000 refute MapSet.member?(retained, hd(keys)) @@ -383,6 +432,8 @@ defmodule PostHog.FeatureFlags.MissingKeyKnowledgeTest do false ) + sync_loader(__MODULE__.Capacity) + refute MapSet.member?( DefinitionLoader.evaluation_state(__MODULE__.Capacity, [returned]).known_missing, returned @@ -394,7 +445,7 @@ defmodule PostHog.FeatureFlags.MissingKeyKnowledgeTest do DefinitionLoader.refresh(__MODULE__.Capacity) - assert :stale_generation = + assert :ok = DefinitionLoader.update_negative_knowledge( __MODULE__.Capacity, initial.generation, @@ -403,6 +454,8 @@ defmodule PostHog.FeatureFlags.MissingKeyKnowledgeTest do true ) + sync_loader(__MODULE__.Capacity) + assert MapSet.size(DefinitionLoader.evaluation_state(__MODULE__.Capacity, keys).known_missing) == 0 end diff --git a/test/posthog/feature_flags_test.exs b/test/posthog/feature_flags_test.exs index d562a85..f26a05a 100644 --- a/test/posthog/feature_flags_test.exs +++ b/test/posthog/feature_flags_test.exs @@ -698,6 +698,22 @@ defmodule PostHog.FeatureFlagsTest do }} = FeatureFlags.get_feature_flag_result("myflag", "foo") end + test "treats malformed nested flag metadata as a missing result" do + expect(API.Mock, :request, fn _client, _method, _url, _opts -> + {:ok, + %{ + status: 200, + body: %{ + "flags" => %{ + "myflag" => %{"enabled" => true, "metadata" => "bad"} + } + } + }} + end) + + assert {:ok, nil} = FeatureFlags.get_feature_flag_result("myflag", "foo") + end + test "returns disabled result when flag not enabled" do expect(API.Mock, :request, fn _client, _method, _url, _opts -> {:ok, From 335c3ebd85a975ea513f1b9cb1d9ea98246fbece Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Wed, 26 Aug 2026 15:01:30 +0200 Subject: [PATCH 8/9] fix(flags): redact secrets and report local reason --- lib/posthog/api.ex | 9 +++++-- lib/posthog/config.ex | 26 +++++++++++++++++++ lib/posthog/feature_flags.ex | 4 +++ lib/posthog/feature_flags/local_evaluator.ex | 1 + lib/posthog/feature_flags/result.ex | 4 +-- lib/posthog/supervisor.ex | 2 +- test/posthog/config_test.exs | 5 +++- .../local_evaluation_integration_test.exs | 2 ++ test/posthog/supervisor_test.exs | 20 ++++++++++++++ 9 files changed, 67 insertions(+), 6 deletions(-) diff --git a/lib/posthog/api.ex b/lib/posthog/api.ex index a6d71b8..bcdb79f 100644 --- a/lib/posthog/api.ex +++ b/lib/posthog/api.ex @@ -14,9 +14,14 @@ defmodule PostHog.API do ) end - def flag_definitions(%__MODULE__.Client{} = client, project_api_key, secret_key, etag) do + def flag_definitions( + %__MODULE__.Client{} = client, + project_api_key, + %PostHog.Config.Secret{} = secret_key, + etag + ) do headers = - [{"authorization", "Bearer #{secret_key}"}] + [{"authorization", "Bearer #{PostHog.Config.Secret.reveal(secret_key)}"}] |> then(fn headers -> if is_binary(etag), do: [{"if-none-match", etag} | headers], else: headers end) diff --git a/lib/posthog/config.ex b/lib/posthog/config.ex index 010cfbb..56bdcf0 100644 --- a/lib/posthog/config.ex +++ b/lib/posthog/config.ex @@ -1,6 +1,28 @@ defmodule PostHog.Config do require Logger + defmodule Secret do + @moduledoc false + @enforce_keys [:value] + defstruct [:value] + + @type t :: %__MODULE__{value: String.t()} + + @doc false + @spec new(String.t()) :: t() + def new(value) when is_binary(value), do: %__MODULE__{value: value} + + @doc false + @spec reveal(t()) :: String.t() + def reveal(%__MODULE__{value: value}), do: value + end + + defimpl Inspect, for: Secret do + import Inspect.Algebra + + def inspect(_secret, _opts), do: concat(["#PostHog.Config.Secret"]) + end + @default_api_host "https://us.i.posthog.com" @shared_schema [ @@ -292,6 +314,10 @@ defmodule PostHog.Config do final_config = config + |> Map.update!(:secret_key, fn + nil -> nil + secret_key -> Secret.new(secret_key) + end) |> Map.put(:api_client, client) |> Map.put(:enabled, not api_key_blank?) |> Map.put( diff --git a/lib/posthog/feature_flags.ex b/lib/posthog/feature_flags.ex index 0fa7d34..5074de2 100644 --- a/lib/posthog/feature_flags.ex +++ b/lib/posthog/feature_flags.ex @@ -519,6 +519,10 @@ defmodule PostHog.FeatureFlags do defp acquire_missing_probe_locks([key | rest], name, callback) do lock = {{__MODULE__, :missing_probe, name, key}, self()} + + # The lock intentionally spans the remote probe. Overlapping evaluations must + # wait for its existence result before deciding whether another fallback is + # needed; sorted keys keep multi-key probes from deadlocking each other. :global.trans(lock, fn -> acquire_missing_probe_locks(rest, name, callback) end) end diff --git a/lib/posthog/feature_flags/local_evaluator.ex b/lib/posthog/feature_flags/local_evaluator.ex index 715020e..477e312 100644 --- a/lib/posthog/feature_flags/local_evaluator.ex +++ b/lib/posthog/feature_flags/local_evaluator.ex @@ -754,6 +754,7 @@ defmodule PostHog.FeatureFlags.LocalEvaluator do version: value(flag, "version"), has_experiment: boolean_or_nil(value(flag, "has_experiment")), minimal_flag_called_events: definitions.minimal_flag_called_events, + reason: "Evaluated locally", locally_evaluated: true } end diff --git a/lib/posthog/feature_flags/result.ex b/lib/posthog/feature_flags/result.ex index 86f7a08..18d8de5 100644 --- a/lib/posthog/feature_flags/result.ex +++ b/lib/posthog/feature_flags/result.ex @@ -10,7 +10,7 @@ defmodule PostHog.FeatureFlags.Result do - `payload` - The JSON payload configured for this flag/variant (nil if not set) - `id` - Numeric flag ID from the PostHog backend (when available) - `version` - Flag version from the PostHog backend (when available) - - `reason` - Reason map describing why this evaluation produced its value + - `reason` - Reason describing why this evaluation produced its value - `request_id` - Request ID returned by the `/flags` endpoint (useful for experiment exposure tracking) - `evaluated_at` - Server-side evaluation timestamp from the response - `has_experiment` - Whether the flag is linked to an experiment. `nil` when @@ -74,7 +74,7 @@ defmodule PostHog.FeatureFlags.Result do payload: json(), id: integer() | nil, version: integer() | nil, - reason: map() | nil, + reason: map() | String.t() | nil, request_id: String.t() | nil, evaluated_at: integer() | nil, has_experiment: boolean() | nil, diff --git a/lib/posthog/supervisor.ex b/lib/posthog/supervisor.ex index b1dafe3..8dd6cf6 100644 --- a/lib/posthog/supervisor.ex +++ b/lib/posthog/supervisor.ex @@ -72,7 +72,7 @@ defmodule PostHog.Supervisor do %{ enabled: true, enable_local_evaluation: true, - secret_key: secret_key + secret_key: %PostHog.Config.Secret{value: secret_key} } = config, callers ) diff --git a/test/posthog/config_test.exs b/test/posthog/config_test.exs index 3d72e7e..c2ec8b8 100644 --- a/test/posthog/config_test.exs +++ b/test/posthog/config_test.exs @@ -37,7 +37,10 @@ defmodule PostHog.ConfigTest do api_client_module: PostHog.API.Mock ) - assert config.secret_key == "secret-key" + assert %PostHog.Config.Secret{} = config.secret_key + assert PostHog.Config.Secret.reveal(config.secret_key) == "secret-key" + refute inspect(config) =~ "secret-key" + assert inspect(config) =~ "#PostHog.Config.Secret" assert config.enable_local_evaluation assert config.feature_flags_poll_interval_ms == 30_000 assert config.flag_definition_cache_provider == nil diff --git a/test/posthog/feature_flags/local_evaluation_integration_test.exs b/test/posthog/feature_flags/local_evaluation_integration_test.exs index b349f2c..2d58c27 100644 --- a/test/posthog/feature_flags/local_evaluation_integration_test.exs +++ b/test/posthog/feature_flags/local_evaluation_integration_test.exs @@ -69,6 +69,7 @@ defmodule PostHog.FeatureFlags.LocalEvaluationIntegrationTest do assert PostHog.Test.all_captured(__MODULE__.Local) == [] assert Evaluations.keys(snapshot) == ["boolean", "variant"] assert snapshot.flags["boolean"].enabled + assert snapshot.flags["boolean"].reason == "Evaluated locally" assert snapshot.flags["variant"].variant == "test" assert Evaluations.get_flag_payload(snapshot, "variant") == %{"answer" => 42} @@ -78,6 +79,7 @@ defmodule PostHog.FeatureFlags.LocalEvaluationIntegrationTest do assert event.event == "$feature_flag_called" assert event.properties[:"$feature_flag_id"] == 11 assert event.properties[:"$feature_flag_version"] == 3 + assert event.properties[:"$feature_flag_reason"] == "Evaluated locally" assert event.properties[:locally_evaluated] == true end diff --git a/test/posthog/supervisor_test.exs b/test/posthog/supervisor_test.exs index 9ec744f..0f257df 100644 --- a/test/posthog/supervisor_test.exs +++ b/test/posthog/supervisor_test.exs @@ -5,6 +5,26 @@ defmodule PostHog.SupervisorTest do @supervisor_name __MODULE__ + test "child specifications and startup failures redact the definition secret" do + secret = "phs_super-secret" + + config = + PostHog.Config.validate!( + api_key: "project-key", + secret_key: secret, + supervisor_name: __MODULE__.Redacted, + enable_local_evaluation: false + ) + + child_spec = PostHog.Supervisor.child_spec(config) + rendered_spec = inspect(child_spec, limit: :infinity) + rendered_failure = Exception.format_exit({:failed_to_start_child, child_spec}) + + refute rendered_spec =~ secret + refute rendered_failure =~ secret + assert rendered_spec =~ "#PostHog.Config.Secret" + end + test "disabled config starts without senders and captures as no-op" do {config, _log} = with_log(fn -> From 8a43ee5d8a95ab7a85dbb607c763822fd5aea1f0 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Wed, 26 Aug 2026 17:48:23 +0200 Subject: [PATCH 9/9] fix(flags): publish missing-key knowledge before unlock --- .../feature_flags/definition_loader.ex | 204 ++++++++++++------ lib/posthog/supervisor.ex | 6 +- .../missing_key_knowledge_test.exs | 14 +- 3 files changed, 153 insertions(+), 71 deletions(-) diff --git a/lib/posthog/feature_flags/definition_loader.ex b/lib/posthog/feature_flags/definition_loader.ex index 3a1d6a9..381af4c 100644 --- a/lib/posthog/feature_flags/definition_loader.ex +++ b/lib/posthog/feature_flags/definition_loader.ex @@ -25,8 +25,141 @@ defmodule PostHog.FeatureFlags.DefinitionLoader do do: concat(["#PostHog.FeatureFlags.DefinitionLoader.Config"]) end + defmodule NegativeKnowledge do + @moduledoc false + use GenServer + + @capacity 1_000 + + def child_spec(opts) do + name = Keyword.fetch!(opts, :supervisor_name) + + %{ + id: __MODULE__, + start: {__MODULE__, :start_link, [name]} + } + end + + def start_link(name), do: GenServer.start_link(__MODULE__, name, name: via(name)) + + def reset(name, generation), + do: GenServer.call(via(name), {:reset, generation}, :infinity) + + def update(name, generation, requested, returned, clean?), + do: + GenServer.call( + via(name), + {:update, generation, requested, returned, clean?}, + :infinity + ) + + def known(name, generation, keys) do + state = cached(name) + + if state.generation == generation do + keys |> MapSet.new() |> MapSet.intersection(state.keys) + else + MapSet.new() + end + end + + @impl GenServer + def init(name) do + state = %{name: name, generation: current_generation(name), keys: MapSet.new(), order: []} + {:ok, publish(state)} + end + + @impl GenServer + def handle_call({:reset, generation}, _from, state) do + state = %{state | generation: generation, keys: MapSet.new(), order: []} + {:reply, :ok, publish(state)} + end + + def handle_call( + {:update, generation, requested, returned, clean?}, + _from, + %{generation: generation} = state + ) do + state = + state + |> Map.put(:generation, generation) + |> update_retained_missing(requested, returned, clean?) + |> publish() + + {:reply, :ok, state} + end + + def handle_call({:update, _generation, _requested, _returned, _clean?}, _from, state), + do: {:reply, :ok, state} + + defp cached(name) do + registry = PostHog.Registry.registry_name(name) + + case Registry.lookup(registry, __MODULE__) do + [{_pid, %{generation: _generation} = state}] -> state + _other -> empty() + end + rescue + ArgumentError -> empty() + end + + defp current_generation(name) do + registry = PostHog.Registry.registry_name(name) + + case Registry.lookup(registry, PostHog.FeatureFlags.DefinitionLoader) do + [{_pid, %{generation: generation}}] -> generation + _other -> nil + end + rescue + ArgumentError -> nil + end + + defp empty, do: %{generation: nil, keys: MapSet.new()} + + defp publish(state) do + registry = PostHog.Registry.registry_name(state.name) + cached = Map.take(state, [:generation, :keys]) + Registry.update_value(registry, __MODULE__, fn _old -> cached end) + state + end + + defp update_retained_missing(state, requested, returned, clean?) do + returned = MapSet.new(returned) + + order = Enum.reject(state.order, &MapSet.member?(returned, &1)) + keys = MapSet.difference(state.keys, returned) + + {order, keys} = + if clean? do + omissions = + requested + |> MapSet.new() + |> MapSet.difference(returned) + |> MapSet.to_list() + |> Enum.sort() + |> Enum.take(-@capacity) + + omission_keys = MapSet.new(omissions) + order = Enum.reject(order, &MapSet.member?(omission_keys, &1)) ++ omissions + {order, MapSet.union(keys, omission_keys)} + else + {order, keys} + end + + overflow = max(length(order) - @capacity, 0) + retained_order = Enum.drop(order, overflow) + + %{ + state + | order: retained_order, + keys: MapSet.intersection(keys, MapSet.new(retained_order)) + } + end + + defp via(name), do: PostHog.Registry.via(name, __MODULE__) + end + @max_quota_backoff_ms 60_000 - @negative_knowledge_capacity 1_000 @type snapshot :: %{ flags: [map()], @@ -69,16 +202,13 @@ defmodule PostHog.FeatureFlags.DefinitionLoader do %{ definitions: state.definitions, generation: state.generation, - known_missing: keys |> MapSet.new() |> MapSet.intersection(state.negative_keys) + known_missing: NegativeKnowledge.known(name, state.generation, keys) } end @doc false def update_negative_knowledge(name, generation, requested_keys, returned_keys, clean?) do - GenServer.cast( - via(name), - {:update_negative_knowledge, generation, requested_keys, returned_keys, clean?} - ) + NegativeKnowledge.update(name, generation, requested_keys, returned_keys, clean?) end defp readable_evaluation_state(name) do @@ -113,7 +243,6 @@ defmodule PostHog.FeatureFlags.DefinitionLoader do %{ definitions: nil, generation: nil, - negative_keys: MapSet.new(), initial_load_complete?: true } end @@ -128,8 +257,6 @@ defmodule PostHog.FeatureFlags.DefinitionLoader do state = %{ definitions: nil, definition_generation: {make_ref(), 0}, - negative_keys: MapSet.new(), - negative_order: [], etag: nil, loaded_at: nil, timer_ref: nil, @@ -139,6 +266,7 @@ defmodule PostHog.FeatureFlags.DefinitionLoader do config: config } + :ok = NegativeKnowledge.reset(config.supervisor_name, state.definition_generation) {:ok, publish_evaluation_state(state), {:continue, :initial_load}} end @@ -156,23 +284,6 @@ defmodule PostHog.FeatureFlags.DefinitionLoader do def handle_call(:ready?, _from, state), do: {:reply, not is_nil(state.definitions), state} def handle_call(:refresh, _from, state), do: {:reply, :ok, refresh_and_schedule(state)} - @impl GenServer - def handle_cast( - {:update_negative_knowledge, generation, requested, returned, clean?}, - %{definition_generation: generation} = state - ) do - state = - state |> update_retained_missing(requested, returned, clean?) |> publish_evaluation_state() - - {:noreply, state} - end - - def handle_cast( - {:update_negative_knowledge, _generation, _requested, _returned, _clean?}, - state - ), - do: {:noreply, state} - @impl GenServer def handle_info({:refresh, generation}, %{timer_generation: generation} = state), do: {:noreply, refresh_and_schedule(state)} @@ -232,7 +343,6 @@ defmodule PostHog.FeatureFlags.DefinitionLoader do %{ definitions: state.definitions, generation: state.definition_generation, - negative_keys: state.negative_keys, initial_load_complete?: state.initial_load_complete? } end @@ -388,12 +498,13 @@ defmodule PostHog.FeatureFlags.DefinitionLoader do end defp successful_definition_refresh(state, snapshot) do + generation = next_definition_generation(state.definition_generation) + :ok = NegativeKnowledge.reset(state.config.supervisor_name, generation) + %{ state | definitions: snapshot, - definition_generation: next_definition_generation(state.definition_generation), - negative_keys: MapSet.new(), - negative_order: [], + definition_generation: generation, loaded_at: DateTime.utc_now(), quota_backoff_ms: nil } @@ -569,39 +680,6 @@ defmodule PostHog.FeatureFlags.DefinitionLoader do defp provider_reason({:unexpected_return, _value}), do: "unexpected return" defp provider_reason(_reason), do: "error" - defp update_retained_missing(state, requested, returned, clean?) do - returned = MapSet.new(returned) - - order = Enum.reject(state.negative_order, &MapSet.member?(returned, &1)) - keys = MapSet.difference(state.negative_keys, returned) - - {order, keys} = - if clean? do - omissions = - requested - |> MapSet.new() - |> MapSet.difference(returned) - |> MapSet.to_list() - |> Enum.sort() - |> Enum.take(-@negative_knowledge_capacity) - - omission_keys = MapSet.new(omissions) - order = Enum.reject(order, &MapSet.member?(omission_keys, &1)) ++ omissions - {order, MapSet.union(keys, omission_keys)} - else - {order, keys} - end - - overflow = max(length(order) - @negative_knowledge_capacity, 0) - retained_order = Enum.drop(order, overflow) - - %{ - state - | negative_order: retained_order, - negative_keys: MapSet.intersection(keys, MapSet.new(retained_order)) - } - end - defp private_config(config) do struct!(Config, %{ api_client: config.api_client, diff --git a/lib/posthog/supervisor.ex b/lib/posthog/supervisor.ex index 8dd6cf6..78e43f1 100644 --- a/lib/posthog/supervisor.ex +++ b/lib/posthog/supervisor.ex @@ -77,7 +77,11 @@ defmodule PostHog.Supervisor do callers ) when is_binary(secret_key) and secret_key != "" do - [{PostHog.FeatureFlags.DefinitionLoader, config: config, callers: callers}] + [ + {PostHog.FeatureFlags.DefinitionLoader.NegativeKnowledge, + supervisor_name: config.supervisor_name}, + {PostHog.FeatureFlags.DefinitionLoader, config: config, callers: callers} + ] end defp definition_loader(_config, _callers), do: [] diff --git a/test/posthog/feature_flags/missing_key_knowledge_test.exs b/test/posthog/feature_flags/missing_key_knowledge_test.exs index 000508e..c6cbf54 100644 --- a/test/posthog/feature_flags/missing_key_knowledge_test.exs +++ b/test/posthog/feature_flags/missing_key_knowledge_test.exs @@ -119,7 +119,7 @@ defmodule PostHog.FeatureFlags.MissingKeyKnowledgeTest do assert {:error, %RuntimeError{message: "offline"}} = scoped(__MODULE__.Failure, ["missing"]) end - test "fallback does not wait for negative bookkeeping behind a stalled refresh" do + test "fallback publishes negative knowledge before releasing a probe during refresh" do owner = self() {:ok, definition_calls} = Agent.start_link(fn -> 0 end) @@ -133,7 +133,7 @@ defmodule PostHog.FeatureFlags.MissingKeyKnowledgeTest do 1 -> send(owner, {:refresh_started, self()}) receive do: (:release_refresh -> :ok) - {:ok, %{status: 304, body: nil, headers: %{}}} + {:ok, %{status: 503, body: nil, headers: %{}}} end {:post, "/flags"} -> @@ -146,14 +146,14 @@ defmodule PostHog.FeatureFlags.MissingKeyKnowledgeTest do assert_receive {:refresh_started, refresh_worker} fallback = Task.async(fn -> scoped(__MODULE__.NonBlocking, ["missing"]) end) - fallback_result = Task.yield(fallback, 500) + assert {:ok, {:ok, snapshot}} = Task.yield(fallback, 500) + assert Evaluations.keys(snapshot) == [] + + assert {:ok, repeated} = scoped(__MODULE__.NonBlocking, ["missing"], "second") + assert Evaluations.keys(repeated) == [] send(refresh_worker, :release_refresh) assert :ok = Task.await(refresh) - - if is_nil(fallback_result), do: Task.await(fallback) - assert {:ok, {:ok, snapshot}} = fallback_result - assert Evaluations.keys(snapshot) == [] end test "dirty omissions do not suppress retries and returned keys remain context specific" do