diff --git a/test/sentry/telemetry_processor_integration_test.exs b/test/sentry/telemetry_processor_integration_test.exs index 46b81182..545fb11d 100644 --- a/test/sentry/telemetry_processor_integration_test.exs +++ b/test/sentry/telemetry_processor_integration_test.exs @@ -343,15 +343,7 @@ defmodule Sentry.TelemetryProcessorIntegrationTest do Sentry.ClientReport.Sender.flush() - on_exit(fn -> - for category <- ~w(log_item error monitor transaction) do - try do - :ets.delete(Sentry.Transport.RateLimiter, category) - catch - :error, :badarg -> :ok - end - end - end) + on_exit(fn -> reset_rate_limits(scope: :scheduler) end) :ok end @@ -391,15 +383,7 @@ defmodule Sentry.TelemetryProcessorIntegrationTest do Sentry.ClientReport.Sender.flush() flush_ref_messages(ctx.ref) - on_exit(fn -> - for category <- ~w(log_item log_byte trace_metric trace_metric_byte) do - try do - :ets.delete(Sentry.Transport.RateLimiter, category) - catch - :error, :badarg -> :ok - end - end - end) + on_exit(fn -> reset_rate_limits(scope: :scheduler) end) :ok end @@ -448,23 +432,7 @@ defmodule Sentry.TelemetryProcessorIntegrationTest do Sentry.ClientReport.Sender.flush() flush_ref_messages(ctx.ref) - rate_limiter_table = Process.get(:rate_limiter_table_name) - - on_exit(fn -> - try do - :ets.delete(rate_limiter_table, "log_item") - :ets.delete(rate_limiter_table, "error") - :ets.delete(rate_limiter_table, "monitor") - :ets.delete(rate_limiter_table, "transaction") - :ets.delete(rate_limiter_table, "trace_metric") - :ets.delete(rate_limiter_table, "log_byte") - :ets.delete(rate_limiter_table, "trace_metric_byte") - catch - :error, :badarg -> :ok - end - end) - - %{rate_limiter_table: rate_limiter_table} + :ok end test "drops rate-limited log events before they enter the buffer", ctx do @@ -474,7 +442,7 @@ defmodule Sentry.TelemetryProcessorIntegrationTest do log_buffer = TelemetryProcessor.get_buffer(ctx.processor, :log) - :ets.insert(ctx.rate_limiter_table, {"log_item", System.system_time(:second) + 60}) + set_rate_limit("log_item") assert {:ok, {:rate_limited, "log_item"}} = TelemetryProcessor.add(ctx.processor, make_log_event("pre-buffer-drop")) @@ -487,7 +455,7 @@ defmodule Sentry.TelemetryProcessorIntegrationTest do error_buffer = TelemetryProcessor.get_buffer(ctx.processor, :error) - :ets.insert(ctx.rate_limiter_table, {"error", System.system_time(:second) + 60}) + set_rate_limit("error") Sentry.capture_message("pre-buffer-drop", result: :none) @@ -514,7 +482,7 @@ defmodule Sentry.TelemetryProcessorIntegrationTest do check_in_buffer = TelemetryProcessor.get_buffer(ctx.processor, :check_in) - :ets.insert(ctx.rate_limiter_table, {"monitor", System.system_time(:second) + 60}) + set_rate_limit("monitor") {:ok, _id} = Sentry.capture_check_in(status: :ok, monitor_slug: "dropped-job") @@ -545,7 +513,7 @@ defmodule Sentry.TelemetryProcessorIntegrationTest do transaction_buffer = TelemetryProcessor.get_buffer(ctx.processor, :transaction) - :ets.insert(ctx.rate_limiter_table, {"transaction", System.system_time(:second) + 60}) + set_rate_limit("transaction") assert {:ok, {:rate_limited, "transaction"}} = TelemetryProcessor.add(ctx.processor, make_transaction()) @@ -562,7 +530,7 @@ defmodule Sentry.TelemetryProcessorIntegrationTest do metric_buffer = TelemetryProcessor.get_buffer(ctx.processor, :metric) - :ets.insert(ctx.rate_limiter_table, {"trace_metric", System.system_time(:second) + 60}) + set_rate_limit("trace_metric") assert {:ok, {:rate_limited, "trace_metric"}} = TelemetryProcessor.add(ctx.processor, make_metric("pre-buffer-drop", 1)) @@ -580,7 +548,7 @@ defmodule Sentry.TelemetryProcessorIntegrationTest do log_buffer = TelemetryProcessor.get_buffer(ctx.processor, :log) - :ets.insert(ctx.rate_limiter_table, {"log_byte", System.system_time(:second) + 60}) + set_rate_limit("log_byte") Logger.info("dropped by a log_byte limit") @@ -598,7 +566,7 @@ defmodule Sentry.TelemetryProcessorIntegrationTest do metric_buffer = TelemetryProcessor.get_buffer(ctx.processor, :metric) - :ets.insert(ctx.rate_limiter_table, {"trace_metric_byte", System.system_time(:second) + 60}) + set_rate_limit("trace_metric_byte") Sentry.Metrics.count("dropped.by.byte.limit", 1) @@ -617,21 +585,6 @@ defmodule Sentry.TelemetryProcessorIntegrationTest do Sentry.ClientReport.Sender.flush() flush_ref_messages(ctx.ref) - # The scheduler runs in its own process (with no `:rate_limiter_table_name` - # in its dictionary), so it reads the default rate limiter table rather - # than this test's uniquely-named one. - on_exit(fn -> - try do - :ets.delete(Sentry.Transport.RateLimiter, "transaction") - :ets.delete(Sentry.Transport.RateLimiter, "log_item") - :ets.delete(Sentry.Transport.RateLimiter, "log_byte") - :ets.delete(Sentry.Transport.RateLimiter, "trace_metric") - :ets.delete(Sentry.Transport.RateLimiter, "trace_metric_byte") - catch - :error, :badarg -> :ok - end - end) - :ok end @@ -648,10 +601,7 @@ defmodule Sentry.TelemetryProcessorIntegrationTest do TelemetryProcessor.add(ctx.processor, transaction) assert Buffer.size(transaction_buffer) == 1 - :ets.insert( - Sentry.Transport.RateLimiter, - {"transaction", System.system_time(:second) + 60} - ) + set_rate_limit("transaction", scope: :scheduler) :sys.resume(scheduler) GenServer.cast(scheduler, :signal) @@ -686,7 +636,7 @@ defmodule Sentry.TelemetryProcessorIntegrationTest do TelemetryProcessor.add(ctx.processor, dropped_log) assert Buffer.size(log_buffer) == 1 - :ets.insert(Sentry.Transport.RateLimiter, {"log_item", System.system_time(:second) + 60}) + set_rate_limit("log_item", scope: :scheduler) :sys.resume(scheduler) GenServer.cast(scheduler, :signal) diff --git a/test/sentry/transport/rate_limiter_test.exs b/test/sentry/transport/rate_limiter_test.exs index ca7e9232..1b7fd8e7 100644 --- a/test/sentry/transport/rate_limiter_test.exs +++ b/test/sentry/transport/rate_limiter_test.exs @@ -1,6 +1,8 @@ defmodule Sentry.Transport.RateLimiterTest do use Sentry.Case, async: true + import Sentry.TestHelpers + alias Sentry.Transport.RateLimiter describe "parse_rate_limits_header/1" do @@ -72,24 +74,21 @@ defmodule Sentry.Transport.RateLimiterTest do describe "rate_limited_for_category?/1" do test "gates log_item on the log_byte limit as well" do - now = System.system_time(:second) - :ets.insert(table_name(), {"log_byte", now + 60}) + set_rate_limit("log_byte") assert RateLimiter.rate_limited_for_category?("log_item") == true assert RateLimiter.rate_limited?("log_item") == false end test "gates trace_metric on the trace_metric_byte limit as well" do - now = System.system_time(:second) - :ets.insert(table_name(), {"trace_metric_byte", now + 60}) + set_rate_limit("trace_metric_byte") assert RateLimiter.rate_limited_for_category?("trace_metric") == true assert RateLimiter.rate_limited?("trace_metric") == false end test "gates a category on itself when it has no companion byte category" do - now = System.system_time(:second) - :ets.insert(table_name(), {"error", now + 60}) + set_rate_limit("error") assert RateLimiter.rate_limited_for_category?("error") == true assert RateLimiter.rate_limited_for_category?("transaction") == false @@ -133,8 +132,7 @@ defmodule Sentry.Transport.RateLimiterTest do describe "rate_limited?/1" do test "returns true for rate-limited category" do - now = System.system_time(:second) - :ets.insert(table_name(), {"error", now + 60}) + set_rate_limit("error") assert RateLimiter.rate_limited?("error") == true end @@ -144,15 +142,13 @@ defmodule Sentry.Transport.RateLimiterTest do end test "returns false for expired rate limit" do - now = System.system_time(:second) - :ets.insert(table_name(), {"error", now - 10}) + set_rate_limit("error", duration: -10) assert RateLimiter.rate_limited?("error") == false end test "returns true when global limit is active" do - now = System.system_time(:second) - :ets.insert(table_name(), {:global, now + 60}) + set_rate_limit(:global) # Any category should be limited assert RateLimiter.rate_limited?("error") == true @@ -160,9 +156,8 @@ defmodule Sentry.Transport.RateLimiterTest do end test "returns true if either category or global limit is active" do - now = System.system_time(:second) - :ets.insert(table_name(), {"error", now + 30}) - :ets.insert(table_name(), {:global, now + 60}) + set_rate_limit("error", duration: 30) + set_rate_limit(:global) assert RateLimiter.rate_limited?("error") == true end diff --git a/test/support/test_helpers.ex b/test/support/test_helpers.ex index 6e6e9832..b90b8d03 100644 --- a/test/support/test_helpers.ex +++ b/test/support/test_helpers.ex @@ -21,6 +21,22 @@ defmodule Sentry.TestHelpers do Sentry.Test.Config.put(config) end + @spec set_rate_limit(String.t() | :global, keyword()) :: :ok + def set_rate_limit(category, opts \\ []) when is_binary(category) or category == :global do + table = rate_limiter_table(Keyword.get(opts, :scope, :local)) + duration = Keyword.get(opts, :duration, 60) + + :ets.insert(table, {category, System.system_time(:second) + duration}) + register_rate_limit_cleanup(table, category) + :ok + end + + @spec reset_rate_limits(keyword()) :: :ok + def reset_rate_limits(opts \\ []) do + :ets.delete_all_objects(rate_limiter_table(Keyword.get(opts, :scope, :local))) + :ok + end + @spec set_mix_shell(module()) :: :ok def set_mix_shell(shell) do mix_shell = Mix.shell() @@ -155,4 +171,23 @@ defmodule Sentry.TestHelpers do wait_until_loop(condition_fn, end_time, min(sleep_time * 2, 50)) end end + + defp rate_limiter_table(:local), do: Process.get(:rate_limiter_table_name) + defp rate_limiter_table(:scheduler), do: Sentry.Transport.RateLimiter + + defp register_rate_limit_cleanup(table, category) do + key = {__MODULE__, :rate_limit_cleanup, table, category} + + unless Process.get(key) do + Process.put(key, true) + + ExUnit.Callbacks.on_exit(fn -> + try do + :ets.delete(table, category) + catch + :error, :badarg -> :ok + end + end) + end + end end