fix: Dedupe feature flag called events by response

.sampo/changesets/resolute-prince-akka.md

@@ -0,0 +1,5 @@
+---
+hex/posthog: patch
+---
+
+Dedupe feature flag called events by response

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

lib/posthog/feature_flags/called_cache.ex

@@ -0,0 +1,80 @@
+defmodule PostHog.FeatureFlags.CalledCache do
+  @moduledoc false
+
+  use GenServer
+
+  @max_size 50_000
+
+  @spec start_link(keyword()) :: GenServer.on_start()
+  def start_link(opts) do
+    supervisor_name = Keyword.fetch!(opts, :supervisor_name)
+
+    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 :ets.insert_new(table, {key}) do
+      true ->
+        rollover_if_full(supervisor_name, table, key)
+        true
+
+      false ->
+        false
+    end
+  rescue
+    ArgumentError -> true
+  catch
+    :exit, _ -> true
+  end
+
+  @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
+
+    {:reply, :ok, state}
+  end
+
+  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 over_max_size?(table) do
+    case :ets.info(table, :size) do
+      size when is_integer(size) -> size > @max_size
+      _ -> false
+    end
+  end
+
+  defp table_name(supervisor_name), do: Module.concat(supervisor_name, CalledCacheTable)
+end

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)

test/posthog/feature_flags/called_cache_test.exs

@@ -0,0 +1,45 @@
+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}
+    table = table(supervisor_name)
+
+    full_cache =
+      1..(@max_size - 1)
+      |> Enum.map(&{{"user-#{&1}", "flag", true}})
+      |> then(&[{seed_key} | &1])
+
+    :ets.insert(table, full_cache)
+
+    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
+
+  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

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