From c27cde9ab2337f025d9cab0e50c82889bbc07a29 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Wed, 24 Jun 2026 19:15:39 +0200 Subject: [PATCH 1/5] fix: dedupe feature flag called events by response --- .sampo/changesets/resolute-prince-akka.md | 5 +++ lib/posthog/feature_flags.ex | 4 +- lib/posthog/feature_flags/called_cache.ex | 44 +++++++++++++++++++ lib/posthog/supervisor.ex | 3 +- .../feature_flags/evaluations_test.exs | 21 ++++++++- 5 files changed, 73 insertions(+), 4 deletions(-) create mode 100644 .sampo/changesets/resolute-prince-akka.md create mode 100644 lib/posthog/feature_flags/called_cache.ex diff --git a/.sampo/changesets/resolute-prince-akka.md b/.sampo/changesets/resolute-prince-akka.md new file mode 100644 index 0000000..d15e887 --- /dev/null +++ b/.sampo/changesets/resolute-prince-akka.md @@ -0,0 +1,5 @@ +--- +hex/posthog: patch +--- + +Dedupe feature flag called events by response diff --git a/lib/posthog/feature_flags.ex b/lib/posthog/feature_flags.ex index 3e97cad..33d4ba2 100644 --- a/lib/posthog/feature_flags.ex +++ b/lib/posthog/feature_flags.ex @@ -620,7 +620,9 @@ defmodule PostHog.FeatureFlags do |> maybe_put(:"$feature_flag_payload", result.payload) |> maybe_put(:"$feature_flag_error", errors) - PostHog.capture(name, "$feature_flag_called", properties) + if PostHog.FeatureFlags.CalledCache.first_seen?(name, distinct_id, result.key, value) do + PostHog.capture(name, "$feature_flag_called", properties) + end if flag_missing? do :ok diff --git a/lib/posthog/feature_flags/called_cache.ex b/lib/posthog/feature_flags/called_cache.ex new file mode 100644 index 0000000..dfff431 --- /dev/null +++ b/lib/posthog/feature_flags/called_cache.ex @@ -0,0 +1,44 @@ +defmodule PostHog.FeatureFlags.CalledCache do + @moduledoc false + + use Agent + + @max_size 50_000 + + @spec start_link(keyword()) :: Agent.on_start() + def start_link(opts) do + supervisor_name = Keyword.fetch!(opts, :supervisor_name) + + Agent.start_link(fn -> MapSet.new() end, + name: PostHog.Registry.via(supervisor_name, __MODULE__) + ) + end + + @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} + + Agent.get_and_update(PostHog.Registry.via(supervisor_name, __MODULE__), fn seen -> + cond do + MapSet.member?(seen, key) -> + {false, seen} + + MapSet.size(seen) >= @max_size -> + {true, MapSet.new([key])} + + true -> + {true, MapSet.put(seen, key)} + end + end) + rescue + error in ArgumentError -> + if String.starts_with?(Exception.message(error), "unknown registry: ") do + true + else + reraise(error, __STACKTRACE__) + end + catch + :exit, _ -> true + end +end diff --git a/lib/posthog/supervisor.ex b/lib/posthog/supervisor.ex index 9a50539..ac0e6ec 100644 --- a/lib/posthog/supervisor.ex +++ b/lib/posthog/supervisor.ex @@ -57,7 +57,8 @@ defmodule PostHog.Supervisor do {Registry, keys: :unique, name: PostHog.Registry.registry_name(config.supervisor_name), - meta: [config: config]} + meta: [config: config]}, + {PostHog.FeatureFlags.CalledCache, supervisor_name: config.supervisor_name} ] ++ senders(config) ++ sources(config) Process.put(:"$callers", callers) diff --git a/test/posthog/feature_flags/evaluations_test.exs b/test/posthog/feature_flags/evaluations_test.exs index 406631f..85d7ad8 100644 --- a/test/posthog/feature_flags/evaluations_test.exs +++ b/test/posthog/feature_flags/evaluations_test.exs @@ -224,12 +224,29 @@ defmodule PostHog.FeatureFlags.EvaluationsTest do assert properties["$feature/variant-flag"] == "control" end - test "fires on every access (no dedup in this PR)", %{snapshot: snapshot} do + test "dedupes repeated access for the same flag value", %{snapshot: snapshot} do Evaluations.enabled?(snapshot, "boolean-flag") Evaluations.enabled?(snapshot, "boolean-flag") Evaluations.get_flag(snapshot, "boolean-flag") - assert length(all_captured()) == 3 + assert [%{event: "$feature_flag_called"}] = all_captured() + 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} + + FeatureFlags.log_feature_flag_usage(PostHog, "foo", true_result) + FeatureFlags.log_feature_flag_usage(PostHog, "foo", false_result) + FeatureFlags.log_feature_flag_usage(PostHog, "foo", true_result) + FeatureFlags.log_feature_flag_usage(PostHog, "foo", false_result) + + events = all_captured() + assert length(events) == 2 + + assert events + |> Enum.map(& &1.properties[:"$feature_flag_response"]) + |> Enum.sort() == [false, true] end end From 9764fbdf1aa39e8aa2d8dc8f96be4f5e3f789921 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Wed, 24 Jun 2026 19:36:06 +0200 Subject: [PATCH 2/5] address called cache review feedback --- lib/posthog/feature_flags/called_cache.ex | 51 ++++++++++++------- .../feature_flags/called_cache_test.exs | 36 +++++++++++++ 2 files changed, 69 insertions(+), 18 deletions(-) create mode 100644 test/posthog/feature_flags/called_cache_test.exs diff --git a/lib/posthog/feature_flags/called_cache.ex b/lib/posthog/feature_flags/called_cache.ex index dfff431..d9dea05 100644 --- a/lib/posthog/feature_flags/called_cache.ex +++ b/lib/posthog/feature_flags/called_cache.ex @@ -19,26 +19,41 @@ defmodule PostHog.FeatureFlags.CalledCache do def first_seen?(supervisor_name, distinct_id, flag_key, value) do key = {to_string(distinct_id), flag_key, value} - Agent.get_and_update(PostHog.Registry.via(supervisor_name, __MODULE__), fn seen -> - cond do - MapSet.member?(seen, key) -> - {false, seen} - - MapSet.size(seen) >= @max_size -> - {true, MapSet.new([key])} - - true -> - {true, MapSet.put(seen, key)} - end - end) - rescue - error in ArgumentError -> - if String.starts_with?(Exception.message(error), "unknown registry: ") do + case cache_pid(supervisor_name) do + nil -> true - else - reraise(error, __STACKTRACE__) - end + + pid -> + Agent.get_and_update(pid, fn seen -> + cond do + MapSet.member?(seen, key) -> + {false, seen} + + MapSet.size(seen) >= @max_size -> + # Intentionally flush instead of evicting individual entries to keep + # the hot path simple. Previously seen values may emit again after + # the cache rolls over. + {true, MapSet.new([key])} + + true -> + {true, MapSet.put(seen, key)} + end + end) + end catch :exit, _ -> true end + + defp cache_pid(supervisor_name) do + registry_name = PostHog.Registry.registry_name(supervisor_name) + + with registry_pid when is_pid(registry_pid) <- Process.whereis(registry_name), + [{pid, _value}] <- Registry.lookup(registry_name, __MODULE__) do + pid + else + _ -> nil + end + rescue + ArgumentError -> nil + end end diff --git a/test/posthog/feature_flags/called_cache_test.exs b/test/posthog/feature_flags/called_cache_test.exs new file mode 100644 index 0000000..4367655 --- /dev/null +++ b/test/posthog/feature_flags/called_cache_test.exs @@ -0,0 +1,36 @@ +defmodule PostHog.FeatureFlags.CalledCacheTest do + use PostHog.Case, async: true + + alias PostHog.FeatureFlags.CalledCache + + @max_size 50_000 + + setup :setup_supervisor + + test "returns true when the supervisor registry is not running" do + supervisor_name = __MODULE__.MissingSupervisor + + refute Process.whereis(PostHog.Registry.registry_name(supervisor_name)) + + assert CalledCache.first_seen?(supervisor_name, "user", "flag", true) + end + + test "flushes the cache when it reaches the maximum size", %{config: config} do + supervisor_name = config.supervisor_name + seed_key = {"seed-user", "flag", true} + + full_cache = + 1..(@max_size - 1) + |> Enum.map(&{"user-#{&1}", "flag", true}) + |> MapSet.new() + |> MapSet.put(seed_key) + + Agent.update(PostHog.Registry.via(supervisor_name, CalledCache), fn _seen -> full_cache end) + + refute CalledCache.first_seen?(supervisor_name, "seed-user", "flag", true) + + assert CalledCache.first_seen?(supervisor_name, "overflow-user", "flag", true) + assert CalledCache.first_seen?(supervisor_name, "seed-user", "flag", true) + refute CalledCache.first_seen?(supervisor_name, "overflow-user", "flag", true) + end +end From 98602d8c9af91a69f20a2883836e0baa3820da4d Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Wed, 24 Jun 2026 19:40:43 +0200 Subject: [PATCH 3/5] fix called cache credo nesting --- lib/posthog/feature_flags/called_cache.ex | 32 ++++++++++++----------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/lib/posthog/feature_flags/called_cache.ex b/lib/posthog/feature_flags/called_cache.ex index d9dea05..cee491e 100644 --- a/lib/posthog/feature_flags/called_cache.ex +++ b/lib/posthog/feature_flags/called_cache.ex @@ -24,26 +24,28 @@ defmodule PostHog.FeatureFlags.CalledCache do true pid -> - Agent.get_and_update(pid, fn seen -> - cond do - MapSet.member?(seen, key) -> - {false, seen} - - MapSet.size(seen) >= @max_size -> - # Intentionally flush instead of evicting individual entries to keep - # the hot path simple. Previously seen values may emit again after - # the cache rolls over. - {true, MapSet.new([key])} - - true -> - {true, MapSet.put(seen, key)} - end - end) + Agent.get_and_update(pid, &mark_seen(&1, key)) end catch :exit, _ -> true end + defp mark_seen(seen, key) do + cond do + MapSet.member?(seen, key) -> + {false, seen} + + MapSet.size(seen) >= @max_size -> + # Intentionally flush instead of evicting individual entries to keep + # the hot path simple. Previously seen values may emit again after + # the cache rolls over. + {true, MapSet.new([key])} + + true -> + {true, MapSet.put(seen, key)} + end + end + defp cache_pid(supervisor_name) do registry_name = PostHog.Registry.registry_name(supervisor_name) From be45ea0c277cd23e037169f5452bad43e4edce25 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Wed, 24 Jun 2026 22:36:37 +0200 Subject: [PATCH 4/5] use ets for called cache --- lib/posthog/feature_flags/called_cache.ex | 75 ++++++++++++------- .../feature_flags/called_cache_test.exs | 16 +++- 2 files changed, 59 insertions(+), 32 deletions(-) diff --git a/lib/posthog/feature_flags/called_cache.ex b/lib/posthog/feature_flags/called_cache.ex index cee491e..0ba41c6 100644 --- a/lib/posthog/feature_flags/called_cache.ex +++ b/lib/posthog/feature_flags/called_cache.ex @@ -1,61 +1,80 @@ defmodule PostHog.FeatureFlags.CalledCache do @moduledoc false - use Agent + use GenServer @max_size 50_000 - @spec start_link(keyword()) :: Agent.on_start() + @spec start_link(keyword()) :: GenServer.on_start() def start_link(opts) do supervisor_name = Keyword.fetch!(opts, :supervisor_name) - Agent.start_link(fn -> MapSet.new() end, + GenServer.start_link(__MODULE__, supervisor_name, name: PostHog.Registry.via(supervisor_name, __MODULE__) ) end + @impl GenServer + def init(supervisor_name) do + table = + supervisor_name + |> table_name() + |> :ets.new([ + :set, + :public, + :named_table, + read_concurrency: true, + write_concurrency: true + ]) + + {:ok, %{table: table}} + end + @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} + table = table_name(supervisor_name) - case cache_pid(supervisor_name) do - nil -> + case :ets.insert_new(table, {key}) do + true -> + rollover_if_full(supervisor_name, table, key) true - pid -> - Agent.get_and_update(pid, &mark_seen(&1, key)) + false -> + false end + rescue + ArgumentError -> true catch :exit, _ -> true end - defp mark_seen(seen, key) do - cond do - MapSet.member?(seen, key) -> - {false, seen} + @impl GenServer + def handle_call({:rollover, key}, _from, %{table: table} = state) do + if over_max_size?(table) do + # Intentionally flush instead of evicting individual entries to keep + # the hot path simple. Previously seen values may emit again after + # the cache rolls over. + :ets.delete_all_objects(table) + :ets.insert(table, {key}) + end - MapSet.size(seen) >= @max_size -> - # Intentionally flush instead of evicting individual entries to keep - # the hot path simple. Previously seen values may emit again after - # the cache rolls over. - {true, MapSet.new([key])} + {:reply, :ok, state} + end - true -> - {true, MapSet.put(seen, key)} + defp rollover_if_full(supervisor_name, table, key) do + if over_max_size?(table) do + GenServer.call(PostHog.Registry.via(supervisor_name, __MODULE__), {:rollover, key}) end end - defp cache_pid(supervisor_name) do - registry_name = PostHog.Registry.registry_name(supervisor_name) - - with registry_pid when is_pid(registry_pid) <- Process.whereis(registry_name), - [{pid, _value}] <- Registry.lookup(registry_name, __MODULE__) do - pid - else - _ -> nil + defp over_max_size?(table) do + case :ets.info(table, :size) do + size when is_integer(size) -> size > @max_size + _ -> false end - rescue - ArgumentError -> nil end + + defp table_name(supervisor_name), do: Module.concat(supervisor_name, CalledCacheTable) end diff --git a/test/posthog/feature_flags/called_cache_test.exs b/test/posthog/feature_flags/called_cache_test.exs index 4367655..58459bd 100644 --- a/test/posthog/feature_flags/called_cache_test.exs +++ b/test/posthog/feature_flags/called_cache_test.exs @@ -18,14 +18,14 @@ 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} + table = table(supervisor_name) full_cache = 1..(@max_size - 1) - |> Enum.map(&{"user-#{&1}", "flag", true}) - |> MapSet.new() - |> MapSet.put(seed_key) + |> Enum.map(&{{"user-#{&1}", "flag", true}}) + |> then(&[{seed_key} | &1]) - Agent.update(PostHog.Registry.via(supervisor_name, CalledCache), fn _seen -> full_cache end) + :ets.insert(table, full_cache) refute CalledCache.first_seen?(supervisor_name, "seed-user", "flag", true) @@ -33,4 +33,12 @@ defmodule PostHog.FeatureFlags.CalledCacheTest do assert CalledCache.first_seen?(supervisor_name, "seed-user", "flag", true) refute CalledCache.first_seen?(supervisor_name, "overflow-user", "flag", true) end + + defp table(supervisor_name) do + [{pid, _value}] = Registry.lookup(PostHog.Registry.registry_name(supervisor_name), CalledCache) + + pid + |> :sys.get_state() + |> Map.fetch!(:table) + end end From b2c04a743db9a2f4e3d90a941e7645eab276dbb4 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Wed, 24 Jun 2026 22:39:33 +0200 Subject: [PATCH 5/5] format called cache test --- test/posthog/feature_flags/called_cache_test.exs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/posthog/feature_flags/called_cache_test.exs b/test/posthog/feature_flags/called_cache_test.exs index 58459bd..1a4ba13 100644 --- a/test/posthog/feature_flags/called_cache_test.exs +++ b/test/posthog/feature_flags/called_cache_test.exs @@ -35,7 +35,8 @@ defmodule PostHog.FeatureFlags.CalledCacheTest do end defp table(supervisor_name) do - [{pid, _value}] = Registry.lookup(PostHog.Registry.registry_name(supervisor_name), CalledCache) + [{pid, _value}] = + Registry.lookup(PostHog.Registry.registry_name(supervisor_name), CalledCache) pid |> :sys.get_state()