From a0720ea57aea997f29484d032a3504d542156ef6 Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Fri, 15 May 2026 11:14:05 +0000 Subject: [PATCH 1/5] feat(tests): introduce a common wait_until helper --- test/sentry/application_test.exs | 21 ++------------- test/sentry/logger_handler/logs_test.exs | 20 -------------- test/support/test_helpers.ex | 34 +++++++++++++++++++++++- 3 files changed, 35 insertions(+), 40 deletions(-) diff --git a/test/sentry/application_test.exs b/test/sentry/application_test.exs index 15704672..bc49c3e8 100644 --- a/test/sentry/application_test.exs +++ b/test/sentry/application_test.exs @@ -1,6 +1,8 @@ defmodule Sentry.ApplicationTest do use ExUnit.Case, async: false + import Sentry.TestHelpers, only: [wait_until: 1] + require Logger describe "auto logger handler when enable_logs is true" do @@ -173,23 +175,4 @@ defmodule Sentry.ApplicationTest do {:ok, _} = Application.ensure_all_started(:sentry) end - - defp wait_until(condition_fn, timeout \\ 1000) do - end_time = System.monotonic_time(:millisecond) + timeout - wait_loop(condition_fn, end_time, 1) - end - - defp wait_loop(condition_fn, end_time, sleep_time) do - cond do - condition_fn.() -> - :ok - - System.monotonic_time(:millisecond) >= end_time -> - :timeout - - true -> - Process.sleep(sleep_time) - wait_loop(condition_fn, end_time, min(sleep_time * 2, 50)) - end - end end diff --git a/test/sentry/logger_handler/logs_test.exs b/test/sentry/logger_handler/logs_test.exs index 8d1fb3f3..0b28173d 100644 --- a/test/sentry/logger_handler/logs_test.exs +++ b/test/sentry/logger_handler/logs_test.exs @@ -389,24 +389,4 @@ defmodule Sentry.LoggerHandler.LogsTest do defp wait_for_buffer_stable(_buffer, expected_size, timeout \\ 1000) do wait_until(fn -> TelemetryProcessor.buffer_size(:log) == expected_size end, timeout) end - - defp wait_until(condition_fn, timeout) do - end_time = System.monotonic_time(:millisecond) + timeout - wait_until_loop(condition_fn, end_time, 1) - end - - defp wait_until_loop(condition_fn, end_time, sleep_time) do - cond do - condition_fn.() -> - :ok - - System.monotonic_time(:millisecond) >= end_time -> - :timeout - - true -> - Process.sleep(sleep_time) - next_sleep = min(sleep_time * 2, 50) - wait_until_loop(condition_fn, end_time, next_sleep) - end - end end diff --git a/test/support/test_helpers.ex b/test/support/test_helpers.ex index 26f7d973..0b61c54f 100644 --- a/test/support/test_helpers.ex +++ b/test/support/test_helpers.ex @@ -79,7 +79,10 @@ defmodule Sentry.TestHelpers do end) config = - [dsn: "http://public:secret@localhost:#{bypass.port}/1", finch_request_opts: [receive_timeout: 2000]] + [ + dsn: "http://public:secret@localhost:#{bypass.port}/1", + finch_request_opts: [receive_timeout: 2000] + ] |> Keyword.merge(extra_config) put_test_config(config) @@ -110,4 +113,33 @@ defmodule Sentry.TestHelpers do def collect_envelopes(ref, expected_count, opts \\ []), do: Sentry.Test.collect_envelopes(ref, expected_count, opts) + + @doc """ + Polls `condition_fn` until it returns a truthy value or `timeout` + (default 1000ms) elapses, using exponential backoff (1ms, doubling, + capped at 50ms). + + Returns `true` when the condition became truthy, `false` on timeout. + Use this to wait on asynchronous state (e.g. a GenServer processing a + `:DOWN`, a buffer draining) instead of a fixed `Process.sleep/1`. + """ + @spec wait_until((-> as_boolean(term())), timeout()) :: boolean() + def wait_until(condition_fn, timeout \\ 1000) when is_function(condition_fn, 0) do + end_time = System.monotonic_time(:millisecond) + timeout + wait_until_loop(condition_fn, end_time, 1) + end + + defp wait_until_loop(condition_fn, end_time, sleep_time) do + cond do + condition_fn.() -> + true + + System.monotonic_time(:millisecond) >= end_time -> + false + + true -> + Process.sleep(sleep_time) + wait_until_loop(condition_fn, end_time, min(sleep_time * 2, 50)) + end + end end From f663d47ad45efa065f31229d37213b4626be091a Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Wed, 13 May 2026 13:44:59 +0000 Subject: [PATCH 2/5] refa(tests): simplify ownership model via NimbleOwnership --- lib/sentry/application.ex | 4 +- lib/sentry/test.ex | 170 +++++++++++++++++++++--------- lib/sentry/test/registry.ex | 202 +++++++++++++++++++++++++++++------- test/sentry/test_test.exs | 4 +- 4 files changed, 293 insertions(+), 87 deletions(-) diff --git a/lib/sentry/application.ex b/lib/sentry/application.ex index b46cda8a..7f5ba009 100644 --- a/lib/sentry/application.ex +++ b/lib/sentry/application.ex @@ -36,8 +36,8 @@ defmodule Sentry.Application do if Config.test_mode?() do if Code.ensure_loaded?(NimbleOwnership) do [ - Sentry.Test.Registry, - {NimbleOwnership, name: Sentry.Test.OwnershipServer} + {NimbleOwnership, name: Sentry.Test.OwnershipServer}, + Sentry.Test.Registry ] else [Sentry.Test.Registry] diff --git a/lib/sentry/test.ex b/lib/sentry/test.ex index e7ff70db..b075eda4 100644 --- a/lib/sentry/test.ex +++ b/lib/sentry/test.ex @@ -56,7 +56,7 @@ defmodule Sentry.Test do @compile {:no_warn_undefined, [Bypass, Plug.Conn, NimbleOwnership]} @ownership_server Sentry.Test.OwnershipServer - @collector_key :sentry_test_collector + @collector_key :sentry_test_scope # Public API @@ -180,7 +180,13 @@ defmodule Sentry.Test do ) scheduler_pid = Sentry.TelemetryProcessor.get_scheduler(processor_name) - Sentry.Test.Config.allow(self(), scheduler_pid) + + if scheduler_pid do + # Goes through the unified `:sentry_test_scope` key, which also + # populates the merged routing ETS row so `Config.namespace/1` + # resolves the scheduler pid back to this test's scope. + Sentry.Test.Registry.claim_allow(self(), scheduler_pid, :soft) + end Process.put(:sentry_telemetry_processor, processor_name) processor_name @@ -297,37 +303,53 @@ defmodule Sentry.Test do ensure_nimble_ownership_loaded!() allowed_pid = resolve_allowed_pid(pid_or_fun) - case NimbleOwnership.allow(@ownership_server, owner_pid, allowed_pid, @collector_key) do + unless owner_collecting?(owner_pid) do + raise ArgumentError, + "owner #{inspect(owner_pid)} is not collecting Sentry reports; " <> + "call Sentry.Test.setup_sentry/1 or " <> + "Sentry.Test.start_collecting_sentry_reports/0 first" + end + + # Single orchestrator call: claims `allowed_pid` for `owner_pid` via + # NimbleOwnership against the unified `:sentry_test_scope` key (which + # also gates the collector callback) and writes the merged routing + # row. After it succeeds, tag the per-test processor for buffered + # event routing — the row is already present so this is a cheap + # `:ets.update_element/3`. + case Sentry.Test.Registry.claim_allow(owner_pid, allowed_pid, :strict) do :ok -> - # Also route per-test config overrides (DSN, before_send hooks, - # the internal collector callback, etc.) through the owner's - # scope so that Sentry callbacks invoked from `allowed_pid` - # resolve to the same configuration the test set up. - Sentry.Test.Config.allow(owner_pid, allowed_pid) + tag_processor_for_allowed_pid(owner_pid, allowed_pid) :ok - {:error, %{reason: {:already_allowed, ^owner_pid}}} -> - # Idempotent re-allow under the same owner. - Sentry.Test.Config.allow(owner_pid, allowed_pid) - :ok + {:error, {:taken, ^allowed_pid}} -> + raise ArgumentError, + "cannot allow #{inspect(allowed_pid)} for #{inspect(owner_pid)}: " <> + "#{inspect(allowed_pid)} is already collecting Sentry reports " <> + "itself (called setup_sentry/1 or start_collecting_sentry_reports/0)" - {:error, %{reason: {:already_allowed, existing_owner}}} -> + {:error, {:taken, existing_owner}} -> raise ArgumentError, "cannot allow #{inspect(allowed_pid)} for #{inspect(owner_pid)}: " <> "already allowed by another live test scope " <> "(owner: #{inspect(existing_owner)})" + end + end - {:error, %{reason: :not_allowed}} -> - raise ArgumentError, - "owner #{inspect(owner_pid)} is not collecting Sentry reports; " <> - "call Sentry.Test.setup_sentry/1 or " <> - "Sentry.Test.start_collecting_sentry_reports/0 first" + defp owner_collecting?(owner_pid) do + # Only `setup_collector/1` stores the per-test collector ETS table + # name (an atom) under `@collector_key`. Lazy ownership registered + # by `Sentry.Test.Registry` for plain config-only tests stores an + # empty map, which we explicitly reject so callers get a useful + # error pointing them at `setup_sentry/1`. + case NimbleOwnership.get_owned(@ownership_server, owner_pid, nil) do + %{} = owned -> + case Map.get(owned, @collector_key) do + name when is_atom(name) and not is_nil(name) -> true + _ -> false + end - {:error, %{reason: :already_an_owner}} -> - raise ArgumentError, - "cannot allow #{inspect(allowed_pid)} for #{inspect(owner_pid)}: " <> - "#{inspect(allowed_pid)} is already collecting Sentry reports " <> - "itself (called setup_sentry/1 or start_collecting_sentry_reports/0)" + _ -> + false end end @@ -345,6 +367,42 @@ defmodule Sentry.Test do end end + # Routes buffered events (logs, metrics) emitted from an allowed + # pid to the owning test's per-test TelemetryProcessor rather than + # the global one. Without this, the buffered pipeline invokes the + # test's collecting callback in the global scheduler pid — which + # is not in the test's NimbleOwnership allow chain — and the + # callback drops the event. + # + # The owner's processor name is looked up from its process + # dictionary; tests set it in `setup_telemetry_processor/0`. If the + # owner has no per-test processor (e.g. legacy + # `start_collecting/1` without `setup_telemetry_processor/0`), the + # tag is skipped and the buffered event still falls back to the + # global processor — the same behaviour as before this change. + defp tag_processor_for_allowed_pid(owner_pid, allowed_pid) do + case fetch_owner_processor(owner_pid) do + nil -> + :ok + + processor_name -> + Sentry.Test.Registry.tag_processor_for(allowed_pid, processor_name) + end + end + + defp fetch_owner_processor(owner_pid) do + case Process.info(owner_pid, :dictionary) do + {:dictionary, dict} -> + case Keyword.get(dict, :sentry_telemetry_processor) do + name when is_atom(name) and not is_nil(name) -> name + _ -> nil + end + + nil -> + nil + end + end + @doc """ Pops all the collected events from the current process. @@ -773,27 +831,35 @@ defmodule Sentry.Test do # :before_send_metric in `Sentry.Config` based on DSN value: when DSN is # `nil`, only these collecting callbacks run; when DSN is set, the user's # callback runs first and its result is collected. - collector = build_collecting_callback() + collector = build_collecting_callback(self()) Sentry.Test.Config.put_override(:_internal_before_send, collector) Sentry.Test.Config.put_override(:_internal_before_send_log, collector) Sentry.Test.Config.put_override(:_internal_before_send_metric, collector) # The TelemetryProcessor's scheduler is not in `$callers` of this test — # allow it explicitly so log/metric events routed through the buffered - # pipeline can find this test's collector. + # pipeline can find this test's collector. Routes through the orchestrator + # so the merged routing row is updated alongside the NimbleOwnership claim. scheduler_pid = get_scheduler_pid() if scheduler_pid do - :ok = - NimbleOwnership.allow(@ownership_server, self(), scheduler_pid, @collector_key) + Sentry.Test.Registry.claim_allow(self(), scheduler_pid, :soft) end # Register cleanup for the collector ETS table only. NimbleOwnership # cleans up the key and allowances automatically when this test exits. + # Drop any worker→processor routing rows that point at this test's + # processor so a test that exits before its allowed pids do does not + # leave stale rows pointing at a stopped per-test processor. + processor_name = + Process.get(:sentry_telemetry_processor, Sentry.TelemetryProcessor.default_name()) + ExUnit.Callbacks.on_exit(fn -> if :ets.whereis(collector_table) != :undefined do :ets.delete(collector_table) end + + Sentry.Test.Registry.drop_processor_routing_for(processor_name) end) :ok @@ -811,32 +877,40 @@ defmodule Sentry.Test do :exit, _ -> nil end - defp find_collector do - pids = [self() | Process.get(:"$callers", [])] - - case NimbleOwnership.fetch_owner(@ownership_server, pids, @collector_key) do - {:ok, owner_pid} -> - @ownership_server - |> NimbleOwnership.get_owned(owner_pid, %{}) - |> Map.get(@collector_key) + # Standalone collecting callback. Records the struct in the owning + # test's collector ETS table, then returns the struct unchanged so it + # flows through any remaining pipeline stages. + # + # The owner pid is captured in the closure at install time so the + # callback always routes to the test that installed it, regardless of + # which process invokes it (the test pid, an allowed worker pid, the + # per-test scheduler pid, or the global TelemetryProcessor scheduler + # pid). + # + # Membership is still enforced via NimbleOwnership: the calling pid + # (or any of its `$callers`) must be in `owner_pid`'s allow chain for + # the collector key. This preserves the pre-existing safety check + # that processes outside the test's allow set never have their + # events collected. + defp build_collecting_callback(owner_pid) do + fn struct -> + with true <- Process.alive?(owner_pid), + true <- caller_allowed_for?(owner_pid), + %{} = owned <- NimbleOwnership.get_owned(@ownership_server, owner_pid, nil), + table when not is_nil(table) <- Map.get(owned, @collector_key) do + collect_struct(table, struct) + end - :error -> - nil + struct end end - # Standalone collecting callback. Records the struct in this test's - # collector ETS table when one is registered for the calling process (or any - # of its callers), then returns the struct unchanged so it flows through any - # remaining pipeline stages. - defp build_collecting_callback do - fn struct -> - case find_collector() do - nil -> :ok - table -> collect_struct(table, struct) - end + defp caller_allowed_for?(owner_pid) do + pids = [self() | Process.get(:"$callers", [])] - struct + case NimbleOwnership.fetch_owner(@ownership_server, pids, @collector_key) do + {:ok, ^owner_pid} -> true + _ -> false end end diff --git a/lib/sentry/test/registry.ex b/lib/sentry/test/registry.ex index ccb1af6e..c4496fe8 100644 --- a/lib/sentry/test/registry.ex +++ b/lib/sentry/test/registry.ex @@ -6,9 +6,19 @@ defmodule Sentry.Test.Registry do require Logger # Bypass and Plug.Conn may not be available at compile time (optional deps). - @compile {:no_warn_undefined, [Bypass, Bypass.Instance, Bypass.Supervisor, Plug.Conn]} + @compile {:no_warn_undefined, + [Bypass, Bypass.Instance, Bypass.Supervisor, Plug.Conn, NimbleOwnership]} - @allows_table :sentry_test_scope_allows + @ownership_server Sentry.Test.OwnershipServer + @scope_key :sentry_test_scope + + # Single merged ETS table replacing the previous + # `:sentry_test_scope_allows` (allowed_pid -> owner_pid) and + # `:sentry_test_allowed_pid_processor_routing` (allowed_pid -> + # processor_name) tables. Rows are 3-tuples + # `{allowed_pid, owner_pid_or_nil, processor_name_or_nil}`. The 3-tuple + # shape keeps `:ets.match_delete` patterns simple. + @routing_table :sentry_test_pid_routing @spec start_link(keyword()) :: GenServer.on_start() def start_link([] = _opts) do @@ -22,10 +32,11 @@ defmodule Sentry.Test.Registry do end @doc """ - Atomic claim of `allowed_pid` for `owner_pid`'s scope. All claims - serialize through this GenServer so the conflict check and the ETS - write happen as one indivisible step — no two concurrent async tests - can both pass a check-and-then-write race for the same allowed_pid. + Atomic claim of `allowed_pid` for `owner_pid`'s scope. Backed by + `NimbleOwnership.allow/4` against the `:sentry_test_scope` key — the + ownership server serializes the conflict check, so two concurrent + async tests cannot both pass a check-and-then-write race for the + same `allowed_pid`. `mode`: * `:strict` — return `{:error, {:taken, existing_owner}}` when a @@ -34,10 +45,11 @@ defmodule Sentry.Test.Registry do * `:soft` — return `:skipped` in the same situation (used by the auto-allow of globally-supervised pids in `Config.put/1`). - Stale entries from owners that have exited without cleanup are - silently replaced so the new owner can claim the pid. - Idempotent: re-claiming a pid you already own returns `:ok`. + + This routes through the Registry GenServer so that owner monitoring + (for ETS row cleanup on owner DOWN) and the cache-row write happen + atomically with the NimbleOwnership claim. """ @spec claim_allow(pid(), pid(), :strict | :soft) :: :ok | :skipped | {:error, {:taken, pid()}} @@ -47,14 +59,18 @@ defmodule Sentry.Test.Registry do end @doc """ - Removes every allow entry whose owner is `owner_pid`. Atomic batch - delete via `:ets.match_delete/2` — safe to call from a test's on_exit - cleanup without serializing through the GenServer. + Removes every routing row whose owner is `owner_pid`. Direct ETS + match_delete — atomic, no GenServer round-trip. + + Kept for the case where a scope wants to drop its allowances + explicitly (e.g. `Sentry.Test.Scope.Registry.unregister/1`); the + `:DOWN` handler also prunes rows automatically when the owner pid + exits. """ @spec drop_allows_for(pid()) :: :ok def drop_allows_for(owner_pid) when is_pid(owner_pid) do - if :ets.whereis(@allows_table) != :undefined do - :ets.match_delete(@allows_table, {:_, owner_pid}) + if :ets.whereis(@routing_table) != :undefined do + :ets.match_delete(@routing_table, {:_, owner_pid, :_}) end :ok @@ -68,56 +84,172 @@ defmodule Sentry.Test.Registry do """ @spec lookup_allow_owner(pid()) :: pid() | nil def lookup_allow_owner(allowed_pid) when is_pid(allowed_pid) do - case :ets.whereis(@allows_table) do + case :ets.whereis(@routing_table) do :undefined -> nil _ref -> - case :ets.lookup(@allows_table, allowed_pid) do - [{^allowed_pid, owner}] when is_pid(owner) -> + case :ets.lookup(@routing_table, allowed_pid) do + [{^allowed_pid, owner, _processor}] when is_pid(owner) -> if Process.alive?(owner), do: owner, else: nil - [] -> + _ -> nil end end end + @doc """ + Tags `allowed_pid` so that buffered events (logs, metrics) emitted + from it are routed to `processor_name` rather than the global + `Sentry.TelemetryProcessor`. Written by `allow_sentry_reports/2` + and consulted by `Sentry.TelemetryProcessor.processor_name/0`. + + Updates the existing routing row's processor field; if no row exists + yet (defensive), inserts a row with `nil` owner. Direct ETS write — + atomic, no GenServer round-trip. + """ + @spec tag_processor_for(pid(), atom()) :: :ok + def tag_processor_for(allowed_pid, processor_name) + when is_pid(allowed_pid) and is_atom(processor_name) do + if :ets.whereis(@routing_table) != :undefined do + unless :ets.update_element(@routing_table, allowed_pid, {3, processor_name}) do + :ets.insert(@routing_table, {allowed_pid, nil, processor_name}) + end + end + + :ok + end + + @doc """ + Returns the per-test processor name that should receive buffered + events from `allowed_pid`, or `nil` if the pid is not tagged or + the routing table is not started (production). + """ + @spec lookup_processor_for(pid()) :: atom() | nil + def lookup_processor_for(allowed_pid) when is_pid(allowed_pid) do + case :ets.whereis(@routing_table) do + :undefined -> + nil + + _ -> + case :ets.lookup(@routing_table, allowed_pid) do + [{^allowed_pid, _owner, processor_name}] + when is_atom(processor_name) and not is_nil(processor_name) -> + processor_name + + _ -> + nil + end + end + end + + @doc """ + Clears the processor field on every routing row that points at + `processor_name`. Used by `setup_collector/1`'s `on_exit/1` so a + test that exits before its allowed pids do does not leave stale + routing rows pointing at a stopped per-test processor. The owner + field is preserved so the allow remains intact (subsequent + buffered events from those pids fall back to the global + processor — matching pre-change behaviour). + """ + @spec drop_processor_routing_for(atom()) :: :ok + def drop_processor_routing_for(processor_name) when is_atom(processor_name) do + if :ets.whereis(@routing_table) != :undefined do + ms = [{{:"$1", :"$2", processor_name}, [], [{{:"$1", :"$2", nil}}]}] + _ = :ets.select_replace(@routing_table, ms) + end + + :ok + end + @impl true def init(nil) do - _allows_table = :ets.new(@allows_table, [:named_table, :public, :set]) + _routing_table = :ets.new(@routing_table, [:named_table, :public, :set]) maybe_start_default_bypass() - {:ok, :no_state} + {:ok, %{owner_monitors: %{}}} end @impl true def handle_call({:claim_allow, owner_pid, allowed_pid, mode}, _from, state) do + state = ensure_owner_monitored(state, owner_pid) + ensure_scope_owner(owner_pid) + reply = - case :ets.lookup(@allows_table, allowed_pid) do - [] -> - true = :ets.insert_new(@allows_table, {allowed_pid, owner_pid}) + case NimbleOwnership.allow(@ownership_server, owner_pid, allowed_pid, @scope_key) do + :ok -> + upsert_owner(allowed_pid, owner_pid) :ok - [{^allowed_pid, ^owner_pid}] -> + {:error, %{reason: {:already_allowed, ^owner_pid}}} -> + upsert_owner(allowed_pid, owner_pid) :ok - [{^allowed_pid, existing_owner}] -> - cond do - not Process.alive?(existing_owner) -> - true = :ets.insert(@allows_table, {allowed_pid, owner_pid}) - :ok - - mode == :strict -> - {:error, {:taken, existing_owner}} + {:error, %{reason: {:already_allowed, other}}} -> + if mode == :strict, do: {:error, {:taken, other}}, else: :skipped - true -> - :skipped - end + {:error, %{reason: :already_an_owner}} -> + # `allowed_pid` is itself a scope owner — treat as a conflict. + if mode == :strict, do: {:error, {:taken, allowed_pid}}, else: :skipped end {:reply, reply, state} end + @impl true + def handle_info({:DOWN, _ref, :process, pid, _reason}, state) do + if :ets.whereis(@routing_table) != :undefined do + :ets.match_delete(@routing_table, {:_, pid, :_}) + end + + {:noreply, %{state | owner_monitors: Map.delete(state.owner_monitors, pid)}} + end + + def handle_info(_msg, state), do: {:noreply, state} + + ## Private helpers + + defp ensure_owner_monitored(%{owner_monitors: monitors} = state, pid) do + if Map.has_key?(monitors, pid) do + state + else + ref = Process.monitor(pid) + %{state | owner_monitors: Map.put(monitors, pid, ref)} + end + end + + # Lazily registers `owner_pid` as the NimbleOwnership owner of + # `:sentry_test_scope` so subsequent `NimbleOwnership.allow/4` calls + # against this owner succeed even when the test never went through + # `Sentry.Test.setup_collector/1` (e.g. a test that uses + # `Sentry.Test.Config.put/1` standalone). When the owner already + # owns the key, the existing metadata is preserved. + defp ensure_scope_owner(owner_pid) do + case NimbleOwnership.get_and_update( + @ownership_server, + owner_pid, + @scope_key, + # Metadata MUST be non-nil so that NimbleOwnership treats + # `owner_pid` as a key owner (its `cond` in `allow/4` checks + # truthiness of the metadata). Preserve any existing value. + fn + nil -> {:ok, %{}} + current -> {:ok, current} + end + ) do + {:ok, _} -> :ok + {:error, _} -> :ok + end + end + + defp upsert_owner(allowed_pid, owner_pid) do + unless :ets.update_element(@routing_table, allowed_pid, {2, owner_pid}) do + :ets.insert(@routing_table, {allowed_pid, owner_pid, nil}) + end + + :ok + end + # Starts a global Bypass instance that acts as a silent HTTP sink for all tests. # This ensures every test has a valid DSN even without calling setup_sentry/1, # preserving backward compatibility where capture_* returns {:ok, ""}. diff --git a/test/sentry/test_test.exs b/test/sentry/test_test.exs index 2d3eac76..981d3546 100644 --- a/test/sentry/test_test.exs +++ b/test/sentry/test_test.exs @@ -253,7 +253,7 @@ defmodule Sentry.TestTest do NimbleOwnership.get_and_update( Sentry.Test.OwnershipServer, self(), - :sentry_test_collector, + :sentry_test_scope, fn _ -> {:ok, :peer_table} end ) @@ -262,7 +262,7 @@ defmodule Sentry.TestTest do Sentry.Test.OwnershipServer, self(), target, - :sentry_test_collector + :sentry_test_scope ) send(parent, {:claimed, self()}) From 60be69709c031fc776d3139ac188f8c4bc21ff68 Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Wed, 13 May 2026 12:24:13 +0000 Subject: [PATCH 3/5] feat(test): make TelemetryProcessor handling work with async tests Closes the gap that left buffered events (logs and metrics) emitted from processes registered via Sentry.Test.allow_sentry_reports/2 silently dropped instead of routed to the test's collector under async tests. Co-Authored-By: Claude Opus 4.7 --- lib/sentry/application.ex | 22 ++++--- lib/sentry/telemetry_processor.ex | 28 ++++++++- lib/sentry/test.ex | 23 ++++--- lib/sentry/test/registry.ex | 74 +++++++++++++--------- lib/sentry/test/scope/registry.ex | 30 ++++----- test/sentry/test_test.exs | 100 +++++++++++++++++++++++++++++- 6 files changed, 215 insertions(+), 62 deletions(-) diff --git a/lib/sentry/application.ex b/lib/sentry/application.ex index 7f5ba009..d5629297 100644 --- a/lib/sentry/application.ex +++ b/lib/sentry/application.ex @@ -53,15 +53,15 @@ defmodule Sentry.Application do [] end - telemetry_processor = + telemetry_processor_opts = [ - {Sentry.TelemetryProcessor, - [ - buffer_capacities: Config.telemetry_buffer_capacities(), - scheduler_weights: Config.telemetry_scheduler_weights(), - transport_capacity: Config.transport_capacity() - ]} + buffer_capacities: Config.telemetry_buffer_capacities(), + scheduler_weights: Config.telemetry_scheduler_weights(), + transport_capacity: Config.transport_capacity() ] + |> maybe_put_test_processor_resolver() + + telemetry_processor = [{Sentry.TelemetryProcessor, telemetry_processor_opts}] children = maybe_test_registry ++ @@ -162,4 +162,12 @@ defmodule Sentry.Application do else defp maybe_rate_limiter, do: [Sentry.Transport.RateLimiter] end + + defp maybe_put_test_processor_resolver(opts) do + if Config.test_mode?() do + Keyword.put(opts, :processor_resolver, &Sentry.Test.Registry.lookup_processor_for/1) + else + opts + end + end end diff --git a/lib/sentry/telemetry_processor.ex b/lib/sentry/telemetry_processor.ex index 81ddb84a..de78419a 100644 --- a/lib/sentry/telemetry_processor.ex +++ b/lib/sentry/telemetry_processor.ex @@ -54,6 +54,9 @@ defmodule Sentry.TelemetryProcessor do | {:scheduler_weights, %{Category.priority() => pos_integer()}} | {:on_envelope, (Sentry.Envelope.t() -> any())} | {:transport_capacity, pos_integer()} + | {:processor_resolver, (pid() -> atom() | nil) | nil} + + @resolver_key {__MODULE__, :resolver} ## Public API @@ -74,6 +77,11 @@ defmodule Sentry.TelemetryProcessor do * `:scheduler_weights` - Map of priority to weight override (optional) * `:on_envelope` - Callback function invoked when envelopes are ready to send (optional) * `:transport_capacity` - Maximum number of items the transport queue can hold (default: 1000). For log envelopes, each log event counts as one item. + * `:processor_resolver` - Optional `(pid() -> atom() | nil)` function used by `add/1` to discover + the per-pid processor name when no value is set in the process dictionary. The resolver is + stored in `:persistent_term` and shared across all processor instances; passing the same + function from every `start_link/1` call is safe and idempotent. Defaults to `nil`, in which + case `add/1` falls back to the default processor name. ## Examples @@ -294,6 +302,11 @@ defmodule Sentry.TelemetryProcessor do on_envelope = Keyword.get(opts, :on_envelope) transport_capacity = Keyword.get(opts, :transport_capacity, 1000) + case Keyword.get(opts, :processor_resolver) do + nil -> :ok + resolver when is_function(resolver, 1) -> :persistent_term.put(@resolver_key, resolver) + end + buffer_names = for category <- Category.all(), into: %{} do {category, buffer_name(processor_name, category)} @@ -374,7 +387,20 @@ defmodule Sentry.TelemetryProcessor do end defp processor_name do - Process.get(:sentry_telemetry_processor, @default_name) + case Process.get(:sentry_telemetry_processor) do + name when is_atom(name) and not is_nil(name) -> + name + + _ -> + resolve_processor_name() || @default_name + end + end + + defp resolve_processor_name do + case :persistent_term.get(@resolver_key, nil) do + nil -> nil + resolver when is_function(resolver, 1) -> resolver.(self()) + end end defp maybe_add_opt(opts, _key, nil), do: opts diff --git a/lib/sentry/test.ex b/lib/sentry/test.ex index b075eda4..92b42784 100644 --- a/lib/sentry/test.ex +++ b/lib/sentry/test.ex @@ -163,22 +163,25 @@ defmodule Sentry.Test do def setup_telemetry_processor do case Process.get(:sentry_telemetry_processor) do name when is_atom(name) and not is_nil(name) -> - if processor_alive?(name), do: name, else: do_setup_telemetry_processor() + if processor_alive?(name), do: name, else: start_telemetry_processor() _ -> - do_setup_telemetry_processor() + start_telemetry_processor() end end - defp do_setup_telemetry_processor do + defp start_telemetry_processor do uid = System.unique_integer([:positive]) processor_name = :"test_telemetry_processor_#{uid}" ExUnit.Callbacks.start_supervised!( - {Sentry.TelemetryProcessor, name: processor_name}, + {Sentry.TelemetryProcessor, + name: processor_name, processor_resolver: &Sentry.Test.Registry.lookup_processor_for/1}, id: processor_name ) + Process.put(:sentry_telemetry_processor, processor_name) + scheduler_pid = Sentry.TelemetryProcessor.get_scheduler(processor_name) if scheduler_pid do @@ -186,9 +189,9 @@ defmodule Sentry.Test do # populates the merged routing ETS row so `Config.namespace/1` # resolves the scheduler pid back to this test's scope. Sentry.Test.Registry.claim_allow(self(), scheduler_pid, :soft) + tag_processor_for_allowed_pid(self(), scheduler_pid) end - Process.put(:sentry_telemetry_processor, processor_name) processor_name end @@ -844,22 +847,24 @@ defmodule Sentry.Test do if scheduler_pid do Sentry.Test.Registry.claim_allow(self(), scheduler_pid, :soft) + tag_processor_for_allowed_pid(self(), scheduler_pid) end # Register cleanup for the collector ETS table only. NimbleOwnership # cleans up the key and allowances automatically when this test exits. # Drop any worker→processor routing rows that point at this test's - # processor so a test that exits before its allowed pids do does not + # processor so a test that exits before its allowed pids do not # leave stale rows pointing at a stopped per-test processor. - processor_name = - Process.get(:sentry_telemetry_processor, Sentry.TelemetryProcessor.default_name()) + processor_name = Process.get(:sentry_telemetry_processor) ExUnit.Callbacks.on_exit(fn -> if :ets.whereis(collector_table) != :undefined do :ets.delete(collector_table) end - Sentry.Test.Registry.drop_processor_routing_for(processor_name) + if is_atom(processor_name) and not is_nil(processor_name) do + Sentry.Test.Registry.drop_processor_routing_for(processor_name) + end end) :ok diff --git a/lib/sentry/test/registry.ex b/lib/sentry/test/registry.ex index c4496fe8..553ccd9e 100644 --- a/lib/sentry/test/registry.ex +++ b/lib/sentry/test/registry.ex @@ -59,21 +59,18 @@ defmodule Sentry.Test.Registry do end @doc """ - Removes every routing row whose owner is `owner_pid`. Direct ETS - match_delete — atomic, no GenServer round-trip. - - Kept for the case where a scope wants to drop its allowances - explicitly (e.g. `Sentry.Test.Scope.Registry.unregister/1`); the - `:DOWN` handler also prunes rows automatically when the owner pid - exits. + Ensures `owner_pid` is monitored by the registry so that the + `:DOWN` handler runs cleanup (routing-table prune + scope-state + erase via `Sentry.Test.Scope.Registry.handle_owner_down/1`) when + the owner exits. Idempotent. + + Called from `Sentry.Test.Scope.Registry.update/1` on first scope + creation so cleanup does not depend on `claim_allow` ever being + invoked for this owner. """ - @spec drop_allows_for(pid()) :: :ok - def drop_allows_for(owner_pid) when is_pid(owner_pid) do - if :ets.whereis(@routing_table) != :undefined do - :ets.match_delete(@routing_table, {:_, owner_pid, :_}) - end - - :ok + @spec monitor_owner(pid()) :: :ok + def monitor_owner(owner_pid) when is_pid(owner_pid) do + GenServer.call(__MODULE__, {:monitor_owner, owner_pid}) end @doc """ @@ -173,35 +170,50 @@ defmodule Sentry.Test.Registry do @impl true def handle_call({:claim_allow, owner_pid, allowed_pid, mode}, _from, state) do state = ensure_owner_monitored(state, owner_pid) - ensure_scope_owner(owner_pid) reply = - case NimbleOwnership.allow(@ownership_server, owner_pid, allowed_pid, @scope_key) do + case ensure_scope_owner(owner_pid) do + {:error, {:taken, existing_owner}} -> + if mode == :strict, do: {:error, {:taken, existing_owner}}, else: :skipped + :ok -> - upsert_owner(allowed_pid, owner_pid) - :ok + case NimbleOwnership.allow(@ownership_server, owner_pid, allowed_pid, @scope_key) do + :ok -> + upsert_owner(allowed_pid, owner_pid) + :ok - {:error, %{reason: {:already_allowed, ^owner_pid}}} -> - upsert_owner(allowed_pid, owner_pid) - :ok + {:error, %{reason: {:already_allowed, ^owner_pid}}} -> + upsert_owner(allowed_pid, owner_pid) + :ok - {:error, %{reason: {:already_allowed, other}}} -> - if mode == :strict, do: {:error, {:taken, other}}, else: :skipped + {:error, %{reason: {:already_allowed, other}}} -> + if mode == :strict, do: {:error, {:taken, other}}, else: :skipped - {:error, %{reason: :already_an_owner}} -> - # `allowed_pid` is itself a scope owner — treat as a conflict. - if mode == :strict, do: {:error, {:taken, allowed_pid}}, else: :skipped + {:error, %{reason: :already_an_owner}} -> + # `allowed_pid` is itself a scope owner — treat as a conflict. + if mode == :strict, do: {:error, {:taken, allowed_pid}}, else: :skipped + + {:error, %{reason: :not_allowed}} -> + if mode == :strict, do: {:error, {:taken, allowed_pid}}, else: :skipped + end end {:reply, reply, state} end + def handle_call({:monitor_owner, owner_pid}, _from, state) do + state = ensure_owner_monitored(state, owner_pid) + {:reply, :ok, state} + end + @impl true def handle_info({:DOWN, _ref, :process, pid, _reason}, state) do if :ets.whereis(@routing_table) != :undefined do :ets.match_delete(@routing_table, {:_, pid, :_}) end + Sentry.Test.Scope.Registry.handle_owner_down(pid) + {:noreply, %{state | owner_monitors: Map.delete(state.owner_monitors, pid)}} end @@ -237,8 +249,14 @@ defmodule Sentry.Test.Registry do current -> {:ok, current} end ) do - {:ok, _} -> :ok - {:error, _} -> :ok + {:ok, _} -> + :ok + + {:error, %{reason: {:already_allowed, existing_owner}}} -> + {:error, {:taken, existing_owner}} + + {:error, _} -> + :ok end end diff --git a/lib/sentry/test/scope/registry.ex b/lib/sentry/test/scope/registry.ex index b9483923..74db9365 100644 --- a/lib/sentry/test/scope/registry.ex +++ b/lib/sentry/test/scope/registry.ex @@ -81,8 +81,9 @@ defmodule Sentry.Test.Scope.Registry do @doc """ Atomically updates the scope owned by `owner_pid`, creating a new - scope on first call and registering cleanup via - `ExUnit.Callbacks.on_exit/1`. + scope on first call and asking `Sentry.Test.Registry` to monitor + the owner pid so cleanup runs from the registry's `:DOWN` handler + when the owner exits. Not concurrency-safe across processes — each test only mutates its own scope from its own process, so in practice there is no @@ -98,7 +99,7 @@ defmodule Sentry.Test.Scope.Registry do :error -> new_scope = Scope.new(owner_pid) bump_counter() - register_cleanup(owner_pid) + TestRegistry.monitor_owner(owner_pid) new_scope end @@ -156,14 +157,22 @@ defmodule Sentry.Test.Scope.Registry do end end - @spec unregister(pid()) :: :ok - def unregister(owner_pid) when is_pid(owner_pid) do + @doc """ + Scope-state half of owner cleanup. Erases the persistent_term scope + entry and decrements the active-scope counter. Called from + `Sentry.Test.Registry`'s `:DOWN` handler — the routing-table half + is pruned there, atomically with monitor-map removal. + + Idempotent: only decrements the counter when the entry was actually + present, so duplicate DOWNs cannot drive it negative. + """ + @spec handle_owner_down(pid()) :: :ok + def handle_owner_down(owner_pid) when is_pid(owner_pid) do case :persistent_term.get({@scope_key, owner_pid}, :__not_set__) do :__not_set__ -> :ok %Scope{} -> - TestRegistry.drop_allows_for(owner_pid) :persistent_term.erase({@scope_key, owner_pid}) decrement_counter() :ok @@ -310,15 +319,6 @@ defmodule Sentry.Test.Scope.Registry do end end - defp register_cleanup(owner_pid) do - ExUnit.Callbacks.on_exit(fn -> unregister(owner_pid) end) - rescue - # `on_exit/1` raises outside an ExUnit test process; in that case the - # caller is responsible for cleanup (or the scope simply lives until the - # owner process dies and `list_active/0` filters it out via `Process.alive?`). - _ -> :ok - end - defp collect_ancestors(_pid, 0, _seen), do: [] defp collect_ancestors(pid, depth, seen) do diff --git a/test/sentry/test_test.exs b/test/sentry/test_test.exs index 981d3546..f415d536 100644 --- a/test/sentry/test_test.exs +++ b/test/sentry/test_test.exs @@ -1,6 +1,8 @@ defmodule Sentry.TestTest do use Sentry.Case, async: false + require Logger + import Sentry.Test.Assertions alias Sentry.Test, as: SentryTest @@ -18,6 +20,13 @@ defmodule Sentry.TestTest do assert Sentry.Config.dedup_events?() == false end + + test "tags the per-test telemetry scheduler for buffered event routing" do + %{telemetry_processor: processor_name} = SentryTest.setup_sentry() + scheduler_pid = Sentry.TelemetryProcessor.get_scheduler(processor_name) + + assert Sentry.Test.Registry.lookup_processor_for(scheduler_pid) == processor_name + end end describe "start_collecting_sentry_reports/0" do @@ -369,11 +378,98 @@ defmodule Sentry.TestTest do end test "collects log events via the TelemetryProcessor pipeline" do - require Logger - Logger.info("pop_sentry_logs test message") assert_sentry_log(:info, "pop_sentry_logs test message") end end + + describe "setup_sentry/1 routes buffered events from allowed processes" do + setup do + SentryTest.setup_sentry() + :ok + end + + test "Sentry.Metrics.count/3 from an allowed pid lands in the test's collector" do + test_pid = self() + done = make_ref() + + pid = + spawn(fn -> + receive do + :go -> + Sentry.Metrics.count("allowance.metric.test", 1) + send(test_pid, done) + end + end) + + ref = Process.monitor(pid) + + assert :ok = SentryTest.allow_sentry_reports(self(), pid) + + send(pid, :go) + assert_receive ^done, 5_000 + assert_receive {:DOWN, ^ref, :process, ^pid, _}, 5_000 + + Sentry.TelemetryProcessor.flush() + Sentry.TelemetryProcessor.flush(Sentry.TelemetryProcessor) + + metrics = SentryTest.pop_sentry_metrics() + + assert Enum.any?(metrics, &(&1.name == "allowance.metric.test")), + "expected the metric emitted from the allowed pid to land in the " <> + "test collector, got: #{inspect(metrics)}" + end + end + + describe "setup_sentry/1 routes buffered logs from allowed processes" do + @describetag :capture_log + + setup do + ctx = SentryTest.setup_sentry(enable_logs: true, logs: [level: :info]) + + handler_name = :"sentry_allow_logs_test_#{System.unique_integer([:positive])}" + + handler_config = %{ + config: %{ + enable_logs: true + } + } + + :ok = :logger.add_handler(handler_name, Sentry.LoggerHandler, handler_config) + + on_exit(fn -> + _ = :logger.remove_handler(handler_name) + end) + + ctx + end + + test "Logger.warning/1 from an allowed pid lands in the test's collector" do + test_pid = self() + done = make_ref() + + pid = + spawn(fn -> + receive do + :go -> + Logger.warning("hello from allowed pid") + send(test_pid, done) + end + end) + + ref = Process.monitor(pid) + + assert :ok = SentryTest.allow_sentry_reports(self(), pid) + + send(pid, :go) + assert_receive ^done, 5_000 + assert_receive {:DOWN, ^ref, :process, ^pid, _}, 5_000 + + Sentry.TelemetryProcessor.flush() + Sentry.TelemetryProcessor.flush(Sentry.TelemetryProcessor) + + assert_sentry_log(:warn, "hello from allowed pid") + end + end end From 6116a80dad259cbecc89e3772aef9ba255f38be5 Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Fri, 15 May 2026 11:02:55 +0000 Subject: [PATCH 4/5] chore(docs): update Sentry.Test docs to reflect changes --- lib/sentry/test.ex | 9 ++++----- lib/sentry/test/registry.ex | 23 ++++++++++++++++++++++- lib/sentry/test/scope/registry.ex | 24 +++++++++++++++++------- 3 files changed, 43 insertions(+), 13 deletions(-) diff --git a/lib/sentry/test.ex b/lib/sentry/test.ex index 92b42784..1a6d9805 100644 --- a/lib/sentry/test.ex +++ b/lib/sentry/test.ex @@ -230,8 +230,7 @@ defmodule Sentry.Test do > #### Deprecated {: .warning} > - > This function is deprecated and will be removed in v13.0.0. - > Use `setup_sentry/1` or `start_collecting_sentry_reports/0` instead. + > This function is deprecated and will be removed in v14.0.0. Use `setup_sentry/1` instead. The `:owner`, `:cleanup`, and `:key` options are no longer supported and are ignored. """ @@ -252,11 +251,11 @@ defmodule Sentry.Test do > #### Deprecated {: .warning} > - > This function is deprecated and will be removed in v13.0.0. - > Cleanup is now handled automatically via `on_exit` callbacks. + > This function is deprecated and will be removed in v14.0.0. + > Cleanup is now handled automatically when the owning test process exits. """ @doc since: "10.2.0" - @doc deprecated: "Cleanup is now automatic via on_exit callbacks" + @doc deprecated: "Cleanup is now automatic when the owning test process exits" @spec cleanup(pid()) :: :ok def cleanup(owner_pid) when is_pid(owner_pid) do :ok diff --git a/lib/sentry/test/registry.ex b/lib/sentry/test/registry.ex index 553ccd9e..8874ab1e 100644 --- a/lib/sentry/test/registry.ex +++ b/lib/sentry/test/registry.ex @@ -167,6 +167,16 @@ defmodule Sentry.Test.Registry do {:ok, %{owner_monitors: %{}}} end + # Serialization note: every claim funnels through this single named + # GenServer and holds it across TWO blocking round-trips to the + # ownership server — `ensure_scope_owner/1`'s + # `NimbleOwnership.get_and_update/4` and `NimbleOwnership.allow/4`. + # This is the deliberate price of atomicity (no two concurrent async + # tests can both pass a check-then-write race for the same + # `allowed_pid`). It is acceptable because claims happen at test + # setup, not per event, and the hot config/buffer read paths + # (`lookup_allow_owner/1`, `lookup_processor_for/1`) bypass this + # GenServer with lock-free direct ETS reads. @impl true def handle_call({:claim_allow, owner_pid, allowed_pid, mode}, _from, state) do state = ensure_owner_monitored(state, owner_pid) @@ -236,6 +246,16 @@ defmodule Sentry.Test.Registry do # `Sentry.Test.setup_collector/1` (e.g. a test that uses # `Sentry.Test.Config.put/1` standalone). When the owner already # owns the key, the existing metadata is preserved. + # + # INVARIANT: the `:sentry_test_scope` key's metadata is overloaded — + # `Sentry.Test.setup_collector/1` stores the per-test collector ETS + # table name (an atom) under it, while this function stores a bare + # `%{}` marker for collector-less scopes. `Sentry.Test`'s + # `owner_collecting?/1` distinguishes the two purely by value type + # (atom = collecting, map = not). Therefore the update fun below MUST + # preserve an existing value (`current -> {:ok, current}`) and MUST + # NOT overwrite it with `%{}`; doing so would silently turn a + # collecting scope into a non-collecting one with no type error. defp ensure_scope_owner(owner_pid) do case NimbleOwnership.get_and_update( @ownership_server, @@ -243,7 +263,8 @@ defmodule Sentry.Test.Registry do @scope_key, # Metadata MUST be non-nil so that NimbleOwnership treats # `owner_pid` as a key owner (its `cond` in `allow/4` checks - # truthiness of the metadata). Preserve any existing value. + # truthiness of the metadata). Preserve any existing value + # (see the INVARIANT above — never clobber a collector atom). fn nil -> {:ok, %{}} current -> {:ok, current} diff --git a/lib/sentry/test/scope/registry.ex b/lib/sentry/test/scope/registry.ex index 74db9365..c4c51f94 100644 --- a/lib/sentry/test/scope/registry.ex +++ b/lib/sentry/test/scope/registry.ex @@ -7,12 +7,19 @@ defmodule Sentry.Test.Scope.Registry do # # * `{:sentry_test_scope, owner_pid} -> %Scope{}` in `:persistent_term` — # one entry per active test scope, owns its overrides. - # * `:sentry_test_scope_allows` ETS table (named, public, set) — - # reverse index `{allowed_pid, owner_pid}` mapping each - # explicitly-routed pid back to the scope that claimed it. - # Owned by `Sentry.Test.Registry`. Direct ETS reads on the config + # * `:sentry_test_pid_routing` ETS table (named, public, set) — + # single merged routing table owned by `Sentry.Test.Registry`. + # Rows are 3-tuples + # `{allowed_pid, owner_pid_or_nil, processor_name_or_nil}`: the + # owner field is the reverse index mapping each explicitly-routed + # pid back to the scope that claimed it (read here via + # `lookup_allow_owner/1`); the processor field routes buffered + # events (logs, metrics) from that pid to a per-test + # `Sentry.TelemetryProcessor`. Direct ETS reads on the config # read path; conflict-checked writes serialize through that - # GenServer for atomic check-and-insert. + # GenServer for atomic check-and-insert. Owner exit is handled by + # that GenServer's `:DOWN` monitor (routing-row prune + + # `handle_owner_down/1`), not `ExUnit.Callbacks.on_exit/1`. # * `@counter_key -> :counters.t()` — atomic counter for cheap # "any active scopes?" short-circuits, so config reads in # production cost essentially nothing. @@ -29,8 +36,11 @@ defmodule Sentry.Test.Scope.Registry do # GenServers started via `start_supervised/1`). # 3. by_allow — walk `[pid | ancestors]`; reverse-allow lookup # for each candidate (the pid was explicitly - # routed onto a scope via `allow/2` or - # auto-allowed in `Config.put/1`). + # routed onto a scope via + # `Sentry.Test.allow_sentry_reports/2` / + # `Sentry.Test.Config.allow/2`, or auto-allowed in + # `Sentry.Test.Config.put/1` — all of which claim + # through `Sentry.Test.Registry.claim_allow/3`). # # Globally-supervised processes (`:logger`, `:logger_sup`, # `Sentry.Supervisor`) have no caller/ancestor link to any test and From 6fa9f76bd137c157443132080d18abfe11181a48 Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Fri, 15 May 2026 11:24:39 +0000 Subject: [PATCH 5/5] chore(tests): improve coverage of Config handling in tests --- test/sentry/test/config_isolation_test.exs | 91 ++++++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/test/sentry/test/config_isolation_test.exs b/test/sentry/test/config_isolation_test.exs index 71ed8b61..b383d422 100644 --- a/test/sentry/test/config_isolation_test.exs +++ b/test/sentry/test/config_isolation_test.exs @@ -169,4 +169,95 @@ defmodule Sentry.Test.ConfigIsolationTest do assert :ok = Sentry.Test.Config.allow(self(), nil) end end + + describe "config scope isolation across an owner process's lifetime" do + test "a worker stops resolving an owner's config once that owner exits" do + parent = self() + worker = spawn_env_probe() + on_exit(fn -> Process.exit(worker, :kill) end) + + owner = + spawn(fn -> + Sentry.Test.Config.put(environment_name: "owner_a_env") + :ok = Sentry.Test.Config.allow(self(), worker) + send(parent, :allowed) + + receive do + :exit -> :ok + end + end) + + assert_receive :allowed, 1_000 + + assert env_of(worker) == "owner_a_env" + + ref = Process.monitor(owner) + send(owner, :exit) + assert_receive {:DOWN, ^ref, :process, ^owner, _}, 1_000 + + assert wait_until(fn -> env_of(worker) != "owner_a_env" end) + end + + test "a worker freed by a finished owner can be reclaimed and rebound by a new owner" do + parent = self() + worker = spawn_env_probe() + on_exit(fn -> Process.exit(worker, :kill) end) + + first_owner = + spawn(fn -> + Sentry.Test.Config.put(environment_name: "first_owner_env") + :ok = Sentry.Test.Config.allow(self(), worker) + send(parent, :claimed) + + receive do + :exit -> :ok + end + end) + + assert_receive :claimed, 1_000 + assert env_of(worker) == "first_owner_env" + + assert_raise Scope.AllowConflictError, fn -> + Sentry.Test.Config.allow(self(), worker) + end + + ref = Process.monitor(first_owner) + send(first_owner, :exit) + assert_receive {:DOWN, ^ref, :process, ^first_owner, _}, 1_000 + + put_test_config(environment_name: "second_owner_env") + + assert wait_until(fn -> + try do + Sentry.Test.Config.allow(self(), worker) == :ok + rescue + Scope.AllowConflictError -> false + end + end) + + assert env_of(worker) == "second_owner_env" + end + end + + defp spawn_env_probe do + spawn(fn -> env_probe_loop() end) + end + + defp env_probe_loop do + receive do + {:env?, reply_to} -> + send(reply_to, {:env, Sentry.Config.environment_name()}) + env_probe_loop() + end + end + + defp env_of(probe) do + send(probe, {:env?, self()}) + + receive do + {:env, env} -> env + after + 100 -> :no_reply + end + end end